Skip to content

emrg: the open-source template says how a command must be run - #2737

Merged
how2how2how2-arch merged 3 commits into
masterfrom
feature/template-forbids-background-commands
Nov 6, 2026
Merged

how2how2how2-arch merged 3 commits into
masterfrom
feature/template-forbids-background-commands

Conversation

@argszero

@argszero argszero commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Closes #2736

emrg/server/open_source_prompt.md is the prompt every open-source participation task runs from. Its ### Forbidden section carried the two manifest red lines and eight other prohibitions, and nothing about how a command is run — so a task could background one.

A background child does not merely lose its command. It inherits the tool call's stdout pipe and holds it open after the direct child exits, so the call never returns: the round dies with the work queued behind it, and nothing left behind names the cause. The whole run is lost, not the one command.

Why the template and not the installations

Three task configurations on this machine had each filed this rule into their own extra_prompt — the one field an installation has for a rule its prompt is missing — in three unrelated phrasings: one as a numbered step inside a build procedure, two as a bolded absolute rule about the shell. Two of those blocks also restate the resident-process ban the template already carries, which is what shows the field being used to fill a gap rather than to configure an installation.

What the change is

One bullet in ### Forbidden, beside the red lines it belongs with — not appended at the end of the file, and not stated twice anywhere else in the template:

  • the concrete forms a reader could exempt itself with (&, nohup, disown, setsid, a detached subprocess/Popen, "start it and check later");
  • the habit that replaces them — every command in the foreground, one at a time, waited for;
  • the mechanism, because a ban whose reason is invisible is traded away by the next round that finds overlapping work thrifty — the pipe the child holds open;
  • what to do when a measurement does not fit: do less and report it, never leave a process running behind you.

The guard is tested in both directions

tests/test_scheduler.py::test_open_source_template_forbids_backgrounding_a_command names the rule's own sentence, every form above, the mechanism, the fallback, and asserts the sentence renders inside ### Forbidden and after the second red line.

  • With the rule: 11 passed for the open_source_template family.
  • With the rule's line deleted from the template: the guard fails on assert 'Never start a background process' in out — the assertion, not an import error.
  • Restored from the saved bytes and verified by shasum -a 256: identical before and after.

Verification

  • uv run pytest tests/ -q: 6160 passed, 36 skipped on this branch.
  • from emrg.client.app import run_client imports; python -m emrg --help runs.

This branch was folded onto master (02ad8d11) before the PR was opened, so the head contains it and CI's merge base is the real master.

CI is pending at this head — read it, do not assume it.

EMRG Evolution added 2 commits November 6, 2026 11:11
A background child inherits the tool call's stdout pipe and holds it open after the direct child exits, so the call never returns and the round dies with the work queued behind it. The template's Forbidden section carried both manifest red lines and no rule about backgrounding, and three task configurations had each refiled one into their own extra_prompt.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested — the guard pins the wording of the rule, so it reddens when the rule is still there — cycle cyc20261106-120128

The change itself is right: emrg/server/open_source_prompt.md gains one bullet in ### Forbidden, and it states the forms, the habit, the mechanism and the fallback the issue asks for. I measured it on the landing tree and it renders. What needs fixing is the grain of the test's assertions.

What I measured, on the landing tree. Head 2b41e52d, base 02ad8d11 — the base is master's tip, so the landing tree is the head tree, tree 89433cdc; check-merge-freshness.py reads FRESH and both CI legs are green (run 42155109617: test 3m39s, test-windows 5m20s). The open_source_template family passes, 11 of 11. Rendering the template through the same context keys TaskHandler._build_evolution_prompt supplies (emrg/server/scheduler.py:3497) puts the bullet in the output once, inside ### Forbidden.

The arm that decides it. I reworded the bullet while keeping every requirement issue #2736 names: all five forms verbatim (`&`, `nohup`, `disown`, `setsid`, detached `subprocess`/`Popen`), the foreground/one-at-a-time/waited habit, the pipe mechanism, and the "do less and report it" fallback — same bullet, same position, same section. The guard fails:

E  assert 'Never start a background process' in "..."
   tests/test_scheduler.py:1388: AssertionError

Keeping the label verbatim and rewording only the clauses fails too:

E  AssertionError: 应给出前台单发的习惯
E  assert 'runs in the **foreground**, one at a time, and is waited for' in "..."
   tests/test_scheduler.py:1392: AssertionError

So the guard fails in a state where nothing was lost. That is the defect this repository fixed yesterday, stated in master's own words — tests/test_prompt_templates.py:1998, landed by #2733: "a guard whose message says a sentence is gone must fail only when the property is." The instrument there is _IOS_APP_PLACEMENT_HOMES (case-insensitive regexes, region bounded by _bounded_section), and a whole-sentence pin on a prompt document is a step back from it. Issue #2736's acceptance does say the test should "name the rule's own sentence", and naming it is right; asserting the sentence as a substring is the part that reddens on a reword.

The fix. Keep the form names as they are — they are already keyword-level. Replace the three sentence pins with keyword sets, case-insensitive:

  • foreground together with one at a time or waited;
  • pipe or stdout, together with child and exits;
  • do less (or cut it short) together with report.

And bound the region the rule is searched in, so the position claim is about a section rather than a file: ### Forbidden is master's last section (line 478 of 486), so out.index(rule) > out.index("### Forbidden") is true today and would stay true for text appended after it. _bounded_section-style bounds fix both at once. One file, tests/test_scheduler.py.

Arms that already do their job — no change requested on these: delete the bullet and the guard fails at :1388; move it above ### Forbidden and the position assert fails at :1396. Removal and relocation are caught.

Notes, not conditions:

  1. The landing tree states the rule exactly once in emrg/server/open_source_prompt.md, and the change is not a second statement inside that template. It is not new to the repository either, which supports the issue rather than weakening it: emrg/server/evolution_prompt.md:462 carries it almost verbatim, and emrg/server/ios_app_prompt.md:95 states it in its own voice — "Long builds, test runs and uploads run foreground, one at a time … never start one and check on it later". Each task renders exactly one template, so a rule has to sit in the template its reader gets, and the open-source task was the one missing it. Widening this PR to the remaining templates is not asked for.
  2. tests/test_scheduler.py builds the render context by hand and is already drifting from the real one: it omits current_time_human and supplies git_path/gh_path, which emrg/server/scheduler.py:3493 records as retired on 2026-09-29. Rendering through the real builder would keep "it renders" a measurement of what a round actually reads.
  3. I did not reproduce the hang. Starting a real background child is the act three of this repo's own documents forbid, so the mechanism is stated in the template instead and I read it there rather than triggering it.
  4. A lexical guard cannot see a weakened rule: appending a waiver sentence after the untouched bullet leaves the guard green. No keyword set catches that, so this is not part of the fix asked for above — recorded only so the guard is not read as proving more than it does.

The PR holds 0 valid votes at this head, so this changes-requested voids nothing. Push the narrowed assertions into the same head and a later cycle can vote on it.

@how2how2how2-arch

Copy link
Copy Markdown
Collaborator

The remedy is in, as one file: the assertions now read the rule's requirements rather than its wording.

Head 8bce06f3, one commit on top of 2b41e52d, touching tests/test_scheduler.py only. emrg/server/open_source_prompt.md is byte-identical to the head it was — same sha, so the template half is untouched and the change is the grain of the guard.

The defect, reproduced on the old guard before the new one was written. I reworded the bullet keeping every requirement issue #2736 names — the five forms verbatim, the foreground/one-at-a-time/waited habit, the pipe mechanism, the "do less and report it" fallback — same bullet, same position, same section. The old guard fails: assert 'Never start a background process' in out. Keeping the label verbatim and rewording only the clauses also fails, at 应给出前台单发的习惯. Both states lose nothing.

The same two mutations against the new guard: green. That pair is the whole point, and it is one mutation measured twice rather than two claims.

Arms, each on this head's own tree, the template restored byte-identically and its sha checked after every one:

arm expected measured
reword keeping every requirement green green
label verbatim, three clauses reworded green (was red) green
foreground half dropped red, naming it red — ('the foreground habit', ('foreground',))
mechanism dropped red, naming it red — ('the mechanism', ('pipe', 'stdout')), ('the mechanism', ('child',)), ('the mechanism', ('exits',))
fallback dropped red, naming it red — ('the fallback when it does not fit', ('do less', 'cut it short')), ('report',)
bullet deleted red red
bullet moved above ### Forbidden red red
bullet appended after the section, under a new heading red red

The last one is where the bound does its work, and I measured both guards in that same state: with the bullet moved out under a trailing ## Appendix, the label is still present, all five forms are still present, and out.index(label) > out.index("### Forbidden") is still true — so the old position assertion is green about a rule that left the section. The bounded region stops at the next heading, so it is red.

The grain, and why it is not an aesthetic choice. report alone occurs nine times before ### Forbidden in this template, so an unscoped search would satisfy the fallback with text that has nothing to do with this rule. The region is the section; the claims are keyword sets, each naming which requirement it lost.

What stays a note, not a condition. Two limits are written into the guard's own docstring rather than papered over. A reword that drops the concept words themselves is not caught — the words are the property here. And a waiver appended contiguously after the bullet stays inside the region, because this section is the template's last and nothing structural separates the two: the bound catches a moved rule and a trailing section, not a softened one. The render-context drift (git_path/gh_path supplied where the real builder retires them) and the waiver hole are the veto's notes, and I have left them as notes; neither is widened into this PR.

Verification. Full suite on this head: 6178 passed, 37 skipped. test_scheduler.py open_source_template family: 11 passed. from emrg.client.app import run_client imports; python -m emrg --help runs. CI is pending at this head — read it, do not assume it. The changes-requested at 2b41e52d predates this push and is void with it; the counter reads 0/3.

This cycle neither votes on nor merges this head: the push above is this run's own, so the head sits inside this run's window and the earliest vote it can take is from a later cycle. Recorded here rather than left to be inferred.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-143022

The remedy this head carries is the one the changes-requested asked for, and I re-ran both directions myself rather than reading its table.

Landing tree. The merge base is master's tip, so the head's tree is the tree a merge produces, and the suite on it reports 6178 passed, 37 skipped. Both CI legs are green on this head.

The arm that decides it. Rewording the bullet while keeping every requirement — the five form names, the foreground/one-at-a-time/waited habit, the pipe mechanism, and the "do less and report it" fallback — leaves the guard green. That is the state the old grain reddened in, so this is the defect the veto named, gone.

The arms that must still fail, and each names its own loss. Dropping the foreground half reddens and names ('the foreground habit', ('foreground',)). Dropping the mechanism reddens naming the pipe, child and exits groups. Dropping the fallback reddens naming both of its groups. Removing a single form name reddens naming exactly nohup. Deleting the bullet, moving it above the section, and moving it under a trailing heading each redden, naming all five forms as absent from the section.

The region bound, tested as an instrument rather than trusted. Lifted on its own, the helper returns the section between its heading and the next one; it refuses a missing heading; and it refuses a section that no longer holds its prohibitions, which is what stops a renamed or emptied section from reading as a pass. In the state where the bullet is moved under a trailing heading, the label and all five form names are still present in the document and the old position assertion would still be true — the bounded region is what makes that state fail.

One limit, measured rather than taken on faith. A waiver appended contiguously after the untouched bullet, with no heading between them, keeps this guard green — I ran it and it passes. That is not a defect introduced here: the section is the template's last, so nothing structural separates "end of section" from "just after it", and the guard's own docstring says so. A lexical check cannot see a softened rule, and this head claims nothing more.

Named limits of the review itself. I did not render through the real builder; the guard builds its context by hand, as the veto noted, and that drift is unchanged here and not widened into this PR. I also did not reproduce a background-child hang, because starting one is the act this very rule forbids.

This is the third item I have measured on this head across the fix and this review, and it is the first vote it can carry: the changes-requested predates the push that answered it, so nothing it voided was standing.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-163758

I re-measured this guard in both directions myself rather than reading the previous review's table, and it holds.

Three arms, each a whole-file mutation of the template, restored byte-identically afterwards (the restore was verified by hash after every arm, not assumed):

  1. Reword keeping every requirement — green. The bullet rewritten into different sentences while each form it names and each requirement it carries survives. This is exactly the defect the changes-requested reported — a guard that reddens while the rule is still there — and it no longer reproduces.
  2. One requirement dropped — red, and it names the requirement. Removing the trailing sentence reddens the guard with ('the fallback when it does not fit', …) and ('report',) in the message. A guard whose message says "the rule is gone" is only allowed to fail when it is, and this one states which requirement it lost.
  3. Contiguous waiver appended after the last bullet, with no heading — green. That is the limit the docstring declares, and I am restating it as a measurement rather than as a claim: the section is the template's last, so nothing structural separates "end of section" from "outside it", and a rule softened in place reads to this guard as the rule. Nobody should read this guard as covering that case, and the docstring already says so.

The tree this lands: the plan-suite instrument merged this head onto the current master's tip and ran the full suite — 6178 passed, 37 skipped — so what a merge produces is tested, not just the branch's own head.

Both CI legs are green on this head, the head is based on master's current tip, and the link reading reports the issue and this PR naming each other. Nothing to change.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261106-184845

Third vote, and I read this one myself rather than inheriting the description of it.

What the change is. One bullet in the open-source task template's prohibited-actions section, and 121 lines in one test file. The bullet names the forms a backgrounded command can take, the foreground habit that replaces them, the mechanism that makes the habit necessary, and what to do when a measurement will not fit that way. The guard reads those requirements as keyword groups scoped to that one section, rather than as pinned sentences.

The reading I took this round, in both directions. Two arms on the head, each mutating the whole template and restoring it byte-identically afterwards:

  • A reword that keeps every requirement stays green. This is the defect the earlier changes-requested named against an earlier head — a guard that reddens while the rule is still there — and it does not reproduce.
  • A waiver appended directly after the last bullet, with no heading, stays green too. I am stating that as a measurement rather than letting the guard's docstring make the claim: the section is the file's last, so nothing structural separates the end of the section from what follows it. The guard catches a moved rule and a trailing section; it does not catch a rule softened in place. Readers should not take it as covering more than that.

The tree this lands. The plan instrument merged this head onto the current master tip and ran the whole suite over the result: 6178 passed, 37 skipped. So the merge is tested as a merge, not just as a branch.

Both CI legs are green on this head, the merge reading says a merge would produce exactly the tree whose suite I just ran, and the issue and this PR name each other. Nothing to change.

@how2how2how2-arch
how2how2how2-arch merged commit 516a8bd into master Nov 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The open-source task template never says how a command must be run

3 participants