From 18f964308736e079db65c55e1539c39ad0ccd567 Mon Sep 17 00:00:00 2001 From: pbean Date: Tue, 28 Jul 2026 00:02:14 -0700 Subject: [PATCH 1/2] feat(model): add awaiting-operator phase, sprint token, and surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Names, at every layer, the state a story reaches when its agent-doable work is finished and committed but its acceptance criteria include external actions only a human can perform. Nothing writes any of it yet: the park path is part 2 of the program, so no run can currently reach the phase. - Phase.AWAITING_OPERATOR, terminal, reachable only from COMMITTING so a park can never skip the gates and the commit a DONE story clears - sprint token ordered immediately before `done` — confirming a parked story is then a forward advance through the sole writer, and nothing can regress `done` back into it — and deliberately not actionable - StoryTask.operator_actions, round-tripped through state.json - run-summary count, `status` text + --json, TUI glyphs and header count - drops the dead STORY_STATUSES set (no readers since STATUS_ORDER landed) refs #335 (1/4) --- CHANGELOG.md | 14 ++++++++ src/bmad_loop/cli.py | 5 ++- src/bmad_loop/engine.py | 12 ++++++- src/bmad_loop/model.py | 26 +++++++++++++- src/bmad_loop/sprintstatus.py | 20 +++++++++-- src/bmad_loop/statemachine.py | 9 ++++- src/bmad_loop/tui/widgets.py | 13 ++++++- tests/test_cli.py | 36 +++++++++++++++++++ tests/test_engine.py | 38 +++++++++++++++++++++ tests/test_model.py | 32 ++++++++++++++++- tests/test_sprintstatus.py | 45 ++++++++++++++++++++++++ tests/test_sprintstatus_advance.py | 30 ++++++++++++++++ tests/test_statemachine.py | 44 +++++++++++++++++++++++- tests/test_tui_app.py | 55 ++++++++++++++++++++++++++++++ tests/test_verify.py | 21 +++++++++++- 15 files changed, 389 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5911a336..8d291ee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,20 @@ breaking changes may land in a minor release. ### Added +- **`awaiting-operator` vocabulary, no writer yet (#335, part 1 of 4).** Names, at every layer, the + state a story reaches when its agent-doable work is finished and committed but its acceptance + criteria include external actions only a human can perform: `Phase.AWAITING_OPERATOR` (terminal, + reachable only from `COMMITTING`), a `awaiting-operator` sprint-status token ordered immediately + before `done` and deliberately not actionable, `operator_actions` on each task, and the matching + run-summary count, `status` output and TUI glyphs. Nothing writes any of it yet — the park path + is part 2 — so no run can currently reach the phase. + + Two consequences worth knowing. A `state.json` carrying the new phase is forward-only: an older + binary rejects it, as with every phase addition. And because the sprint token is now _ordered_ + rather than unknown, a review session writing `awaiting-operator` onto a board the orchestrator + had already advanced to `done` is classified as a deliberate sign-off regression and escalates + (#334), where it previously fell through to a retry. + - **Defer notifications name where the work survives (#333).** A deferred story's rollback parked the attempt on an `attempt-preserve/*` branch (or a `refs/attempt-preserve-dirty/*` snapshot), but the ref only ever reached `journal.jsonl` — the notification carried a bare reason, leaving diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 94158d92..6eecf837 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -1581,8 +1581,11 @@ def cmd_status(args: argparse.Namespace) -> int: extra = task.defer_reason or task.commit_sha or "" if task.preserve_ref: extra = f"{extra} [{task.preserve_ref}]".lstrip() + # 17 = len("awaiting-operator"), the longest Phase token; a narrower + # field does not truncate, it shifts every following column on that one + # row, which reads as corrupt output rather than as a long status. print( - f" {key:40s} {task.phase:16s} dev×{task.attempt} review×{task.review_cycle} " + f" {key:40s} {task.phase:17s} dev×{task.attempt} review×{task.review_cycle} " f"{tokens} {extra}" ) if state.source == "stories": diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index fe917c40..68928cfd 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -107,6 +107,11 @@ class RunSummary: # direction this field exists to fix. There is one construction site; a # TypeError beats a plausible zero. weighted_tokens: int + # stories that committed but owe human-only external actions (Phase + # AWAITING_OPERATOR). Defaulted, unlike weighted_tokens: a miscounted 0 here + # is a count that is genuinely 0 on every run that never parks a story, so + # the default is honest rather than silently wrong. + awaiting_operator: int = 0 crashed: bool = False crash_error: str | None = None @@ -124,9 +129,13 @@ def render(self) -> str: # shutdown-only flush). Splitting this into "0 weighted (0 raw)" # would assert free work twice over; one plain zero is honest. tokens = "0 tokens" + # Appended only when non-zero: a run that parks nothing is the norm, and + # a standing ", 0 awaiting operator" would train readers to skip the very + # clause that matters on the run where it is not zero. + parked = f", {self.awaiting_operator} awaiting operator" if self.awaiting_operator else "" lines = [ f"run {self.run_id}: {self.done} done, {self.deferred} deferred, " - f"{self.escalated} escalated, {tokens}" + f"{self.escalated} escalated{parked}, {tokens}" ] if self.crashed: lines.append(f"CRASHED: {self.crash_error}") @@ -626,6 +635,7 @@ def summary(self) -> RunSummary: done=sum(1 for t in tasks if t.phase == Phase.DONE), deferred=sum(1 for t in tasks if t.phase == Phase.DEFERRED), escalated=sum(1 for t in tasks if t.phase == Phase.ESCALATED), + awaiting_operator=sum(1 for t in tasks if t.phase == Phase.AWAITING_OPERATOR), paused=self.state.paused, paused_reason=self.state.paused_reason or "", total_tokens=sum(t.tokens.total for t in tasks), diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index a12bc308..e1b01d67 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -32,9 +32,20 @@ class Phase(StrEnum): DONE = "done" DEFERRED = "deferred" ESCALATED = "escalated" + # the story's agent-doable work is finished and COMMITTED, but its acceptance + # criteria include external actions only a human can perform (buy a domain, + # publish a DNS record). Terminal like DONE — the run moves on rather than + # halting — but not DONE: the outstanding actions are recorded on the task. + # Deliberately NOT a pause: an operator-owed story must never block the + # stories behind it (contrast the spec's `Block If:` -> blocked -> CRITICAL + # -> pause channel, which is run-halting by design). + AWAITING_OPERATOR = "awaiting-operator" -TERMINAL_PHASES = frozenset({Phase.DONE, Phase.DEFERRED, Phase.ESCALATED}) +# Terminal = the orchestrator will not drive this story further in this run. It +# says nothing about success: DONE and AWAITING_OPERATOR carry a commit, DEFERRED +# and ESCALATED do not. +TERMINAL_PHASES = frozenset({Phase.DONE, Phase.DEFERRED, Phase.ESCALATED, Phase.AWAITING_OPERATOR}) # Pause stages recorded in RunState.paused_stage PAUSE_SPEC_APPROVAL = "spec-approval" @@ -173,6 +184,17 @@ class StoryTask: baseline_untracked: list[str] | None = None spec_file: str | None = None commit_sha: str | None = None + # the external, human-only actions this story still owes when it parks at + # Phase.AWAITING_OPERATOR — one free-text instruction per entry, as the dev + # session enumerated them in the spec's `operator_actions:` frontmatter. + # Plain strings in v1: a per-action deterministic `check:` command is a + # deliberate v2 question, and strings-now/objects-later is the cheaper + # migration than the reverse. Empty on every other phase — a park is defined + # by owing at least one action, so this being non-empty is what distinguishes + # AWAITING_OPERATOR from DONE. Survives the resume serialization round-trip + # (it is the durable record of what the human owes, and nothing re-derives it + # once the session that wrote the spec is gone). + operator_actions: list[str] = field(default_factory=list) defer_reason: str | None = None # the recovery ref this attempt's work was parked on by the last auto-rollback # — an `attempt-preserve/*` branch (commits above baseline) or, when the tree @@ -299,6 +321,7 @@ def to_dict(self) -> dict[str, Any]: "baseline_untracked": self.baseline_untracked, "spec_file": self._serialized_spec_file(), "commit_sha": self.commit_sha, + "operator_actions": self.operator_actions, "defer_reason": self.defer_reason, "preserve_ref": self.preserve_ref, "preserve_partial": self.preserve_partial, @@ -349,6 +372,7 @@ def from_dict(cls, d: dict[str, Any]) -> "StoryTask": ), spec_file=d.get("spec_file"), commit_sha=d.get("commit_sha"), + operator_actions=[str(a) for a in d.get("operator_actions", [])], defer_reason=d.get("defer_reason"), preserve_ref=d.get("preserve_ref"), preserve_partial=bool(d.get("preserve_partial", False)), diff --git a/src/bmad_loop/sprintstatus.py b/src/bmad_loop/sprintstatus.py index f3693226..263d44e4 100644 --- a/src/bmad_loop/sprintstatus.py +++ b/src/bmad_loop/sprintstatus.py @@ -24,11 +24,25 @@ SHORT_REF_RE = re.compile(r"^(\d+)[-.](\d+)([a-z]?)$") # short story ref: 3-1, 3.1, 3-1a BARE_NUM_RE = re.compile(r"^(\d+)([a-z]?)$") # a lone story number, needs --epic -STORY_STATUSES = {"backlog", "ready-for-dev", "in-progress", "review", "done"} # Lifecycle order, earliest -> latest. `advance` never moves a story backward -# through this sequence (matches sync-sprint-status's "never regress"). -STATUS_ORDER = ("backlog", "ready-for-dev", "in-progress", "review", "done") +# through this sequence (matches sync-sprint-status's "never regress"), and it is +# the only ordering any caller may use — a token absent from it cannot be ordered +# at all, so every consumer treats "unknown" conservatively rather than guessing. +# `awaiting-operator` sits immediately before `done`: parking is the last stop on +# the way to finished, so confirming a parked story is a legal forward advance +# through the sole writer, while nothing can ever regress `done` back into it. +STATUS_ORDER = ( + "backlog", + "ready-for-dev", + "in-progress", + "review", + "awaiting-operator", + "done", +) LEGACY_STORY_STATUSES = {"drafted": "ready-for-dev"} +# Statuses a story may be PICKED UP from. `awaiting-operator` is deliberately +# absent: the story's agent-doable work is already committed, so re-driving it +# would redo finished work while the human's external actions stay outstanding. ACTIONABLE_STATUSES = {"backlog", "ready-for-dev"} diff --git a/src/bmad_loop/statemachine.py b/src/bmad_loop/statemachine.py index 6572f26a..dd8952a8 100644 --- a/src/bmad_loop/statemachine.py +++ b/src/bmad_loop/statemachine.py @@ -31,13 +31,20 @@ class IllegalTransition(Exception): Phase.ESCALATED, } ), - Phase.COMMITTING: frozenset({Phase.DONE, Phase.ESCALATED}), + # AWAITING_OPERATOR: a story owing human-only external actions parks on the + # NORMAL commit path — the work commits, then the final phase is chosen by + # whether the task carries operator_actions. Reachable only from COMMITTING + # precisely so a park can never skip the gates/commit a DONE story clears. + Phase.COMMITTING: frozenset({Phase.DONE, Phase.ESCALATED, Phase.AWAITING_OPERATOR}), Phase.TRIAGE_RUNNING: frozenset({Phase.TRIAGE_VERIFY}), # TRIAGE_RUNNING: invalid triage output retries with feedback, like DEV_VERIFY Phase.TRIAGE_VERIFY: frozenset({Phase.TRIAGE_RUNNING, Phase.DONE, Phase.ESCALATED}), Phase.DONE: frozenset(), Phase.DEFERRED: frozenset(), Phase.ESCALATED: frozenset(), + # terminal: `bmad-loop confirm` completes a parked story out of band, not by + # transitioning the (by then finished) run's task. + Phase.AWAITING_OPERATOR: frozenset(), } diff --git a/src/bmad_loop/tui/widgets.py b/src/bmad_loop/tui/widgets.py index 626ebb16..c1452709 100644 --- a/src/bmad_loop/tui/widgets.py +++ b/src/bmad_loop/tui/widgets.py @@ -160,7 +160,7 @@ def show_run( if state.current_epic is not None: text.append(f" epic {state.current_epic}", style="dim") - counts = {Phase.DONE: 0, Phase.DEFERRED: 0, Phase.ESCALATED: 0} + counts = {Phase.DONE: 0, Phase.DEFERRED: 0, Phase.ESCALATED: 0, Phase.AWAITING_OPERATOR: 0} weight = state.cache_read_weight() weighted = raw = 0 for task in state.tasks.values(): @@ -174,6 +174,11 @@ def show_run( text.append(f" deferred {counts[Phase.DEFERRED]}", style="yellow") style = "red" if counts[Phase.ESCALATED] else "dim" text.append(f" escalated {counts[Phase.ESCALATED]}", style=style) + # Only when non-zero, unlike the three above: the header line is already + # at the width the narrowest supported pane can hold, and a standing + # "awaiting 0" would cost that room on every run that never parks a story. + if counts[Phase.AWAITING_OPERATOR]: + text.append(f" awaiting {counts[Phase.AWAITING_OPERATOR]}", style="yellow") text.append(f" {weighted:,} tokens ({raw:,} raw)", style="dim") # The agent line: who is driving (or, when idle, who is configured to). @@ -600,6 +605,7 @@ def validate_header(doc: dict) -> Text: "in-progress": "▶", "review": "◆", "ready-for-dev": "○", + "awaiting-operator": "⏸", "backlog": "·", "optional": "·", } @@ -609,6 +615,9 @@ def validate_header(doc: dict) -> Text: "in-progress": "cyan", "review": "magenta", "ready-for-dev": "cyan", + # yellow, matching the run header's deferred count: work is finished but the + # story is not, and something outside the loop has to happen next. + "awaiting-operator": "yellow", "backlog": "dim", "optional": "dim", } @@ -719,6 +728,7 @@ def update_sprint(self, ss: SprintStatus | None) -> None: "in-progress": "▶", "in-review": "◆", "done": "✓", + "awaiting-operator": "⏸", "blocked": "✖", "ambiguous": "⚠", "sentinel": "⚠", @@ -731,6 +741,7 @@ def update_sprint(self, ss: SprintStatus | None) -> None: "in-progress": "cyan", "in-review": "magenta", "done": "green", + "awaiting-operator": "yellow", "blocked": "bold red", "ambiguous": "bold red", "sentinel": "bold red", diff --git a/tests/test_cli.py b/tests/test_cli.py index 8f300979..d692873e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1124,6 +1124,42 @@ def test_status_text_names_the_preserve_ref_beside_the_reason(project, capsys): assert "review did not converge within budget [attempt-preserve/run-1-abcd1234]" in out +def test_status_reports_a_parked_story_on_both_surfaces(project, capsys): + """The `--json` projection is already phase-agnostic (`str(task.phase)`), so a + new phase needs no schema change and no version bump — but "needs no change" + is a claim about the contract, and this is what holds it: the parked task + reports its own token, carries its commit, and is not confused with a defer. + + On the text side the phase cell is measured, not just matched: + `awaiting-operator` is the longest phase token there is, and a field too + narrow for it does not truncate — it pushes every following cell one column + right on that one row, which reads as corrupt output rather than as a long + status. `_story_row` splits on whitespace and so cannot see that; comparing + the two rows' column offsets can. + """ + from bmad_loop.model import Phase, StoryTask + + parked = StoryTask(story_key="1-1-login", epic=1, phase=Phase.AWAITING_OPERATOR) + parked.commit_sha = "abc1234" + parked.operator_actions = ["publish the DKIM TXT record"] + done = StoryTask(story_key="1-2-logout", epic=1, phase=Phase.DONE) + _make_run_with_tokens(project, {"1-1-login": parked, "1-2-logout": done}, weight=0.1) + + doc = _status_json(project, capsys) + entry_parked, _ = doc["tasks"] + assert entry_parked["phase"] == "awaiting-operator" + assert entry_parked["commit_sha"] == "abc1234" # parked work is committed work + assert entry_parked["defer_reason"] is None # a park is not a failure + assert doc["schema_version"] == 1 # additive: no consumer breaks + + assert cli.main(["status", "--project", str(project.project)]) == 0 + out = capsys.readouterr().out + assert _story_row(out)[1] == "awaiting-operator" + (parked_line,) = [ln for ln in out.splitlines() if ln.startswith(" 1-1-login ")] + (done_line,) = [ln for ln in out.splitlines() if ln.startswith(" 1-2-logout ")] + assert parked_line.index("dev×") == done_line.index("dev×") + + def test_status_json_stories_mode_is_pure_json(project, capsys): """--json must skip every text trailer (stories board, backlog, decisions nudge) — the stories-mode board would otherwise corrupt the document.""" diff --git a/tests/test_engine.py b/tests/test_engine.py index d4ca51aa..b648862c 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -1183,6 +1183,44 @@ def test_summary_weights_per_task_not_over_the_aggregate(project): assert summary.weighted_tokens == 30 # NOT 32 +def test_summary_counts_parked_stories_apart_from_done(project): + """A story that committed but owes human external actions is neither `done` + (its acceptance criteria are not met) nor `deferred`/`escalated` (nothing + failed). Without its own count it would land in none of the three and vanish + from a summary that still counts it in the task total. + + Constructed rather than driven: PR-1 ships the vocabulary with no writer, so + COMMITTING -> AWAITING_OPERATOR is unreachable through the engine loop. + """ + engine = _cache_heavy_engine(project, snapshot_weight=0.5, live_weight=0.5, usage=TokenUsage()) + for key, phase in ( + ("1-1-a", Phase.DONE), + ("1-2-b", Phase.AWAITING_OPERATOR), + ("1-3-c", Phase.AWAITING_OPERATOR), + ): + engine.state.tasks[key] = StoryTask(story_key=key, epic=1, phase=phase) + + summary = engine.summary() + + assert summary.awaiting_operator == 2 + # and it did NOT inflate any of the three existing counts + assert summary.done == 1 and summary.deferred == 0 and summary.escalated == 0 + + +def test_run_summary_render_names_parked_stories_only_when_there_are_any(project): + """The clause earns its space by being absent on the runs it would be 0 on — + a standing ", 0 awaiting operator" on every run trains readers to skip the + one clause that matters on the run where it is not zero.""" + engine = _cache_heavy_engine(project, snapshot_weight=0.5, live_weight=0.5, usage=TokenUsage()) + assert "awaiting operator" not in engine.summary().render() + + engine.state.tasks["1-2-b"] = StoryTask( + story_key="1-2-b", epic=1, phase=Phase.AWAITING_OPERATOR + ) + + assert "1 awaiting operator" in engine.summary().render() + + def test_run_summary_render_labels_both_units(project): """render() feeds stdout, the ATTENTION file and the desktop notification from one place, so this covers all three.""" diff --git a/tests/test_model.py b/tests/test_model.py index 1a676588..d0fa65ae 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -2,7 +2,7 @@ import pytest -from bmad_loop.model import RunState, SessionRecord, StoryTask, TokenUsage +from bmad_loop.model import Phase, RunState, SessionRecord, StoryTask, TokenUsage def _state(**kw) -> RunState: @@ -193,6 +193,36 @@ def test_preserve_ref_defaults_none_for_legacy_state(): assert restored.preserve_ref is None and restored.preserve_partial is False +def test_operator_actions_round_trip(): + task = StoryTask( + story_key="1-1-a", + epic=1, + phase=Phase.AWAITING_OPERATOR, + operator_actions=["buy the domain", "publish the DKIM TXT record"], + ) + restored = StoryTask.from_dict(task.to_dict()) + # the phase travels as its plain token, so a parked run resumes parked + assert restored.phase is Phase.AWAITING_OPERATOR + assert task.to_dict()["phase"] == "awaiting-operator" + # order is meaningful — the human works the list top-down + assert restored.operator_actions == ["buy the domain", "publish the DKIM TXT record"] + + +def test_operator_actions_defaults_empty_for_legacy_state(): + doc = StoryTask(story_key="1-1-a", epic=1).to_dict() + del doc["operator_actions"] # state.json from before the field existed + assert StoryTask.from_dict(doc).operator_actions == [] + + +def test_operator_actions_are_not_shared_between_tasks(): + """`field(default_factory=list)` rather than a shared `[]` default: two parked + stories in one run must not accumulate each other's actions.""" + one = StoryTask(story_key="1-1-a", epic=1) + other = StoryTask(story_key="1-2-b", epic=1) + one.operator_actions.append("buy the domain") + assert other.operator_actions == [] + + def test_token_budget_warned_round_trips(): task = StoryTask(story_key="1-1-a", epic=1, token_budget_warned=True) assert StoryTask.from_dict(task.to_dict()).token_budget_warned is True diff --git a/tests/test_sprintstatus.py b/tests/test_sprintstatus.py index d318bf71..5fca3302 100644 --- a/tests/test_sprintstatus.py +++ b/tests/test_sprintstatus.py @@ -112,6 +112,51 @@ def test_next_actionable_order_and_skip(project): assert sprintstatus.next_actionable(ss, skip={"1-2-b", "1-3-c"}) is None +def test_awaiting_operator_is_ordered_just_below_done(): + """The token's whole contract is its POSITION: last stop before `done`, so + confirming a parked story is a forward advance and nothing can regress `done` + back into it. Asserting the index relationships rather than the literal tuple + keeps this from breaking every time a status is added elsewhere.""" + order = sprintstatus.STATUS_ORDER + assert order.index("awaiting-operator") == order.index("done") - 1 + assert order.index("review") < order.index("awaiting-operator") + + +def test_parked_story_is_never_picked_as_next_actionable(project): + """A parked story's agent-doable work is already committed — picking it up + again would redo finished work while the human's external actions stay + outstanding. The board must walk straight past it to the next real story. + + Ablation (repo rule, inverse form — the gate here is an ABSENCE, so the + ablation ADDS rather than deletes): put "awaiting-operator" into + ACTIONABLE_STATUSES and this test must fail on the first assert. It does — + which is what proves the assertions gate on actionability rather than on + file order (1-1-a is first in the file, so a test that merely returned the + first pickable story would pass either way). + """ + write_sprint( + project, + {"1-1-a": "awaiting-operator", "1-2-b": "ready-for-dev"}, + ) + ss = sprintstatus.load(project.sprint_status) + assert sprintstatus.next_actionable(ss).key == "1-2-b" + # and with the only other story taken, there is nothing left to run — the + # parked story is not a fallback either + assert sprintstatus.next_actionable(ss, skip={"1-2-b"}) is None + + +def test_parked_story_cannot_be_targeted_by_a_selector(project): + """`--story` naming a parked story is a user error with a specific message, + not a silent re-drive: select_actionable filters on the same set.""" + write_sprint(project, {"1-1-a": "awaiting-operator"}) + ss = sprintstatus.load(project.sprint_status) + with pytest.raises( + sprintstatus.SprintStatusError, + match=r"story 1-1 matched 1-1-a but its status is 'awaiting-operator'", + ): + sprintstatus.select_actionable(ss, None, "1-1") + + def test_next_actionable_epic_filter(project): # document order (epic 5 before epic 9), not numeric; the epic filter returns # epic 9's first actionable story even though 5-1 is earlier in the file. diff --git a/tests/test_sprintstatus_advance.py b/tests/test_sprintstatus_advance.py index a42830d9..67c5e9bb 100644 --- a/tests/test_sprintstatus_advance.py +++ b/tests/test_sprintstatus_advance.py @@ -72,6 +72,36 @@ def test_advance_never_regresses(tmp_path): assert sprintstatus.story_status(p, "4-1-thing") == "review" +def test_advance_confirms_a_parked_story_forward_to_done(tmp_path): + """The exit move `bmad-loop confirm` will need: because `awaiting-operator` + sits below `done` in STATUS_ORDER, completing a parked story is an ordinary + forward advance through the sole writer — no invariant exception required.""" + p = _write(tmp_path) + sprintstatus.advance(p, "3-2-digest-delivery", "awaiting-operator") + assert sprintstatus.story_status(p, "3-2-digest-delivery") == "awaiting-operator" + + out = sprintstatus.advance(p, "3-2-digest-delivery", "done") + + assert out == "done" + assert sprintstatus.story_status(p, "3-2-digest-delivery") == "done" + + +def test_advance_never_regresses_done_into_awaiting_operator(tmp_path): + """The other half of the ordering: once a story is `done`, nothing walks the + board back to `awaiting-operator`. This is a real hardening, not a restatement + — before the token joined STATUS_ORDER it was unordered, so the never-regress + guard's `target in STATUS_ORDER` arm short-circuited and this write went + through. (Demoting a done story is Phase 4's `operator.on_review_demotion` + question, and it will need its own deliberate, allowlisted writer.)""" + p = _write(tmp_path) + before = p.read_text() + + out = sprintstatus.advance(p, "3-1-login", "awaiting-operator") # already done + + assert out == "done" + assert p.read_text() == before + + def test_advance_returns_current_when_line_not_rewritable(tmp_path): """A quoted story key parses via YAML (story_status finds it) but the line-edit writer can't rewrite it. advance() must report the unchanged status, not falsely diff --git a/tests/test_statemachine.py b/tests/test_statemachine.py index 909ee10f..9aef92ae 100644 --- a/tests/test_statemachine.py +++ b/tests/test_statemachine.py @@ -1,6 +1,6 @@ import pytest -from bmad_loop.model import Phase, StoryTask +from bmad_loop.model import TERMINAL_PHASES, Phase, StoryTask from bmad_loop.statemachine import TRANSITIONS, IllegalTransition, advance @@ -8,6 +8,16 @@ def test_table_covers_every_phase(): assert set(TRANSITIONS) == set(Phase) +def test_terminal_phases_are_exactly_the_dead_ends(): + """`TERMINAL_PHASES` and the transition table are two spellings of the same + fact, in two modules, with nothing linking them: a new terminal phase added + to the table with no outgoing edges but forgotten in the frozenset would be + driven as if it were still live (`StoryTask.terminal` is False), and every + other test would still pass. Pin them to each other.""" + dead_ends = {phase for phase, targets in TRANSITIONS.items() if not targets} + assert dead_ends == set(TERMINAL_PHASES) + + @pytest.mark.parametrize("source", list(Phase)) @pytest.mark.parametrize("target", list(Phase)) def test_exhaustive_transitions(source, target): @@ -35,6 +45,38 @@ def test_happy_path_sequence(): assert task.terminal +def test_park_path_sequence(): + """A story owing human-only external actions rides the NORMAL commit path and + parks at the end of it: COMMITTING is the only phase it is reachable from, so + a park can never skip the gates and the commit a DONE story clears.""" + task = StoryTask(story_key="1-1-x", epic=1) + for phase in ( + Phase.DEV_RUNNING, + Phase.DEV_VERIFY, + Phase.REVIEW_RUNNING, + Phase.REVIEW_VERIFY, + Phase.COMMITTING, + Phase.AWAITING_OPERATOR, + ): + advance(task, phase) + assert task.terminal + + +@pytest.mark.parametrize( + "source", + [p for p in Phase if p is not Phase.COMMITTING], +) +def test_awaiting_operator_is_reachable_only_from_committing(source): + """The explicit inverse of the happy path above. `test_exhaustive_transitions` + covers this pairwise too, but only as one cell of an N-squared grid whose + expectation is read out of the table under test — this states the rule + independently, so widening the table cannot silently widen the test.""" + task = StoryTask(story_key="1-1-x", epic=1, phase=source) + with pytest.raises(IllegalTransition): + advance(task, Phase.AWAITING_OPERATOR) + assert task.phase == source + + def test_triage_path_sequence(): task = StoryTask(story_key="sweep-triage", epic=0) for phase in ( diff --git a/tests/test_tui_app.py b/tests/test_tui_app.py index 6896b393..20c12b11 100644 --- a/tests/test_tui_app.py +++ b/tests/test_tui_app.py @@ -2331,7 +2331,32 @@ def test_sprint_story_label_split_suffix(): assert sprint_story_label(half).plain == "· 6a-build" +def test_sprint_glyphs_cover_every_lifecycle_status(): + """Both maps are read through `.get(..., "?")` — deliberately, because the + board is LLM-maintained and an unknown token must render, not raise. The cost + is that a lifecycle status nobody added a glyph for renders a silent `?` and + no test notices. Scope the coverage claim to STATUS_ORDER, which is exactly + the set the orchestrator itself writes, and leave the fallback for the rest. + """ + from bmad_loop.sprintstatus import STATUS_ORDER + from bmad_loop.tui.widgets import SPRINT_GLYPHS, SPRINT_STYLES + + assert set(STATUS_ORDER) <= set(SPRINT_GLYPHS) + assert set(SPRINT_GLYPHS) == set(SPRINT_STYLES) # never a glyph without a color + + +def test_sprint_story_label_awaiting_operator(): + from bmad_loop.sprintstatus import Story + + parked = Story(key="2-7-dns", epic=2, num=7, slug="dns", status="awaiting-operator") + label = sprint_story_label(parked) + assert label.plain == "⏸ 7-dns" + # not the "?"/dim unknown-token fallback + assert label.style == "yellow" + + def test_story_cells_render(): + assert story_state_cell("awaiting-operator").plain == "⏸ awaiting-operator" assert story_state_cell("done").plain == "✓ done" assert story_state_cell("sentinel:unresolved").plain.startswith("⚠") assert story_checkpoint_cell(True, False).plain == "S·" @@ -2690,6 +2715,36 @@ async def test_graceful_stop_pending_shows_in_header_and_note(project, monkeypat ) +async def test_header_counts_parked_stories_only_when_there_are_any(project): + """The header's done/deferred/escalated trio is the TUI's mirror of + RunSummary's counts; a parked story belongs to none of them, so without its + own cell it would show up in `tasks N` and nowhere else. Conditional for the + same reason the summary clause is: the line is already at the width the + narrowest supported pane holds. + + Drives `show_run` directly — the count line is a pure function of the state it + is handed, so routing a second run through the dashboard's polling only adds + scheduling to what this is actually asserting.""" + done = StoryTask(story_key="1-2-beta", epic=1, phase=Phase.DONE) + parked = StoryTask(story_key="1-1-alpha", epic=1, phase=Phase.AWAITING_OPERATOR) + + def _state(tasks): + return RunState(run_id="r1", project=str(project.project), started_at="now", tasks=tasks) + + app = BmadLoopApp(project.project) + async with app.run_test(): + header = dashboard(app).query_one("#runheader", RunHeader) + + header.show_run("r1", "finished", _state({"1-2-beta": done})) + assert "awaiting" not in str(header.content) + + header.show_run("r1", "finished", _state({"1-2-beta": done, "1-1-alpha": parked})) + content = str(header.content) + assert "awaiting 1" in content + assert "tasks 2" in content + assert "done 1" in content # the park did not absorb the done story + + async def test_active_agent_shows_in_header_and_task_cell(project, monkeypatch): # End to end: a RUNNING run with an open, adapter-stamped session-start paints # the header's live agent line and the task row's agent cell diff --git a/tests/test_verify.py b/tests/test_verify.py index d81b5657..be543f61 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -473,7 +473,7 @@ def test_verify_review_signoff_regression_needs_the_launch_flag(project): "board, why", [ ({"1-2-b": "done"}, "story absent from the board -> story_status returns None"), - ({"1-1-a": "awaiting-operator"}, "token outside STATUS_ORDER (hand-edited/future board)"), + ({"1-1-a": "needs-signoff"}, "token outside STATUS_ORDER (hand-edited board)"), ], ) def test_verify_review_unrecognized_sprint_status_retries(project, board, why): @@ -491,6 +491,25 @@ def test_verify_review_unrecognized_sprint_status_retries(project, board, why): assert not out.ok and out.retryable and out.contradiction is False, why +def test_verify_review_awaiting_operator_board_is_a_regression(project): + """`awaiting-operator` joined STATUS_ORDER below `done`, so a review writing + it onto a board the orchestrator had already signed off is now ORDERED, and + therefore a deliberate regression — not the unknown token it used to be. + + This is the one behavior the vocabulary PR changes, and it is the intended + one: until the park path exists, nothing legitimately writes this token, so a + review that writes it has revoked a sign-off the same way `in-progress` does. + When the park path lands, `operator.on_review_demotion` is what re-routes it. + """ + task = _signoff_regression_task(project, sprint_status="awaiting-operator") + + out = verify.verify_review(task, project, Policy(), sprint_reached_done=True) + + assert not out.ok and not out.retryable + assert out.contradiction is True and out.severity == "CRITICAL" + assert "'awaiting-operator'" in out.reason + + # --------------------------------------------------- verify command exit codes (issue #126) From 67f98a337d013b69cda9b47ce08a845b36bc0b72 Mon Sep 17 00:00:00 2001 From: pbean Date: Tue, 28 Jul 2026 06:44:36 -0700 Subject: [PATCH 2/2] docs(model): scope the operator_actions non-empty rule to its writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #347. The field comment stated "a park is defined by owing at least one action" as a live invariant, but this PR ships no writer and no validator, so nothing can hold it — which is exactly why a reviewer read the empty-list fixtures as invalid state. Attribute the rule to the park path that will enforce it and say plainly that the field is unvalidated on both load and write. Also: fix the article before `awaiting-operator` in the CHANGELOG, and pin that the v1 status document deliberately does not project the actions themselves (the fixture set them but asserted nothing). --- CHANGELOG.md | 2 +- src/bmad_loop/model.py | 13 ++++++++----- tests/test_cli.py | 3 +++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d291ee6..12013d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ breaking changes may land in a minor release. - **`awaiting-operator` vocabulary, no writer yet (#335, part 1 of 4).** Names, at every layer, the state a story reaches when its agent-doable work is finished and committed but its acceptance criteria include external actions only a human can perform: `Phase.AWAITING_OPERATOR` (terminal, - reachable only from `COMMITTING`), a `awaiting-operator` sprint-status token ordered immediately + reachable only from `COMMITTING`), an `awaiting-operator` sprint-status token ordered immediately before `done` and deliberately not actionable, `operator_actions` on each task, and the matching run-summary count, `status` output and TUI glyphs. Nothing writes any of it yet — the park path is part 2 — so no run can currently reach the phase. diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index e1b01d67..12a55156 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -189,11 +189,14 @@ class StoryTask: # session enumerated them in the spec's `operator_actions:` frontmatter. # Plain strings in v1: a per-action deterministic `check:` command is a # deliberate v2 question, and strings-now/objects-later is the cheaper - # migration than the reverse. Empty on every other phase — a park is defined - # by owing at least one action, so this being non-empty is what distinguishes - # AWAITING_OPERATOR from DONE. Survives the resume serialization round-trip - # (it is the durable record of what the human owes, and nothing re-derives it - # once the session that wrote the spec is gone). + # migration than the reverse. Empty on every other phase: owing at least one + # action is what selects AWAITING_OPERATOR over DONE when the park path picks + # a committing story's final phase. That choice is the only enforcement — + # nothing validates this field on load or on write, so a task carrying the + # phase with no actions is unreachable rather than rejected. Survives the + # resume serialization round-trip (it is the durable record of what the human + # owes, and nothing re-derives it once the session that wrote the spec is + # gone). operator_actions: list[str] = field(default_factory=list) defer_reason: str | None = None # the recovery ref this attempt's work was parked on by the last auto-rollback diff --git a/tests/test_cli.py b/tests/test_cli.py index d692873e..a8776ede 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1151,6 +1151,9 @@ def test_status_reports_a_parked_story_on_both_surfaces(project, capsys): assert entry_parked["commit_sha"] == "abc1234" # parked work is committed work assert entry_parked["defer_reason"] is None # a park is not a failure assert doc["schema_version"] == 1 # additive: no consumer breaks + # the v1 document carries the phase, not the actions themselves — the + # registry and `confirm --list` are their surface (part 3) + assert "operator_actions" not in entry_parked assert cli.main(["status", "--project", str(project.project)]) == 0 out = capsys.readouterr().out