diff --git a/emrg/server/open_source_prompt.md b/emrg/server/open_source_prompt.md index 1557ac39..6a8c34d2 100644 --- a/emrg/server/open_source_prompt.md +++ b/emrg/server/open_source_prompt.md @@ -484,3 +484,4 @@ When the platform's CLI is unavailable — not installed, unauthenticated, netwo - 🛑 Contributors: the gatekeeping verbs in R1's right-hand column are the whole prohibition. - 🛑 **Never write, restore or introduce anything that stops or restarts the emrg server / `emrgd`** — not in a test, not in a script, not in any other code path: `stop_all()`, `stop_daemon()`, `emrg server stop`, `emrg server restart`, or a test that reaches a real teardown. The server is the body of the running instance, not a resource a task owns: stopping it drops the host's connection, rebuilds every scheduled handler, and takes this task's own cycle down with it. This is `MANIFESTO.md` 第四条附则二; it is **not subject to any evolution mechanism**, and no task may modify, delete or weaken it. **To test this area safely**, call the teardown with its effects mocked (the shape `tests/test_daemon.py::test_shutdown_all_*` uses) — a test must never start or stop a real daemon; the autouse fixtures `tests/conftest.py::_guard_stop_all_hermeticity` and `_guard_no_live_daemon_is_signalled` are the mechanical backstop. - 🛑 **Never write, restore or introduce anything that triggers the real auto-upgrade chain** — not in a test, not in a script, not in any other code path: a real GitHub releases request, a real read or write of `~/.emrg/install/version.txt`, a real `emrg-upgrade` session write, or `UpgradeManager.tick()` / the daemon's `_run_upgrade_session` reached without full isolation. This is `MANIFESTO.md` 第四条附则三; it is **not subject to any evolution mechanism**. To test this area safely, stub every side-effect endpoint (the releases client, `VERSION_FILE`, the `emrg-upgrade` session, `run_session_cb`); the autouse fixture `tests/conftest.py::_guard_upgrade_hermeticity` is the backstop that turns an unstubbed call into an assertion failure rather than a live request. +- 🛑 **Never start a background process** — no `&`, no `nohup`, no `disown`, no `setsid`, no detached `subprocess`/`Popen`, and no "start it and check later". Every command runs in the **foreground**, one at a time, and is waited for. 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 that was queued behind it — nothing left behind names the cause. When a measurement does not fit that way, do less and report it; never leave a process running behind you. diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 64efa98d..21342d3b 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -7,6 +7,7 @@ import json import logging import os +import re import subprocess import sys import tempfile @@ -1359,6 +1360,126 @@ def test_open_source_template_measures_the_shipped_artifact(): assert out.index("A defect is a claim about what the project ships") < out.index("##### 3.1.4 Fork, clone and branch"), "规则应落在 §3.1.3 内" +#: The section that owns prohibitions in `open_source_prompt.md`, and every heading that +#: could end it. The section is the template's **last**, so an end bound exists only when a +#: later edit adds a heading after it — which is exactly the case that has to fail. +_FORBIDDEN_HEADING = "### Forbidden" +_HEADING_AFTER = re.compile(r"\n#{2,4} ") + +#: Every form a reader could exempt itself with, verbatim. These are already keywords, so a +#: reword of the sentence around them cannot drop one without dropping the form itself. +_BACKGROUND_FORMS = ( + "`&`", "`nohup`", "`disown`", "`setsid`", "detached `subprocess`/`Popen`", +) + +#: What the rule must still say, as keyword sets rather than pinned sentences: each entry is +#: (what it carries, groups that must each match) and a group is satisfied by any of its +#: members. A reword that keeps a requirement keeps every one of its groups; a reword that +#: drops it loses one, and the failure names which. +_BACKGROUND_CLAIMS = ( + ("the ban itself", (("background",),)), + ("the foreground habit", (("foreground",), ("one at a time", "waited"))), + ("the mechanism", (("pipe", "stdout"), ("child",), ("exits",))), + ("the fallback when it does not fit", (("do less", "cut it short"), ("report",))), +) + + +def _forbidden_section(text: str) -> str: + """One rendered template's `### Forbidden` region, with both ends controlled. + + `text.split(heading, 1)[1]` returns the whole tail when no heading follows, so a region + written as "everything after the heading" reads as a tightening and behaves as none: + this section is the template's *last*, and `out.index(rule) > out.index(heading)` stays + true for anything appended after it — including the rule itself, moved out. So the + region is the section up to the next heading when one exists, and the control below + refuses a region that is not a prohibitions list, which is what a renamed section, an + emptied one, or a heading that lost its bullets produces. + """ + after_heading = text.split(_FORBIDDEN_HEADING, 1) + assert len(after_heading) == 2, ( + f"the template must still carry the {_FORBIDDEN_HEADING!r} heading — this region is " + "anchored on it" + ) + rest = after_heading[1] + heading_after = _HEADING_AFTER.search(rest) + region = rest[: heading_after.start()] if heading_after else rest + assert "\U0001f6d1" in region, ( + f"{_FORBIDDEN_HEADING!r} must still be followed by the prohibitions it owns: without " + "them the region is empty or holds something else, and the assertions below would be " + "reading nothing" + ) + return region + + +def test_open_source_template_forbids_backgrounding_a_command(): + """open_source_prompt.md §Forbidden says how a command must be run (issue #2736). + + Three task configurations independently refiled this rule into their own + `extra_prompt` — the one field an installation has for a rule its prompt is + missing — in three unrelated phrasings, while two of them also restated the + resident-process ban the template already carries. 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. + + Two grains are deliberate, and both were measured rather than chosen. The assertions + are **keywords, not sentences**: three whole-sentence pins reddened on a reword that + kept every requirement — the defect a neighbouring guard in this same tree fixed + (#2733): a guard whose message says the rule is gone must fail only when it is. And + they are **scoped to the section**: `report` alone occurs nine times before + `### Forbidden`, so an unscoped search would satisfy the fallback with text that has + nothing to do with this rule. + + Named limits. A reword that drops the concept words themselves is not caught: the words + are the property here, so a rule restated without them is a rule this guard cannot read. + And a waiver appended *contiguously* after the bullet stays inside the region, because + the 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. + """ + import jinja2 + + template_path = ( + Path(__file__).resolve().parent.parent + / "emrg" / "server" / "open_source_prompt.md" + ) + env = jinja2.Environment(undefined=jinja2.Undefined) + template = env.from_string(template_path.read_text(encoding="utf-8")) + out = template.render( + instance_id="test", host_name="host", repo_url="https://github.com/x/y.git", owner="x", repo="y", + local_source="/tmp/os", source_dir="/tmp/os", session_id="s1", + timestamp="20260814", + task={"role": "committer", "agent": "aitokenpool"}, + agent={}, git_path="git", gh_path="gh", + ) + region = _forbidden_section(out) + folded = region.lower() + + # 1) the rule names every form a reader could exempt itself with + missing_forms = [form for form in _BACKGROUND_FORMS if form not in region] + assert not missing_forms, ( + f"§Forbidden no longer names {missing_forms} as a form a command must not take — " + "a reader that cannot see its own case named here decides that case is different" + ) + + # 2) each requirement the issue states, as the words that carry it + missing_claims = [] + for label, groups in _BACKGROUND_CLAIMS: + for group in groups: + if not any(member.lower() in folded for member in group): + missing_claims.append((label, group)) + assert not missing_claims, ( + "§Forbidden no longer carries these requirements of the rule " + f"(requirement, words that must appear): {missing_claims} — the ban needs the habit " + "that replaces the forms, the mechanism behind it, and what to do when a measurement " + "does not fit, or the next round trades the rule away" + ) + + # 3) it stays inside the section that owns prohibitions, grouped after the two red lines + assert region.index("`nohup`") > region.index("_guard_upgrade_hermeticity"), ( + "the rule must sit with the two manifest red lines, under §Forbidden — above them it " + "is a different section's rule, and the region stops containing it" + ) + + def test_open_source_template_distinguishes_an_unrunnable_toolchain(): """open_source_prompt.md §3.1.6 resolves the runner before reading a result, and treats a suite that could not start as a skip rather than a failure (issue #2515).