diff --git a/CHANGELOG.md b/CHANGELOG.md index ff817c6f..31b1e3bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,19 @@ whose seams had diverged enough that several ports needed a different fix, and t ### Added +- **A deferred-work entry can block a story: `gate:`.** An entry that must land before specific + stories run could only say so in prose (`HARD GATE: must land before 3-2`), and prose stopped + nothing — `run` drove the story anyway. A `gate: 3-2, 3-3` line names the blocked story keys, and + it is enforced on both sides: `bmad-loop validate` fails (`deferred.hard-gate`) for every + actionable story a token matches, in both queue modes, and `run` pauses (`story-gate`) rather + than dispatch a gated story — so the refusal no longer depends on remembering to run the + preflight. The pause happens before the story is recorded, so closing the entry and resuming + runs it. Sweeps are exempt: they are what closes the gating entry. The only deferred check that + gates rather than advises; cleared by closing the entry or dropping the token. A gate that can + enforce nothing — an unusable token, an empty `gate:` line, a `gate:` not written lowercase at + the start of a line, or a prose-only `HARD GATE:` — warns instead + (`deferred.hard-gate-unstructured`). Silent on a ledger that gates nothing, as before. + - **Deferred review findings are harvested from spec frontmatter (#433).** BMAD-METHOD#2640 moved `defer`-triaged findings into the spec's unfiled `deferred:` list. A successful dev, review, repair or review-timeout-salvage pass now files each as `### DW-` (`spec-deferrals-harvested`), @@ -103,6 +116,12 @@ whose seams had diverged enough that several ports needed a different fix, and t ### Changed +- **An unreadable deferred-work ledger fails `validate` instead of warning + (`deferred.ledger-unreadable`).** The hard gate rides on the same bytes, so a warning exited 0 + with the gate never evaluated — a fail-open on the one deferred check that refuses, and one that + cannot be narrowed by asking whether the project uses gates, because the file that would answer + is the unreadable one. `run` pauses on the same fault, so preflight and dispatch now agree. + - **Every spec-frontmatter status read goes through `status_of` (#358 follow-up).** Five inline status reads remained in the engine and the generic adapter, each reading a blank `status:` as the token `none` — the defect #358 fixed at the shared reader. Three were neutral; the pair that was diff --git a/README.md b/README.md index 3c808e96..fe158d89 100644 --- a/README.md +++ b/README.md @@ -278,6 +278,23 @@ The orchestrator writes the same annotation a bundle close writes — `status: d A declaration in a shape nothing can read — a bare `closes_deferred: DW-5` where a list belongs — depends on which channel it is in. In a **story spec** it is journaled and dropped, like an unknown id: the spec is generated mid-run by a dev skill, and a malformed field there must not be able to fail a story that succeeded. In **`stories.yaml`** it is a schema error, exactly like every other manifest field of the wrong type, and the manifest fails to load — the breakdown is hand-authored before the run, where `bmad-loop validate` reports it up front and a typo is still cheap to fix. `validate` warns about unknown ids in both queue modes and about a malformed spec declaration; a malformed manifest is the manifest's own `queue.stories-manifest` failure. +**Blocking a story from the ledger.** Some entries do not merely defer work, they block it: a leg nobody has wired yet is not a nice-to-have for the first story that consumes it. Entries said so in prose (`HARD GATE: must land before 3-2`) long before anything could act on it, and prose gates nothing — `run` picked the story off the board and drove it anyway, and the gate surfaced afterwards, in a diff built on the missing leg. A `gate:` field line makes the claim enforceable: + +```markdown +### DW-1: wire the blob-storage credentials + +status: open +gate: 3-2, 3-3 +``` + +Until that entry lands, `bmad-loop validate` **fails** for every actionable story a token matches, and `run` **pauses** rather than dispatch one — the gate is enforced at dispatch as well as at preflight, so it no longer depends on remembering to run `validate` first. A token gates a story key when it is that key, or is its prefix at a key boundary — a `-`, or the split-story suffix (one lowercase letter then `-`). So `3-2` covers the sprint key `3-2-invite-link-student-surface`, the stories-mode id `3-2`, and both halves of a split (`3-2a-…`/`3-2b-…`), but never `3-20-later-story`; the split arm is there because breakdown can split a gated story after the gate was written, and a gate that quietly stops matching is worse than none. Closing the entry clears it, and so does dropping the token. This is the only deferred-work check that is a gate rather than an advisory: the `closes_deferred` checks above describe traceability that may be wrong and must never block a run, while this one describes work that must not start. + +Only an explicit `status: done ` retires a gate. An entry whose status the format cannot read — `status: opne`, or no `status:` line at all — still gates, because an unreadable status is not evidence the work landed; letting it read as closed would have meant one keystroke silently disabling the refusal. + +The dispatch pause (`story-gate`, reviewable in the TUI like any other gate) fires before the story is recorded as touched, so closing the entry — by hand or with `bmad-loop sweep` — and resuming runs it. **Sweeps themselves are never gated**: a sweep is what closes the gating entry, so gating it would deadlock the gate against its own remedy. A story whose session already completed **finishes** when the run resumes — its recorded result replays through to commit rather than stranding half-done work; the gate is about work that must not _start_. A resume that instead **restarts** a story, discarding its worktree or resetting to baseline and re-running from scratch, is a start and is asked again: so a gate landing while a run was down still stops the story that run had picked but never got underway, and stops a re-drive of one whose escalation or wedge you have just resolved. + +Four shapes declare a gate nothing can enforce, and each is a warning while the entry is unlanded: a token that cannot name a story key (a space-separated `gate: 3-2 3-3`, which is one bad token rather than two good ones, or an unmatchable `gate: 3.2` — note `.` and `_` are fine inside a sprint slug, so `gate: 3-2-a_b` is a real gate); a `gate:` line with nothing usable after the colon; a `gate:` not written lowercase at the very start of a line (`Gate:`, or indented — surfaced rather than accepted, so a fenced example inside an entry cannot become a refusal); and prose declaring `HARD GATE:` on an entry that carries no `gate:` line. The prose arm matches mid-line, because `reason:` prose is hard-wrapped and that is where a real declaration lands — but not directly after a quote character, so an entry that merely cites the phrase stays silent. + > **Ledger outside the repo.** If `implementation_artifacts` is configured outside the project tree, the ledger is shared between worktrees and cannot be part of any commit. The annotation is written all the same, at the same moment, and the run journals `deferred-close-external-ledger` so its absence from git history is not a surprise. A location that cannot be read or written when the write comes due (a shared mount that has gone away) closes nothing and is journaled — those entries stay `open` for a sweep to re-verify, and an outage is never read as "no such entries". **Answering missed decisions later.** An unattended sweep (`--no-prompt`) skips decisions, and an interactive one can be abandoned before you answer them all — those answers would otherwise be lost, since triage re-derives the decision set from the ledger every run. `bmad-loop decisions` (or press `d` in the TUI) surfaces every decision past sweeps left unanswered, reconstructed from their triage output, and lets you answer them out of band. A `close` is applied immediately; a `build`/`keep-open` is saved to `.bmad-loop/decisions.json` and consumed by the next sweep (build → bundle, keep-open → recorded) with no re-prompt. `--list` shows them without answering; `bmad-loop status` reports the outstanding count. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 2ee41089..70779485 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -112,6 +112,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Skills accumulate an append-only ledger (`deferred-work.md`, `DW-` entries): split-off goals, pre-existing findings, "needs human decision" items. - Story-declared closure (`closes_deferred: [DW-5, DW-6]`, human-authored on a `stories.yaml` entry or in a story spec's frontmatter — the two are unioned): when the story commits, each declared entry flips to `status: done ` + `resolution: resolved by story ` — the annotation a sweep bundle writes — so the ledger stops being one-way. Written at the commit boundary, so an in-repo ledger carries the annotation in the story's own commit and a story that fails, is rejected by review, or escalates closes nothing. Declared, never inferred from a diff; re-read at the commit so a declaration edited after implementation still counts; idempotent across a resume; an unknown id, an unreadable entry status or a non-list declaration in a story spec is journaled, never fatal, and `bmad-loop validate` warns about all of them before the run starts. (A non-list `closes_deferred` in `stories.yaml` is different: the manifest is a schema the parser owns, so it is refused outright, before the run.) An artifact dir outside the repo cannot be committed — the annotation is written anyway and journaled (`deferred-close-external-ledger`). +- Hard gates (`gate: 3-2, 3-3` on an entry): until the entry lands, `bmad-loop validate` fails (`deferred.hard-gate`) for every actionable story a token matches and `run` pauses (`story-gate`) rather than dispatch one — a token gates a key it equals or prefixes at a key boundary (`-`, or a split-story suffix), so `3-2` covers `3-2-invite-link`, the stories-mode id `3-2` and both halves of a `3-2a`/`3-2b` split, but never `3-20-later`. Only an explicit `status: done` retires a gate; an unreadable status (`opne`, or no status line) still gates. The dispatch pause precedes the story's own run record, so closing the entry and resuming runs it; sweeps are exempt, since a sweep is what closes the gating entry. The only deferred check that gates rather than advises; cleared by closing the entry or dropping the token. A warning (`deferred.hard-gate-unstructured`) covers the four gates nothing can enforce: a token that cannot name a story key (`3-2 3-3`, or an unmatchable `3.2` — `.`/`_` are legal inside a sprint slug), an empty `gate:` line, a `gate:` not lowercase at the start of a line, and prose declaring `HARD GATE:` (matched mid-line, since `reason:` prose wraps — but not straight after a quote, so a citation stays silent) on an entry with no `gate:` line. - `bmad-loop sweep` triages every open entry against the actual code (ledger statuses treated as unreliable) → partition: already-resolved (auto-closed with evidence) / bundles / blocked / skip / decisions. - Bundles run the full pipeline (dev `--dw-bundle` → review → verify → commit); the review gate checks every bundle entry is `status: done`. - Interactive decision walkthrough (build / close / keep-open per option, with a recommendation); answers written back as `decision:` lines. Unattended runs leave decisions open. diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 953a4716..fa877583 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -440,9 +440,11 @@ artifacts the engine already wrote. interactive agent as `R`; **Re-arm & resume** (offered once the resolve agent has recorded a resolution) re-arms and resumes — deleting a sentinel with a preserved copy for a clean re-dispatch. Both refuse a still-live engine. -- **Spec-approval / epic gate** — reuses the spec viewer (view the finalized spec, - then **Approve & resume**), so the pre-existing sprint-mode gates inherit the same - richer surface. +- **Spec-approval / epic / story gate** — reuses the spec viewer (view the finalized + spec, then **Approve & resume**), so the pre-existing sprint-mode gates inherit the + same richer surface. A story gate fires before the story is recorded, so it has no + spec to show; read its reason — which names the blocking entries and the remedy — in + the run-header banner or the resume confirmation. `p` and `R` overlap for an escalation (both reach Resolve); `p` also exposes Re-arm & resume inline once a resolution exists. Pause badges in the run list and diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index 8574b9fc..aa443652 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -91,6 +91,8 @@ "deferred.closes-unknown", "deferred.closes-malformed", "deferred.closes-entry-unreadable", + "deferred.hard-gate", + "deferred.hard-gate-unstructured", "deferred.ledger-unreadable", } ) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 64b02b70..a345a639 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -341,9 +341,9 @@ def cmd_validate(args: argparse.Namespace) -> int: if paths: if stories_on: _validate_stories_queue(project, paths, spec_folder, dev_trees, report) - _validate_closes_deferred(paths, report, spec_folder=spec_folder) + _validate_deferred_ledger(paths, report, spec_folder=spec_folder) else: - _validate_closes_deferred(paths, report) + _validate_deferred_ledger(paths, report) _validate_operator_registry(project, paths, report) try: ss = sprintstatus.load(paths.sprint_status) @@ -1034,8 +1034,293 @@ def _validate_operator_registry( ) +def _validate_deferred_ledger( + paths: bmadconfig.ProjectPaths, + report: ValidationReport, + *, + spec_folder: str | None = None, +) -> None: + """Read the deferred-work ledger once, then run every check that needs it. + + The read sits here rather than inside each check because an unreadable ledger + is one fault, not one per reader: two checks opening the same file would + report the same outage twice under two ids. + + A ledger that cannot be read at all is reported. Staying quiet there is not + the same trade as staying quiet about an unparseable manifest: the manifest is + already reported by ``queue.stories-manifest``, while nothing else in + ``validate`` reads the ledger, so silence meant reporting success for + preflights that checked nothing. + + The gate runs first for one reason only: an operator who has both a blocked + story and a stale traceability field should meet the refusal before the + advisory. Presentation, nothing more — ``_validate_closes_deferred``'s early + return leaves *that function*, so it could never have skipped a sibling call + here, and swapping the two lines changes no severity and no exit code. + """ + ledger = paths.deferred_work + try: + text = ledger.read_text(encoding="utf-8") if ledger.is_file() else "" + except (OSError, UnicodeDecodeError) as e: + # Split from the manifest read in the checks below, which is silent for a + # good reason that does not apply here: nothing else in `validate` reads + # the ledger, so returning quietly reported success for preflights that + # checked nothing, against the very file the run's closure will fail on + # (#284 round-5 review, finding 6). + # + # A problem rather than a warning, escalated from the severity this id + # carried while the read served only `closes_deferred`. The hard gate now + # rides on the same bytes, and a warning exits 0 having evaluated no gate + # at all — a fail-open on the one deferred check that is a refusal, and one + # that cannot be narrowed by asking whether the project uses gates, because + # the file that would answer is the unreadable one. `Engine._loop` refuses + # the same way for the same reason, so preflight and dispatch agree about + # this file instead of `validate` reporting a run that then pauses at its + # first story. Nothing is lost by failing early: the message's last clause + # is literal — the run's own closure reads this file too. + report.fail( + "deferred.ledger-unreadable", + f"{ledger} cannot be read ({e}) — neither closes_deferred declarations nor " + "`gate:` hard gates were checked against it, so an open entry could be " + "gating an actionable story unseen; the run's own closure will fail the same way", + {"ledger": str(ledger), "error": str(e)}, + ) + return + _validate_hard_gates(paths, text, report, spec_folder=spec_folder) + _validate_closes_deferred(paths, text, report, spec_folder=spec_folder) + + +def _validate_hard_gates( + paths: bmadconfig.ProjectPaths, + text: str, + report: ValidationReport, + *, + spec_folder: str | None = None, +) -> None: + """FAIL when the queue would dispatch a story an unlanded ledger entry gates. + + The preflight half of a two-sided refusal: ``Engine._refuse_gated_story`` + enforces the same gate at dispatch, so a ``run`` that skipped ``validate`` + pauses instead of proceeding. This side exists to move the answer earlier — + the operator learns before the run starts, and learns about *every* gated + story on the queue rather than just the first one picked. + + The two must keep agreeing about what "unlanded" means (only an explicit + ``done`` retires a gate) and about an unreadable ledger (both refuse); a + ``validate`` that passed a run which then paused at its first story would + teach operators to trust neither. + + A ledger entry could always *say* it blocked a story — ``HARD GATE: must land + before 3-2`` in its reason — and saying it stopped nothing. ``run`` took the + story off the board and drove it, and the gate was discovered afterwards, in + the diff of work built on a leg nobody had wired. A ``gate:`` line makes the + claim matchable and this check makes it a refusal. + + The only deferred check that is a gate rather than an advisory, and the + severity is the whole point: the ``closes-*`` siblings describe traceability + that is wrong, which must never block a run, while this one describes work + that must not start, which is exactly what a non-zero exit is for. + + Silent on a ledger nobody has gated, so the zero-config output is unchanged. + Once a gate exists the passing case reports itself — a gate that only ever + speaks when it fires is indistinguishable, on the day it matters, from one + nobody remembered to write. + """ + declared = [(entry, deferredwork.gates(entry)) for entry in deferredwork.parse_ledger(text)] + for entry, entry_gates in declared: + if entry.done: + continue # a landed entry gates nothing; that is what closing it means + _report_unstructured_gate(entry, entry_gates, report) + # Keyed on enforceable tokens, not on `gate:` lines: a ledger whose only gate is + # malformed enforced nothing, and an `ok` there would be the same false all-clear + # the warning above exists to break. Closed entries still count, deliberately — + # the passing case has to keep speaking after the gate lands, or `ok` and "nobody + # ever wrote a gate" become the same silence. Reading the queue sits behind this + # test so a project that gates nothing pays neither the walk nor its failure modes. + if not any(entry_gates.tokens for _, entry_gates in declared): + return + story_keys = _actionable_story_keys(paths, spec_folder) + if story_keys is None: + # The queue could not be read, so nothing was compared. `queue.sprint-status` + # and `queue.stories-manifest` already fail for it; adding an `ok` here would + # say "no story is gated" about a queue this check never saw. + return + gating = [e.id for e, g in declared if not e.done and g.tokens] + gated = False + for entry, entry_gates in declared: + # `done`, not `not open` — the tri-state is the whole point. An entry whose + # status the format cannot read (`status: opne`, or no status line) is not + # evidence the work landed, and skipping it let one typo disable the gate + # *and* emit an `ok` naming the entry as clear. Only an explicit `done` + # retires a gate; everything else holds until someone writes that word. + if entry.done: + continue + for story_key in story_keys: + hits = [t for t in entry_gates.tokens if deferredwork.gates_story(t, story_key)] + if not hits: + continue + gated = True + report.fail( + "deferred.hard-gate", + f"{entry.id} ({entry.title}) {_gate_status_clause(entry)} and gates " + f"{story_key} (gate: {', '.join(hits)}) — that story must not run until " + f"the entry lands. Close it in {paths.deferred_work.name} " + f"(`status: done `), or drop the token from its `gate:` line if it " + f"no longer blocks this work", + { + "dw_id": entry.id, + "title": entry.title, + "story_key": story_key, + "tokens": hits, + }, + ) + if not gated: + report.ok( + "deferred.hard-gate", + f"deferred-work gates OK: no actionable story is gated by an unlanded entry " + f"({', '.join(gating) if gating else 'no unlanded gated entries'})", + {"gating_ids": gating, "actionable": list(story_keys)}, + ) + + +def _gate_status_clause(entry: deferredwork.DWEntry) -> str: + """How the failure names *why* this entry still gates. + + An unreadable status is reported as what it is rather than folded into "is + open": the remedy differs — the operator with a typo fixes the `status:` line, + and telling them the entry "is open" sends them to close work that may already + have landed. Naming the offending value is what makes a one-character typo + findable in a ledger of fifty entries. + """ + if entry.open: + return "is open" + if not entry.status: + return "has no `status:` line, so it cannot be read as landed" + return f"has an unreadable status (`{entry.status}`), so it cannot be read as landed" + + +def _report_unstructured_gate( + entry: deferredwork.DWEntry, + entry_gates: deferredwork.EntryGates, + report: ValidationReport, +) -> None: + """Warn about a hard gate the mechanical check cannot enforce. + + Three causes, one id, because the remedy is the same line in the same file. + A ``HARD GATE:`` written as prose is the pre-``gate:`` convention still + holding nothing back. A token that cannot name a story key, and a ``gate:`` + line with nothing after the colon, are that same nothing with the field's + syntax around it — worse, because to anyone scanning the entry they read as a + gate already in force. + + An entry carrying a valid token *and* an unenforceable one is still reported, + and each cause is reported on its own rather than the first one winning: the + valid half gates what it names, and the operator's belief about the other half + is exactly the thing that goes wrong quietly. That applies to an empty line as + much as to a malformed token — ``gate: 3-2`` followed by a bare ``gate:`` used + to report neither, because the entry had tokens and so read as fully gated. + + A fourth cause, and the only one that is about a line the parser never saw: a + ``gate:`` the strict field anchor misses (``Gate:``, or indented). It is a + warning rather than an accepted gate on purpose — see :data:`_GATE_NEAR_RE`. + + Runs for every entry that is not ``done``, which is the same set the refusal + holds against. Keying it on ``open`` instead would have left an entry with an + unreadable status silent about a malformed token as well as about its gate. + """ + reasons: list[str] = [] + if entry_gates.malformed: + reasons.append( + f"declares `gate:` tokens that cannot name a story: {', '.join(entry_gates.malformed)}" + ) + if entry_gates.empty == 1: + reasons.append("declares an empty `gate:` line, which names no story") + elif entry_gates.empty: + reasons.append(f"declares {entry_gates.empty} empty `gate:` lines, which name no story") + if entry_gates.near_miss: + reasons.append( + f"spells {entry_gates.near_miss} `gate:` line(s) in a form the field does not " + f"read (the field is a lowercase `gate:` at the very start of a line)" + ) + prose_only = not entry_gates.tokens and not reasons + if prose_only and deferredwork.declares_prose_gate(entry): + reasons.append("declares a `HARD GATE:` in prose but carries no `gate:` line") + if not reasons: + return + report.warn( + "deferred.hard-gate-unstructured", + f"{entry.id} ({entry.title}) {' and '.join(reasons)} — so `validate` cannot refuse " + f"the gated story and nothing holds it back; name the blocked stories on a `gate:` " + f"line (comma-separated) to make the gate enforceable", + { + "dw_id": entry.id, + "malformed": list(entry_gates.malformed), + "empty": entry_gates.empty, + "near_miss": entry_gates.near_miss, + }, + ) + + +def _actionable_story_keys( + paths: bmadconfig.ProjectPaths, spec_folder: str | None +) -> list[str] | None: + """The story keys this queue could dispatch, in queue order, in either mode. + + ``None`` when the queue could not be read, which is not the same answer as an + empty list: ``queue.sprint-status`` and ``queue.stories-manifest`` own queue + readability, so this check stays quiet rather than raising — but a caller that + read ``[]`` as "nothing is gated" would report an all-clear about a queue it + never saw. The whole walk is inside the guard for the same reason: the + per-story ``resolve_story_spec`` globs the filesystem too, and leaving it + outside turned a degraded check into a traceback out of ``validate``. + + Stories mode has no status column — the manifest is a flat schedule and the + story's own spec carries the status — so actionability comes from + :func:`stories._classify`, the predicate the scheduler itself picks with. + Dropping only ``done`` is not the same line the sprint board draws: + ``ACTIONABLE_STATUSES`` is a two-element allowlist, so ``blocked`` and the + rest are already out on that side. In stories mode a ``blocked``, sentinel, + ambiguous or unknown-status entry is one :func:`stories.schedule` refuses to + dispatch (``SCHEDULE_WEDGED``), so treating every non-``done`` state as + actionable made ``validate`` exit nonzero over a gate on a story the queue + could not run — and made the two queue modes disagree about what a gate + refuses. + + What is shared is that per-entry predicate and deliberately NOT the scan's + stop rule: ``schedule`` gives up at the FIRST wedged entry, and mirroring that + here would drop every later story from this list. Two reasons not to. A wedge + is a property of some *other* story, and ``run --story `` scans that entry + alone (``selector``), so a later story really is reachable while the wedge + stands — while ``validate`` takes no story selector and so cannot know which + run is coming. And stopping would let one blocked entry near the top of a + manifest silence the gate check for everything below it, which is the failure + this check exists to prevent, arriving by a quieter route than the one it + fixed. Over-reporting a real gate on a story that needs an unrelated + resolution first is the cheaper wrong answer, and dispatch still refuses + independently. + """ + if spec_folder is not None: + keys: list[str] = [] + try: + folder = stories_mod.resolve_spec_folder(paths.project, spec_folder) + for entry in stories_mod.load_stories(folder).entries: + state = stories_mod.resolve_story_spec(folder, entry.id) + if stories_mod._classify(state) != "actionable": + continue + keys.append(entry.id) + except (OSError, UnicodeDecodeError, stories_mod.StoriesError): + return None + return keys + try: + ss = sprintstatus.load(paths.sprint_status) + except (sprintstatus.SprintStatusError, OSError, UnicodeDecodeError): + return None + return [s.key for s in ss.stories if s.status in sprintstatus.ACTIONABLE_STATUSES] + + def _validate_closes_deferred( paths: bmadconfig.ProjectPaths, + text: str, report: ValidationReport, *, spec_folder: str | None = None, @@ -1070,31 +1355,12 @@ def _validate_closes_deferred( close nothing and say nothing. Covering only two of the three left the third to be discovered in the journal after the run it should have preceded. - A ledger that cannot be read at all is reported as well. Staying quiet there - is not the same trade as staying quiet about an unparseable manifest: the - manifest is already reported by ``queue.stories-manifest``, while nothing - else in ``validate`` reads the ledger, so silence meant reporting success for - a preflight that checked nothing. - Never a failure. The annotation is traceability, not a gate, so a stale reference must not be able to block a run that would otherwise start. + ``text`` is the ledger snapshot :func:`_validate_deferred_ledger` already + read; an unreadable ledger never reaches here. """ ledger = paths.deferred_work - try: - text = ledger.read_text(encoding="utf-8") if ledger.is_file() else "" - except (OSError, UnicodeDecodeError) as e: - # Split from the manifest read below, which is silent for a good reason - # that does not apply here: nothing else in `validate` reads the ledger, so - # returning quietly reported success for a preflight that checked nothing, - # against the very file the run's closure will fail on - # (#284 round-5 review, finding 6). - report.warn( - "deferred.ledger-unreadable", - f"{ledger} cannot be read ({e}) — closes_deferred declarations were not " - "checked against it, and the run's own closure will fail the same way", - {"ledger": str(ledger), "error": str(e)}, - ) - return try: sources = ( _stories_declarations(paths, spec_folder) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index c085e3d1..da98933c 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -72,6 +72,95 @@ for polish and nice-to-haves. When a deferred item is later completed, set its `status:` to `done` with the date (e.g. `status: done 2026-06-20`) — do not delete the entry. +## Hard gates: `gate:` + +Some entries are not merely deferred — they **block** specific stories. An +infrastructure leg nobody has wired yet is not a nice-to-have for the first story +that consumes it; that story must not run at all until the entry lands. Say so +with a `gate:` line naming the blocked story keys: + +```markdown +### DW-1: wire the blob-storage credentials + +status: open +gate: 3-2, 3-3 +``` + +`gate:` is optional and most entries have none. Its value is a **comma-separated** +list of story-key tokens; several `gate:` lines in one entry union, so an entry +blocking three stories may list them on one line or on three. A token matches a +story key when it **is** that key, or is its prefix at a key boundary — either a +`-`, or the split-story suffix (one lowercase letter then `-`). So `3-2` gates the +sprint key `3-2-invite-link-student-surface`, the stories-mode id `3-2`, and both +halves of a split (`3-2a-…` / `3-2b-…`), but never `3-20-later-story`. The split +arm matters because breakdown can split a story _after_ the gate was written, and +a gate that quietly stops matching is worse than one that was never there. The +prefix must end at a story **number** for the split arm to apply, so a word id +like `auth` does not gate `authz-login`. + +Like `source_spec:`, a `gate:` line is never edited or dropped when an entry is +otherwise touched: removing it un-gates the story silently, which is the exact +failure this field exists to prevent. + +Until the entry lands, the gate is enforced twice. `bmad-loop validate` **fails** +(`deferred.hard-gate`) for every story a token matches that the queue would +actually dispatch — sprint-status stories at `backlog` / `ready-for-dev`, or +manifest entries whose spec is not yet written or sits at `draft` / +`ready-for-dev` / `in-progress` / `in-review`. A `blocked` manifest entry is not +gated, nor is one the scheduler would stop on anyway (two specs matching one id, +or a skeletal sentinel from a failed planning halt): the queue cannot reach that +story, so a gate refusing it would report work held back that was never going to +run. A `run` that never called `validate` **pauses** (`story-gate`) rather +than dispatch a gated story. Two things clear it: closing the entry +(`status: done `), or removing the token because it no longer blocks that +work. This is the one deferred-work check that gates rather than advises: +everything else here is traceability that may be wrong, while this is work that +must not start. + +**Only an explicit `done` retires a gate.** A status the format cannot read — +`status: opne`, or an entry with no `status:` line — is not evidence the work +landed, so the gate still holds. Write the status word exactly. + +A sweep is never gated by the ledger it is draining, whatever any entry's `gate:` +says: closing the gating entry is what a sweep is for, so gating it would +deadlock the gate against its own remedy. + +**Quoting the field is safe.** A `gate:` line inside a fenced code block is an +example, not a declaration, so an entry that documents this convention gates +nothing. This holds for a whole quoted entry too — heading, `status:` and `gate:` +inside one fence, the shape shown above: the fenced heading starts no entry, and +a quoted heading or bullet does not end the entry that quotes it, so a real +`gate:` below the example keeps gating. One exception worth knowing when you +write an entry: a fence you open and never close is not treated as a fence at +all, because swallowing the rest of the entry could silently disable a real +`gate:` line below it — and, at file scope, hide every entry after it. Close your +fences. + +Four shapes declare a gate that nothing can enforce, and all four are reported as +`deferred.hard-gate-unstructured` while the entry is unlanded: + +- a token nothing can match. It must look like a story key + (`[A-Za-z0-9][A-Za-z0-9._-]*`, no spaces) **and** be a shape a key can actually + take — alphanumeric segments joined by `-`, or a full sprint key. So a + space-separated `gate: 3-2 3-3` is one bad token rather than two good ones, and + `gate: 3.2` / `gate: 3_2` are rejected: no key spells its numbers that way. + Inside a sprint slug those characters are fine — `gate: 3-2-a_b` is a real gate; +- a `gate:` line with nothing usable after the colon (`gate:`, `gate: ,`) — each + such line is reported, including one sitting beside a line that does name a + story, since the half that names nothing is the half you are wrong about; +- a `gate:` that is not lowercase at the very start of its line — `Gate: 3-2`, or + a line that indents `gate: 3-2`. These are reported rather than read as + declarations: the field is a fixed spelling, and guessing at near-misses is how + a line that was never meant to gate ends up refusing a story; +- prose declaring `HARD GATE:` — the convention that predates this field — + anywhere on a line of an entry that carries no `gate:` line. It is matched + mid-line because `reason:` prose is hard-wrapped, but never directly after a + quote character (`"`, `'`, `` ` ``, `«`, or a curly quote): an entry that merely + _cites_ the phrase stays silent, as does one that writes it without the colon. + +Each reads like a gate already in force while holding nothing back. Add or repair +the `gate:` line to make it enforceable. + ## Sweep annotations `bmad-loop sweep` runs (the orchestrator and its bundle dev sessions) add two diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index e28b923e..5ce7fe79 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -13,11 +13,14 @@ import hashlib import re +from bisect import bisect_right from collections.abc import Sequence from dataclasses import dataclass from datetime import date as calendar_date from pathlib import Path +from . import sprintstatus +from .fences import fenced_spans from .platform_util import atomic_write_text HEADING_RE = re.compile(r"^### (DW-\d+): (.+?)\s*$", re.MULTILINE) @@ -33,6 +36,74 @@ _FLAT_SOURCE_BODY = r"source_spec:[ \t]" FLAT_ENTRY_RE = re.compile(rf"^[-*][ \t]+{_FLAT_SOURCE_BODY}", re.IGNORECASE | re.MULTILINE) STATUS_RE = re.compile(r"^status:[ \t]*(.*)$", re.MULTILINE) +# The mechanical half of a hard gate. An entry could always *say* it blocked a +# story — `HARD GATE: must land before 3-2` in the reason line — and saying it +# stopped nothing: the queue picked the story up anyway, and the gate surfaced +# afterwards in the diff of work built on a leg nobody had wired. `gate:` names +# the blocked story keys in a form a check can match, so the claim can refuse. +# Parsed exactly like `status:`: a field line, read inside `parse_ledger`'s +# canonical span, so a line under a flat-append bullet belongs to that block and +# not to the entry above it. +GATE_RE = re.compile(r"^gate:[ \t]*(.*)$", re.MULTILINE) +# A story key as either queue spells one: a sprint key (`3-2-invite-link`), the +# stories-mode id it starts with (`3-2`), or a bare slug. Whitespace and +# separators are deliberately out — a token nothing can match is the same silent +# no-op the field exists to end, so it is surfaced rather than dropped. +GATE_TOKEN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") +# The second half of "can this token gate anything", and a different miss from the +# one above: `GATE_TOKEN_RE` rejects the spellings a *line* cannot carry (a space, +# a bare separator), this rejects the ones no *key* can carry. `gate: 3.2` passes +# the first and can never match `gates_story` against any legal key, so it used to +# report a green `ok` while gating nothing — the field's own silent no-op, one +# keystroke away from the shape that works. +# +# Two arms, because BMAD spells a story key two ways and they are NOT +# interchangeable. A stories-mode id is alphanumeric segments joined by single +# dashes (`_STORIES_ID_RE`), so `3.2` and `3_2` are out. A sprint key's slug is +# unconstrained (`sprintstatus.STORY_RE`'s trailing group), so `3-2-foo.bar` and +# `3-2-a_b` are LEGAL keys that gate correctly — which is why this is a +# whole-token shape test and not a ban on `.`/`_`. Only those characters in the +# *number* prefix are unmatchable; banning them outright would refuse real gates. +# +# Sound in the direction that matters: a token matching either arm is itself a +# legal key, so a story it could gate can exist. `gates_story`'s prefix and split +# arms only ever extend a key rightward past a `-`, and every such prefix of a +# legal key matches one of these arms too. +# +# `sprintstatus` is imported for its regex; `stories.ID_RE` is copied rather than +# imported because `stories` imports *this* module (a cycle). The copy is pinned +# to the original by a drift test rather than to a comment. +_STORIES_ID_RE = re.compile(r"^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$") +# The tokens `gates_story`'s split arm may fire for: a bare `-`, both +# numeric. `sprintstatus.STORY_RE` attaches the split letter straight after the +# story *number*, so that is the only token a split can extend. "Ends in a digit" +# is a weaker test that reads the same shape into a slug — `3-2-v2` would take the +# arm and refuse `3-2-v2a-followup`, a different and legal key. +_SPLITTABLE_TOKEN_RE = re.compile(r"^\d+-\d+$") +# A `gate:` line the strict field pattern above will never see. `GATE_RE` is +# anchored to a lowercase `gate:` in column 0, exactly like `status:`, and that +# strictness fails in opposite directions for the two fields: a missed `status:` +# leaves an entry unresolved, which now gates conservatively, while a missed +# `gate:` leaves no gate at all. `Gate: 3-2` and an indented ` gate: 3-2` are +# therefore surfaced as unenforceable rather than silently absent — and surfaced +# rather than *accepted*, because accepting an indented line would read a fenced +# example inside an entry as a live gate and refuse a story nobody meant to block. +_GATE_NEAR_RE = re.compile(r"^[ \t]*gate[ \t]*:", re.IGNORECASE | re.MULTILINE) +# The prose convention `gate:` replaces, matched anywhere on a line rather than +# at its start: real ledgers hard-wrap their `reason:` prose, so the declaration +# routinely lands mid-line and a line-anchored pattern misses exactly the entries +# that have one. The quote lookbehind is what keeps that from over-firing — an +# entry *citing* the phrase (`names a "HARD GATE: ..."`) is discussion, not a +# declaration — and the colon does the rest of the work, since a sentence about +# "this HARD GATE is textual only" never reaches the pattern at all. +# The class covers the backtick and the curly quotes as well as the ASCII pair: +# a ledger is markdown, so `HARD GATE:` is the citation form an author reaches for +# first, and an LLM-written entry curls its quotes. Missing them made the warning +# fire on entries documenting the convention — including this repo's own docs. +# The lookbehind only reaches an *inline* citation, though; the block form of the +# same quoting is a fence, and no character precedes a line inside one. Callers +# read this through `declares_prose_gate`, which masks those out. +HARD_GATE_PROSE_RE = re.compile(r"""(? bool: return self.status.split()[0] == "open" if self.status else False + @property + def done(self) -> bool: + """Whether the entry has landed. + + Deliberately NOT ``not open``. A status line the format does not + understand — ``status: opne``, or no status line at all — is neither open + nor done, and the readers that ask want *opposite* answers about it: + :func:`open_ids` drops it (it may already be finished), while a gate on it + has to hold (it may not be). Deriving one from the other is what let + ``gate:`` fail open on a one-character typo — the entry read as closed, so + the gate was skipped and ``validate`` reported an all-clear naming it. + """ + return self.status.split()[0] == "done" if self.status else False + + +@dataclass(frozen=True) +class _Examples: + """The ledger's fenced worked examples, indexed for repeated offset queries. + + ``fenced_spans`` returns its ranges in increasing order and non-overlapping (a + fence cannot open inside an open one), so a query is a binary search for the + last span starting at or before the offset. Kept as an index rather than a bare + list because both scales are in play at once: a parse asks once per heading and + several times per entry, so a linear membership test would leave the parse + quadratic whenever a ledger's examples grow with its entries. + """ + + spans: tuple[tuple[int, int], ...] + starts: tuple[int, ...] + + def covers(self, offset: int) -> bool: + i = bisect_right(self.starts, offset) + return i > 0 and offset < self.spans[i - 1][1] + + +def _example_spans(text: str) -> _Examples: + """The ledger's fenced worked examples, as offset ranges. + + Read at WHOLE-FILE scope, which is the whole point. A fence that opens above + a quoted ``### DW-n:`` heading is stranded in the *previous* entry once spans + are carved, so an entry-local query reads the example as live — a phantom + entry whose ``gate:`` refuses a story nobody deferred. `deferred-work-format.md` + ships exactly that shape (a complete entry inside a ```markdown fence), so + quoting it into a ledger is the expected trigger, not a corner case. + + ``unclosed_hides_rest=False`` repeats the answer `gates()` gives one level + down, and here for a stronger reason: under ``True`` a single stray opener + would erase every heading below it, dropping real open work out of + ``open_ids()`` in silence. A phantom entry from an unterminated fence is + today's behaviour and is visible; a vanished ledger is neither. + + Walked once per :func:`parse_ledger` and passed down to the offset checks. The + walk covers the whole file, so recomputing it per offset made the parse + quadratic in the number of entries — and `Engine._refuse_gated_story` re-parses + before every story dispatch, so a mature ledger paid it on the dispatch path. + """ + spans = tuple(fenced_spans(text, unclosed_hides_rest=False)) + return _Examples(spans=spans, starts=tuple(s for s, _ in spans)) + + +def _example(examples: _Examples, offset: int) -> bool: + """Whether ``offset`` sits in a fenced worked example rather than the ledger. + + Takes the index rather than the text: the answer must come from the same + whole-file walk for every offset in one parse, and a signature that re-derived + it per call is what made that expensive enough to matter. + """ + return examples.covers(offset) + + +def _unfenced( + pattern: re.Pattern[str], + text: str, + start: int, + end: int, + examples: _Examples, +) -> re.Match[str] | None: + """First match of ``pattern`` within ``text[start:end]`` that is not quoted. + + Not `search()` plus a check: the first match may be the quoted one, and the + real boundary sits after it. Bounded by ``endpos`` so a match beyond the span + cannot claim it, while ``examples`` still describes fence state from offset 0. + """ + for m in pattern.finditer(text, start, end): + if not _example(examples, m.start()): + return m + return None + def parse_ledger(text: str) -> list[DWEntry]: """Extract DW entries; non-conforming sections are skipped, an entry - without a status line parses with status "" (not open).""" + without a status line parses with status "" (not open). + + Fenced matches are skipped by every scan below, not just the heading one: a + heading or flat bullet quoted inside an example must not start an entry, end + one, or bound a block out of one. Filtering only the headings would trade the + phantom entry for a truncation — a fenced ``## heading`` would still cut a + real entry short at its own boundary, and a `gate:` line below the example + would fall outside the span and stop gating, which is the failure this field + exists to end. + """ entries = [] - headings = list(HEADING_RE.finditer(text)) + examples = _example_spans(text) + headings = [m for m in HEADING_RE.finditer(text) if not _example(examples, m.start())] for i, m in enumerate(headings): end = headings[i + 1].start() if i + 1 < len(headings) else len(text) # an entry also ends at any intervening heading (e.g. a "## Deferred # from:" section header between freeform and DW-format content) - other = ANY_HEADING_RE.search(text, m.end(), end) + other = _unfenced(ANY_HEADING_RE, text, m.end(), end, examples) if other: end = other.start() # ...and at a flat appender block, which belongs to no canonical entry @@ -82,12 +270,18 @@ def parse_ledger(text: str) -> list[DWEntry]: # done (open_ids() drops it, classify() calls it malformed), which trades # one lost flat block for one lost tracked entry. An entry with no status # line has nothing to protect, so the whole span is fair game. - status_m = STATUS_RE.search(text, m.end(), end) - flat = FLAT_ENTRY_RE.search(text, status_m.end() if status_m else m.end(), end) + status_m = _unfenced(STATUS_RE, text, m.end(), end, examples) + flat = _unfenced( + FLAT_ENTRY_RE, text, status_m.end() if status_m else m.end(), end, examples + ) if flat: end = flat.start() body = text[m.start() : end] - status_m = STATUS_RE.search(body) + # Re-read rather than reuse the probe above: `end` may have moved, and the + # status must be the one inside the final span. Searched over `text` at + # absolute offsets because `_example` reads fence state from the top of the + # file — a body slice cannot see an opener that sits above the heading. + status_m = _unfenced(STATUS_RE, text, m.start(), end, examples) entries.append( DWEntry( id=m.group(1), @@ -95,6 +289,10 @@ def parse_ledger(text: str) -> list[DWEntry]: status=status_m.group(1).strip() if status_m else "", body=body, span=(m.start(), end), + status_span=( + (status_m.start() - m.start(), status_m.end() - m.start()) if status_m else None + ), + examples=examples, ) ) return entries @@ -104,6 +302,184 @@ def open_ids(text: str) -> set[str]: return {e.id for e in parse_ledger(text) if e.open} +@dataclass(frozen=True) +class EntryGates: + """One entry's ``gate:`` declaration, split by what a check can act on. + + Every shape that is not an enforceable token is reported by ``validate``, + because none of them is a *weaker* gate than a valid one — each is the prose + gate again wearing the field's clothes, and silence about it is what let the + story run. ``lines`` is what distinguishes "declared nothing usable" from + "declared nothing at all": an entry with no ``gate:`` line has made no claim, + while ``gate:`` with an empty value has made one and inertly. + + ``empty`` counts those inert lines individually rather than folding them into + an entry-wide verdict, because the two coexist: ``gate: 3-2`` followed by a + bare ``gate:`` has both a gate in force and a line that names nothing, and an + aggregate answer can only report one of them. Reporting the tokens and + swallowing the empty line is the worse half to lose — the operator who wrote + it believes a second story is held back. + """ + + tokens: tuple[str, ...] = () + malformed: tuple[str, ...] = () + lines: int = 0 + empty: int = 0 + near_miss: int = 0 + + @property + def inert(self) -> bool: + """Every ``gate:`` line named nothing — ``gate:`` or ``gate: ,`` and no others.""" + return self.lines > 0 and not self.tokens and not self.malformed + + +def _quoted(entry: DWEntry, offset: int) -> bool: + """Whether a BODY-relative ``offset`` sits in a fenced example. + + The single rule every gate scan in this module reads through, so that a fence + means the same thing to all of them: an entry documenting the field quotes it, + and a quoted example is not a declaration. Sharing it is the point — the prose + scan was left on the raw body once, on the reasoning that a warning is cheap + and its quote lookbehind was guard enough. It is not: that lookbehind reaches + an inline citation only, so an entry explaining the old convention in a fenced + block was told to convert a gate it was not declaring. + + Asked at FILE scope, like the heading and status reads in :func:`parse_ledger` + and for the same reason: a body slice cannot see a fence opened above the + heading, so the two views can disagree about the same line. They disagree in + the direction that matters — a stray unclosed ``` above the heading leaves the + entry standing at file scope while the body reads a later matched ``~~~`` pair + as a real fence, masking a live ``gate:`` into an example. A gate lost in + silence is the failure this field exists to end; a spurious refusal in an entry + whose markdown is already malformed is the cheaper wrong answer. + """ + return entry.examples.covers(entry.span[0] + offset) + + +def declares_prose_gate(entry: DWEntry) -> bool: + """Whether the entry declares a gate in the pre-``gate:`` prose convention. + + :data:`HARD_GATE_PROSE_RE` filtered the way every other gate scan here is + filtered. Lives beside them rather than at the caller so the fence rule has + one implementation: ``validate`` is the only reader today, and a second one + reaching for the bare pattern would reintroduce exactly the half-applied rule + this replaced. + """ + return any(not _quoted(entry, m.start()) for m in HARD_GATE_PROSE_RE.finditer(entry.body)) + + +def gates(entry: DWEntry) -> EntryGates: + """Every ``gate:`` token in one entry's canonical span, order-preserving. + + Multiple ``gate:`` lines union: an entry blocking three stories may list them + on one line or on three, and to a line-oriented file neither spelling is the + wrong one. Within a line the separator is a comma, and only a comma — a + space-separated ``gate: 3-2 3-3`` lands in ``malformed`` rather than being + read leniently, so the operator is told the spelling gated nothing instead of + finding out from a story that ran. + + Duplicates collapse (an id repeated across lines is one claim, not two); + empty items drop, so a trailing separator is not a token — but the *line* is + still counted, which is how an all-empty declaration stays reportable. + + ``near_miss`` counts the lines this function deliberately did NOT read as a + declaration: a `gate:` the strict field anchor misses (see + :data:`_GATE_NEAR_RE`). They are counted rather than parsed so the operator is + told the spelling gated nothing — the same trade the space-separated token + makes, one level up. + """ + tokens: list[str] = [] + malformed: list[str] = [] + lines = 0 + empty = 0 + + # Both scans below skip fenced matches: an entry documenting this field quotes + # it, and a quoted example is not a declaration — a fenced `gate: 3-2` sits in + # column 0, right where the anchor looks, and the answer here is a *refusal*. + for m in GATE_RE.finditer(entry.body): + if _quoted(entry, m.start()): + continue + lines += 1 + named = False + for raw in m.group(1).split(","): + token = raw.strip() + if not token: + continue + named = True + bucket = tokens if _matchable_token(token) else malformed + if token not in bucket: + bucket.append(token) + if not named: + empty += 1 + near_miss = sum( + # `^` puts every match at a line start, so this asks whether the same line + # would have satisfied `GATE_RE` — i.e. whether it is the canonical spelling + # already counted above — without re-running the anchor against a slice. + not entry.body.startswith("gate:", m.start()) + for m in _GATE_NEAR_RE.finditer(entry.body) + if not _quoted(entry, m.start()) + ) + return EntryGates( + tokens=tuple(tokens), + malformed=tuple(malformed), + lines=lines, + empty=empty, + near_miss=near_miss, + ) + + +def _matchable_token(token: str) -> bool: + """Whether ``token`` could gate any legal story key — the test that decides + :attr:`EntryGates.tokens` vs :attr:`EntryGates.malformed`. + + Both halves are required and neither implies the other: ``GATE_TOKEN_RE`` + alone admits ``3.2``, which nothing can match, and the key shapes alone admit + ``3-2 3-3`` via the sprint slug, which is one token pretending to be two. + """ + if not GATE_TOKEN_RE.match(token): + return False + return bool(_STORIES_ID_RE.match(token) or sprintstatus.STORY_RE.match(token)) + + +def gates_story(token: str, story_key: str) -> bool: + """Whether ``token`` gates ``story_key``: equal, or its prefix at a key boundary. + + The prefix arm is what lets one token reach both queues — stories mode keys on + the bare id (``3-2``) while sprint mode keys on the full ``3-2-invite-link``, + and an author gating "story 3-2" means the story, not the spelling. The + boundary is required rather than a bare ``startswith`` so ``3-2`` cannot sweep + in its numeric neighbours: ``3-20-later`` is a different story. + + Two boundaries count, because BMAD spells a story key two ways. The plain one + is ``-``. The other is a **split**: ``sprintstatus.STORY_RE`` lets an oversized + story become ``3-2a-...`` / ``3-2b-...`` at breakdown time, and a token that + only knew ``-`` would lose its gate the moment the gated story was split — + silently, which is the worst thing a gate can do. One lowercase ASCII letter + followed by ``-`` is therefore also a boundary. Exactly one letter, and the + ``-`` after it is required, so ``3-2ab-x`` and a bare ``3-2a`` are not swept in. + + The split arm applies only to a token that *is* a bare ``-``, + because that is the only place a split letter can attach: ``STORY_RE`` puts it + straight after the story *number*. Without that guard the arm reads any + trailing letter as a split and gates a story nobody named — ``stories.ID_RE`` + admits word ids, so ``gate: auth`` refused ``authz-login``, and a hard failure + on an unrelated story is the one way this check can be worse than the prose it + replaced. "Ends in a digit" is the same guard written too loosely: the digit + can belong to a *slug*, so ``gate: 3-2-v2`` took the arm and refused + ``3-2-v2a-followup`` — a different, legal key — and ``gate: 3`` refused the + distinct stories id ``3a-task``. + """ + if story_key == token or story_key.startswith(f"{token}-"): + return True + # The `startswith` guard is load-bearing, not redundant with the slice below: + # `story_key[len(token):]` says nothing about what preceded it, so without it + # `3-2` would gate `9-9a-x` on the tail alone. + if not story_key.startswith(token) or not _SPLITTABLE_TOKEN_RE.match(token): + return False + rest = story_key[len(token) :] + return len(rest) >= 2 and "a" <= rest[0] <= "z" and rest[1] == "-" + + def parse_declaration(raw: object) -> tuple[tuple[str, ...], str | None]: """The single reading of a ``closes_deferred:`` declaration (#234), shared by the ``stories.yaml`` parser, the engine's close hook, and ``validate``. @@ -210,9 +586,8 @@ def _find_entry(text: str, dw_id: str) -> DWEntry | None: def _insert_after_status(text: str, entry: DWEntry, line: str) -> str: """Insert a field line right after the entry's status line (or at the end of the entry when no status line exists).""" - status_m = STATUS_RE.search(entry.body) - if status_m: - pos = entry.span[0] + status_m.end() + if entry.status_span: + pos = entry.span[0] + entry.status_span[1] return text[:pos] + "\n" + line + text[pos:] insert_at = entry.span[0] + len(entry.body.rstrip()) return text[:insert_at] + "\n" + line + text[insert_at:] @@ -330,11 +705,10 @@ def _apply_done( entry = _find_entry(text, dw_id) if entry is None or not entry.open: return None - status_m = STATUS_RE.search(entry.body) - assert status_m is not None # open implies a status line - start = entry.span[0] + status_m.start() - end = entry.span[0] + status_m.end() - previous_status_line = status_m.group(0) + assert entry.status_span is not None # open implies a status line + start = entry.span[0] + entry.status_span[0] + end = entry.span[0] + entry.status_span[1] + previous_status_line = entry.body[entry.status_span[0] : entry.status_span[1]] if undo_owner is not None and LINE_BREAK_RE.search(previous_status_line): # An undo marker must never preserve a value that becomes more than one line # under the ledger readers' shared splitlines semantics. Standard closes @@ -453,24 +827,24 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: entry = _find_entry(text, dw_id) if entry is None or entry.open: return False - status_m = STATUS_RE.search(entry.body) - if status_m is None: + if entry.status_span is None: # parse_ledger deliberately tolerates status-less entries. This primitive # is later called from _defer, where an AttributeError would crash the run # instead of completing the deferral. return False + status_line = entry.body[entry.status_span[0] : entry.status_span[1]] try: _require_canonical_status(entry.status) except ValueError: # Only a canonical status written by mark_done is eligible for undo. # Preserve malformed or human-authored statuses for validation/reporting. return False - res_m = _MARK_DONE_TAIL_RE.match(entry.body, status_m.end()) + res_m = _MARK_DONE_TAIL_RE.match(entry.body, entry.status_span[1]) if res_m is None: return False if res_m.group(1).strip() != _one_line(note).strip() or res_m.group(2) != undo_owner: return False - if status_m.group(0) != f"status: done {res_m.group(3)}": + if status_line != f"status: done {res_m.group(3)}": return False try: previous_status_line = bytes.fromhex(res_m.group(4)).decode("utf-8") @@ -482,7 +856,7 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: previous_status = previous_status_m.group(1).strip() if previous_status_m else "" if not previous_status or previous_status.split()[0] != "open": return False - start = entry.span[0] + status_m.start() + start = entry.span[0] + entry.status_span[0] end = entry.span[0] + res_m.end() atomic_write_text(path, text[:start] + previous_status_line + text[end:]) return True @@ -799,11 +1173,23 @@ def _heading_entry(struck: bool, hid: str, rest: str, body: str, section: str) - def parse_legacy(text: str) -> list[LegacyEntry]: - """Extract legacy (non-DW) deferred items. Canonical DW entries are - masked out first, so mixed ledgers parse both ways without overlap.""" + """Extract legacy (non-DW) deferred items. Canonical DW entries and fenced + examples are masked out first, so mixed ledgers parse both ways without + overlap and a quoted example contributes nothing to either reading. + + The fenced half is not symmetry for its own sake. `parse_ledger` used to hand + a quoted example over as a phantom canonical entry, whose span masked the + example here by accident; once it stopped doing that, the same quotation + surfaced on this side instead — a bullet or `### DW-n:` heading inside a fence + read as a legacy finding (#514). + """ masked = text - for e in parse_ledger(text): - s, t = e.span + # `unclosed_hides_rest=False` for the reason the canonical side uses it: one + # stray opener must not blank every legacy finding below it out of view. The + # delimiter lines survive as a lone backtick or tilde plus spaces, which no + # pattern below can start an item on — masking them too made no test disagree. + spans = [e.span for e in parse_ledger(text)] + fenced_spans(text, unclosed_hides_rest=False) + for s, t in spans: masked = masked[:s] + re.sub(r"[^\n]", " ", masked[s:t]) + masked[t:] found: list[tuple[dict, tuple[int, int]]] = [] diff --git a/src/bmad_loop/devcontract.py b/src/bmad_loop/devcontract.py index 8914bbea..3fd16138 100644 --- a/src/bmad_loop/devcontract.py +++ b/src/bmad_loop/devcontract.py @@ -28,6 +28,7 @@ from typing import Any from . import deferredwork +from .fences import fenced as _fenced from .frontmatter import _edit_frontmatter_block, status_of from .platform_util import atomic_replace from .verify import DEV_WORKFLOW, operator_actions_of, read_frontmatter @@ -113,37 +114,6 @@ class AutoRunResult: detail: str # the prose body after the heading, trimmed (human-readable) -# A fence line: up to three spaces of indent, then a maximal run of >= 3 backticks -# or tildes (its char AND length both matter per CommonMark), then the rest of the -# line — an info string on an opener; on a close, only whitespace is allowed. -_FENCE_LINE_RE = re.compile(r"^ {0,3}(`{3,}|~{3,})([^\n]*)$", re.MULTILINE) - - -def _fenced(text: str, offset: int) -> bool: - """True when `offset` falls inside a ``` / ~~~ fenced code block. - - A fence opens on a line of three-or-more backticks or tildes (indentable up - to three spaces; a tab would make an indented code block instead). Per - CommonMark it closes only on a later line using the SAME character, at least - as long as the opener, with no trailing non-whitespace — so a shorter run, a - different fence char, or an info-bearing line inside the block is content, - not a close. Tracking the open fence's char+length (not a bare line-parity - count) is what stops a nested-or-mismatched inner fence from flipping state - early and exposing a quoted `## Auto Run Result` as a real heading — a - destructive misread on the strip path.""" - open_marker: str | None = None - for m in _FENCE_LINE_RE.finditer(text): - if m.start() >= offset: - break - marker, rest = m.group(1), m.group(2) - if open_marker is None: - open_marker = marker # opening fence — an info string is allowed - elif marker[0] == open_marker[0] and len(marker) >= len(open_marker) and not rest.strip(): - open_marker = None # valid closing fence - # else: a shorter / mismatched / info-bearing fence line — literal content - return open_marker is not None - - def _section_headings( text: str, pattern: re.Pattern[str] = AUTO_RUN_HEADING_RE ) -> list[re.Match[str]]: diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index a3726720..934ad7a1 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -42,6 +42,7 @@ PAUSE_EPIC_BOUNDARY, PAUSE_ESCALATION, PAUSE_SPEC_APPROVAL, + PAUSE_STORY_GATE, Phase, RunState, SessionRecord, @@ -832,6 +833,10 @@ def _loop(self) -> None: if story is None: self._maybe_auto_sweep("run-end", "run-end") return + # Before ANY state mutation for this story, and deliberately so — see + # _refuse_gated_story. The story is not in state.tasks yet, so a resume + # re-picks it and re-asks the ledger. + self._refuse_gated_story(story.key) if self.state.current_epic is not None and story.epic != self.state.current_epic: self._epic_boundary(self.state.current_epic, story.epic) self.state.current_epic = story.epic @@ -853,6 +858,117 @@ def _dispatched_count(self) -> int: resume would reset the counter and let the run dispatch past its cap.""" return len(self.state.tasks) + def _refuse_gated_story(self, story_key: str) -> None: + """Pause the run rather than dispatch a story an unlanded ledger entry gates. + + The enforcing half of ``gate:``. ``bmad-loop validate`` refuses the same + story at preflight, but a preflight is only as strong as the operator's + habit — ``run`` never called it, and ``_pick_next`` reads the board alone, + so before this the field's whole promise rested on someone remembering to + type a second command. + + **Placement is load-bearing.** Called from ``_loop`` before + ``state.tasks[key] = task``, so the gated story is *not* recorded as + touched by this run. That is what makes the refusal re-askable: a resume + re-picks the same story and re-reads the ledger, so closing the entry and + resuming runs it. Registering the task first — the obvious placement, next + to ``_run_story`` — would put the key in ``_pick_next``'s ``base_skip``, + and the gate would fire once and then silently retire the story for the + rest of the run and every resume of it. A gate that drops the work it was + protecting is worse than no gate. + + **Pause, not skip.** ``validate`` fails the whole preflight over one gated + story, and the two surfaces have to agree or the operator learns to + distrust both. It raises the reserved :data:`PAUSE_STORY_GATE` stage, which + the TUI already renders and routes to its gate viewer. + + The ledger is re-read here rather than carried from preflight: a sweep (or + a human) may have closed the entry since, and a gate answering from a stale + snapshot would refuse work that has landed. + + **Unreadable ledger pauses too.** Degrading to "not gated" would let the + one deferred check that is a refusal be disabled by a broken file, and the + question "does this project use gates?" is answerable only from the file + that will not open. ``deferred.ledger-unreadable`` is a ``validate`` + problem for the same reason. + + Two exemptions, both deliberate. ``SweepEngine`` overrides ``_loop`` and so + never reaches this call — it must not, because the sweep is the only + automated closer of the gating entry (``sweep.py`` `_close_resolved` / + bundle close), and gating the sweep would deadlock the gate against its own + remedy. And a resumed story *finishes* rather than stranding a half-done + session with a live worktree; the gate applies to work that must not + *start*, which is the same line ``validate`` draws when it passes a story + the board has already finished. + + That second exemption belongs to ``_finish_inflight``'s finishing arms — + the defer replay, the spec-approval continuation, the recorded-session + replay, the commit completion — and not to its restart arm, which finishes + nothing: it discards the worktree (or resets to baseline) and re-runs the + story from scratch. So the restart arm re-asks this gate, and + unconditionally. ``_pick_next`` cannot ask for it, having skipped the key as + touched, and the run's own crash must not be what disables the one deferred + check that refuses. + + Both call sites ask **before** their caller mutates anything, and that is + one rule rather than two coincidences. In ``_loop`` it keeps the refusal + re-askable; in the restart arm it keeps the ledger readable, because that + arm's in-place rollback is ``git reset --hard `` and a gate + committed while the run was down is a commit *after* that baseline. + + Deliberately no "but did a session really run?" test there. Every available + signal is wrong somewhere: ``attempt`` is bumped before the session launches, + ``sessions`` is written only after one returns, and ``rearmed`` covers a + stories-mode wedge (``StoriesEngine._pause_wedged``) that reaches ESCALATED + with no session at all. The arm's own unwinding is the stronger guarantee. + """ + ledger = self.paths.deferred_work + try: + text = ledger.read_text(encoding="utf-8") if ledger.is_file() else "" + except (OSError, UnicodeDecodeError) as e: + self.journal.append("story-gate-unreadable", story_key=story_key, error=str(e)) + reason = ( + f"{ledger} cannot be read ({e}), so the `gate:` hard gates protecting " + f"{story_key} could not be evaluated — fix the file, then " + f"`bmad-loop resume {self.state.run_id}`" + ) + gates.notify(self.policy, self.run_dir, f"story gated: {story_key}", reason) + raise RunPaused(reason, PAUSE_STORY_GATE, story_key) from e + blocking = [ + (entry.id, hits) + for entry, hits in ( + ( + entry, + [ + token + for token in deferredwork.gates(entry).tokens + if deferredwork.gates_story(token, story_key) + ], + ) + # `done`, not `not open`: an entry whose status the format cannot + # read is not evidence the work landed, and reading it as closed + # would let a one-character typo disable the gate. + for entry in deferredwork.parse_ledger(text) + if not entry.done + ) + if hits + ] + if not blocking: + return + named = ", ".join(f"{dw_id} (gate: {', '.join(hits)})" for dw_id, hits in blocking) + reason = ( + f"{story_key} is gated by unlanded deferred work: {named} — close the " + f"entry in {ledger.name} (`status: done `) or run `bmad-loop sweep`, " + f"then `bmad-loop resume {self.state.run_id}`" + ) + self.journal.append( + "story-gated", + story_key=story_key, + dw_ids=[dw_id for dw_id, _ in blocking], + ) + gates.notify(self.policy, self.run_dir, f"story gated: {story_key}", reason) + raise RunPaused(reason, PAUSE_STORY_GATE, story_key) + def _pick_next(self): ss = load_sprint_status(self.paths.sprint_status) if ss.unknown_keys: @@ -1117,6 +1233,30 @@ def _finish_inflight(self) -> None: else: self._finalize_commit_phase(task) else: + # This arm is the one that does not finish work: it discards the + # worktree or resets the tree to baseline and re-runs the story + # from scratch, so what follows is a *start* and gets the same + # question `_loop` asks. Unconditionally — any test for "did a + # session really run?" is wrong somewhere: `attempt` is bumped + # before the session launches, `sessions` is written only after one + # returns, and `rearmed` covers a stories-mode wedge that reached + # ESCALATED with no session at all. + # + # Asked BEFORE the unwinding below, for the same reason `_loop` + # asks before it registers the task: the in-place rollback is + # `git reset --hard `, and a `gate:` committed while the + # run was down lives in a commit *after* that baseline. Rolling + # back first would rewind a tracked ledger and put the question to + # a file the human never wrote — `keep=(".bmad-loop",)` guards only + # untracked deletion, which is exactly why `verify.safe_rollback` + # has to restore `policy.toml` by hand. It also keeps the pause + # honest under the default `rollback_on_failure = false`, where + # `_rollback_or_pause` would otherwise pause for manual recovery + # and never reach the gate. The cost is that a refused isolated + # task keeps its half-built worktree mounted until a resume gets + # past the gate — the same thing an escalation pause does, and the + # cheaper of the two mistakes. + self._refuse_gated_story(task.story_key) self.journal.append( "resume-restart", story_key=task.story_key, phase=str(task.phase) ) diff --git a/src/bmad_loop/fences.py b/src/bmad_loop/fences.py new file mode 100644 index 00000000..a531540c --- /dev/null +++ b/src/bmad_loop/fences.py @@ -0,0 +1,103 @@ +"""Whether a markdown offset sits inside a fenced code block. + +A leaf module with no bmad-loop imports, deliberately: the two readers that need +this — `devcontract` (is a `## Auto Run Result` heading real, or quoted?) and +`deferredwork` (is a `gate:` line a declaration, or an example?) — sit on opposite +sides of an import edge (`devcontract` imports `deferredwork`), so neither can +host it for the other. `devcontract._section_headings` already argued the case in +prose: "a second copy of `_fenced`'s open-marker walk is exactly the kind of +near-duplicate that drifts." This module is that argument taken one step further +once a second subsystem needed the same walk. +""" + +from __future__ import annotations + +import re + +# A fence line: up to three spaces of indent, then a maximal run of >= 3 backticks +# or tildes (its char AND length both matter per CommonMark), then the rest of the +# line — an info string on an opener; on a close, only whitespace is allowed. +FENCE_LINE_RE = re.compile(r"^ {0,3}(`{3,}|~{3,})([^\n]*)$", re.MULTILINE) + + +def _delimits(marker: str, rest: str) -> bool: + """Whether a matched line delimits a fence at all, or is ordinary text. + + CommonMark forbids a backtick anywhere in the info string of a BACKTICK fence, + and only there — the rule exists so that inline code is not read as opening a + block. Tilde fences carry no such restriction. Checked here rather than folded + into `FENCE_LINE_RE` so the pattern stays one readable alternation instead of + two near-identical arms with different info-string classes. + + The miss runs the wrong way for `deferredwork`: a line of prose quoting a + fence opens a block CommonMark never opens, and a real `gate:` above the next + closing run is then read as an example and silently stops gating — the lost + gate the field exists to prevent, not the spurious refusal it tolerates. + """ + return marker[0] == "~" or "`" not in rest + + +def fenced(text: str, offset: int, *, unclosed_hides_rest: bool = True) -> bool: + """True when ``offset`` falls inside a ``` / ~~~ fenced code block. + + A fence opens on a line of three-or-more backticks or tildes (indentable up + to three spaces; a tab would make an indented code block instead). Per + CommonMark it closes only on a later line using the SAME character, at least + as long as the opener, with no trailing non-whitespace — so a shorter run, a + different fence char, or an info-bearing line inside the block is content, + not a close. Tracking the open fence's char+length (not a bare line-parity + count) is what stops a nested-or-mismatched inner fence from flipping state + early and exposing a quoted heading as a real one. + + ``unclosed_hides_rest`` decides the one case CommonMark leaves to the reader: + a fence that opens and never closes. The two callers need opposite answers, + and both are choosing the direction where being wrong is survivable, so this + is a parameter rather than a policy: + + - ``True`` (``devcontract``): everything after the opener is content. Reading + a quoted ``## Auto Run Result`` as a real section is a *destructive* misread + — it strips or terminates a spec — so an ambiguous tail must stay inert. + - ``False`` (``deferredwork``): the opener is ordinary text. A `gate:` line + below a stray fence must keep gating, because a gate lost in silence is the + exact failure that field exists to end; a spurious refusal in an entry whose + markdown is already malformed is the cheaper wrong answer. + """ + return any( + s <= offset < e for s, e in fenced_spans(text, unclosed_hides_rest=unclosed_hides_rest) + ) + + +def fenced_spans(text: str, *, unclosed_hides_rest: bool = True) -> list[tuple[int, int]]: + """Half-open ``[start, end)`` ranges of ``text`` that sit inside a fenced block. + + The walk itself, which `fenced()` reduces to one offset and + `deferredwork.parse_legacy` blanks out wholesale before scanning line by line. + Keeping it here is the point of the module: a reader that needs the ranges and + a reader that needs one answer must not disagree about where a block ends. + + The bounds follow the delimiters' roles rather than their extents. A span opens + one character past the opener's line start, so the opener itself reads as + outside the block — it is markup that a scanner may still want to see. It ends + one character past the closer's line start, which puts the closer *inside*: the + scanners this serves anchor at column 0, and a closing delimiter is the one + line of a block that can never be mistaken for the content it terminates. + """ + spans: list[tuple[int, int]] = [] + open_marker: str | None = None + start = 0 + for m in FENCE_LINE_RE.finditer(text): + marker, rest = m.group(1), m.group(2) + if not _delimits(marker, rest): + continue # inline code, not a fence line + if open_marker is None: + open_marker, start = marker, m.start() + 1 # opener — an info string is allowed + elif marker[0] == open_marker[0] and len(marker) >= len(open_marker) and not rest.strip(): + spans.append((start, m.start() + 1)) # valid closing fence + open_marker = None + # else: a shorter / mismatched / info-bearing fence line — literal content + if open_marker is not None and unclosed_hides_rest: + # Past the last offset, not up to it: an unclosed fence has no end, and + # `fenced()` answered True for an offset at or beyond `len(text)` before + # these ranges existed. Slicing clamps, so a mask is unaffected either way. + spans.append((start, len(text) + 1)) + return spans diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index 7f0ebbb3..a62d62b0 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -52,6 +52,9 @@ class Phase(StrEnum): PAUSE_SPEC_APPROVAL = "spec-approval" PAUSE_EPIC_BOUNDARY = "epic-boundary" PAUSE_ESCALATION = "escalation" +# Raised by Engine._refuse_gated_story: the picked story is named by the `gate:` +# line of a deferred-work entry that has not landed. Produced before the story is +# recorded in state.tasks, so a resume re-picks it and re-reads the ledger. PAUSE_STORY_GATE = "story-gate" # stories-mode HITL checkpoints (independent per story). PLAN fires after a # spec_checkpoint story's plan-halt leg (ready-for-dev, awaiting human plan diff --git a/tests/conftest.py b/tests/conftest.py index 0a86eae3..bf11b295 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -876,6 +876,30 @@ def write_ledger(paths: ProjectPaths, statuses: dict[str, str], commit: bool = T git(paths.project, "commit", "-q", "-m", "ledger") +def write_gated_ledger(paths: ProjectPaths, entries, commit: bool = True) -> None: + """`write_ledger` plus the lines a hard gate is written on: `entries` maps a + DW id to `(status, extra_field_lines)`, appended verbatim after `status:` so a + test can spell a `gate:` line, a prose `HARD GATE:`, or a deliberately broken + one exactly as a human would. + + Committed by default, like `write_ledger` and for the same reason: the engine + paths that dispatch a story need a clean tree. `validate` reads the file + directly and never looks at git, so its callers pass `commit=False` and skip + the git round-trip. + """ + parts = ["# Deferred Work\n"] + for dw_id, (status, extra) in entries.items(): + tail = "".join(f"{line}\n" for line in extra) + parts.append( + f"### {dw_id}: item {dw_id}\n\norigin: test, 2026-06-01\n" + f"location: src.txt:1\nreason: test entry.\nstatus: {status}\n{tail}" + ) + paths.deferred_work.write_text("\n".join(parts), encoding="utf-8") + if commit: + git(paths.project, "add", "-A") + git(paths.project, "commit", "-q", "-m", "ledger") + + def mark_ledger_done(paths: ProjectPaths, dw_ids, date: str = "2026-06-11") -> None: from bmad_loop import deferredwork diff --git a/tests/test_cli.py b/tests/test_cli.py index ad6e6ee0..b0e91986 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -21,6 +21,7 @@ machine_json, mark_ledger_done, spec_path, + write_gated_ledger, write_ledger, write_spec, write_sprint, @@ -3826,13 +3827,20 @@ def test_validate_warns_on_unknown_closes_deferred_in_sprint_mode(project, capsy assert findings[0]["detail"] == {"source": "spec spec-1-1-a.md", "unknown_ids": ["DW-99"]} -def test_validate_warns_when_the_ledger_itself_is_unreadable(project, capsys, monkeypatch): +def test_validate_fails_when_the_ledger_itself_is_unreadable(project, capsys, monkeypatch): """The ledger read shared a `try` with the manifest read, and that arm returns silently — correctly for the manifest, which `queue.stories-manifest` already reports, but nothing else in `validate` reads the ledger. So an unreadable one produced no finding at all: preflight reported success for a check that examined nothing, against the very file the run's closure will fail on - (#284 round-5 review, finding 6).""" + (#284 round-5 review, finding 6). + + A problem, not a warning. A warning exits 0 with the hard gate never evaluated, + which is a fail-open on the one deferred check that refuses — and it cannot be + narrowed by asking whether the project gates anything, because the file that + would answer is the unreadable one. `Engine._refuse_gated_story` pauses on the + same fault, and the two surfaces have to give the same verdict about the same + file.""" install_bmad_config(project) _write_policy(project.project) write_sprint(project, {"1-1-a": "ready-for-dev"}) @@ -3846,10 +3854,15 @@ def test_validate_warns_when_the_ledger_itself_is_unreadable(project, capsys, mo doc = json.loads(capsys.readouterr().out) findings = [f for f in doc["findings"] if f["check"] == "deferred.ledger-unreadable"] assert len(findings) == 1 - assert findings[0]["severity"] == "warning" # advisory: still never a gate + assert findings[0]["severity"] == "problem" # the gate rode on these bytes assert findings[0]["detail"]["ledger"] == str(project.deferred_work) + # the same bytes now back the hard gate, so the message has to say the gate went + # unchecked too — a warning that names only closes_deferred reads as though the + # refusal had run and found nothing + assert "gate:" in findings[0]["message"] and "hard gates" in findings[0]["message"] # and the declaration checks it could not run stay quiet rather than guessing assert not [f for f in doc["findings"] if f["check"] == "deferred.closes-unknown"] + assert not [f for f in doc["findings"] if f["check"].startswith("deferred.hard-gate")] def test_validate_warns_on_a_malformed_closes_deferred_declaration(project, capsys): @@ -3888,6 +3901,514 @@ def test_validate_sprint_mode_silent_without_declarations(project, capsys): assert [f for f in doc["findings"] if f["check"].startswith("deferred.closes")] == [] +def _hard_gate_findings(capsys, check="deferred.hard-gate"): + doc = json.loads(capsys.readouterr().out) + return [f for f in doc["findings"] if f["check"] == check] + + +def _validate_gated_sprint(project, capsys, board, ledger): + """Run validate over a sprint project with `board` on the queue and `ledger` + (write_gated_ledger's shape) on disk; returns the parsed findings.""" + install_bmad_config(project) + _write_policy(project.project) + write_sprint(project, board) + write_gated_ledger(project, ledger, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) # rc varies by host (binary/skills) — parse the document + return json.loads(capsys.readouterr().out)["findings"] + + +def test_validate_fails_when_an_open_entry_gates_an_actionable_story(project, capsys): + """The gate the ledger could only ever *say* before: an entry whose prose read + "HARD GATE: must run before 3-2" stopped nothing, and `run` drove the story on + a leg nobody had wired. With `gate:` the claim is matchable, and unlike every + other deferred check this one is a problem — it describes work that must not + start, which is what a non-zero exit is for.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link-student-surface": "ready-for-dev"}, + {"DW-1": ("open", ["gate: 3-2"])}, + ) + + gates = [f for f in findings if f["check"] == "deferred.hard-gate"] + assert len(gates) == 1 + assert gates[0]["severity"] == "problem" # the one deferred check that gates + assert gates[0]["detail"] == { + "dw_id": "DW-1", + "title": "item DW-1", + "story_key": "3-2-invite-link-student-surface", + "tokens": ["3-2"], + } + # names the entry, the story, the token, and both ways out + assert "DW-1 (item DW-1)" in gates[0]["message"] + assert "3-2-invite-link-student-surface" in gates[0]["message"] + assert "gate: 3-2" in gates[0]["message"] + assert "status: done " in gates[0]["message"] + + +def test_validate_passes_when_the_gated_story_is_already_done(project, capsys): + """A gate is about work that must not *start*. A story the board has already + finished is past the point the entry was protecting, so it reports the passing + case rather than a refusal nobody can act on.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link-student-surface": "done"}, + {"DW-1": ("open", ["gate: 3-2"])}, + ) + + gates = [f for f in findings if f["check"] == "deferred.hard-gate"] + assert len(gates) == 1 and gates[0]["severity"] == "ok" + assert gates[0]["detail"] == {"gating_ids": ["DW-1"], "actionable": []} + + +def test_validate_passes_when_the_gate_entry_is_closed(project, capsys): + """Closing the entry is the primary remedy the failure names, so it has to be + the one that clears it: the same board passes once DW-1 lands.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link-student-surface": "ready-for-dev"}, + {"DW-1": ("done 2026-08-01", ["gate: 3-2"])}, + ) + + gates = [f for f in findings if f["check"] == "deferred.hard-gate"] + assert len(gates) == 1 and gates[0]["severity"] == "ok" + assert gates[0]["detail"]["gating_ids"] == [] + + +def test_validate_ignores_an_unstructured_gate_on_a_closed_entry(project, capsys): + """The `entry.open` skip guards the warning as well as the refusal, and only the + refusal half was pinned — an ablation of the skip left every closed entry + warning about gates that already landed, with nothing red.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + { + "DW-1": ("done 2026-08-01", ["gate: 3-2 3-3"]), + "DW-2": ("done 2026-08-01", ["HARD GATE: must land before 3-2"]), + "DW-3": ("done 2026-08-01", ["gate:"]), + }, + ) + + assert not [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + + +@pytest.mark.parametrize("status", ["opne", "opened", "in progress", ""]) +def test_validate_hard_gate_holds_on_a_status_the_format_cannot_read(project, capsys, status): + """`entry.open` is False for a typo'd status, so the entry was skipped — the + gate silently did not apply, AND the check went on to emit an `ok` naming the + board as clear. One character disabled the refusal and replaced it with an + all-clear, which is the exact silent miss `gate:` exists to end. + + Only an explicit `done` retires a gate; a status the format cannot read is not + evidence the work landed.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": (status, ["gate: 3-2"])}, + ) + + gates = [f for f in findings if f["check"] == "deferred.hard-gate"] + assert len(gates) == 1 and gates[0]["severity"] == "problem" + assert gates[0]["detail"]["story_key"] == "3-2-invite-link" + # and the message sends the operator to the `status:` line rather than telling + # them to close work that may already have landed + assert "cannot be read as landed" in gates[0]["message"] + if status: + assert f"`{status}`" in gates[0]["message"] + + +def test_validate_warns_on_a_gate_line_the_field_anchor_cannot_read(project, capsys): + """`Gate: 3-2` and an indented ` gate: 3-2` produced no finding of any kind — + the field failing open, where a missed `status:` fails closed. Surfaced rather + than accepted: reading an indented line as a declaration would turn a fenced + example inside an entry into a refusal of a story nobody meant to block.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["Gate: 3-2", " gate: 3-3"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + assert unstructured[0]["detail"]["near_miss"] == 2 + assert "the very start of a line" in unstructured[0]["message"] + # nothing enforceable was declared, so there is no passing gate to report either + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + + +def test_validate_does_not_report_an_all_clear_for_an_unmatchable_token(project, capsys): + """`gate: 3.2` is one keystroke from the shape that works and can never match + any key. It used to land in `tokens`, which made the ledger "gated", and the + check then reported a green `ok` — an all-clear earned by a gate that held + nothing. It is a malformed token now, so the operator is told.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["gate: 3.2"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["detail"]["malformed"] == ["3.2"] + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + + +def test_validate_does_not_refuse_a_story_over_a_quoted_gate_example(project, capsys): + """End to end: an entry documenting the field must not refuse the story its + example names. A false refusal wedges a run, which is the one way this check + can be worse than the prose gate it replaced — and the entry most likely to + carry a quoted `gate:` is the one written to explain `gate:`.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["```markdown", "gate: 3-2", "```"])}, + ) + + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + assert not [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + + +def test_validate_hard_gate_token_stops_at_the_key_boundary(project, capsys): + """`3-2` gates the story it names and both halves of that story once breakdown + splits it, and not its numeric neighbours. A bare `startswith` would sweep + `3-20-...` in and block unrelated work; a `-`-only boundary would drop the gate + the moment 3-2 became 3-2a/3-2b, which is the same gate failing silently.""" + findings = _validate_gated_sprint( + project, + capsys, + { + "3-2-invite-link-student-surface": "ready-for-dev", + "3-2a-split-half": "ready-for-dev", + "3-20-later-story": "ready-for-dev", + }, + {"DW-1": ("open", ["gate: 3-2"])}, + ) + + gated = [f["detail"]["story_key"] for f in findings if f["check"] == "deferred.hard-gate"] + assert gated == ["3-2-invite-link-student-surface", "3-2a-split-half"] + + +def test_validate_unions_multiple_gate_lines(project, capsys): + """One entry may block several stories, spelled across lines or on one line. + A line-oriented file gives an author no reason to prefer either, so both read + the same — and each gated story is its own refusal.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev", "4-1-receipts": "backlog"}, + {"DW-1": ("open", ["gate: 3-2", "gate: 4-1, 9-9"])}, + ) + + gated = [f["detail"]["story_key"] for f in findings if f["check"] == "deferred.hard-gate"] + assert gated == ["3-2-invite-link", "4-1-receipts"] # 9-9 is on no board + + +def test_validate_warns_on_a_prose_only_hard_gate(project, capsys): + """The convention `gate:` replaces. A ledger that says HARD GATE in prose is + making a claim nothing enforces, so preflight names it — otherwise the entry + reads, to anyone scanning it, as a gate already in force.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["HARD GATE: must land before story 3-2"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + assert unstructured[0]["detail"] == { + "dw_id": "DW-1", + "malformed": [], + "empty": 0, + "near_miss": 0, + } + assert "`gate:` line" in unstructured[0]["message"] + # nothing enforceable exists, so there is no passing gate to report either + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + + +def test_validate_warns_on_a_mid_line_hard_gate(project, capsys): + """Real ledgers hard-wrap their `reason:` prose, so a declaration routinely + lands mid-line. A line-anchored detector missed exactly the entries that had + one — which is the whole population this warning exists for.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["reason: wired late. HARD GATE: must land before 3-2."])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + + +def test_validate_ignores_a_hard_gate_quoted_in_a_fenced_example(project, capsys): + """The citation an author writes as a block rather than inline. `gates()` + already masks a fenced `gate:` out of the refusal; leaving the prose scan on + the raw body made the entry documenting the migration — the one place both + spellings appear together — warn about its own example.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + { + "DW-1": ( + "open", + ["reason: documents the old convention:", "```", "HARD GATE: before 3-2", "```"], + ) + }, + ) + + assert not [f for f in findings if f["check"].startswith("deferred.hard-gate")] + + +def test_validate_ignores_an_entry_that_only_cites_a_hard_gate(project, capsys): + """An entry *about* the convention is not declaring one — the ledger's own + "no mechanical check enforces a HARD GATE" entry must not warn about itself. + The quote is what separates the two, and a colon-less mention never reaches + the pattern at all.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + { + "DW-1": ("open", ['reason: an entry naming a "HARD GATE: before X" binds nothing.']), + "DW-2": ("open", ["reason: this HARD GATE is textual only, nothing enforces it."]), + }, + ) + + assert not [f for f in findings if f["check"].startswith("deferred.hard-gate")] + + +def test_validate_warns_on_an_empty_gate_line(project, capsys): + """`gate:` with nothing after it is a claim made inertly. It reads, to anyone + scanning the entry, as a gate already in force, and silence would make it + indistinguishable from an entry that never gated anything.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["gate:"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + assert "empty `gate:` line" in unstructured[0]["message"] + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + + +def test_validate_warns_on_a_malformed_gate_token(project, capsys): + """`gate: 3-2 3-3` looks like two tokens and is one that matches nothing. + Reading it leniently would guess at a separator the format never promised, so + it is surfaced instead — a token that gates nothing is the prose gate again, + wearing the field's syntax.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", ["gate: 3-2 3-3"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + assert unstructured[0]["detail"] == { + "dw_id": "DW-1", + "malformed": ["3-2 3-3"], + "empty": 0, + "near_miss": 0, + } + # and it is NOT reported as an enforced gate: nothing matched + assert not [f for f in findings if f["check"] == "deferred.hard-gate"] + + +def test_validate_warns_on_an_empty_gate_line_beside_an_enforced_one(project, capsys): + """The entry gates 3-2 and names nothing on its second line. Reporting only the + token would leave the operator believing both lines hold — the belief the field + exists to end — so the empty line is reported even though the entry is gating.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev", "4-1-billing": "ready-for-dev"}, + {"DW-1": ("open", ["gate: 3-2", "gate:"])}, + ) + + unstructured = [f for f in findings if f["check"] == "deferred.hard-gate-unstructured"] + assert len(unstructured) == 1 and unstructured[0]["severity"] == "warning" + assert "empty `gate:` line" in unstructured[0]["message"] + assert unstructured[0]["detail"]["empty"] == 1 + # the valid half still gates, so this is additive to the refusal, not instead of it + gated = [f for f in findings if f["check"] == "deferred.hard-gate"] + assert [f["severity"] for f in gated] == ["problem"] + assert gated[0]["detail"]["story_key"] == "3-2-invite-link" + + +def test_validate_does_not_gate_a_word_id_that_merely_shares_a_prefix(project, capsys): + """`stories.ID_RE` admits word ids, and the split-story arm used to read the `z` + of `authz-login` as a split letter — FAILING validate for a story nobody gated. + A false refusal wedges a run, which is worse than the prose gate it replaced. + + Stories mode deliberately: a sprint board cannot express this. `authz-login` + does not match `sprintstatus.STORY_RE`, so it never reaches `ss.stories`, and a + sprint fixture stays green with the digit guard ablated — which is exactly how + the first version of this test was written, and it measured nothing.""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + _setup_stories_fixture(project, [_stories_entry("authz-login")]) + write_gated_ledger(project, {"DW-1": ("open", ["gate: auth"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + findings = _hard_gate_findings(capsys) + assert [f["severity"] for f in findings] == ["ok"] + + +def test_validate_silent_when_the_ledger_declares_no_gate(project, capsys): + """Zero-config output stays byte-identical: a project that has never written a + `gate:` line gets no new lines at all, in either direction.""" + findings = _validate_gated_sprint( + project, + capsys, + {"3-2-invite-link": "ready-for-dev"}, + {"DW-1": ("open", []), "DW-2": ("done 2026-08-01", [])}, + ) + + assert not [f for f in findings if f["check"].startswith("deferred.hard-gate")] + + +def test_validate_hard_gate_runs_in_stories_mode(project, capsys): + """Stories mode dispatches manifest ids rather than board keys, and the same + token has to reach both — an epic driven from stories.yaml is exactly where a + gated leg would otherwise be run first and discovered later.""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + _setup_stories_fixture(project, [_stories_entry("1")]) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + findings = _hard_gate_findings(capsys) + assert len(findings) == 1 and findings[0]["severity"] == "problem" + assert findings[0]["detail"]["story_key"] == "1" + + +def test_validate_survives_an_unreadable_story_spec_in_stories_mode(project, capsys, monkeypatch): + """`resolve_story_spec` globs the filesystem per story, so it has to sit inside + the same guard as the manifest read. Outside it, a gated stories-mode project on + a mount that raises turned a degraded advisory into a traceback out of + `validate` — no findings, no JSON, which is worse than the check being skipped.""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + _setup_stories_fixture(project, [_stories_entry("1")]) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1"])}, commit=False) + monkeypatch.setattr( + cli.stories_mod, + "resolve_story_spec", + lambda *a, **k: (_ for _ in ()).throw(OSError("EIO")), + ) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) # must not raise + + # the queue was never read, so the check says nothing rather than an all-clear + assert _hard_gate_findings(capsys) == [] + + +def test_validate_reports_no_gate_all_clear_when_the_queue_is_unreadable(project, capsys): + """`_actionable_story_keys` degrades on an unreadable queue, and an empty list + read as "nothing is gated" produced an `ok` naming the very entry it claimed was + clear. `queue.*` owns the outage; this check must not answer for a queue it + never saw.""" + install_bmad_config(project) + _write_policy(project.project) + project.sprint_status.write_text("development_status: [oh no\n", encoding="utf-8") + write_gated_ledger(project, {"DW-1": ("open", ["gate: 3-2"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + assert _hard_gate_findings(capsys) == [] + + +def test_validate_stories_mode_skips_a_done_story(project, capsys): + """The manifest carries no status — the story's own spec does. Without reading + it, a finished epic would fail validate forever over gates on work that already + landed. `done` is only the clearest case of the general rule the sibling test + pins: actionability is `stories._classify`, not "anything but done".""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + folder = _setup_stories_fixture(project, [_stories_entry("1")]) + (folder / "stories" / "1-slug.md").write_text( + "---\ntitle: 'test'\nstatus: 'done'\n---\n\n## Intent\n\ntest\n", encoding="utf-8" + ) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + findings = _hard_gate_findings(capsys) + assert len(findings) == 1 and findings[0]["severity"] == "ok" + + +@pytest.mark.parametrize("status", ["blocked", "opne"]) +def test_validate_stories_mode_skips_a_story_the_scheduler_would_wedge(project, capsys, status): + """A gate only means something for a story the queue can dispatch. `blocked` + and an unrecognized status both STOP the stories scan (`SCHEDULE_WEDGED`), so + refusing over them made `validate` exit nonzero about a story that could not + move, and made the two queue modes disagree — the sprint arm's + `ACTIONABLE_STATUSES` is a two-element allowlist that already excludes both. + + Parametrized over the two arms `_classify` reaches "wedged" by: a status it + knows and refuses, and one it cannot read at all. A single case would let the + other regress, since only the second depends on the unknown-status branch.""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + folder = _setup_stories_fixture(project, [_stories_entry("1")]) + (folder / "stories" / "1-slug.md").write_text( + f"---\ntitle: 'test'\nstatus: '{status}'\n---\n\n## Intent\n\ntest\n", encoding="utf-8" + ) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + findings = _hard_gate_findings(capsys) + assert len(findings) == 1 and findings[0]["severity"] == "ok" + + +def test_validate_stories_mode_still_gates_a_story_behind_a_wedged_one(project, capsys): + """The deliberate half of the parity, pinned so it is not "fixed" into a + `break`. `schedule()` gives up at the first wedged entry, but preflight shares + its per-entry predicate and NOT its stop rule: story 2 is reachable right now + via `run --story 2`, which scans that entry alone, and `validate` takes no + story selector so it cannot know which run is coming. Stopping here would also + let one blocked entry at the top of a manifest silence the gate check for + every story below it — the silent miss this field exists to end.""" + install_bmad_config(project) + _write_policy(project.project, STORIES_POLICY) + folder = _setup_stories_fixture(project, [_stories_entry("1"), _stories_entry("2")]) + (folder / "stories" / "1-slug.md").write_text( + "---\ntitle: 'test'\nstatus: 'blocked'\n---\n\n## Intent\n\ntest\n", encoding="utf-8" + ) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 2"])}, commit=False) + args = argparse.Namespace(project=str(project.project), spec=None, json=True) + + cli.cmd_validate(args) + + findings = _hard_gate_findings(capsys) + assert len(findings) == 1 and findings[0]["severity"] == "problem" + assert findings[0]["detail"]["story_key"] == "2" + + OPENCODE_QUALIFIED_POLICY = '[adapter]\nname = "opencode"\nmodel = "anthropic/claude-haiku-4-5"\n' diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index e176032a..06a162c5 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -4,7 +4,7 @@ import pytest -from bmad_loop import deferredwork +from bmad_loop import deferredwork, fences from bmad_loop.deferredwork import ( _ISO_DATE_RE, LINE_BREAK_RE, @@ -1529,3 +1529,602 @@ def test_mark_done_many_skips_an_already_done_entry(tmp_path): assert again == [] body = next(e for e in parse_ledger(p.read_text(encoding="utf-8")) if e.id == "DW-1").body assert body.count("resolution: resolved by story 1") == 1 + + +def _gated(*lines: str): + text = ( + "# Deferred Work\n\n### DW-1: gated entry\n\n" + "origin: test\nlocation: n/a\nreason: test\nstatus: open\n" + + "".join(f"{x}\n" for x in lines) + ) + (entry,) = parse_ledger(text) + return deferredwork.gates(entry) + + +def test_gates_unions_every_line_and_splits_on_commas(): + """Several `gate:` lines are one claim, not competing ones: a line-oriented + file gives an author no reason to prefer one line over three. Duplicates + collapse and a trailing separator is not a token.""" + g = _gated("gate: 3-2, 3-3", "gate:\t4-1,3-2,") + + assert g.tokens == ("3-2", "3-3", "4-1") + assert g.malformed == () + assert not g.inert + + +@pytest.mark.parametrize("line", ["gate:", "gate: ", "gate: ,", "gate: , ,"]) +def test_gates_reports_a_line_that_names_nothing(line): + """`gate:` with nothing usable after it is a claim made inertly, and the + parse alone cannot tell it apart from an entry that never gated anything — + `lines` is what keeps it reportable. Left silent, it reads to anyone scanning + the entry as a gate already in force.""" + g = _gated(line) + + assert g.tokens == () and g.malformed == () + assert g.inert + assert g.empty == 1 + + +def test_gates_counts_an_empty_line_beside_a_valid_one(): + """An entry can gate one story and name nothing on the next line. `inert` is an + entry-wide verdict and answers False here — the entry does have a token — so the + empty line needs its own count or the operator who wrote it is never told the + second gate holds nothing back.""" + g = _gated("gate: 3-2", "gate:") + + assert g.tokens == ("3-2",) + assert g.lines == 2 + assert not g.inert # the entry-wide verdict cannot express this case... + assert g.empty == 1 # ...which is why the per-line count exists + + +def test_gates_reports_a_token_that_cannot_name_a_story(): + """The separator is a comma and only a comma. Reading `3-2 3-3` leniently + would guess at one the format never promised, so it lands in `malformed` — + surfaced by validate rather than silently gating nothing.""" + g = _gated("gate: 3-2 3-3, ../etc, 4-1") + + assert g.tokens == ("4-1",) + assert g.malformed == ("3-2 3-3", "../etc") + + +@pytest.mark.parametrize("fence", ["```markdown", "```", "~~~"]) +def test_a_fenced_example_is_not_a_gate_declaration(fence): + """An entry whose subject IS this field quotes it, and a quoted example sits in + column 0 — right where the strict field anchor looks. The sibling + `HARD_GATE_PROSE_RE` already needed quote guards for exactly this (its comment + records the warning firing on entries documenting the convention, this repo's + own docs included), and `gate:` is worse off: the answer here is a refusal, so + an entry explaining the field would fail validate and pause a run.""" + close = "```" if fence.startswith("`") else "~~~" + g = _gated(fence, "gate: 3-2", close) + + assert g.tokens == () and g.near_miss == 0 and g.lines == 0 + + +@pytest.mark.parametrize(("outer", "inner"), [("```", "~~~"), ("~~~", "```")]) +def test_a_stray_opener_above_the_heading_does_not_mask_a_live_gate(outer, inner): + """The two views of the same line, and the reason `_quoted` asks at FILE scope. + + The stray `outer` opener never closes (`inner` is the other fence char and + cannot close it), and at whole-file scope `unclosed_hides_rest=False` reads it + as ordinary text — which is why the heading below it still carves an entry. A + body slice starts at that heading, cannot see the opener, and so reads the + matched `inner` pair as a real fence, masking the `gate:` between them into an + example. That drops a live gate in silence, which is the failure the field + exists to end; `parse_ledger` already reads headings and `status:` at file + scope for exactly this reason. Found by differential fuzz against the + whole-file predicate, not by inspection.""" + text = f"# Deferred Work\n\n{outer}\n### DW-2: title\n{inner}\ngate: 3-2\n{inner}\n" + + (entry,) = parse_ledger(text) + + assert deferredwork.gates(entry).tokens == ("3-2",) + + +def test_a_stray_opener_above_the_heading_does_not_mask_a_prose_gate(): + """The prose scan shares `_quoted`, so it shares the file-scope question too — + pinned separately because the two scans reach it by different call paths.""" + text = "# Deferred Work\n\n```\n### DW-2: title\n~~~\nHARD GATE: before 3-2\n~~~\n" + + (entry,) = parse_ledger(text) + + assert deferredwork.declares_prose_gate(entry) is True + + +def test_a_fence_hides_only_itself(): + """The mask must not reach past the block. A real declaration on either side of + a quoted example still gates — otherwise the fix for a false refusal would have + bought a lost gate, which is the worse of the two.""" + g = _gated("gate: 4-1", "```", "gate: 3-2", "```", "gate: 5-1") + + assert g.tokens == ("4-1", "5-1") + + +def test_an_unclosed_fence_swallows_no_gate(): + """The deliberate asymmetry. Masking an unterminated fence to end-of-entry + would let one stray ``` silently disable every gate below it — the exact + silent miss this field exists to end. A malformed-markdown entry keeping a + readable gate is the cheaper wrong answer.""" + g = _gated("```", "an example nobody closed", "gate: 4-1") + + assert g.tokens == ("4-1",) + + +def test_a_line_with_an_info_string_does_not_close_a_fence(): + """A closer carries no info string (CommonMark), and the rule has to be tested + at EQUAL fence length or the length rule answers first and the assertion + measures nothing. Here every line is a 3-backtick run: without the + info-string requirement, the ```python would close the block early, re-expose + `gate: 4-1`, and leave the trailing ``` opening an unclosed fence.""" + g = _gated("```", "gate: 3-2", "```python", "gate: 4-1", "```") + + assert g.tokens == () + + +def test_a_four_space_backtick_run_is_indented_code_and_cannot_silence_a_gate(): + """CommonMark indents a fence up to three spaces; at four it is indented code, + not a delimiter. `FENCE_LINE_RE`'s ` {0,3}` is the only thing enforcing that, + and it enforces it in the fail-OPEN direction: were the run accepted, two such + lines would wrap a real column-0 `gate:` and mask it out of existence — a gate + lost in silence, the exact failure this field exists to end. The devcontract + half already reasoned this through (reviewer guard #53): the limit is safe + because fenced content in a list is co-indented and can never match a + column-0 anchor, so only the delimiter rule needs pinning.""" + g = _gated(" ```", "gate: 4-1", " ```") + + assert g.tokens == ("4-1",) + + +def test_a_fenced_worked_example_is_not_an_entry(): + """A complete example — heading, status and gate inside one fence — is the + shape `deferred-work-format.md` ships for authors to copy, so a ledger quoting + it is expected rather than exotic. Read entry-locally it used to become a real + entry: `HEADING_RE` split the file first, stranding the opening fence in the + PREVIOUS entry, so the example's body saw no open fence and its `gate:` went + live — a phantom entry refusing a story nobody deferred.""" + text = ( + "# Deferred Work\n\n### DW-01: a real entry\nstatus: open\n\n" + "The format, for reference:\n\n" + "```markdown\n### DW-99: worked example\nstatus: open\ngate: 3-2\n```\n" + ) + + (entry,) = parse_ledger(text) + + assert entry.id == "DW-01" + assert open_ids(text) == {"DW-01"} + assert deferredwork.gates(entry).tokens == () + + +def test_a_fenced_heading_does_not_bound_the_entry_that_quotes_it(): + """The other half of skipping fenced headings, and the one that fails OPEN. + `ANY_HEADING_RE` ends an entry at any intervening heading; left fence-blind it + would end this one at the quoted `### DW-99`, dropping the real `gate:` below + the example out of the span entirely. Trading a phantom entry for a lost gate + would have been the worse of the two bugs.""" + text = ( + "# Deferred Work\n\n### DW-01: a real entry\nstatus: open\n\n" + "```markdown\n### DW-99: worked example\nstatus: done 2026-01-01\n```\n\n" + "gate: 3-2\n" + ) + + (entry,) = parse_ledger(text) + + assert deferredwork.gates(entry).tokens == ("3-2",) + + +def test_a_fenced_flat_bullet_does_not_bound_the_entry_that_quotes_it(): + """Same failure through the #304 flat-appender boundary: a quoted bullet is an + example of the appender's shape, not an appended block, and bounding the entry + at it would again strand the `gate:` below. The real block must still be + bounded out — `test_flat_boundary_still_applies_after_a_quoted_block_inside_the_entry` + holds that end.""" + text = ( + "# Deferred Work\n\n### DW-01: a real entry\nstatus: open\n\n" + "```markdown\n- source_spec: `example.md`\n summary: quoted\n" + " evidence: e\n```\n\ngate: 3-2\n" + ) + + (entry,) = parse_ledger(text) + + assert deferredwork.gates(entry).tokens == ("3-2",) + + +def test_a_stray_unclosed_fence_does_not_erase_the_entries_below_it(): + """Why `_example` asks with `unclosed_hides_rest=False`. Under the opposite + answer one unterminated fence would swallow every heading after it, and those + entries would vanish from `open_ids()` — real open work reported as landed, in + silence. A phantom entry from a stray opener is today's behaviour and is + visible on the page; a disappeared ledger is neither.""" + text = ( + "# Deferred Work\n\n### DW-01: oops\nstatus: open\n```\n\n" + "### DW-02: still real\nstatus: open\ngate: 3-2\n" + ) + + first, second = parse_ledger(text) + + assert (first.id, second.id) == ("DW-01", "DW-02") + assert open_ids(text) == {"DW-01", "DW-02"} + assert deferredwork.gates(second).tokens == ("3-2",) + + +def test_a_fenced_status_line_is_not_the_status_of_the_entry_quoting_it(): + """Skipping fenced headings moves the example INSIDE the quoting entry instead + of splitting it off, which hands `STATUS_RE` a second candidate it never used + to see. An entry with no status of its own must not inherit the example's: + reading `done` there would drop live work out of `open_ids()` on the strength + of a quotation.""" + text = ( + "# Deferred Work\n\n### DW-01: no status of its own\n\norigin: test\n\n" + "```markdown\n### DW-99: worked example\nstatus: done 2026-01-01\n```\n" + ) + + (entry,) = parse_ledger(text) + + assert entry.status == "" + assert not entry.done and not entry.open + + +def test_a_backtick_run_carrying_backticks_is_inline_code_not_a_fence(): + """CommonMark forbids a backtick anywhere in a BACKTICK fence's info string, + exactly so a line of inline code does not open a block. Without the rule this + line opens one, the trailing ``` closes it, and the `gate:` between them reads + as a quoted example — a gate lost in silence, which is the failure this field + exists to end, not the spurious refusal the fenced path is allowed to make.""" + g = _gated("```gate:``` is the field name.", "gate: 3-2", "```") + + assert g.tokens == ("3-2",) + + +def test_a_tilde_fence_may_carry_backticks_in_its_info_string(): + """The other arm of the same rule, and the reason it is not simply "no + backticks in an info string": the restriction is backtick-only, because a + tilde run cannot appear in inline code. Dropping the fence-char test would + leave this example live and refuse a story the entry only documented.""" + g = _gated("~~~ `inline`", "gate: 3-2", "~~~") + + assert g.tokens == () + + +def test_a_fenced_example_is_not_a_legacy_finding_either(): + """The far side of skipping fenced headings (#514). While `parse_ledger` read a + quoted example as a phantom canonical entry, that entry's span masked the + quotation out of this reader by accident; removing the phantom removed the + accident, and the same bullet surfaced here as a legacy finding instead. The + real block below must still parse — over-masking would lose a tracked item, + which is the failure `parse_legacy` exists to prevent.""" + text = ( + "# Deferred Work\n\nThe format, for reference:\n\n" + "```markdown\n### DW-1: wire the blob-storage credentials\nstatus: open\n" + "gate: 3-2\n- source_spec: `example.md`\n summary: quoted\n evidence: e\n```\n\n" + "- source_spec: `real.md`\n summary: real finding\n evidence: e\n" + ) + + (legacy,) = parse_legacy(text) + + assert legacy.title == "real finding" + assert parse_ledger(text) == [] + + +def test_a_stray_unclosed_fence_does_not_hide_legacy_findings_below_it(): + """`parse_legacy` asks `fences.fenced_spans` with `unclosed_hides_rest=False` + for the reason the canonical side does: under the opposite answer one + unterminated fence would blank every finding after it out of the ledger, and + a lost legacy item is as silent as a lost entry.""" + text = "# Deferred Work\n\n```\n\n- source_spec: `real.md`\n summary: real finding\n evidence: e\n" + + (legacy,) = parse_legacy(text) + + assert legacy.title == "real finding" + + +QUOTED_STATUS_LEDGER = """\ +# Deferred Work + +### DW-1: an entry that quotes the format in an example + +summary: shows an operator what an entry looks like +evidence: e + +``` +status: open +``` + +status: open +gate: 3-2 +""" + + +def test_a_close_rewrites_the_live_status_and_not_a_quoted_one(tmp_path): + """The reader picks the status with a fence-aware lookup, so a writer that ran + `STATUS_RE.search(body)` again would pick the *first* raw match — the quoted + one. That split is worse than either half alone: `mark_done_many` reports the + id as closed while `open_ids` still lists it, so a sweep or story close can + never actually close the entry, and a `gate:` it carries refuses its story on + every following pass. Asserted on `open_ids` rather than on the return value, + because the return value is exactly what the bug got right.""" + path = write_ledger(tmp_path, QUOTED_STATUS_LEDGER) + + assert mark_done_many(path, ["DW-1"], "2026-06-11", "fixed") == ["DW-1"] + + text = path.read_text(encoding="utf-8") + assert open_ids(text) == set() + (entry,) = parse_ledger(text) + assert entry.status == "done 2026-06-11" + # the quoted example is documentation, and a close must not edit it + assert "```\nstatus: open\n```" in text + + +def test_a_reopen_restores_the_live_status_of_an_entry_that_quotes_an_example(tmp_path): + """The undo path reads its marker at an offset taken from the status line, so + it has to start from the same line the close wrote. Round-tripped rather than + asserted field-by-field: the close and the reopen must agree about *which* + line they own, and only the round trip pins that they do.""" + path = write_ledger(tmp_path, QUOTED_STATUS_LEDGER) + close_reopenable(path, "DW-1", "fixed") + assert open_ids(path.read_text(encoding="utf-8")) == set() + + assert mark_open(path, "DW-1", "fixed", OPERATION_ID) is True + + assert path.read_text(encoding="utf-8") == QUOTED_STATUS_LEDGER + + +def test_a_decision_lands_after_the_live_status_of_an_entry_that_quotes_an_example( + tmp_path, +): + """`_insert_after_status` is the third writer that used to re-derive the status + line. Inserting after the quoted one would bury the decision inside the fenced + example, where every reader — the parser and the human — treats it as prose.""" + path = write_ledger(tmp_path, QUOTED_STATUS_LEDGER) + + assert append_decision(path, "DW-1", "2026-06-11", "keep", "still worth doing") is True + + text = path.read_text(encoding="utf-8") + assert "```\nstatus: open\n```" in text + assert "status: open\ndecision: 2026-06-11 keep — still worth doing" in text + + +def test_the_example_index_answers_exactly_at_its_span_bounds(): + """The binary search replaced a linear `any(s <= offset < e)` test, and the + whole suite passes with its upper bound moved by one character — no heading or + field line ever begins at that offset, so no behavioural test can reach it. + Pinned directly for the same reason a differential fuzz found it and the tests + did not: an index that is right about every real offset and wrong about the + boundary is one refactor away from being wrong about a real one.""" + text = "before\n```\nquoted\n```\nafter\n" + examples = deferredwork._example_spans(text) + + ((start, end),) = examples.spans + assert examples.covers(start - 1) is False + assert examples.covers(start) is True + assert examples.covers(end - 1) is True + assert examples.covers(end) is False + # and the index must not answer for a ledger that quotes nothing + assert deferredwork._example_spans("### DW-1: t\nstatus: open\n").covers(0) is False + + +def test_parse_ledger_walks_the_fences_once_however_many_entries(monkeypatch): + """Asserted as a call count, not a duration: the property is that the fence + walk is hoisted out of the per-offset checks, and a timing threshold would + both flake and stop meaning that. Reading fence state per offset made + `parse_ledger` quadratic in entries, and `Engine._refuse_gated_story` re-parses + before every story dispatch, so a mature ledger paid it on the dispatch path. + + Both bindings are patched because the module imported the name at import time + (`from .fences import fenced_spans`), so patching only `fences` would miss the + direct call and only `deferredwork` would miss any walk reached via + `fences.fenced`.""" + walks: list[int] = [] + real = fences.fenced_spans + + def counting(text: str, **kw: object): + walks.append(len(text)) + return real(text, **kw) # type: ignore[arg-type] + + monkeypatch.setattr(fences, "fenced_spans", counting) + monkeypatch.setattr(deferredwork, "fenced_spans", counting) + text = "# Deferred Work\n" + "".join( + f"\n### DW-{i}: entry {i}\n\norigin: o\nreason: r\nstatus: open\n" for i in range(1, 26) + ) + + assert len(parse_ledger(text)) == 25 + assert len(walks) == 1 + + +def test_gate_token_shape_copy_agrees_with_the_stories_id_it_mirrors(): + """`_STORIES_ID_RE` is a copy of `stories.ID_RE`, taken because `stories` + imports this module and the reverse would cycle. Pinned to the original rather + than to a comment: if the manifest ever admits a new id shape, a gate on one + would start reporting `malformed` and refuse nothing.""" + from bmad_loop import stories + + assert deferredwork._STORIES_ID_RE.pattern == stories.ID_RE.pattern + + +@pytest.mark.parametrize("token", ["3.2", "3_2", "3.2-invite", "3_2-invite"]) +def test_gates_reject_a_token_no_story_key_can_carry(token): + """Shape-valid and unmatchable. `GATE_TOKEN_RE` admits `.` and `_` because a + sprint slug may contain them, so `gate: 3.2` used to land in `tokens` — where + it matched nothing, gated nothing, and reported a green `ok` for doing so. + A `.`/`_` in the *number* prefix is what no legal key can carry.""" + g = _gated(f"gate: {token}") + + assert g.tokens == () + assert g.malformed == (token,) + + +@pytest.mark.parametrize("token", ["3-2-foo.bar", "3-2-a_b", "authz-login", "3"]) +def test_gates_keep_a_token_a_sprint_slug_can_actually_spell(token): + """The trap in the fix above: `sprintstatus.STORY_RE`'s slug is unconstrained, + so `3-2-foo.bar` and `3-2-a_b` are LEGAL keys that gate correctly. Banning `.` + and `_` outright — the obvious reading of "reject 3.2" — would refuse real + gates, turning a fail-open into a false refusal.""" + g = _gated(f"gate: {token}") + + assert g.tokens == (token,) + assert g.malformed == () + + +@pytest.mark.parametrize("line", ["Gate: 3-2", "GATE: 3-2", " gate: 3-2", "\tgate: 3-2"]) +def test_gates_count_a_line_the_field_anchor_will_never_read(line): + """`GATE_RE` is a lowercase `gate:` in column 0. Every other spelling produced + ZERO findings — no gate, no warning, nothing — which is the field failing open, + where a missed `status:` now fails closed. Counted, not parsed: accepting an + indented line would read a fenced example inside an entry as a live gate.""" + g = _gated(line) + + assert g.tokens == () # deliberately NOT enforced... + assert g.near_miss == 1 # ...but no longer silent + assert g.lines == 0 + + +def test_gates_do_not_count_the_canonical_spelling_as_a_near_miss(): + """The near-miss pattern is a superset of the field pattern, so the canonical + line matches both. Counting it would warn about every gate that works.""" + g = _gated("gate: 3-2") + + assert g.tokens == ("3-2",) and g.near_miss == 0 + + +@pytest.mark.parametrize( + ("status", "is_open", "is_done"), + [ + ("open", True, False), + ("done 2026-08-01", False, True), + ("opne", False, False), # a typo is neither, and must not read as landed + ("", False, False), # no status line at all + ], +) +def test_entry_status_is_a_tri_state_not_a_boolean(status, is_open, is_done): + """`done` is deliberately not `not open`. The readers want opposite answers + about an unreadable status — `open_ids` drops it, a gate on it has to hold — + and deriving one from the other let `status: opne` disable a gate silently.""" + line = f"status: {status}\n" if status else "" + (entry,) = parse_ledger(f"# DW\n\n### DW-1: t\n\norigin: t\nreason: t\n{line}") + + assert entry.open is is_open + assert entry.done is is_done + + +def test_gates_stop_at_the_canonical_span_boundary(): + """A `gate:` line below a flat-append bullet belongs to that block, not to the + entry above it — the same boundary `status:` is read within. Absorbing it would + let an unrelated appended finding block a story nobody gated.""" + text = ( + "# Deferred Work\n\n### DW-1: canonical\n\n" + "origin: test\nlocation: n/a\nreason: test\nstatus: open\n\n" + "- source_spec: `s.md`\n summary: finding\ngate: 3-2\n" + ) + + (entry,) = parse_ledger(text) + + assert deferredwork.gates(entry).tokens == () + + +@pytest.mark.parametrize( + ("token", "story_key", "gated"), + [ + ("3-2", "3-2", True), # stories-mode id: the token IS the key + ("3-2", "3-2-invite-link-student-surface", True), # sprint key: `-` prefix + ("3-2", "3-20-later-story", False), # the boundary the `-` buys + # A split story is still the gated story. STORY_RE lets breakdown turn an + # oversized 3-2 into 3-2a/3-2b, and a token that only knew `-` would lose + # its gate at exactly that moment — silently, which is the one thing a + # gate must never do. + ("3-2", "3-2a-split-half", True), + ("3-2", "3-2b-other-half", True), + ("3-2", "3-2A-upper", False), # the split suffix is lowercase ASCII + ("3-2", "3-2ab-two-letters", False), # exactly one letter, or it is a slug + ("3-2", "3-2a", False), # the `-` after the letter is required + ("3-2", "9-9a-elsewhere", False), # the split arm still needs the prefix + ("3", "3-2-invite-link", True), # a whole epic is a legal token + ("3-2-invite", "3-2-invite-link", True), + ("3-2-invite", "3-2-invited", False), + # The split arm needs the token to end at a story NUMBER, because that is + # the only place STORY_RE can attach a split letter. `stories.ID_RE` admits + # word ids, so without the digit guard the arm read the `z` of `authz` as a + # split and FAILED validate for a story nobody gated — the one way this + # check can be worse than the prose it replaced. + ("auth", "authz-login", False), + ("api", "apis-v2", False), + # ...and "ends in a digit" was that same guard written too loosely: the + # digit can belong to a slug, so the arm read a slug boundary as a split + # and refused keys the entry never named. + ("3-2-v2", "3-2-v2a-followup", False), # `2` closes the slug `v2`, not a story + ("3", "3a-task", False), # a distinct stories id, not a split of `3` + ("3-2-v2", "3-2-v2-followup", True), # the plain `-` arm is untouched by that + ("3-2a", "3-2ab-x", False), # a token already carrying a split letter + ("3-2a", "3-2a-x", True), # ...still gates its own `-` boundary + ("", "a-b", False), # an empty token names nothing, so it gates nothing + ], +) +def test_gates_story_matches_on_key_boundaries(token, story_key, gated): + assert deferredwork.gates_story(token, story_key) is gated + + +@pytest.mark.parametrize( + ("body", "declared"), + [ + ("HARD GATE: must land before 3-2", True), # the bare convention + ("reason: wired late. HARD GATE: must land before 3-2", True), # hard-wrapped prose + ('reason: an entry naming a "HARD GATE: before X" is enforced by nothing', False), + ("reason: an entry naming a 'HARD GATE: before X'", False), + ("reason: «HARD GATE: before X» is only prose", False), + ("reason: this HARD GATE is textual only, nothing enforces it", False), + # A ledger is markdown, so the backtick is the citation form an author + # reaches for first, and an LLM-written entry curls its quotes. Both used + # to warn, so an entry documenting the convention accused itself. + ("reason: a `HARD GATE:` is prose only", False), + ("reason: cites “HARD GATE: before X” only", False), + ("reason: cites ‘HARD GATE: before X’ only", False), + # KNOWN LIMIT, pinned rather than left to surprise someone: the lookbehind + # is one character wide, so a citation that spaces its opening quote off + # the phrase — the French convention, `«` + U+00A0 — still reads as a + # declaration. The remedy for such an entry is a `gate:` line, which + # silences the warning either way. + ("reason: «\u00a0HARD GATE: before X\u00a0» is only prose", True), + ], +) +def test_hard_gate_prose_detects_a_declaration_not_a_citation(body, declared): + """Matched anywhere on a line, because real ledgers hard-wrap `reason:` and the + declaration lands mid-line. The quote lookbehind is what keeps that honest — an + entry *citing* the phrase is discussion — and the colon excludes prose that + merely talks about a hard gate.""" + assert bool(deferredwork.HARD_GATE_PROSE_RE.search(body)) is declared + + +def _prose_gated(*lines: str) -> bool: + text = ( + "# Deferred Work\n\n### DW-1: gated entry\n\n" + "origin: test\nlocation: n/a\nreason: test\nstatus: open\n" + + "".join(f"{x}\n" for x in lines) + ) + (entry,) = parse_ledger(text) + return deferredwork.declares_prose_gate(entry) + + +@pytest.mark.parametrize("fence", ["```markdown", "```", "~~~"]) +def test_a_fenced_prose_gate_is_not_a_declaration(fence): + """The block form of the citation the quote lookbehind already handles inline. + Nothing precedes a line inside a fence, so an entry documenting the old + convention in an example was told to convert a gate it was not declaring — + the same rule `gates()` applies to `gate:`, left half-applied.""" + close = "```" if fence.startswith("`") else "~~~" + body = f"{fence}\nHARD GATE: must land before 3-2\n{close}\n" + + # the pattern itself still matches: the mask is what answers, not a lucky miss + assert deferredwork.HARD_GATE_PROSE_RE.search(body) + assert _prose_gated(fence, "HARD GATE: must land before 3-2", close) is False + + +def test_a_fence_hides_only_the_prose_gate_it_quotes(): + """Masking must not reach past the block, or the fix for a spurious warning + would buy a missed one — an entry that both explains the convention and uses + it is exactly the entry this warning is for.""" + assert _prose_gated("```", "HARD GATE: an example", "```", "HARD GATE: for real") is True + + +def test_an_unclosed_fence_swallows_no_prose_gate(): + """Parity with `gates()`: `unclosed_hides_rest=False`, so one stray ``` cannot + silence every declaration below it.""" + assert _prose_gated("```", "an example nobody closed", "HARD GATE: for real") is True diff --git a/tests/test_engine.py b/tests/test_engine.py index 790a0d43..cd3abf0c 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -26,6 +26,7 @@ review_effect, set_sprint, spec_path, + write_gated_ledger, write_ledger, write_spec, write_sprint, @@ -40,6 +41,7 @@ PAUSE_EPIC_BOUNDARY, PAUSE_ESCALATION, PAUSE_SPEC_APPROVAL, + PAUSE_STORY_GATE, Phase, RunState, SessionRecord, @@ -6451,6 +6453,334 @@ def test_critical_escalation_pauses_and_resume_continues(project): assert resumed.state.finished +def test_dispatch_refuses_a_story_an_unlanded_entry_gates(project): + """The enforcing half of `gate:`. Before this, `_pick_next` read the board + alone: the ledger could say a story was blocked and `run` drove it anyway, and + the gate was discovered afterwards in the diff of work built on a leg nobody + had wired. `validate` refuses the same story, but only if someone ran it.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + engine, adapter = make_engine(project, [dev_effect(project, "1-1-a")]) + + summary = engine.run() + + assert summary.paused + saved = load_state(engine.run_dir) + assert saved.paused_stage == PAUSE_STORY_GATE + assert saved.paused_story_key == "1-1-a" + # the refusal has to precede the work, not follow it + assert adapter.sessions == [] + # ...and precede the *record* of the work: the story is deliberately NOT in + # state.tasks, which is what `_pick_next`'s base_skip keys on. Registering it + # first would fire the gate once and then retire the story for this run and + # every resume of it — a gate that drops the work it was protecting. + assert saved.tasks == {} + events = [e for e in engine.journal.entries() if e["kind"] == "story-gated"] + assert len(events) == 1 and events[0]["dw_ids"] == ["DW-1"] + assert "DW-1" in saved.paused_reason and "bmad-loop sweep" in saved.paused_reason + + +def test_the_gate_still_holds_when_a_resume_has_not_closed_the_entry(project): + """A resume that fixed nothing must not get the story through. + + This is what the placement buys, stated as behavior. Recording the task before + the check — the obvious placement, next to `_run_story` — leaves a non-terminal + task behind, and `_finish_inflight` runs *before* the loop and drives exactly + those: the resume would dispatch the gated story without ever consulting the + ledger again. Refusing before the story is recorded is what makes the gate a + standing condition rather than a one-shot speed bump. + """ + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + engine, _ = make_engine(project, []) + assert engine.run().paused + + resumed, adapter = resume_engine(project, engine, [dev_effect(project, "1-1-a")]) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert adapter.sessions == [] # the entry is still open; nothing may run + assert load_state(resumed.run_dir).paused_stage == PAUSE_STORY_GATE + + +def test_a_gated_story_runs_once_the_entry_lands(project): + """Closing the entry is the primary remedy the pause names, so it has to be + the one that clears it — a gate nobody can get past is a wedge, not a gate. + (That the refusal survives a resume which changed nothing is the test above; + this one is the release.)""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + engine, _ = make_engine(project, []) + assert engine.run().paused + + write_gated_ledger(project, {"DW-1": ("done 2026-08-01", ["gate: 1-1"])}) + resumed, _ = resume_engine( + project, + engine, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = resumed.run() + + assert summary.done == 1 and not summary.paused + + +def test_dispatch_gate_holds_on_a_status_the_format_cannot_read(project): + """`status: opne` is not evidence the work landed. The check keys on an + explicit `done` rather than on `not open` precisely so a one-character typo + cannot disable the gate — the silent no-op the whole field exists to end.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("opne", ["gate: 1-1"])}) + engine, adapter = make_engine(project, [dev_effect(project, "1-1-a")]) + + summary = engine.run() + + assert summary.paused + assert load_state(engine.run_dir).paused_stage == PAUSE_STORY_GATE + assert adapter.sessions == [] + + +def test_dispatch_gate_does_not_fire_for_a_story_it_does_not_name(project): + """A false refusal wedges a run, which is worse than the prose gate this + replaced. An entry gating 2-1 must let 1-1-a through untouched.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 2-1"])}) + engine, _ = make_engine( + project, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + + summary = engine.run() + + assert summary.done == 1 and not summary.paused + + +def test_dispatch_pauses_when_the_ledger_cannot_be_read(project, monkeypatch): + """Degrading to "not gated" would let a broken file disable the one deferred + check that refuses, and "does this project use gates?" is answerable only from + the file that will not open. `validate` reports the same fault as a problem.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + write_gated_ledger(project, {"DW-1": ("open", ["gate: 9-9"])}) + engine, adapter = make_engine(project, [dev_effect(project, "1-1-a")]) + fault_read_text(monkeypatch, project.deferred_work) + + summary = engine.run() + + assert summary.paused + saved = load_state(engine.run_dir) + assert saved.paused_stage == PAUSE_STORY_GATE + assert adapter.sessions == [] + assert "cannot be read" in saved.paused_reason + assert [e["kind"] for e in engine.journal.entries()].count("story-gate-unreadable") == 1 + + +def test_resume_re_gates_a_story_registered_but_never_started(project): + """`_loop` saves the task and *then* calls `_run_story`, so a host death in + that window — or anywhere before `_dev_phase`'s advance, which spans the + isolated worktree mount — persists a PENDING task no session ever touched. + `_pick_next` skips it (it is in `base_skip`), so only `_finish_inflight` + drives it, and its restart arm calls `_run_story` directly. A gate that + landed while the run was down would never be asked. + + The exemption below is for work already *in flight*; this task is not. Its + state is byte-identical to one the loop would have re-picked and re-gated a + microsecond earlier, and the run's own crash is not a reason to skip it. + """ + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, _ = make_engine(project, []) + # exactly what _loop persists between `state.tasks[key] = task` and _run_story + engine.state.tasks["1-1-a"] = StoryTask(story_key="1-1-a", epic=1) + engine._save() + # the gate lands while the run is down + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + + resumed, adapter = resume_engine( + project, + engine, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert adapter.sessions == [] + saved = load_state(resumed.run_dir) + assert saved.paused_stage == PAUSE_STORY_GATE + assert saved.paused_story_key == "1-1-a" + + +def test_resume_re_gates_a_story_whose_attempt_never_reached_a_session(project): + """The second window, and why the arm asks unconditionally rather than testing + the attempt counter: `_dev_phase` persists `attempt == 1` with the DEV_RUNNING + advance, but the session does not launch until `adapter.run()` — past + `_restore_patch`, the prompt build and the pre_session plugin gate, any of which + can be slow or can pause. A host death in there records an attempt no session + ever backed, and the restart arm rolls the task back and re-runs it from + scratch, so this is a start too.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, _ = make_engine(project, []) + # attempt counted, no session record: the state _dev_phase saves before launch + engine.state.tasks["1-1-a"] = StoryTask( + story_key="1-1-a", epic=1, phase=Phase.DEV_RUNNING, attempt=1 + ) + engine._save() + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + + resumed, adapter = resume_engine( + project, + engine, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert adapter.sessions == [] + assert load_state(resumed.run_dir).paused_stage == PAUSE_STORY_GATE + + +def test_restart_arm_gate_re_asks_until_the_entry_lands(project): + """The restart arm's refusal must be a standing condition, not a one-shot. + + `_finish_inflight` drives every non-terminal task, and a refused task stays + non-terminal — so each resume re-reads the ledger, and closing the entry is + what releases it. Were the refusal to retire the story instead, the gate would + drop the very work it was protecting; that is the same property the `_loop` + side buys by refusing before it records the task.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, _ = make_engine(project, []) + engine.state.tasks["1-1-a"] = StoryTask(story_key="1-1-a", epic=1) + engine._save() + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + + first, adapter1 = resume_engine(project, engine, [dev_effect(project, "1-1-a")]) + assert first.run().paused and adapter1.sessions == [] + + # a resume that changed nothing must not get the story through + second, adapter2 = resume_engine(project, first, [dev_effect(project, "1-1-a")]) + assert second.run().paused and adapter2.sessions == [] + assert load_state(second.run_dir).paused_stage == PAUSE_STORY_GATE + + # ...and closing the entry releases it, so the gate is not a wedge + write_gated_ledger(project, {"DW-1": ("done 2026-08-01", ["gate: 1-1"])}) + third, _ = resume_engine( + project, + second, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = third.run() + + assert summary.done == 1 and not summary.paused + + +def test_resume_reads_the_gate_before_the_restart_rollback_rewinds_the_ledger(project): + """Order matters, not just placement. The restart arm's in-place rollback is + `git reset --hard `, and `keep=(".bmad-loop",)` guards only untracked + deletion — tracked content under it is reverted anyway, which is why + `verify.safe_rollback` restores `policy.toml` by hand. A tracked ledger has no + such rescue: a `gate:` committed while the run was down lives in a commit + *after* the baseline, so a rollback that ran first would rewind the ledger and + the gate would read a file the human never wrote. Ask before the arm mutates + anything — the same rule `_loop` follows.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", "board") # board predates the baseline + engine, _ = make_engine(project, []) # default test policy: rollback_on_failure=True + baseline = rev_parse_head(project.project) + task = StoryTask(story_key="1-1-a", epic=1, phase=Phase.DEV_RUNNING, attempt=1) + task.baseline_commit = baseline + task.baseline_untracked = [] + engine.state.tasks["1-1-a"] = task + engine._save() + # the gate is committed while the run is down — i.e. after the task's baseline + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + assert rev_parse_head(project.project) != baseline # the gate is a later commit + + resumed, adapter = resume_engine( + project, + engine, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert adapter.sessions == [] + assert load_state(resumed.run_dir).paused_stage == PAUSE_STORY_GATE + + +def test_resume_still_finishes_a_story_whose_session_already_completed(project): + """The other side of the line, and the exemption stated as behavior. It belongs + to `_finish_inflight`'s *finishing* arms, not to every non-terminal task: here + the review session completed and its result is on disk, so the resume replays + that record straight into the decision path. Gating it would abandon a verified + session's work over an entry whose remedy is a later story — and the story is + not starting, it is ending. The restart arm is the opposite case: it discards + the work and re-runs, so it re-asks (the tests above).""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + engine, _ = make_engine( + project, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + post_sessions = [] + original_emit = engine._emit + + def crashing_emit(stage, *args, **kwargs): + if stage == "post_session": + post_sessions.append(stage) + if len(post_sessions) == 2: # the review session's post_session window + raise RuntimeError("host died in the post-session window") + return original_emit(stage, *args, **kwargs) + + engine._emit = crashing_emit + assert engine.run().crashed + assert load_state(engine.run_dir).tasks["1-1-a"].phase == Phase.REVIEW_RUNNING + # the gate lands while the run is down, on a story whose work is already done + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + + resumed, adapter = resume_engine(project, engine, []) + summary = resumed.run() + + assert summary.done == 1 and not summary.paused + assert adapter.sessions == [] # replayed the recorded result; nothing re-run + kinds = [e["kind"] for e in resumed.journal.entries()] + assert "resume-verify" in kinds and "resume-restart" not in kinds + + +def test_resume_re_gates_a_human_armed_re_drive(project): + """A resolved escalation re-drives through the restart arm — the escalated + attempt is rolled back and the story re-runs from scratch — so it is a start, + and the gate is asked. Resolving an escalation is not evidence that the gating + entry landed, and `validate` refuses this story on the same ledger no matter + what its task record remembers; the two surfaces have to agree. + + `rearmed` is also the signal a "has this story ever run a session?" test would + most want to trust, and it cannot be trusted: `StoriesEngine._pause_wedged` + reaches ESCALATED with `attempt == 0` and no session at all, so exempting + re-drives would wave through a wedged story's very first dispatch.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + escalating = SessionResult( + status="completed", + result_json={ + "workflow": "auto-dev", + "escalations": [{"type": "missing-config", "severity": "CRITICAL", "detail": "boom"}], + }, + ) + engine, _ = make_engine(project, [escalating]) + assert engine.run().escalated == 1 + rearm_escalation(engine.run_dir) # the resolve workflow's re-arm step + assert load_state(engine.run_dir).tasks["1-1-a"].attempt == 0 # the confusable state + # a gate lands on the story while the operator is resolving it + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1-1"])}) + + resumed, adapter = resume_engine( + project, + engine, + [dev_effect(project, "1-1-a"), review_effect(project, "1-1-a", clean=True)], + ) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert adapter.sessions == [] # the re-drive is a start, and it was refused + assert load_state(resumed.run_dir).paused_stage == PAUSE_STORY_GATE + + def test_epic_boundary_gate_pause_and_resume(project): write_sprint( project, diff --git a/tests/test_stories_engine.py b/tests/test_stories_engine.py index 7abefa06..103b9d44 100644 --- a/tests/test_stories_engine.py +++ b/tests/test_stories_engine.py @@ -8,7 +8,7 @@ import pytest import yaml -from conftest import attach_profile, git, install_build_auto_skill, write_spec +from conftest import attach_profile, git, install_build_auto_skill, write_gated_ledger, write_spec from bmad_loop.adapters.base import SessionResult from bmad_loop.adapters.mock import MockAdapter @@ -25,6 +25,7 @@ PAUSE_PLAN_CHECKPOINT, PAUSE_SPEC_APPROVAL, PAUSE_STORY_CHECKPOINT, + PAUSE_STORY_GATE, Phase, RunState, StoryTask, @@ -1102,6 +1103,43 @@ def test_blocked_resolve_rearm_then_redispatch_to_done(project): ] +def test_resolved_wedge_is_still_gated_on_redispatch(project): + """The state that makes a "has this story ever run?" test unbuildable, and so + the reason `_finish_inflight`'s restart arm asks the gate unconditionally. + + `_pause_wedged` records an ESCALATED task *before any session runs this pick*: + `attempt == 0`, no session records, and after `resolve` also `rearmed`. Every + signal that would exempt a re-drive is therefore set on a story whose first + dispatch has not happened — so exempting re-drives would wave a wedged story + straight past a gate that landed while the run was down. Story 1's re-dispatch + is a start like any other, and story 2 must not be leapfrogged either: the gate + pauses the run rather than skipping the story, exactly as `validate` fails the + whole preflight.""" + from bmad_loop import runs + + folder = setup_stories(project, [entry("1"), entry("2")]) + write_spec(folder / "stories" / "1-slug.md", "blocked", rev_parse_head(project.project)) + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", "story 1 blocked") + + engine, _ = make_engine(project, []) + assert engine.run().paused + wedged = load_state(engine.run_dir).tasks["1"] + assert wedged.phase == Phase.ESCALATED and wedged.attempt == 0 and not wedged.sessions + + runs.rearm_escalation(engine.run_dir, "1") # human fixed the frozen spec + assert load_state(engine.run_dir).tasks["1"].rearmed # ...and the re-drive is armed + # a gate on story 1 lands while the run is down + write_gated_ledger(project, {"DW-1": ("open", ["gate: 1"])}) + + resumed, radapter = resume_engine(project, engine, [stories_dev_effect(), stories_dev_effect()]) + summary = resumed.run() + + assert summary.paused and summary.done == 0 + assert radapter.sessions == [] + assert load_state(resumed.run_dir).paused_stage == PAUSE_STORY_GATE + + def test_sentinel_rearm_deletes_by_recorded_verdict_e2e(project): """C2 (E2E): a pick-time sentinel wedge records task.sentinel_kind on disk; a subsequent rearm clears the sentinel by that recorded verdict (not the basename) diff --git a/tests/test_sweep.py b/tests/test_sweep.py index f3020cff..7580fbce 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -965,6 +965,50 @@ def test_sweep_happy_path(project): assert "fix both" in intent and "DW-2" in intent and "### DW-3" in intent +def test_sweep_is_exempt_from_the_dispatch_hard_gate(project): + """The sweep must never be gated by the ledger it exists to drain. + + `Engine._refuse_gated_story` refuses a picked story named by an unlanded + `gate:` entry. `SweepEngine` overrides `_loop` and so never reaches that call — + exemption by omission, which is exactly the kind of thing a later refactor + "unifies" away. Gating the sweep would deadlock the gate against its own + remedy: closing DW-1 is what the pause tells the operator to run a sweep for, + and here DW-1 gates the sweep's own unit keys. + + Written behaviorally rather than as "the method was not called" so it also + fails if the refusal arrives by some other route. + """ + paths = project + paths.deferred_work.write_text( + "# Deferred Work\n\n" + "### DW-1: item DW-1\n\norigin: test, 2026-06-01\nlocation: src.txt:1\n" + "reason: test entry.\nstatus: open\ngate: sweep-triage, dw-fix-things\n", + encoding="utf-8", + ) + git(paths.project, "add", "-A") + git(paths.project, "commit", "-q", "-m", "ledger") + plan = triage_result( + ["DW-1"], + bundles=[{"name": "fix-things", "dw_ids": ["DW-1"], "intent": "fix it"}], + ) + engine, _ = make_sweep( + project, + [ + triage_effect(plan), + bundle_dev_effect(project, "fix-things", ["DW-1"]), + bundle_review_effect(project, "fix-things"), + ], + ) + + summary = engine.run() + + assert not summary.paused, "the sweep must not be gated by the ledger it drains" + assert engine.state.tasks["sweep-triage"].phase == Phase.DONE + assert engine.state.tasks["dw-fix-things"].phase == Phase.DONE + # and the gating entry is closed — the remedy the story-gate pause points at + assert ledger_entries(project)["DW-1"].status.startswith("done") + + def test_generic_skill_bundle_orchestrator_closes_ledger(project): """B4: on the generic bmad-dev-auto path the bundle session never edits the ledger; the orchestrator marks each owned dw id done only after the dev attempt