feat: story-declared deferred-work closure — the advisory contract (#234)

CHANGELOG.md

@@ -9,6 +9,31 @@ breaking changes may land in a minor release.
 
 ### Added
 
+- **Stories can close deferred-work entries (#234).** The ledger was one-way: entries are filed
+  automatically but were only ever marked resolved by hand. A story can now declare the entries
+  its work closes — `closes_deferred: [DW-5, DW-6]`, on its `stories.yaml` entry (stories mode)
+  or in the story spec's frontmatter, the two unioned — and when the story commits, the
+  orchestrator flips each declared entry to `status: done <date>` + `resolution: resolved by
+story <id>`, the same annotation a sweep bundle writes. Both sprint and stories mode.
+
+  Advisory by contract: the annotation is written at the commit boundary — behind every verify
+  gate, checkpoint and review cycle, just before the squash — so an in-repo ledger carries it in
+  the story's own commit (worktree isolation included) and a story that fails, is rejected by
+  review, or escalates closes nothing; a commit that then fails takes the annotation back with
+  it. The declaration is re-read at that boundary, so a late edit counts in both directions, and
+  an id already `done` is a silent no-op across a resume. Nothing about it can fail a story:
+  unknown ids, duplicate ids, unreadable entry statuses, an unreadable ledger location and a
+  non-list spec declaration are journaled warnings (a non-list `closes_deferred` in `stories.yaml`
+  is a manifest schema error like any other). A ledger outside the repo is written at the same
+  moment; its annotation is part of no commit, which is journaled
+  (`deferred-close-external-ledger`).
+
+  `bmad-loop validate` warns up front in both queue modes (`deferred.closes-unknown`,
+  `deferred.closes-malformed`, `deferred.closes-entry-unreadable`, `deferred.ledger-unreadable`);
+  warnings never change the exit code. The field is human-authored, like `spec_checkpoint` — no
+  upstream skill emits it yet (BMAD-METHOD#2619 proposes it at breakdown), and re-deriving
+  `stories.yaml` drops it unless the intent is logged in `.memlog.md`.
+
 - **`review.on_timeout` policy knob (#271).** A timeout-like review verdict (`timeout` /
   `stalled` / `over_budget`) previously always burned a review cycle (RETRY) until
   `max_review_cycles`, then deferred — even when the dev product was already finalized and

README.md

@@ -173,7 +173,20 @@ Turn it on per project with `[stories] source = "stories"` + `spec_folder = "<ep
 - **`spec_checkpoint`** — pause _before_ code, to review the plan. The dev session halts right after planning (`Halt after planning.`) with the spec at `ready-for-dev`; the run pauses at a **plan checkpoint**. Approve to resume straight to implementation, or request a replan (resets the spec to `draft` so the next dispatch re-plans).
 - **`done_checkpoint`** — pause _after_ the story commits, to review the result before the loop moves on (skipped automatically when it is the last story).
 
-A story may set both (it pauses twice); `gates.mode` pauses stack on top. A blocked story escalates exactly as in sprint mode — `bmad-loop resolve`, then re-arm + resume — now with the story's title/description and the blocking condition surfaced; a pre-planning-halt **sentinel** spec is auto-deleted (a copy preserved under the run dir) on re-arm for a clean re-dispatch.
+An entry may also carry **`closes_deferred`** — the `DW-<n>` ledger ids that story's work closes (see [Deferred-work sweeps](#deferred-work-sweeps)):
+
+```yaml
+- id: '3-2'
+  title: Export digests
+  description: …
+  closes_deferred: [DW-5, DW-6]
+```
+
+Declaring it here rather than in the story spec is what lets the annotation happen without touching each generated spec: `bmad-dev-auto` generates the spec and knows nothing of the ledger, whereas the breakdown is authored while the ledger is in view. A spec that _does_ carry the field in frontmatter is honored too — the two are unioned.
+
+Like `spec_checkpoint` and `done_checkpoint`, this is a **human-authored, caller-only field**. Breakdown time, with the ledger open, is where it belongs — but it is not a deadline: the declaration is read when the story commits, so adding one to a story spec's frontmatter mid-run is honored, and withdrawing one before the commit means it is not closed. Upstream Story Breakdown does not emit it yet ([BMAD-METHOD#2619](https://github.com/bmad-code-org/BMAD-METHOD/issues/2619)), and re-deriving `stories.yaml` rewrites the file — so record the intent in `.memlog.md` alongside the story, or the declaration is lost on the next re-derive.
+
+A story may set both checkpoints (it pauses twice); `gates.mode` pauses stack on top. A blocked story escalates exactly as in sprint mode — `bmad-loop resolve`, then re-arm + resume — now with the story's title/description and the blocking condition surfaced; a pre-planning-halt **sentinel** spec is auto-deleted (a copy preserved under the run dir) on re-arm for a clean re-dispatch.
 
 `bmad-loop run --dry-run --spec <folder>` prints the linear schedule (list order, checkpoint markers, live on-disk state); `bmad-loop status` shows the same stories board.
 
@@ -235,6 +248,22 @@ bmad-loop sweep [--no-prompt] [--decisions-only] [--max-bundles N] [--repeat] [-
               `status: done` in the ledger.
 ```
 
+**Closing entries from a story.** Sweeps are not the only way an entry gets closed. A story spec can declare the entries its work closes, in frontmatter:
+
+```yaml
+closes_deferred: [DW-5, DW-6] # DW-<n> ids this story closes
+```
+
+In stories mode the same declaration can live on the `stories.yaml` entry instead (`closes_deferred: [DW-5, DW-6]`), which is where it belongs for an unattended run — the breakdown is authored while the ledger is in view, whereas the story spec is generated later by a dev skill that knows nothing of the ledger. Both channels are unioned. Either way the field is human-authored — like `spec_checkpoint` / `done_checkpoint`, no upstream skill emits it yet, and re-deriving `stories.yaml` drops it unless the intent is logged in `.memlog.md`.
+
+The orchestrator writes the same annotation a bundle close writes — `status: done <date>` and `resolution: resolved by story <id>` — into each declared entry. Without it the ledger is one-way: entries are filed automatically but only ever marked resolved by hand, so a multi-epic run ends with entries that were satisfied epics ago still reading `open`.
+
+**Closure happens at the commit boundary**, after artifact verification, the verify commands, every checkpoint, the review loop and the pre-commit workflows — and just before the story's commit is squashed, so an in-repo ledger carries the annotation in that same commit. A story that fails, blocks, is rejected by review, or escalates closes nothing, and there is nothing to undo. Closure is declared, never inferred from the diff; a resume re-driving the same close is a no-op; and an id that matches no entry is journaled rather than failing the story. The declaration is re-read at that boundary, so an edit made after the story was implemented is the one that counts.
+
+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.
+
+> **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.
 
 Sweeps are their own resumable runs (`bmad-loop resume <id>`). `[sweep] auto` in the policy fires an unattended sweep automatically at epic boundaries or run end; a failed/paused child sweep never interrupts the parent run.

docs/FEATURES.md

@@ -104,6 +104,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se
 ### Deferred-work sweeps
 
 - Skills accumulate an append-only ledger (`deferred-work.md`, `DW-<n>` 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, the orchestrator flips each declared entry to `status: done <date>` + `resolution: resolved by story <id>` — the same annotation a sweep bundle writes — so the ledger stops being one-way (filed automatically, marked resolved by hand). Written at the commit boundary, after verification/review/checkpoints and just before the squash, 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 (a non-list declaration in `stories.yaml` is a manifest schema error like any other, caught before the run). `bmad-loop validate` warns about all of those before the run starts, in both queue modes. An artifact dir configured outside the repo is shared across worktrees and cannot be committed — the annotation is written all the same and journaled (`deferred-close-external-ledger`).
 - `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.

src/bmad_loop/checks.py

@@ -75,6 +75,10 @@
         "skills.stories-dispatch",
         "skills.stories-dispatch-missing",
         "skills.stories-dispatch-stale",
+        "deferred.closes-unknown",
+        "deferred.closes-malformed",
+        "deferred.closes-entry-unreadable",
+        "deferred.ledger-unreadable",
     }
 )
 

src/bmad_loop/cli.py

@@ -20,6 +20,7 @@
     decisions,
     deferredwork,
     envvars,
+    frontmatter,
     gates,
     install,
     machine,
@@ -229,7 +230,9 @@ def cmd_validate(args: argparse.Namespace) -> int:
             _validate_stories_queue(
                 project, paths, spec_folder, [p.skill_tree for p in profiles], report
             )
+            _validate_closes_deferred(paths, report, spec_folder=spec_folder)
         else:
+            _validate_closes_deferred(paths, report)
             try:
                 ss = sprintstatus.load(paths.sprint_status)
                 actionable = [s for s in ss.stories if s.status in sprintstatus.ACTIONABLE_STATUSES]
@@ -603,6 +606,154 @@ def _validate_stories_queue(
     report.extend(stories_probs)
 
 
+def _spec_closes_deferred(path: Path) -> tuple[tuple[str, ...], str | None]:
+    """One story spec's ``closes_deferred:`` declaration, degrading an unreadable
+    spec to an empty one — other gates own spec readability, and an advisory
+    check must never be the thing that crashes the preflight it is advising."""
+    try:
+        raw = frontmatter.read_frontmatter(path).get("closes_deferred")
+    except (OSError, UnicodeDecodeError):
+        return (), None
+    return deferredwork.parse_declaration(raw)
+
+
+def _validate_closes_deferred(
+    paths: bmadconfig.ProjectPaths,
+    report: ValidationReport,
+    *,
+    spec_folder: str | None = None,
+) -> None:
+    """Warn when a story declares ``closes_deferred:`` ids the deferred-work
+    ledger does not carry, or declares them in a shape nothing can read (#234).
+
+    At clean close the orchestrator flips each declared id to ``status: done
+    <date>`` + a ``resolution:`` line. An id that names no entry — a typo, or an
+    entry reworded/renumbered since the spec was written — annotates nothing, and
+    the run says so only in the journal, where nobody looks until the retro that
+    the annotation exists to spare. Saying it at preflight is the whole point:
+    the declaration is fixable before the run, not after.
+
+    Runs in **both** queue modes, because the declaration exists in both. With
+    ``spec_folder`` (stories mode) each manifest entry is checked together with
+    its id-resolved spec, unioned — the manifest half is what makes this genuinely
+    a *pre*-flight, since those ids are readable before the story has ever been
+    dispatched. Without it (sprint mode) the story specs already sitting in the
+    artifacts dir are scanned instead; those are written by `create-story` ahead
+    of the run, which is exactly while a typo is still cheap to fix. Reporting
+    only in stories mode left sprint-mode operators with nothing but a journal
+    line after the fact.
+
+    An id present but already ``done`` is a declaration a prior run already
+    satisfied (a resume re-drives the same close), so it stays silent — the same
+    classification the engine's close hook makes, for the same reason. Everything
+    else it can journal is reported here: an absent id, an id whose ledger entry
+    carries neither an ``open`` nor a ``done`` status (nothing will be marked, and
+    the remedy is in the ledger rather than in the declaration), and a
+    wrong-container declaration, which names real intent that would otherwise
+    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.
+    """
+    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)
+            if spec_folder is not None
+            else _sprint_declarations(paths)
+        )
+    except (OSError, UnicodeDecodeError, stories_mod.StoriesError):
+        # An unparseable manifest is already a `queue.stories-manifest` failure from
+        # the gate above — don't double-report it.
+        return
+    for label, ids, error in sources:
+        if error:
+            report.warn(
+                "deferred.closes-malformed",
+                f"{label} declares closes_deferred in a shape that cannot be read: "
+                f"{error} — nothing will be marked resolved for it",
+                {"source": label, "error": error},
+            )
+        declared = deferredwork.classify(text, ids)
+        if declared.malformed:
+            malformed = list(declared.malformed)
+            report.warn(
+                "deferred.closes-entry-unreadable",
+                f"{label} declares closes_deferred ids whose {ledger.name} entries carry "
+                f"neither an `open` nor a `done` status: {', '.join(malformed)} — nothing "
+                "will be marked resolved for them until the entry status is repaired",
+                {"source": label, "dw_ids": malformed},
+            )
+        if declared.unknown:
+            unknown = list(declared.unknown)
+            report.warn(
+                "deferred.closes-unknown",
+                f"{label} declares closes_deferred ids that are not in "
+                f"{ledger.name}: {', '.join(unknown)} — nothing will be marked "
+                f"resolved for them (typo, or a renumbered/reworded entry?)",
+                {"source": label, "unknown_ids": unknown},
+            )
+
+
+def _stories_declarations(
+    paths: bmadconfig.ProjectPaths, spec_folder: str
+) -> list[tuple[str, tuple[str, ...], str | None]]:
+    """Per manifest entry: the union of its ``stories.yaml`` ids and its
+    id-resolved spec's, deduped across both channels."""
+    folder = stories_mod.resolve_spec_folder(paths.project, spec_folder)
+    out = []
+    for entry in stories_mod.load_stories(folder).entries:
+        ids = list(entry.closes_deferred)
+        error = None
+        state = stories_mod.resolve_story_spec(folder, entry.id)
+        if state.kind == stories_mod.KIND_PRESENT and state.path is not None:
+            spec_ids, error = _spec_closes_deferred(state.path)
+            ids += spec_ids
+        out.append((f"story {entry.id}", tuple(dict.fromkeys(ids)), error))
+    return out
+
+
+def _sprint_declarations(
+    paths: bmadconfig.ProjectPaths,
+) -> list[tuple[str, tuple[str, ...], str | None]]:
+    """Sprint mode has no manifest, so the declarations that exist yet are the
+    ones in story specs already on disk — the flat ``*.md`` layout the artifacts
+    dir holds."""
+    impl = paths.implementation_artifacts
+    if not impl.is_dir():
+        return []
+    out = []
+    for path in sorted(impl.glob("*.md")):
+        if path == paths.deferred_work:
+            continue
+        ids, error = _spec_closes_deferred(path)
+        if ids or error:
+            out.append((f"spec {path.name}", ids, error))
+    return out
+
+
 def _warn_unknown_keys(ss: sprintstatus.SprintStatus) -> None:
     """Surface sprint-status keys the parser could not classify. Silently
     dropping one reads to the operator as "that story is done, or not mine to