From f676c9fe2aecccbf6d49688f663f17ec7154e618 Mon Sep 17 00:00:00 2001 From: istos Date: Sun, 2 Aug 2026 12:39:07 +0200 Subject: [PATCH 1/3] prompts: say that a headless run is one turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A work agent ended its turn to wait for a four-minute test suite it had backgrounded, and the run ended with it: 314 lines of passing work staged and never committed, an empty branch, a halted phase. Nothing in the prompts said the run was a single non-interactive turn, so waiting looked like a sensible strategy. All four core prompts now open with the same block: this is one turn, the process exits with your reply, and nothing you meant to finish afterwards survives. It names the trap directly — do not background a command and end the turn to wait for it, do not promise to come back — and each template follows it with what its own run loses: work.md commits around long-running commands, act-pr.md commits and pushes, review-pr.md posts its verdict inside the turn, review.md writes its report there. The block sits before the task body and carries no braces, so str.format still renders it and the NOT READY / ADDRESSED / PR REVIEW / RELEVANCE REVIEW marker lines keep their place after it. Co-Authored-By: Claude Opus 5 --- manager/core/prompts/act-pr.md | 21 +++++ manager/core/prompts/review-pr.md | 17 ++++ manager/core/prompts/review.md | 17 ++++ manager/core/prompts/work.md | 19 +++++ tests/test_prompt_one_turn.py | 132 ++++++++++++++++++++++++++++++ 5 files changed, 206 insertions(+) create mode 100644 tests/test_prompt_one_turn.py diff --git a/manager/core/prompts/act-pr.md b/manager/core/prompts/act-pr.md index 4a8e6dd..8791220 100644 --- a/manager/core/prompts/act-pr.md +++ b/manager/core/prompts/act-pr.md @@ -2,6 +2,27 @@ You are addressing review feedback on a pull request for a task from this repository's task board. You are in the task's git worktree on branch `{branch}`. The PR is {pr}. + +**This run is a single non-interactive turn.** Nobody is watching it and +there is no second turn: when your reply ends, the process exits. Work +you meant to finish afterwards is lost with it, and the board judges the +run by what you actually left behind. + +Two habits end runs early, so neither is allowed here: +- Do not start something in the background and end your turn to wait for + it. There is no monitor, no notification and no resume. If a check + takes minutes, run it in the foreground and wait for it inside this + turn. +- Do not promise to come back to something. There is no coming back — do + it now, or say plainly in your report that it is not done. + +So commit early and push often: work that is not committed dies with the +process, and a commit you never pushed never reaches the PR. Commit +before you start anything long-running — the test suite especially — +then commit and push again after it. A commit is cheap and a lost run is +not, and an early commit can always be improved on later in the same +turn. + The task, for what the work was supposed to be: --- TASK --- diff --git a/manager/core/prompts/review-pr.md b/manager/core/prompts/review-pr.md index 43abdb2..5c0fae2 100644 --- a/manager/core/prompts/review-pr.md +++ b/manager/core/prompts/review-pr.md @@ -1,6 +1,23 @@ You are reviewing a pull request for a task on this repository's task board — you are NOT implementing anything. +**This run is a single non-interactive turn.** Nobody is watching it and +there is no second turn: when your reply ends, the process exits. Work +you meant to finish afterwards is lost with it, and the board judges the +run by what you actually left behind. + +Two habits end runs early, so neither is allowed here: +- Do not start something in the background and end your turn to wait for + it. There is no monitor, no notification and no resume. If a check + takes minutes, run it in the foreground and wait for it inside this + turn. +- Do not promise to come back to something. There is no coming back — do + it now, or say plainly in your report that it is not done. + +So post your verdict to GitHub during the turn, before the reply that +ends it: a verdict you only described in your report never reached the +PR. + The task is `{filename}`, its branch is `{branch}`, and its PR is {pr}. The task content, for what the work was supposed to be: diff --git a/manager/core/prompts/review.md b/manager/core/prompts/review.md index 7a2c1f1..ad995bb 100644 --- a/manager/core/prompts/review.md +++ b/manager/core/prompts/review.md @@ -1,6 +1,23 @@ You are reviewing a task on this repository's task board for continued relevance — you are NOT implementing it. +**This run is a single non-interactive turn.** Nobody is watching it and +there is no second turn: when your reply ends, the process exits. Work +you meant to finish afterwards is lost with it, and the board judges the +run by what you actually left behind. + +Two habits end runs early, so neither is allowed here: +- Do not start something in the background and end your turn to wait for + it. There is no monitor, no notification and no resume. If a check + takes minutes, run it in the foreground and wait for it inside this + turn. +- Do not promise to come back to something. There is no coming back — do + it now, or say plainly in your report that it is not done. + +Your report is the only thing this run leaves behind, so finish your +investigation and write it in the reply that ends the turn. There is no +later message to put it in. + The task is `{stage}/{filename}`. Its content: --- TASK --- diff --git a/manager/core/prompts/work.md b/manager/core/prompts/work.md index 80421ac..22d5a0b 100644 --- a/manager/core/prompts/work.md +++ b/manager/core/prompts/work.md @@ -4,6 +4,25 @@ You are in an isolated git worktree on branch `{branch}` created for this task. All your work happens here: commit to this branch, do not push, do not merge, and do not switch branches. +**This run is a single non-interactive turn.** Nobody is watching it and +there is no second turn: when your reply ends, the process exits. Work +you meant to finish afterwards is lost with it, and the board judges the +run by what you actually left behind. + +Two habits end runs early, so neither is allowed here: +- Do not start something in the background and end your turn to wait for + it. There is no monitor, no notification and no resume. If a check + takes minutes, run it in the foreground and wait for it inside this + turn. +- Do not promise to come back to something. There is no coming back — do + it now, or say plainly in your report that it is not done. + +So commit early and commit often: work that is not committed dies with +the process. Commit before you start anything long-running — the test +suite especially — and commit again after it. A commit is cheap and a +lost run is not, and an early commit can always be improved on later in +the same turn. + Read AGENTS.md at the repo root first and follow it, including its definition of done — run whatever checks it names until they pass. diff --git a/tests/test_prompt_one_turn.py b/tests/test_prompt_one_turn.py new file mode 100644 index 0000000..439749d --- /dev/null +++ b/tests/test_prompt_one_turn.py @@ -0,0 +1,132 @@ +"""A headless run is one non-interactive turn, and the prompts say so. + +Every core prompt template carries the identical "this run is a single +non-interactive turn" block, before the task body, naming the trap that +killed card 47: backgrounding a long command and ending the turn to wait +for it. Each template then says what its own run loses when the turn ends +early — a commit, a push, a posted verdict, the report itself — and none +of it disturbs the marker lines the board parses out of the same output. +""" + +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +PROMPTS = REPO / "manager" / "core" / "prompts" +TEMPLATES = ("work.md", "review.md", "review-pr.md", "act-pr.md") + +# The shared block runs from the first line to the last; both ends are +# fixed so a template that paraphrases either one fails here. +START = "**This run is a single non-interactive turn.**" +END = "or say plainly in your report that it is not done." + +BODY = "--- TASK ---" + + +def _text(name: str) -> str: + return (PROMPTS / name).read_text(encoding="utf-8") + + +def _block(name: str) -> str: + text = _text(name) + start = text.find(START) + if start < 0: + raise AssertionError(f"{name} never says the run is a single turn") + end = text.find(END, start) + if end < 0: + raise AssertionError(f"{name} lost the end of the one-turn block") + return text[start:end + len(END)] + + +class SharedBlock(unittest.TestCase): + def test_identical_across_all_templates(self): + reference = _block(TEMPLATES[0]) + for name in TEMPLATES[1:]: + self.assertEqual(_block(name), reference, + f"{name} drifted from work.md's one-turn block") + + def test_it_states_the_shape_of_the_run(self): + block = " ".join(_block("work.md").split()) + self.assertIn("no second turn", block) + self.assertIn("the process exits", block) + + def test_it_names_the_trap(self): + block = " ".join(_block("work.md").split()) + self.assertIn("Do not start something in the background and end " + "your turn to wait for it", block) + self.assertIn("no monitor", block) + self.assertIn("run it in the foreground", block) + self.assertIn("Do not promise to come back to something", block) + + def test_it_comes_before_the_task_body(self): + """A brief read after the task is the one the agent skims.""" + for name in TEMPLATES: + text = _text(name) + self.assertLess(text.find(START), text.find(BODY), + f"{name} says it after the task body") + + def test_block_survives_str_format(self): + """Prompts are filled via str.format, so a literal brace here + would break every launch.""" + block = _block("work.md") + self.assertNotIn("{", block) + self.assertNotIn("}", block) + + +class WhatEachRunLoses(unittest.TestCase): + """The block is the reason; each template gives the instruction that + follows from it for the run it drives.""" + + def test_work_commits_around_long_commands(self): + text = " ".join(_text("work.md").split()) + self.assertIn("commit early and commit often", text) + self.assertIn("Commit before you start anything long-running", text) + self.assertIn("commit again after it", text) + + def test_act_pr_commits_and_pushes(self): + text = " ".join(_text("act-pr.md").split()) + self.assertIn("commit early and push often", text) + self.assertIn("Commit before you start anything long-running", text) + self.assertIn("never pushed never reaches the PR", text) + + def test_review_pr_posts_its_verdict_inside_the_turn(self): + text = " ".join(_text("review-pr.md").split()) + self.assertIn("post your verdict to GitHub during the turn", text) + + def test_review_writes_its_report_inside_the_turn(self): + text = " ".join(_text("review.md").split()) + self.assertIn("Your report is the only thing this run leaves behind", + text) + + +class MarkersUndisturbed(unittest.TestCase): + """agents.py parses the marker lines out of the same output; the new + prose must not compete for 'the first line'.""" + + MARKERS = { + "work.md": "NOT READY: ", + "act-pr.md": "ADDRESSED: ", + "review-pr.md": "PR REVIEW: ", + "review.md": "RELEVANCE REVIEW: ", + } + + def test_markers_still_follow_the_new_block(self): + for name, marker in self.MARKERS.items(): + text = _text(name) + self.assertIn(marker, text, f"{name} lost its marker line") + self.assertLess(text.find(START), text.find(marker), + f"{name} now states its marker before the block") + + def test_not_ready_keeps_its_place_in_work(self): + """The NOT READY instruction is still the only thing in work.md + claiming a reply's first line, and it still sits after the task.""" + text = _text("work.md") + self.assertEqual(text.count("FIRST line"), 1) + self.assertLess(text.find("--- END TASK ---"), text.find("FIRST line")) + self.assertNotIn("FIRST line", _block("work.md")) + self.assertNotIn("NOT READY", _block("work.md")) + + +if __name__ == "__main__": + unittest.main() From ccffbec8317ec38ed190b97e27cc3819f98e6706 Mon Sep 17 00:00:00 2001 From: istos Date: Sun, 2 Aug 2026 13:32:08 +0200 Subject: [PATCH 2/3] A work agent refuses a phase card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `▸ run phase` guarded its own door and left the neighbour's open: `/api/agent/start` accepted a phase card, cut `task/` and handed a list of other cards to a work agent as a brief. It did that once, and the agent implemented two cards at once in a worktree nobody was watching. The refusal is a server rule, in `_validate` with the stage check — ahead of the claim and well ahead of the worktree, so it costs nothing and leaves nothing to clean up — and it names ▸ run phase rather than just saying no. Which headless kinds a phase card may host is now decided kind by kind where the guard lives: ▸ start work and ↻ act on PR refuse it (both are work agents), while ◔ still true? and ◔ review PR are allowed — the latter now told the phase's own branch, since its PR is from `phase/` and `task/` was never cut. The guard is about starting, so a card retyped under a running agent is left alone. And the card says which state it is in: an `in-progress/` phase nobody has started read exactly like one mid-run, the header chip being absent in both cases. It now wears `not started` (or `held`) in the settled register, with the line under it saying what ▸ run phase would do — never the accent, the breathing mark or the caret, which mean work is happening. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 26 ++ manager/core/agents.py | 83 +++++- manager/core/board.html | 35 ++- tests/test_phase_card_refuses_work.py | 385 ++++++++++++++++++++++++++ tests/test_phase_members_hidden.py | 18 +- 5 files changed, 532 insertions(+), 15 deletions(-) create mode 100644 tests/test_phase_card_refuses_work.py diff --git a/AGENTS.md b/AGENTS.md index 2a02516..d52a02c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -378,6 +378,21 @@ there, and firing it is the deliberate reassignment. An unclaimed card claims itself on launch. One agent per task at a time, and a work agent's worktree must not already exist when it starts. +It also refuses a **phase card**, and the refusal names **▸ run phase** — +a phase card's body is a list of other cards, so a work agent handed one +implements the table of contents. The board's own page offers one action +or the other and never both, but a UI layer can be stale or bypassed, so +the rule lives where every other launch refusal lives: with the stage +check, ahead of the claim and well ahead of the worktree, costing nothing +and leaving nothing to clean up. Which headless kinds a phase card may +host is decided rather than left to omission: **▸ start work** and **↻ act +on PR** refuse it (both are work agents, and a phase's work belongs to its +members' cards), while the read-only pair — **◔ still true?** and **◔ +review PR**, the latter told the phase's own branch — are allowed. The +guard is about *starting*: a card that gains `**Type:** Phase` while an +ordinary run is in flight is left alone, and that run ends as it would +have. + 1. The board creates a git worktree at `.worktrees//` on a new branch `task/` from the newest main it can see: with an `origin` remote it fetches `origin/main` first (bounded by @@ -951,6 +966,17 @@ and they are the phase's own interface: watching is only trustworthy if its halt is impossible to miss, so it is told three times at three altitudes, exactly as a dead run is. +**And the card itself says which state it is in.** The header chip only +appears while there is something to say, so it cannot tell "the phase has +not been started" from "there is no phase here at all" — which left an +`in-progress/` phase card nobody had run looking exactly like one mid-run. +So the card carries the distinction quietly, in the pill and the line +under it: `not started` (`▸ run phase` cuts its branch and starts the +first card), `held`, the accent and the member in flight while it runs, +and `halted` in `--alarm`. Only a run wears the working vocabulary — the +breathing mark, the accent border, the caret — because only a run is work +happening. + **And the card does not move while its work runs.** A phase card stands for cards the Board no longer draws, so dragging it to another stage — or onto the archive tray, which is a move like any other — while a member has diff --git a/manager/core/agents.py b/manager/core/agents.py index 0c41729..7b1ffb4 100644 --- a/manager/core/agents.py +++ b/manager/core/agents.py @@ -148,13 +148,65 @@ def working_on(files: set[str]) -> list[dict]: return [record for record in records if _alive(record)] -def _validate(filename: str, stage: str, allowed: set[str], why: str | None = None) -> None: +# ── what a phase card may host ───────────────────────────────────────── +# +# A phase card is a list of other cards. Handed to a work agent as a brief +# it reads as a table of contents, and the agent does what it is told — +# which is how one run once implemented two cards at once in a worktree +# nobody was watching. `▸ run phase` already guards its own door (a card +# that is not a phase is refused there); this is the other half of that +# gate, and it is decided kind by kind rather than left to omission: +# +# - **▸ start work** (`start_agent`) refuses. A phase is run with ▸ run +# phase, which works its list into a branch of its own; its members are +# worked on their own cards. +# - **↻ act on PR** (`start_pr_fix`) refuses. It is the same work agent +# with a push, and the phase's PR carries its members' commits — review +# feedback on it belongs on the member's own card, or on the phase +# branch by hand. +# - **◔ still true?** (`start_review`) is allowed. Read-only, no worktree: +# asking whether a phase is still worth running is a fair question, and +# the report is appended to the card like any other. +# - **◔ review PR** (`start_pr_review`) is allowed. Read-only, and the PR +# into `main` is the one thing the whole run exists to produce — the +# launch just has to name the phase's own branch rather than a +# `task/` that was never cut. +# +# It is about *starting*: a card that gains `**Type:** Phase` while an +# ordinary run is in flight is left alone, and the run ends as it would +# have. +PHASE_RUNS_WITH = ("a list of other cards, not a brief. Run it with ▸ run " + "phase, which cuts a branch of its own and works the list " + "into it; its members are worked on their own cards") + + +def is_phase_card(filename: str, stage: str) -> bool: + """Is this card a phase card? `**Type:** Phase` is the whole of it — + the same reading `phases._phase_card` refuses a non-phase by, so the + two gates cannot disagree. A phase card whose list is empty or unwritten + is still a coordinator, and still no brief for a work agent.""" + try: + return bool(read_task(config.TASKS / stage / filename, stage)["isPhase"]) + except OSError: + return False + + +def _validate(filename: str, stage: str, allowed: set[str], why: str | None = None, + *, phase: str | None = None) -> None: + """The refusals every launch shares, before anything exists to clean up. + + `phase` is what a phase card should do instead — pass it and this kind + refuses one, naming that; omit it and the kind is one a phase card may + host. See the note above for which is which and why. + """ if Path(filename).name != filename or not filename.endswith(".md"): raise ValueError("bad filename") if stage not in allowed: raise ValueError(why or f"agents cannot start from {stage}/") if not (config.TASKS / stage / filename).is_file(): raise ValueError(f"{filename} is not in {stage}/ — refresh the board") + if phase and is_phase_card(filename, stage): + raise ValueError(f"{filename} is a phase card — {phase}") _assert_no_running_agent(filename) @@ -304,9 +356,12 @@ def claim_for_launch(filename: str, stage: str, takeover: bool = False) -> None: def start_agent(filename: str, stage: str, takeover: bool = False) -> dict: - # Moving a card to in-progress is the commitment; only then does work start. + # Moving a card to in-progress is the commitment; only then does work + # start — and a phase card is refused here, in the same breath and ahead + # of the claim, so the refusal costs nothing and leaves nothing behind. _validate(filename, stage, {"in-progress"}, - "work starts from in-progress/ — move the card there first") + "work starts from in-progress/ — move the card there first", + phase=PHASE_RUNS_WITH) claim_for_launch(filename, stage, takeover) stem = filename[:-3] @@ -386,7 +441,11 @@ def start_agent(filename: str, stage: str, takeover: bool = False) -> dict: def start_review(filename: str, stage: str) -> dict: - """Fire a read-only agent that checks the task against the codebase.""" + """Fire a read-only agent that checks the task against the codebase. + + Every stage, and a phase card too: no `phase=` here is the deliberate + answer, not an omission. Nothing is written but the report. + """ _validate(filename, stage, config.STAGE_DIRS) task = read_task(config.TASKS / stage / filename, stage) @@ -593,14 +652,21 @@ def _reap_agent(agent_id: str, proc: subprocess.Popen, log_file) -> None: def start_pr_review(filename: str, stage: str) -> dict: """Fire a read-only agent that reviews the task's PR and posts the - verdict to GitHub as well as back to the board.""" + verdict to GitHub as well as back to the board. + + A phase card is allowed here, deliberately: the PR into `main` is what + the whole run exists to produce, and reading it writes nothing. + """ _validate(filename, stage, {"review"}, "PR reviews run on cards in review/") task = read_task(config.TASKS / stage / filename, stage) if not task.get("pr"): raise ValueError(f"{filename} has no PR yet — nothing to review") - branch = f"task/{filename[:-3]}" + # …but it must be told the branch its PR is actually from: the prompt + # asks GitHub for the diff by branch, and a phase's is its own. + branch = (phase_branch(filename) if task["isPhase"] + else f"task/{filename[:-3]}") name = _pick_name(filename) agent_id = f"review-pr-{filename[:-3]}-{time.strftime('%H%M%S')}" log_path = config.AGENT_DIR / "logs" / f"{agent_id}.log" @@ -632,7 +698,10 @@ def start_pr_fix(filename: str, stage: str) -> dict: working in the task's existing worktree (recreated from the branch if it was cleaned up), committing and pushing to update the PR.""" _validate(filename, stage, {"review"}, - "acting on a PR happens from review/") + "acting on a PR happens from review/", + phase="↻ act on PR is a work agent, and a phase's PR carries " + "its members' commits — address the review on the member's " + "own card, or on the phase branch by hand") task = read_task(config.TASKS / stage / filename, stage) if not task.get("pr"): raise ValueError(f"{filename} has no PR to act on") diff --git a/manager/core/board.html b/manager/core/board.html index 2bc83bd..b1ef4ea 100644 --- a/manager/core/board.html +++ b/manager/core/board.html @@ -1347,8 +1347,14 @@ function phaseSummary(task) { } /* …and what it is doing right now: the member in flight while a run is on, - the halt while one is held. Null when the phase has no story yet — a - phase waiting in to-do/ is just a card. */ + the halt while one is held — and the quiet case this exists for, that + nobody has started it. A phase card sitting in in-progress/ that has not + been run used to look exactly like one that is running: the header chip + is the only other place that difference is written, and it is absent both + when a phase has not started and when there is no phase at all. So the + card says it itself. `idle` is not work — it takes no accent, no + breathing mark and no caret, because nothing is happening. Null when the + phase has no story to tell here: one waiting in to-do/ is just a card. */ function phaseFlight(task) { const snap = (S.state.phases || {})[task.file]; if (!snap) return null; @@ -1356,7 +1362,14 @@ function phaseFlight(task) { return { bad: true, line: 'halted' + (snap.haltedAt ? ` at #${snap.haltedAt}` : '') + ' — ' + (snap.haltedWhy || snap.halted) }; } - if (!snap.running) return null; + if (!snap.running) { + if (task.stage !== 'in-progress') return null; + return snap.stopped + ? { idle: true, pill: 'held', + line: 'held — ▸ run phase carries on from where it stopped' } + : { idle: true, pill: 'not started', + line: 'not started — ▸ run phase cuts its branch and starts the first card' }; + } const at = phaseProgress(snap); if (at.on) return { bad: false, line: `on #${at.on.number || at.on.file} — ${at.on.title}` }; if ((snap.waitingOn || []).length) { @@ -1392,7 +1405,8 @@ function cardFor(task) { // a running phase has an agent alive on a card this view no longer draws, // so the phase card wears that state on its members' behalf const flight = task.isPhase ? phaseFlight(task) : null; - const working = !!(agent && agent.mode !== 'review') || !!(flight && !flight.bad); + const working = !!(agent && agent.mode !== 'review') + || !!(flight && !flight.bad && !flight.idle); const verdict = task.stage === 'review' ? prVerdict(task) : null; const failure = failedRun(task); // the server holds this, not the tab that clicked: while merge & clean up @@ -1419,6 +1433,15 @@ function cardFor(task) { pill = { text: 'changes asked', tint: 'var(--alarm)', bg: mix('var(--alarm)', 16) }; tint = 'var(--alarm)'; } + if (flight && flight.idle) { + // the state the card could not tell from a run in flight: a phase in + // in-progress/ that nobody has started, or one held. It is the absence + // of work, so it wears the settled register — never the accent, which + // means an agent is alive. + pill = { text: flight.pill, tint: 'var(--idle)', bg: 'var(--sunken)', + title: flight.line }; + tint = 'var(--idle)'; + } if (failure) { // the newest thing that happened here, and the only actionable one: // it outranks a PR verdict from before the run died @@ -1610,7 +1633,7 @@ function cardFor(task) { // the header chip carries, at the altitude a person is already reading liveLine = `
·` + `${esc(flight.line)}` + - `${flight.bad ? '' : '▌'}
`; + `${flight.bad || flight.idle ? '' : '▌'}`; } // tool chips: destinations, not statuses — they live in the card's footer @@ -2503,7 +2526,7 @@ function laneFor(task) { // a run in flight, in the same well the cards use for the same fact const well = !halted && flight ? `
·${esc(flight.line)}` + - `▌
` + `${flight.idle ? '' : '▌'}` : ''; el.innerHTML = head + halt + well + diff --git a/tests/test_phase_card_refuses_work.py b/tests/test_phase_card_refuses_work.py new file mode 100644 index 0000000..c7edbf2 --- /dev/null +++ b/tests/test_phase_card_refuses_work.py @@ -0,0 +1,385 @@ +"""A work agent must refuse a phase card (task 54). + +`▸ run phase` guards its own door — a card that is not a phase is refused +there. This is the other half of that gate, on the neighbour's door: a +phase card's body is a list of other cards, so a work agent handed one +implements the table of contents, which is exactly what happened the first +time a phase reached the board. The refusal is a server rule because the +page that offers one action or the other can be stale or bypassed. + +Which headless kinds a phase card may host is *decided* here rather than +left to omission, so each of the four has a case saying which it is. + + python3 -m unittest discover -s tests -v +""" + +from __future__ import annotations + +import json +import re +import shutil +import subprocess +import sys +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO / "manager" / "core")) + +import agents # noqa: E402 +import config # noqa: E402 +import phases # noqa: E402 +import state # noqa: E402 + +from tests.test_phase_runs import (ONE, PHASE, PHASE_BRANCH, PhaseCase, # noqa: E402 + card, git, wait_for) + +BOARD = REPO / "manager" / "core" / "board.html" +NODE = shutil.which("node") + +ALONE = "77-alone.md" +PR_URL = "https://github.com/acme/widget/pull/7" + +# Reads, says something, writes nothing — what a read-only kind does. +REPORTS = """#!/usr/bin/env python3 +print("RELEVANCE REVIEW: Still relevant") +""" + +# Commits, then waits for a file beside the worktrees directory before +# exiting — a run held open for as long as a test needs it in flight. +BLOCKS = """#!/usr/bin/env python3 +import os, subprocess, time +cwd = os.environ["AGENT_CWD"] +open(os.path.join(cwd, "work.txt"), "w").write("work\\n") +subprocess.run(["git", "-C", cwd, "add", "-A"], check=True) +subprocess.run(["git", "-C", cwd, "-c", "user.email=a@b", "-c", "user.name=stub", + "commit", "-q", "-m", "work"], check=True) +go = os.path.join(os.path.dirname(os.path.dirname(cwd)), "worktrees", "go") +for _ in range(1200): + if os.path.exists(go): + break + time.sleep(0.05) +print("WORK REPORT: the work is committed") +""" + + +class LaunchCase(PhaseCase): + """PhaseCase's world — a real repo, a real adapter, a real phase card in + in-progress/ — with the launches aimed at the card rather than the run.""" + + def only_in(self, filename: str, text: str, stage: str) -> None: + """One card, one stage — a rewrite that moves it rather than + leaving the board reading the same number twice.""" + for slug in config.STAGE_DIRS: + path = self.tasks / slug / filename + if path.is_file(): + path.unlink() + self.write(filename, text, stage) + + def ordinary(self, stage: str = "in-progress", **kw) -> None: + """A card that is not a phase, in the stage a launch wants it in.""" + self.only_in(ALONE, card("77 — On its own", + status=config.STAGE_LABELS[stage], **kw), stage) + + def as_phase(self, filename: str, stage: str, listed: str | None = None) -> None: + """The same card, now typed Phase — what a person writes when they + decide a card coordinates rather than builds.""" + self.only_in(filename, card("77 — On its own", kind="Phase", + status=config.STAGE_LABELS[stage], + cards=listed), stage) + + def with_pr(self, filename: str, title: str, stage: str, **kw) -> None: + text = card(title, status=config.STAGE_LABELS[stage], **kw) + self.only_in(filename, text.replace( + "**Priority:** High\n", + f"**Priority:** High\n**PR:** {PR_URL}\n"), stage) + + def running(self) -> list[dict]: + return [r for r in state.AGENTS.values() if r["status"] == "running"] + + def worktrees(self) -> list[str]: + return [line.split(" ", 1)[1] for line in + git(self.repo, "worktree", "list", "--porcelain").stdout.splitlines() + if line.startswith("worktree ")] + + +class TheWorkAgentRefusesAPhaseCard(LaunchCase): + def test_a_phase_card_is_refused_and_told_what_to_run_instead(self): + with self.assertRaises(ValueError) as caught: + agents.start_agent(PHASE, "in-progress") + + self.assertIn("▸ run phase", str(caught.exception)) + self.assertIn("phase card", str(caught.exception)) + + def test_the_refusal_leaves_nothing_behind(self): + """Refused with `_validate`, so it costs nothing: no branch, no + worktree, no process, nothing for anyone to clean up.""" + with self.assertRaises(ValueError): + agents.start_agent(PHASE, "in-progress") + + self.assertFalse(self.branch_exists(f"task/{PHASE[:-3]}")) + self.assertFalse((config.WORKTREES / PHASE[:-3]).exists()) + self.assertEqual(self.worktrees(), [str(self.repo)]) + self.assertEqual(state.AGENTS, {}, "no run was ever recorded") + + def test_it_refuses_ahead_of_the_claim(self): + """The order the shape depends on: a refusal must not write the card + it refused. In team mode a launch claims an unheld card, so an + assignee appearing here would mean the guard ran too late.""" + self.patch(COMMIT_MOVES=True) + + with self.assertRaises(ValueError): + agents.start_agent(PHASE, "in-progress") + + self.assertNotIn("**Assignee:**", self.text(PHASE)) + + def test_a_takeover_is_refused_just_the_same(self): + """The deliberate second click reassigns a card; it does not make a + table of contents into a brief.""" + self.patch(COMMIT_MOVES=True) + + with self.assertRaises(ValueError) as caught: + agents.start_agent(PHASE, "in-progress", takeover=True) + + self.assertIn("▸ run phase", str(caught.exception)) + self.assertNotIn("**Assignee:**", self.text(PHASE)) + + def test_a_phase_card_with_no_list_yet_is_still_a_phase_card(self): + """`**Type:** Phase` is the whole of the reading, the same one + `phases._phase_card` refuses a non-phase by. A list not written yet + is an authoring mistake to fix, not an invitation to build it.""" + self.as_phase(ALONE, "in-progress") + + with self.assertRaises(ValueError) as caught: + agents.start_agent(ALONE, "in-progress") + + self.assertIn("▸ run phase", str(caught.exception)) + + def test_an_ordinary_card_starts_work_exactly_as_it_did(self): + self.ordinary() + + agent = agents.start_agent(ALONE, "in-progress") + self.settle() + + self.assertEqual(agent["branch"], f"task/{ALONE[:-3]}") + self.assertTrue(self.branch_exists(f"task/{ALONE[:-3]}")) + self.assertEqual(self.stage_of(ALONE), "review", + "the ordinary path is untouched: commits, then review/") + + def test_the_two_gates_are_mirror_images(self): + """The bug was an asymmetry, so the symmetry is the test: each door + refuses the card the other one is for, and says so.""" + self.ordinary() + + with self.assertRaises(ValueError) as work: + agents.start_agent(PHASE, "in-progress") + with self.assertRaises(ValueError) as run: + phases.start_phase(ALONE, "in-progress") + + self.assertIn("phase card", str(work.exception)) + self.assertIn("not a phase", str(run.exception)) + self.assertEqual(state.AGENTS, {}) + self.assertFalse(self.branch_exists(f"phase/{ALONE[:-3]}")) + + +class ARunInFlightIsLeftAlone(LaunchCase): + """The guard is about starting. A card retyped under a running agent is + a person's edit, not a reason to break the run underneath it.""" + + def test_a_card_that_becomes_a_phase_mid_run_still_lands(self): + self.ordinary() + self.adapter_is(BLOCKS) + go = config.WORKTREES / "go" + + agents.start_agent(ALONE, "in-progress") + self.assertTrue( + wait_for(lambda: (config.WORKTREES / ALONE[:-3] / "work.txt").is_file()), + "the agent never got as far as its commit") + # the edit, while the process is alive and the reaper has not run + self.as_phase(ALONE, "in-progress") + self.assertTrue(self.running(), "the run ended before the edit landed") + go.write_text("done\n", encoding="utf-8") + self.settle() + + record = next(iter(state.AGENTS.values())) + self.assertEqual(record["rc"], 0) + self.assertEqual(record["status"], "done", "the run ended as it would have") + self.assertEqual(self.stage_of(ALONE), "review", + "and the card moved on, phase line or not") + + def tearDown(self): + # never leave a blocked adapter behind if an assertion jumped the wire + (config.WORKTREES / "go").parent.mkdir(parents=True, exist_ok=True) + (config.WORKTREES / "go").write_text("done\n", encoding="utf-8") + wait_for(lambda: not self.running(), timeout=10) + + +class WhichKindsAPhaseCardMayHost(LaunchCase): + """Four kinds, four deliberate answers — the two that would work on the + card refuse it, the two that only read it are allowed.""" + + def test_still_true_is_allowed_on_a_phase_card(self): + self.adapter_is(REPORTS) + + agent = agents.start_review(PHASE, "in-progress") + self.assertTrue(wait_for(lambda: not self.running())) + + self.assertEqual(agent["mode"], "review") + self.assertIsNone(agent["worktree"], "read-only: nothing is cut for it") + self.assertIn("Relevance review", self.text(PHASE)) + + def test_reviewing_the_phase_pr_is_allowed_and_names_the_phase_branch(self): + """The prompt asks GitHub for the diff by branch, and a phase's PR + is from its own branch — `task/` was never cut.""" + self.adapter_is(REPORTS) + self.with_pr(PHASE, "40 — Ship the site", "review", kind="Phase", + cards="- 31 — Stand up site/\n") + + agent = agents.start_pr_review(PHASE, "review") + self.assertTrue(wait_for(lambda: not self.running())) + + self.assertEqual(agent["branch"], PHASE_BRANCH) + + def test_an_ordinary_cards_pr_review_still_names_its_task_branch(self): + self.adapter_is(REPORTS) + self.with_pr(ONE, "31 — Stand up site/", "review") + + agent = agents.start_pr_review(ONE, "review") + self.assertTrue(wait_for(lambda: not self.running())) + + self.assertEqual(agent["branch"], f"task/{ONE[:-3]}") + + def test_acting_on_a_phase_pr_is_refused_and_says_where_it_belongs(self): + """↻ act on PR is the same work agent with a push, and a phase's PR + carries its members' commits.""" + self.with_pr(PHASE, "40 — Ship the site", "review", kind="Phase", + cards="- 31 — Stand up site/\n") + + with self.assertRaises(ValueError) as caught: + agents.start_pr_fix(PHASE, "review") + + self.assertIn("phase card", str(caught.exception)) + self.assertIn("member", str(caught.exception)) + self.assertFalse((config.WORKTREES / PHASE[:-3]).exists()) + self.assertEqual(state.AGENTS, {}) + + +@unittest.skipUnless(NODE, "node is needed to run the page's own rules") +class TheCardSaysWhichStateItIsIn(unittest.TestCase): + """A phase card in in-progress/ that nobody has started used to look + exactly like one mid-run. These run the page's own function over the + snapshots the server sends.""" + + PARTS = (r"function phaseProgress\(p\) \{.*?\n\}", + r"function phaseFlight\(task\) \{.*?\n\}") + + @classmethod + def setUpClass(cls): + cls.html = BOARD.read_text(encoding="utf-8") + parts = [] + for pattern in cls.PARTS: + match = re.search(pattern, cls.html, re.S) + if match is None: + raise AssertionError(f"board.html no longer defines {pattern!r}") + parts.append(match.group(0)) + cls.src = "\n".join(parts) + cls.card_fn = re.search(r"function cardFor\(task\) \{.*?\n\}", + cls.html, re.S).group(0) + + def flight(self, snapshot: dict | None, stage: str = "in-progress") -> object: + task = {"file": PHASE, "stage": stage, "isPhase": True} + script = (self.src + "\nvar S = { state: { phases: " + + json.dumps({PHASE: snapshot} if snapshot else {}) + + " } };\nconsole.log(JSON.stringify(phaseFlight(" + + json.dumps(task) + ")));\n") + out = subprocess.run([NODE, "-e", script], capture_output=True, text=True) + self.assertEqual(out.returncode, 0, out.stderr) + return json.loads(out.stdout) + + def snapshot(self, **extra) -> dict: + return {"file": PHASE, "members": [], "halted": None, "haltedAt": None, + "haltedWhy": None, "running": False, "stopped": False, + "started": False, **extra} + + def test_a_phase_nobody_has_started_says_so(self): + flight = self.flight(self.snapshot()) + + self.assertTrue(flight["idle"], "nothing is happening, so nothing breathes") + self.assertEqual(flight["pill"], "not started") + self.assertIn("▸ run phase", flight["line"]) + + def test_a_held_phase_is_its_own_state(self): + flight = self.flight(self.snapshot(started=True, stopped=True)) + + self.assertTrue(flight["idle"]) + self.assertEqual(flight["pill"], "held") + + def test_a_running_phase_is_unchanged(self): + flight = self.flight(self.snapshot( + started=True, running=True, + members=[{"number": "31", "file": ONE, "title": "Stand up site/", + "state": "running"}])) + + self.assertFalse(flight.get("idle"), "a run is work: it keeps the accent") + self.assertIn("on #31", flight["line"]) + + def test_a_halted_phase_is_unchanged(self): + flight = self.flight(self.snapshot(started=True, halted="31: its CI is red", + haltedAt="31", haltedWhy="its CI is red")) + + self.assertTrue(flight["bad"]) + self.assertFalse(flight.get("idle")) + + def test_a_phase_the_runner_has_not_read_says_nothing(self): + """Before the first beat there is no snapshot, and a card that + guessed would be worse than one that waits.""" + self.assertIsNone(self.flight(None)) + + def test_a_settled_phase_card_is_not_told_it_never_started(self): + """review/ and done/ are past the question — the card there is + waiting on a person, which its stage already says.""" + self.assertIsNone(self.flight(self.snapshot(started=True), stage="review")) + self.assertIsNone(self.flight(self.snapshot(started=True), stage="done")) + + def test_idle_wears_the_settled_register_and_never_the_accent(self): + block = re.search(r"if \(flight && flight\.idle\) \{.*?\n \}", + self.card_fn, re.S).group(0) + self.assertIn("var(--idle)", block) + for colour in ("--accent", "--alarm", "--calm"): + self.assertNotIn(colour, block, + "not started is neither work, an alarm nor a verdict") + self.assertIn("!flight.bad && !flight.idle", self.card_fn, + "an unstarted phase must not read as an agent working") + self.assertIn("flight.bad || flight.idle ? '' :", self.card_fn, + "…and nothing is still arriving, so there is no caret") + + def test_the_halt_and_the_failure_still_outrank_it(self): + idle = self.card_fn.index("if (flight && flight.idle)") + self.assertLess(idle, self.card_fn.index("if (failure)")) + self.assertLess(idle, self.card_fn.index("if (flight && flight.bad)")) + + +class TheDocumentedGuard(unittest.TestCase): + """The doctrine this card restores: the file-carried gates exist because + a UI layer can be stale, so AGENTS.md has to say this one is there.""" + + @classmethod + def setUpClass(cls): + cls.doc = (REPO / "AGENTS.md").read_text(encoding="utf-8") + # the doc is hard-wrapped, so read it as the sentence it is + cls.flat = re.sub(r"\s+", " ", cls.doc.replace("**", "")) + + def test_the_refusal_is_written_down_with_the_action_it_names(self): + self.assertIn("It also refuses a phase card", self.flat) + self.assertIn("the refusal names ▸ run phase", self.flat) + + def test_the_kinds_a_phase_card_may_host_are_named(self): + self.assertIn("▸ start work and ↻ act on PR refuse it", self.flat) + self.assertIn("◔ still true? and ◔ review PR", self.flat) + + def test_the_unstarted_card_is_described(self): + self.assertIn("`not started`", self.doc) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_phase_members_hidden.py b/tests/test_phase_members_hidden.py index 960c0ad..2b66848 100644 --- a/tests/test_phase_members_hidden.py +++ b/tests/test_phase_members_hidden.py @@ -416,10 +416,24 @@ class ThePhaseCardsSummary(BoardViewCase): self.assertIsNone(self.flight()) - def test_a_phase_whose_run_ended_has_nothing_to_say(self): + def test_a_phase_in_progress_with_no_run_says_it_has_not_started(self): + """It said nothing here once, which made a phase nobody had started + look exactly like one mid-run — the header chip is absent in both + cases. Task 54 gave the card the distinction; it is quiet, and it is + not work, so nothing about it breathes.""" self.a_phase_of_two() - self.assertIsNone(self.flight(phases=self.snapshot(running=False))) + flight = self.flight(phases=self.snapshot(running=False)) + + self.assertTrue(flight["idle"]) + self.assertEqual(flight["pill"], "not started") + + def test_a_phase_that_was_held_says_that_instead(self): + self.a_phase_of_two() + + flight = self.flight(phases=self.snapshot(running=False, stopped=True)) + + self.assertEqual(flight["pill"], "held") class Wiring(unittest.TestCase): From 64f3e2b3bd9c743a9c39bf33e02f81343008bf0d Mon Sep 17 00:00:00 2001 From: istos Date: Sun, 2 Aug 2026 13:42:38 +0200 Subject: [PATCH 3/3] A board whose agents cannot run anything says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BOARD_AGENT_COMMANDS is the one setting a headless agent cannot work around, and since the install stopped asking for it a project the detector does not recognise starts with it empty — correctly, but silently, until a run ended with an agent explaining it could not verify its work. Say it twice, in the two places it is learnable: a quiet `no agent commands` chip in the header (`--idle`, like the drive's "no driver", never `--alarm` — nothing is failing, something is unconfigured), and a note appended to the ticker line of the launches that would have run those commands, work and act-pr. Neither blocks anything: an agent that only edits files is still useful. What counts as empty is answered once, by `config.agent_commands()`, which splits exactly as the adapters' own `split_commands()` does — so whitespace and a lone comma are nothing configured on the board as well as at the launch, and the page reads the server's boolean rather than the raw setting. --- AGENTS.md | 13 ++ manager/core/.env.example | 4 +- manager/core/agents.py | 26 ++- manager/core/board.html | 23 +++ manager/core/config.py | 10 ++ manager/core/httpd.py | 4 + tests/test_no_agent_commands.py | 282 ++++++++++++++++++++++++++++++++ 7 files changed, 358 insertions(+), 4 deletions(-) create mode 100644 tests/test_no_agent_commands.py diff --git a/AGENTS.md b/AGENTS.md index d52a02c..25d06cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -193,6 +193,19 @@ in. A wrong guess costs nothing an empty value would not, since a prefix that matches nothing denies exactly the same way. An existing value is never overwritten, so `--setup` cannot undo a hand-edit. +Empty is honest, but honest and invisible is a board that lets an agent +discover the problem on your behalf, slowly — so the board says it out +loud. With `BOARD_AGENT_COMMANDS` holding nothing (unset, whitespace, or +a lone comma, which is how the adapters read it too) the header carries a +quiet `no agent commands` chip naming the setting and the file that holds +it, `manager/local/.env`, and every launch of the two intents that would +have run those commands — **▸ start work** and **↻ act on PR** — appends +the same sentence to its line in the ticker. It is a configuration fact, +not an alarm: it wears the settled register rather than `--alarm`, it +appears only while the setting is empty, and it never refuses a launch. +An agent that only edits files is still useful, and bench does not +decline work because a project is unconfigured. + Bare Enter takes the default, Ctrl-D skips the rest, and what lands is `core/.env.example` with the answers substituted into their lines: every other key, every comment, so the written file is where the project reads diff --git a/manager/core/.env.example b/manager/core/.env.example index ca16723..450f50d 100644 --- a/manager/core/.env.example +++ b/manager/core/.env.example @@ -46,7 +46,9 @@ BOARD_AGENT_MODEL_REVIEW= # (each adapter renders them into its vendor's permission rules; the # git/gh grants per launch intent are built in). Headless runs have no # human at a permission prompt, so a test runner missing from this list -# is a test the work agent cannot run. +# is a test the work agent cannot run. Left empty — or holding only +# whitespace or a lone comma — the board says so: a quiet `no agent +# commands` chip in the header, and a note on every work launch. BOARD_AGENT_COMMANDS=python3 -m unittest # What counts as a definition-of-done check (the Focus view's CHECKS diff --git a/manager/core/agents.py b/manager/core/agents.py index 7b1ffb4..73a2c26 100644 --- a/manager/core/agents.py +++ b/manager/core/agents.py @@ -254,6 +254,21 @@ def _launch(mode: str, prompt: str, cwd: Path, agent_id: str, filename: str, log return proc, log_file, model +def _no_commands_note() -> str | None: + """What a launch owes the ticker when the project configured nothing for + its agents to run: this run can edit and commit, but it cannot check its + own work. Only the two intents that would have run the commands say it + (work and act-pr); a read-only kind never had them. + + A note, never a refusal — an agent that only edits files is still + useful, and bench does not decline work because a project is + unconfigured. The header says the same thing standing still.""" + if config.agent_commands(): + return None + return ("no project commands configured, so it cannot run this project's " + "tests — set BOARD_AGENT_COMMANDS in manager/local/.env") + + def _fresh_branch_point() -> tuple[str | None, str | None]: """Where a brand-new task branch should start: the newest main that exists. With an `origin` remote, fetch its main (bounded by @@ -429,8 +444,9 @@ def start_agent(filename: str, stage: str, takeover: bool = False) -> dict: summary = (f"{name} is back on {filename} — continuing branch {branch}" if continuing else f"{name} started on {filename} (branch {branch})") - if base_note: - summary += f" — {base_note}" + for note in (base_note, _no_commands_note()): + if note: + summary += f" — {note}" state.record_board_event({ "kind": "agent", "actor": "agent", "file": filename, "summary": summary, @@ -739,9 +755,13 @@ def start_pr_fix(filename: str, stage: str) -> dict: } with state.LOCK: state.AGENTS[agent_id] = record + summary = f"{name} is acting on the review of {filename}'s PR" + note = _no_commands_note() + if note: + summary += f" — {note}" state.record_board_event({ "kind": "agent", "actor": "agent", "file": filename, - "summary": f"{name} is acting on the review of {filename}'s PR", + "summary": summary, }) threading.Thread(target=_reap_pr_fix, args=(agent_id, proc, log_file), daemon=True).start() diff --git a/manager/core/board.html b/manager/core/board.html index b1ef4ea..6736540 100644 --- a/manager/core/board.html +++ b/manager/core/board.html @@ -661,6 +661,7 @@ +
@@ -1066,6 +1067,27 @@ function renderSync() { `${esc(detail.split(' — ')[0].replace(/^sync[^:]*:\s*/, ''))}`; } +/* The commands a headless agent may run are the one thing it cannot work + around: with none configured a work agent still edits and commits, but it + can never run this project's tests, and nothing said so until a run had + already ended saying it. Same shape as the drive's "no driver" — a quiet + statement of what this project has not set up, in the settled register, + never `--alarm`: nothing is failing here. A project that set it, by + detection or by hand, sees nothing at all. */ +function renderAgentCommands() { + const el = $('#cmdchip'); + const missing = S.state && S.state.hasAgentCommands === false; + el.hidden = !missing; + if (!missing) return; + el.title = 'Headless agents have no project commands to run here, so a ' + + "work agent cannot run this project's tests. Set BOARD_AGENT_COMMANDS " + + 'in manager/local/.env — comma-separated command prefixes, e.g. ' + + '"npm test". Work still runs; the agent simply cannot check itself.'; + el.innerHTML = `` + + `no agent commands` + + `BOARD_AGENT_COMMANDS`; +} + /* ── a phase in flight ────────────────────────────────────────────────── */ /* The phases the header has something to say about: one running, or one @@ -1214,6 +1236,7 @@ function render() { renderPhases(); renderViews(); renderSync(); + renderAgentCommands(); if (S.view === 'flight') renderFlight(); else if (S.view === 'focus') renderFocus(); else renderCards(); diff --git a/manager/core/config.py b/manager/core/config.py index 3e80f24..d74acd1 100644 --- a/manager/core/config.py +++ b/manager/core/config.py @@ -105,6 +105,16 @@ ADAPTER = setting("BOARD_AGENT_ADAPTER", "claude") # the adapter's own knowledge; this list is the project's half. AGENT_COMMANDS = setting("BOARD_AGENT_COMMANDS", "python3 -m unittest") + +def agent_commands() -> list[str]: + """The prefixes as an adapter reads them: comma-separated, blanks + dropped — so whitespace, or a lone comma, is exactly nothing + configured. The adapters split the same string in their own standalone + copies of `split_commands()`; this is core's, and it is what the board + asks so the page and the launch can never disagree about *empty*.""" + return [part.strip() for part in AGENT_COMMANDS.split(",") if part.strip()] + + # Model per launch intent — an opaque vendor-native name core passes to the # adapter untranslated (what names mean anything is vendor knowledge). Empty # = inherit the vendor's own default, exactly today's behaviour. A per-intent diff --git a/manager/core/httpd.py b/manager/core/httpd.py index 870cce1..ba87ad9 100644 --- a/manager/core/httpd.py +++ b/manager/core/httpd.py @@ -42,6 +42,10 @@ def state_payload() -> dict: "phases": phases.public_state(), "drive": drive.public(), "hasDriver": config.driver_path() is not None, + # whether this project gave its headless agents anything to run. The + # one setting an agent cannot work around, so the board says it is + # missing rather than letting a run discover it — see the header chip + "hasAgentCommands": bool(config.agent_commands()), "branches": github.task_branches(), "commands": config.commands(), "commandRuns": commands.public(), diff --git a/tests/test_no_agent_commands.py b/tests/test_no_agent_commands.py new file mode 100644 index 0000000..85be4bb --- /dev/null +++ b/tests/test_no_agent_commands.py @@ -0,0 +1,282 @@ +"""A board whose agents cannot run anything says so (task 46). + +`BOARD_AGENT_COMMANDS` is the one setting a headless agent cannot work +around: a test runner missing from it is a test the work agent cannot run. +Since the install stopped asking for it, a project the detector does not +recognise starts with it empty — and until this card nothing mentioned that +until a run had already ended with an agent explaining it could not verify +its work. + +Three places have to agree on *empty*: the splitter (core's, and the +standalone copies each adapter carries), the state payload the page reads, +and the launch that says it in the ticker. So the suite is one class each, +plus the page's own function run under node. + + python3 -m unittest discover -s tests -v +""" + +from __future__ import annotations + +import importlib.util +import json +import re +import shutil +import subprocess +import sys +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +CORE = REPO / "manager" / "core" +sys.path.insert(0, str(CORE)) + +import agents # noqa: E402 +import config # noqa: E402 + +from tests.test_phase_runs import PhaseCase, card, git # noqa: E402 + +BOARD = CORE / "board.html" +NODE = shutil.which("node") +ALONE = "77-alone.md" +PR_URL = "https://github.com/acme/widget/pull/7" + +# The forms of "nothing configured" a person can actually produce: never +# set, cleared, left as whitespace, or reduced to the separator. +NOTHING = ("", " ", "\t", ",", " , ", ",,", " ,\t,") +SOMETHING = {"npm test": ["npm test"], + " npm test ": ["npm test"], + "npm test, make check": ["npm test", "make check"], + "python3 -m unittest,": ["python3 -m unittest"]} + + +def _load(name: str, path: Path): + spec = importlib.util.spec_from_file_location(name, path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class WhatCountsAsNothing(unittest.TestCase): + """`config.agent_commands()` is the board's half of the split the + adapters already do. If the two disagreed, a board could show a chip a + launch contradicts, or stay silent about a run with nothing to run.""" + + def commands(self, raw: str) -> list[str]: + saved = config.AGENT_COMMANDS + self.addCleanup(setattr, config, "AGENT_COMMANDS", saved) + config.AGENT_COMMANDS = raw + return config.agent_commands() + + def test_every_shape_of_empty_is_empty(self): + for raw in NOTHING: + self.assertEqual(self.commands(raw), [], + f"{raw!r} configures nothing") + + def test_a_value_survives_stripped(self): + for raw, expected in SOMETHING.items(): + self.assertEqual(self.commands(raw), expected) + + def test_it_agrees_with_the_adapters(self): + """Both shipped adapters carry a standalone `split_commands()` (the + hooks run outside the board's imports). Same answers, or the board + is guessing about somebody else's rules.""" + splitters = [ + _load("claude_hook_settings", + CORE / "adapters" / "claude" / "hook_settings.py").split_commands, + _load("opencode_permission_config", + CORE / "adapters" / "opencode" / "permission_config.py").split_commands, + ] + for raw in (*NOTHING, *SOMETHING): + for split in splitters: + self.assertEqual(self.commands(raw), split(raw), + f"core and an adapter disagree about {raw!r}") + + +class TheStatePayloadSaysWhichItIs(unittest.TestCase): + """One boolean, beside `hasDriver` — the same shape of fact, answered by + the server so the page never re-parses the setting itself.""" + + def flag(self, raw: str) -> bool: + import httpd + saved = config.AGENT_COMMANDS + self.addCleanup(setattr, config, "AGENT_COMMANDS", saved) + config.AGENT_COMMANDS = raw + return httpd.state_payload()["hasAgentCommands"] + + def test_empty_is_reported_as_empty(self): + for raw in NOTHING: + self.assertFalse(self.flag(raw), f"{raw!r} is nothing configured") + + def test_a_configured_board_reports_true(self): + self.assertTrue(self.flag("npm test")) + self.assertTrue(self.flag("python3 -m unittest")) + + def test_the_page_does_not_read_the_raw_setting(self): + html = BOARD.read_text(encoding="utf-8") + self.assertNotIn("state.agentCommands", html) + self.assertIn("hasAgentCommands", html) + + +@unittest.skipUnless(NODE, "node is needed to run the page's own rules") +class TheHeaderChip(unittest.TestCase): + """The indicator itself, run as the page runs it: a stub element in + place of the DOM, a state payload in place of the server.""" + + @classmethod + def setUpClass(cls): + cls.html = BOARD.read_text(encoding="utf-8") + match = re.search(r"function renderAgentCommands\(\) \{.*?\n\}", + cls.html, re.S) + if match is None: + raise AssertionError("board.html no longer defines renderAgentCommands") + cls.src = match.group(0) + + def chip(self, state: dict | None) -> dict: + script = ( + "const el = {hidden: null, title: '', innerHTML: ''};\n" + "function $(sel) { if (sel !== '#cmdchip') " + "throw new Error('unexpected ' + sel); return el; }\n" + "var S = " + json.dumps({"state": state}) + ";\n" + + self.src + "\nrenderAgentCommands();\n" + "console.log(JSON.stringify(el));\n") + out = subprocess.run([NODE, "-e", script], capture_output=True, text=True) + self.assertEqual(out.returncode, 0, out.stderr) + return json.loads(out.stdout) + + def test_an_empty_setting_is_said_once_naming_setting_and_file(self): + chip = self.chip({"hasAgentCommands": False}) + + self.assertFalse(chip["hidden"]) + self.assertIn("BOARD_AGENT_COMMANDS", chip["innerHTML"] + chip["title"]) + self.assertIn("manager/local/.env", chip["title"], + "a reader who has never opened that file has to be told " + "which file it is") + + def test_a_configured_board_shows_nothing_at_all(self): + chip = self.chip({"hasAgentCommands": True}) + + self.assertTrue(chip["hidden"]) + self.assertEqual(chip["innerHTML"], "", + "nothing is drawn, not even hidden") + + def test_a_payload_that_does_not_say_is_not_guessed_at(self): + """An older server, or the first frame before any state: the board + says nothing rather than accusing a project of being unconfigured.""" + self.assertTrue(self.chip(None)["hidden"]) + self.assertTrue(self.chip({})["hidden"]) + + def test_it_is_not_an_alarm(self): + """Nothing is failing — something is unconfigured. `--alarm` is + reserved for blocked, failed or HIGH.""" + self.assertNotIn("--alarm", self.src) + self.assertNotIn("--accent", self.src, "and nothing here is working") + self.assertIn("var(--idle)", self.src) + + def test_the_chip_exists_and_is_drawn_every_frame(self): + self.assertIn('id="cmdchip"', self.html) + render = re.search(r"function render\(\) \{.*?\n\}", self.html, re.S).group(0) + self.assertIn("renderAgentCommands();", render) + + +class ALaunchWithNothingToRun(PhaseCase): + """The sharper half: the moment it matters is the launch. It is a note + in the ticker beside the run's own line — never a refusal, because an + agent that only edits files is still useful.""" + + SETTING = "BOARD_AGENT_COMMANDS in manager/local/.env" + + def setUp(self): + super().setUp() + self.write(ALONE, card("77 — On its own", status="In Progress"), + "in-progress") + + def said(self) -> list[str]: + return [s for s in self.summaries() if self.SETTING in s] + + def test_a_work_launch_says_it_and_still_runs(self): + self.patch(AGENT_COMMANDS="") + + agents.start_agent(ALONE, "in-progress") + self.settle() + + self.assertEqual(len(self.said()), 1, self.summaries()) + self.assertIn("cannot run this project's tests", self.said()[0]) + self.assertEqual(self.stage_of(ALONE), "review", + "the launch was not blocked: it ran, committed and landed") + + def test_a_configured_board_never_mentions_it(self): + self.patch(AGENT_COMMANDS="npm test") + + agents.start_agent(ALONE, "in-progress") + self.settle() + + self.assertEqual(self.said(), []) + self.assertFalse([s for s in self.summaries() + if "BOARD_AGENT_COMMANDS" in s]) + + def test_whitespace_and_a_lone_comma_count_as_nothing_here_too(self): + for raw in (" ", ","): + with self.subTest(raw=raw): + self.patch(AGENT_COMMANDS=raw) + self.assertIsNotNone(agents._no_commands_note()) + + def test_the_note_rides_beside_the_branch_point_note(self): + """Two things worth saying about one launch, one line: the note is + appended, it does not replace what the launch already said.""" + self.patch(AGENT_COMMANDS="") + + agents.start_agent(ALONE, "in-progress") + self.settle() + + line = self.said()[0] + self.assertIn(f"started on {ALONE}", line) + self.assertIn(f"task/{ALONE[:-3]}", line) + + def test_acting_on_a_pr_says_it_too(self): + """↻ act on PR is a work agent with a push — same intent, same + commands, same silence to break.""" + self.patch(AGENT_COMMANDS="") + text = card("77 — On its own", status="Review").replace( + "**Priority:** High\n", f"**Priority:** High\n**PR:** {PR_URL}\n") + (self.tasks / "in-progress" / ALONE).unlink() + self.write(ALONE, text, "review") + git(self.repo, "branch", f"task/{ALONE[:-3]}") + + agents.start_pr_fix(ALONE, "review") + self.settle() + + self.assertEqual(len(self.said()), 1, self.summaries()) + self.assertIn("acting on the review", self.said()[0]) + + def test_a_read_only_kind_says_nothing(self): + """`◔ still true?` never had the project's commands: telling it + about them would be noise on every card in every stage.""" + self.patch(AGENT_COMMANDS="") + self.adapter_is("#!/usr/bin/env python3\n" + "print('RELEVANCE REVIEW: Still relevant')\n") + + agents.start_review(ALONE, "in-progress") + self.settle() + + self.assertEqual(self.said(), []) + + +class TheDocumentedTrade(unittest.TestCase): + """The install stopped asking on purpose; AGENTS.md carries the reason, + so it has to carry what the board now does about it.""" + + @classmethod + def setUpClass(cls): + doc = (REPO / "AGENTS.md").read_text(encoding="utf-8") + cls.flat = re.sub(r"\s+", " ", doc.replace("**", "").replace("`", "")) + + def test_the_indicator_is_written_down(self): + self.assertIn("no agent commands", self.flat) + self.assertIn("BOARD_AGENT_COMMANDS", self.flat) + + def test_it_says_the_launch_is_not_blocked(self): + self.assertIn("never refuses a launch", self.flat) + + +if __name__ == "__main__": + unittest.main()