diff --git a/AGENTS.md b/AGENTS.md index 2a02516..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 @@ -378,6 +391,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 +979,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/.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 0c41729..73a2c26 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) @@ -202,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 @@ -304,9 +371,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] @@ -374,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, @@ -386,7 +457,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 +668,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 +714,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") @@ -670,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 2bc83bd..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(); @@ -1347,8 +1370,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 +1385,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 +1428,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 +1456,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 +1656,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 +2549,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/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/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_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() 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): 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()