From 29ba68419f330203e3a8cca9699458b14fd2ea58 Mon Sep 17 00:00:00 2001 From: pbean Date: Mon, 6 Jul 2026 13:18:38 -0700 Subject: [PATCH] feat(resolve): intent-gap patch-restore re-drive + inline-review-layer preflight Integrate the July 4-6 upstream bmad-dev-auto changes into bmad-loop. Phase A - provisioning + doc alignment: - install: require bmad-review-verification-gap (BMAD-METHOD #2550) and a bmad-dev-auto customize.toml marker in the base-skill preflight; the three inline step-04 review hunters are now all bmm prerequisites - devcontract: refresh the in-review reconcile note (restore path re-arms TO in-review pre-session; reconcile semantics unchanged) - verify: document finalize_commit's intentional residual-artifact squash (#2563) - docs: README skill table, FEATURES review-layer + resolve notes, CHANGELOG Phase B - intent-gap patch-restore re-drive (BMAD-METHOD #2564): A review-stage intent-gap halt now saves the reverted attempt as a patch file. When the attempted reading was correct, bmad-loop resolve re-arms the spec to in-review and re-applies that patch onto the baseline before the re-drive, so the session resumes review (step-04) on the restored diff instead of re-implementing from scratch. - model.StoryTask.restore_patch latch; verify.apply_patch; runs.rearm_escalation restore branch (in-review vs ready-for-dev + restore journal field) - engine: Engine._restore_patch applied in the dev loop gated on feedback-is-None (escalates rather than dispatch on apply failure; cleared on commit) - cli: --restore-patch flag + strict validation + resolution.json passthrough - resolve.read_resolution; bmad-loop-resolve SKILL.md guidance Sweep-workflow extension tracked in #75. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 16 ++ README.md | 21 ++- docs/FEATURES.md | 5 +- src/bmad_loop/cli.py | 54 +++++- .../data/skills/bmad-loop-resolve/SKILL.md | 36 +++- src/bmad_loop/devcontract.py | 13 +- src/bmad_loop/engine.py | 52 +++++- src/bmad_loop/install.py | 27 +-- src/bmad_loop/model.py | 10 ++ src/bmad_loop/resolve.py | 15 ++ src/bmad_loop/runs.py | 40 +++-- src/bmad_loop/verify.py | 28 ++++ tests/test_cli.py | 88 ++++++++++ tests/test_engine.py | 158 ++++++++++++++++++ tests/test_install.py | 25 ++- tests/test_model.py | 11 ++ tests/test_runs.py | 63 +++++++ tests/test_verify.py | 43 +++++ 18 files changed, 663 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff2b2c..24eafbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to `bmad-loop` are documented here. The format is based on [Semantic Versioning](https://semver.org/spec/v2.0.0.html). While the project is pre-1.0, breaking changes may land in a minor release. +## [Unreleased] + +### Added + +- **Intent-gap patch-restore recovery.** When review halts on an `intent gap`, `bmad-dev-auto` + now saves the attempted change as a patch before reverting (BMAD-METHOD#2564). If the attempted + reading was correct, `bmad-loop resolve` re-arms the spec to `in-review` and re-applies the patch + onto baseline after every reset, so the re-driven session resumes review on the restored diff + instead of re-implementing. New `--restore-patch ` flag for the `--no-interactive` path; a + patch that fails to apply escalates instead of running on a half-restored tree. +- **Preflight covers the inline review layers.** `bmad-loop validate` (and run-start) now require the + three upstream review-hunter skills `bmad-dev-auto`'s step-04 invokes — `bmad-review-adversarial-general`, + `bmad-review-edge-case-hunter`, and `bmad-review-verification-gap` (new in BMAD-METHOD#2550) — plus a + `customize.toml` in `bmad-dev-auto` (its review-layer config, BMAD-METHOD#2535/#2550). A pre-July bmm + install missing any is reported with remediation before a run stalls. + ## [0.8.1] — 2026-07-05 ### Fixed diff --git a/README.md b/README.md index 57f7066..b679f41 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Inspired by the original [bmad-automator](https://github.com/bmad-code-org/bmad- - **Python 3.11+**, **tmux**, and a supported coding CLI — `claude` by default; `codex` and `gemini` via [profiles](#other-coding-clis). - **Linux or macOS** (or **Windows via WSL**, which _is_ Linux — it runs as-is). tmux is the one terminal-multiplexer backend today, but it now sits behind a pluggable **registry** of OS seams (transport, process lifecycle, hook interpreter), so a native-Windows backend slots in as new files + a registration line each, with no engine edits — see [Porting bmad-loop to a new OS](docs/porting-to-a-new-os.md). Native Windows is not yet shipped. -- A **BMAD v6 project** (`_bmad/bmm/config.yaml`, a `sprint-status.yaml` from `bmad-sprint-planning`) with the upstream `bmad-dev-auto` skill and the bmad-loop skill module from this repo installed (`bmad-loop-resolve`, `bmad-loop-sweep` — see [Installing the skill module](#installing-the-skill-module)). Standard BMAD skills stay untouched. +- A **BMAD v6 project** (`_bmad/bmm/config.yaml`, a `sprint-status.yaml` from `bmad-sprint-planning`) with the upstream `bmad-dev-auto` skill (and the three review-hunter skills its step-04 invokes inline: `bmad-review-adversarial-general`, `bmad-review-edge-case-hunter`, `bmad-review-verification-gap`) and the bmad-loop skill module from this repo installed (`bmad-loop-resolve`, `bmad-loop-sweep` — see [Installing the skill module](#installing-the-skill-module)). Standard BMAD skills stay untouched. ## Quick start @@ -215,14 +215,19 @@ Bundle dev sessions can themselves append new deferred entries (split-off goals, ## Installing the skill module -The orchestrator drives the upstream `bmad-dev-auto` skill as its inner dev primitive — unmodified, so there is no fork to keep in sync; it both implements and (re-invoked on the done spec) runs the follow-up review — plus its own bundled `bmad-loop-*` skills for escalation, sweep, and setup. Your standard BMAD install is never modified. The three bundled skills ship in the `bmad-loop` wheel (canonical source: `src/bmad_loop/data/skills/`, BMAD module code `bmad-loop`) so `bmad-loop init` lays them down for you; `bmad-dev-auto` is a prerequisite installed by the BMad Method (bmm) module: +The orchestrator drives the upstream `bmad-dev-auto` skill as its inner dev primitive — unmodified, so there is no fork to keep in sync; it both implements and (re-invoked on the done spec) runs the follow-up review — plus its own bundled `bmad-loop-*` skills for escalation, sweep, and setup. Your standard BMAD install is never modified. The three bundled skills ship in the `bmad-loop` wheel (canonical source: `src/bmad_loop/data/skills/`, BMAD module code `bmad-loop`) so `bmad-loop init` lays them down for you; `bmad-dev-auto` and the three review-hunter skills its step-04 invokes inline are prerequisites installed by the BMad Method (bmm) module: -| Skill | Role | -| ------------------- | ------------------------------------------------------------------------------------------- | -| `bmad-dev-auto` | unattended implementation + follow-up review (**upstream** — bmm prerequisite, not bundled) | -| `bmad-loop-resolve` | interactive CRITICAL-escalation resolution (`/bmad-loop-resolve `) | -| `bmad-loop-sweep` | deferred-work ledger triage (automation-only) | -| `bmad-loop-setup` | registers the module in `_bmad/` config + help | +| Skill | Role | +| --------------------------------- | ----------------------------------------------------------------------------------------------- | +| `bmad-dev-auto` | unattended implementation + follow-up review (**upstream** — bmm prerequisite, not bundled) | +| `bmad-review-adversarial-general` | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled) | +| `bmad-review-edge-case-hunter` | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled) | +| `bmad-review-verification-gap` | inline step-04 review layer, newest (BMAD-METHOD#2550) (**upstream** — bmm prereq, not bundled) | +| `bmad-loop-resolve` | interactive CRITICAL-escalation resolution (`/bmad-loop-resolve `) | +| `bmad-loop-sweep` | deferred-work ledger triage (automation-only) | +| `bmad-loop-setup` | registers the module in `_bmad/` config + help | + +`bmad-loop validate` preflights all four upstream skills; a target project on a pre-July bmm install missing `bmad-review-verification-gap` (or a `bmad-dev-auto` without its `customize.toml` review-layer config) is reported with bmm-module remediation before any run starts. **Via uv + `bmad-loop init` (self-sufficient).** Installing the tool and running `init` is all you need — `init` installs the `bmad-loop-*` skills into `.claude/skills/` (claude) and/or `.agents/skills/` (codex/gemini) for the CLIs you select, alongside the hooks and policy: diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 79a1b9a..680d1ee 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -48,9 +48,9 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se ### Adversarial review (review stage) - The follow-up review is a re-invocation of `bmad-dev-auto` on the `done` spec — a fresh-context session with no anchoring bias from the implementer (BMAD-METHOD#2508 routes a `done` spec to a fresh step-04 review pass), so there is no separate review skill. -- Two parallel adversarial layers (Blind Hunter / Edge Case Hunter) → verify findings against code → triage → auto-apply patches → log → defer ambiguity → commit. +- Parallel adversarial layers resolved from the skill's `customize.toml` (defaults: Adversarial-General, Edge-Case-Hunter, and Verification-Gap — the third added by BMAD-METHOD#2550) → verify findings against code → triage → auto-apply patches → log → defer ambiguity → commit. Each layer invokes an upstream `bmad-review-*` skill, so all three are bmm prerequisites the `bmad-loop validate` preflight checks for. - Bounded review loop (`limits.max_review_cycles`, default 3 cycles); done when the pass finishes `done` and no longer recommends a follow-up. This bound is also the oscillation guard for skill-recommended follow-up review. -- Optional (`[review].enabled`, default `true`): set `false` to skip the follow-up review session. The dev pass's own inline review (same two layers, in-context) is then the only review and it finalizes the story to `done` — one session per story instead of two. Verify commands still gate the commit. Applies to story runs and deferred-work sweeps alike. +- Optional (`[review].enabled`, default `true`): set `false` to skip the follow-up review session. The dev pass's own inline review (same layers, in-context) is then the only review and it finalizes the story to `done` — one session per story instead of two. Verify commands still gate the commit. Applies to story runs and deferred-work sweeps alike. - Trigger (`[review].trigger`, default `recommended`): when review is enabled, decides _when_ the follow-up pass runs. `recommended` runs it only when `bmad-dev-auto` sets `followup_review_recommended` on a `done` spec (it self-reviews inline and flags an independent pass only when its review-driven changes were significant — BMAD-METHOD#2505). `always` runs it on every story (pre-0.7.0 behavior). ### Failure handling & resilience @@ -59,6 +59,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Plateau-defer: when review won't converge, the story is skipped, the spec stashed into the run dir, deferred-work preserved, the run continues. - Typed escalations: `CRITICAL` pauses the run + notifies (desktop + `ATTENTION` file); `PREFERENCE` is journaled and continues. - CRITICAL resolution: `bmad-loop resolve ` opens an interactive resolve agent seeded with the escalation + frozen spec; you disambiguate, it re-arms the story (`escalated → pending`, spec reset to `ready-for-dev`) and resumes. `--no-interactive` skips to re-arm if you fixed the spec yourself. +- Intent-gap patch-restore (BMAD-METHOD#2564): when review halts on an `intent gap`, `bmad-dev-auto` saves the attempted change as a patch file (in the implementation-artifacts folder, referenced from the halt output) before reverting the tree. If the attempted reading turns out to be correct, the resolve agent adds `"restore_patch": ""` to its `resolution.json`; the orchestrator then re-arms the spec to `in-review` (not `ready-for-dev`) and re-applies the patch onto the baseline after every reset, so the re-driven session resumes _review_ on the restored diff instead of re-implementing from scratch. A hand-driven `bmad-loop resolve --no-interactive --restore-patch ` does the same. A patch that fails to apply escalates rather than dispatching a session onto a half-restored tree. ### Git worktree isolation (opt-in) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 035fbf4..b756616 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -601,6 +601,38 @@ def _confirm(question: str) -> bool: return ans in ("y", "yes") +def _resolve_restore_patch( + project: Path, run_dir: Path, story_key: str, args: argparse.Namespace +) -> tuple[str | None, str | None]: + """Determine the intent-gap patch-restore latch (BMAD-METHOD #2564) for a re-arm. + + Precedence: the explicit ``--restore-patch`` flag (hand-driven recovery) wins; + otherwise, on the interactive path, the resolve agent may have recorded a + ``restore_patch`` field in resolution.json. Returns ``(latch, error)``: a + validated absolute patch path to latch (None = ordinary from-scratch re-drive), + or an error string when a supplied path is missing / outside the project — the + caller aborts strictly rather than silently re-driving from scratch when the + operator asked to restore.""" + raw = getattr(args, "restore_patch", None) + if raw is None and args.interactive: + doc = resolve.read_resolution(run_dir, story_key) + if doc is not None: + val = doc.get("restore_patch") + raw = str(val) if val else None + if not raw: + return None, None + patch = Path(raw) + if not patch.is_absolute(): + patch = project / patch + patch = patch.resolve() + if not patch.is_file() or not patch.is_relative_to(project.resolve()): + return None, ( + f"restore patch {raw!r} is not a file under the project — refusing to " + "re-arm (fix the path, or re-run without a restore to re-drive from scratch)" + ) + return str(patch), None + + def cmd_resolve(args: argparse.Namespace) -> int: from .model import PAUSE_ESCALATION, Phase @@ -671,16 +703,26 @@ def cmd_resolve(args: argparse.Namespace) -> int: file=sys.stderr, ) + # intent-gap patch-restore latch (#2564): validate a supplied patch path before + # prompting, so a bad path fails loudly instead of after a confirmation. + restore_patch, err = _resolve_restore_patch(project, run_dir, story_key, args) + if err is not None: + print(err, file=sys.stderr) + return 1 + # confirm-then-resume (args.resume: None = ask, True = auto, False = re-arm only) if args.resume is None and not _confirm(f"re-arm {story_key} and resume run {args.run_id}?"): print("cancelled — run is still paused at the escalation") return 0 try: - runs.rearm_escalation(run_dir, story_key) + runs.rearm_escalation(run_dir, story_key, restore_patch=restore_patch) except runs.RearmError as e: print(f"error: {e}", file=sys.stderr) return 1 - print(f"re-armed {story_key}") + print( + f"re-armed {story_key}" + + (" (restoring the attempted change for review)" if restore_patch else "") + ) if args.resume is False: print(f"resume when ready: bmad-loop resume {args.run_id}") return 0 @@ -1298,6 +1340,14 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: action="store_false", help="skip the resolve agent (spec already fixed by hand); just re-arm + resume", ) + resolve_p.add_argument( + "--restore-patch", + metavar="PATH", + help="intent-gap patch-restore (#2564): re-arm the spec to `in-review` and " + "re-apply this saved patch before the re-drive, resuming review on the " + "attempted change instead of re-implementing (hand-driven; the interactive " + "agent supplies it via resolution.json)", + ) resolve_p.add_argument( "--resume", action=argparse.BooleanOptionalAction, diff --git a/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md b/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md index 1b4795f..76009b7 100644 --- a/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md +++ b/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md @@ -53,10 +53,14 @@ action of a successful resolution. Schema: "story_key": "", "decision": "", "spec_file": "", - "spec_updated": true + "spec_updated": true, + "restore_patch": "" } ``` +`restore_patch` is **optional** and used only for the intent-gap patch-restore +case below — omit it entirely for an ordinary resolution. + ## What you MUST do 1. **Read the context**, then read the **frozen spec** at `spec_file` in full — @@ -79,6 +83,36 @@ action of a successful resolution. Schema: orchestrator will offer to **re-arm the story and resume the run** (a clean rebuild against the corrected spec). +## Special case: a review-stage `intent gap` with a saved patch + +When the escalation came from the **review step** halting on an `intent gap`, the +dev session first **saved its attempted change as a patch file** (in the +implementation-artifacts folder) before reverting the tree — the escalation +`detail` and the spec's `## Review Triage Log` reference the patch path. That +patch is concrete evidence: it shows exactly which reading of the intent the run +implemented. Use it two ways: + +- **As evidence.** Read the patch (and the diff it represents) to see what the + guessed reading produced — often clearer input for writing the clarification + than the questions alone. +- **When the attempted reading was actually correct.** Sometimes the run's guess + is the right one and only the _intent_ was silent. Present this as an explicit + option to the human: _"the implementation read it as X, which is in fact what we + want — amend the intent to say X, and resume **review** on the already-written + change instead of re-implementing it."_ If the human chooses this: + 1. Still **amend the intent** in the spec so it unambiguously says X (step 4 + above is unchanged — the frozen intent must match the restored code). + 2. Add **`"restore_patch": ""`** to `resolution.json` + (copy the path verbatim from the escalation detail / triage log). + + The orchestrator then re-arms the spec to `in-review`, re-applies the patch onto + the baseline, and re-dispatches — the session resumes at the review step on the + restored diff. **Do NOT `git apply` the patch yourself and do NOT set the spec + status** — the orchestrator does both deterministically at re-arm. + +If the attempted reading was wrong (the common case), omit `restore_patch` +entirely: the orchestrator re-drives from scratch against the corrected intent. + ## What you MUST NOT do - **Do NOT** write the orchestrator's `result.json` — that is a dev/review diff --git a/src/bmad_loop/devcontract.py b/src/bmad_loop/devcontract.py index cf83f9c..f6e4f8e 100644 --- a/src/bmad_loop/devcontract.py +++ b/src/bmad_loop/devcontract.py @@ -48,11 +48,14 @@ # allowlist: anything else (already-`done`, `blocked`, or an unknown custom token) # is left untouched, so reconciliation can never override a status the skill set on # purpose. `""` covers a blank or missing frontmatter `status:` — `reset_spec_status` -# fills/inserts the line in that case. `in-review` is included because on the sole -# (generic `bmad-dev-auto`) path it is only ever the transient marker step-04 sets at -# its start; the skill self-finalizes to `done`. The legacy `bmad-loop-dev` fork that -# used `in-review` as a deliberate review-handoff terminal is retired, so nothing -# leaves `in-review` on purpose anymore. +# fills/inserts the line in that case. `in-review` is included because step-04 sets +# it transiently at the start of a review pass; the skill self-finalizes to `done`, +# so a spec left AT `in-review` with a prose `done` result is a mid-review interrupt +# safe to reconcile forward. The intent-gap patch-restore re-drive (BMAD-METHOD +# #2564) deliberately re-arms the spec TO `in-review` before a session (so step-01 +# routes straight to step-04 on the restored diff) — but that is the pre-session +# status the re-driven skill then advances past; it never LEAVES a spec at +# `in-review` as a terminal, so the reconcile allowlist semantics are unchanged. RECONCILABLE_FROM = frozenset({"", "draft", "ready-for-dev", "in-progress", "in-review"}) # The leading `---\n …frontmatter… \n---` block, captured in three parts so the diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 4114b4c..3f4c1ff 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -864,6 +864,44 @@ def _safe_reset(self, task: StoryTask, *, preserve: tuple[str, ...] = ()) -> Non preserve=preserve, ) + def _restore_patch(self, task: StoryTask) -> None: + """Re-apply the latched intent-gap patch (BMAD-METHOD #2564) onto the + baseline tree so the re-driven session resumes review (step-04) on the + restored diff instead of re-implementing. No-op unless a restore is latched. + + Applied from inside `_dev_phase`'s loop, right before each dispatch that + runs against a fresh baseline (the first attempt and every non-fixable + rollback retry — the loop gates this on ``feedback is None``, so a + fixable-feedback retry that KEEPS the attempt's tree is never double-applied, + and the patch file's own untracked/tracked content is excluded from + `baseline_untracked` because that snapshot is taken before the first apply). + This is the plan's "apply after every baseline reset" seam, placed here + rather than in `_rollback_or_pause` so the patch always lands after the + clean baseline_untracked snapshot — avoiding a mid-re-drive reset preserving + the patch's own new files and then colliding with the re-apply. + + On apply failure we escalate rather than dispatch a session onto a + half-restored tree; the task is mid-dispatch (DEV_RUNNING), so step it to + the escalatable DEV_VERIFY phase first (`_escalate` raises RunPaused).""" + if not task.restore_patch: + return + patch = Path(task.restore_patch) + if not patch.is_absolute(): + patch = self.workspace.root / patch + try: + verify.apply_patch(self.workspace.root, patch) + except verify.GitError as e: + self.journal.append( + "attempt-restore-failed", + story_key=task.story_key, + patch=task.restore_patch, + error=str(e), + ) + if task.phase == Phase.DEV_RUNNING: + advance(task, Phase.DEV_VERIFY) # PENDING/DEV_RUNNING can't escalate directly + self._escalate(task, f"intent-gap restore patch failed to apply: {e}") + self.journal.append("attempt-restored", story_key=task.story_key, patch=task.restore_patch) + def _prune_preserve_refs(self) -> None: """Bounded retention for both recovery-ref families at run start — the attempt-preserve/* branches and the refs/attempt-preserve-dirty/* @@ -1388,6 +1426,15 @@ def _dev_phase(self, task: StoryTask, resume_result: SessionResult | None = None result = resume_result resume_result = None else: + # intent-gap patch-restore (#2564): re-lay the saved attempt onto + # the baseline before dispatch so the re-driven session resumes + # review on the restored diff. `feedback is None` ⇒ the tree is at + # baseline (fresh attempt or a non-fixable rollback below), NOT a + # fixable-feedback retry that kept the attempt's tree — so this + # never double-applies. No-op unless a restore is latched; escalates + # (never dispatches) if the patch fails to apply. + if feedback is None: + self._restore_patch(task) result = self._run_session( task, role="dev", @@ -1682,8 +1729,11 @@ def _commit(self, task: StoryTask) -> None: sha = verify.finalize_commit(self.workspace.root, task.baseline_commit, message) task.commit_sha = sha or task.baseline_commit # the corrected spec is now durable in HEAD; later attempts need no - # special preservation, so drop the re-drive latch. + # special preservation, so drop the re-drive latch. The restored diff + # is likewise committed, so clear its latch too — a subsequent re-arm + # (if any) decides afresh whether to restore again. task.resolved_redrive = False + task.restore_patch = None except verify.GitError as e: self._escalate(task, f"commit failed: {e}") advance(task, Phase.DONE) diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 83d504b..df31405 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -67,14 +67,20 @@ # BMad Method (bmm) module installs them. Each must exist in every active CLI skill # tree and carry its marker files (a half-installed or pre-automation skill is # caught by the `bmad-loop validate` preflight). `{skill: (marker-rel-path, ...)}`. -# - bmad-dev-auto: the inner dev primitive — always required. -# - the two review hunters bmad-dev-auto's step-04 invokes inline on EVERY dev +# - bmad-dev-auto: the inner dev primitive — always required. Markers pin BOTH a +# step file (catches a truncated copy) AND customize.toml, the layer/handoff +# config step-04 resolves review_layers from (BMAD-METHOD #2535/#2550): a +# pre-July bmm install predating it would let every dev run's step-04 fail. +# - the three review hunters bmad-dev-auto's step-04 invokes inline on EVERY dev # run (and on each follow-up review re-invocation) — also always required, no -# longer gated on a separate review session. +# longer gated on a separate review session. bmad-review-verification-gap is +# the newest layer (BMAD-METHOD #2550): a target project missing it makes the +# verification-gap review layer fail on every run. DEV_BASE_SKILLS = { - "bmad-dev-auto": ("step-04-review.md",), + "bmad-dev-auto": ("step-04-review.md", "customize.toml"), "bmad-review-adversarial-general": (), "bmad-review-edge-case-hunter": (), + "bmad-review-verification-gap": (), } # Every non-bundled skill that might need copying into an isolated worktree. BASE_SKILLS = dict(DEV_BASE_SKILLS) @@ -83,12 +89,13 @@ def missing_base_skills(project: Path, trees: Sequence[str]) -> list[str]: """Problems for the upstream skills the orchestrator drives but doesn't bundle. - The dev primitive (bmad-dev-auto) and the two adversarial review hunters it - invokes inline are installed by the BMad Method module, not by `bmad-loop - init`. Each must exist in every active CLI skill tree and carry its marker - files. Returns one human-readable problem string per missing/incomplete skill; - empty list means OK. Run as a preflight so a missing skill fails loudly with - remediation instead of stalling as an `Unknown command` until the run times out. + The dev primitive (bmad-dev-auto) and the three review hunters it invokes + inline — adversarial-general, edge-case-hunter, and verification-gap — are + installed by the BMad Method module, not by `bmad-loop init`. Each must exist + in every active CLI skill tree and carry its marker files. Returns one + human-readable problem string per missing/incomplete skill; empty list means + OK. Run as a preflight so a missing skill fails loudly with remediation instead + of stalling as an `Unknown command` until the run times out. """ required = dict(DEV_BASE_SKILLS) problems: list[str] = [] diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index 1c9d55c..48cfda1 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -152,6 +152,14 @@ class StoryTask: # tracked content, so a mid-re-drive retry/defer reset can't silently revert # the human correction. Survives the resume serialization round-trip. resolved_redrive: bool = False + # intent-gap patch-restore re-drive (BMAD-METHOD #2564): a repo-relative-or- + # absolute path to the patch file bmad-dev-auto saved of the reverted attempt. + # Latched by runs.rearm_escalation when the human confirms the attempted reading + # was correct; the engine re-applies it onto the baseline after every reset of + # the re-drive so the re-driven session resumes review (step-04) on the restored + # diff, and clears it once the corrected work commits. None = ordinary + # from-scratch re-drive. Survives the resume serialization round-trip. + restore_patch: str | None = None # sweep bundles only: the deferred-work ids this task closes and the # rendered intent file handed to dev sessions dw_ids: list[str] = field(default_factory=list) @@ -203,6 +211,7 @@ def to_dict(self) -> dict[str, Any]: "defer_reason": self.defer_reason, "rearmed": self.rearmed, "resolved_redrive": self.resolved_redrive, + "restore_patch": self.restore_patch, "dw_ids": self.dw_ids, "bundle_file": self.bundle_file, "worktree_path": self.worktree_path, @@ -245,6 +254,7 @@ def from_dict(cls, d: dict[str, Any]) -> "StoryTask": defer_reason=d.get("defer_reason"), rearmed=bool(d.get("rearmed", False)), resolved_redrive=bool(d.get("resolved_redrive", False)), + restore_patch=d.get("restore_patch"), dw_ids=[str(i) for i in d.get("dw_ids", [])], bundle_file=d.get("bundle_file"), worktree_path=str(d.get("worktree_path", "")), diff --git a/src/bmad_loop/resolve.py b/src/bmad_loop/resolve.py index 24acd71..d2c5b4a 100644 --- a/src/bmad_loop/resolve.py +++ b/src/bmad_loop/resolve.py @@ -38,6 +38,21 @@ def resolution_path(run_dir: Path, story_key: str) -> Path: return _story_dir(run_dir, story_key) / "resolution.json" +def read_resolution(run_dir: Path, story_key: str) -> dict[str, Any] | None: + """Parse the resolve agent's ``resolution.json`` marker, or None when it is + absent or unreadable. The caller reads the optional ``restore_patch`` field + (the intent-gap patch-restore path, BMAD-METHOD #2564) from the returned dict; + it validates that path itself before acting on it.""" + path = resolution_path(run_dir, story_key) + if not path.is_file(): + return None + try: + doc = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return None + return doc if isinstance(doc, dict) else None + + def _gather_escalations(run_dir: Path, state: RunState, story_key: str) -> list[dict[str, Any]]: """The CRITICAL escalations recorded by this story's sessions, newest first. diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 5f89893..015508f 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -505,17 +505,31 @@ class RearmError(Exception): """The run/story is not in a re-armable escalation state.""" -def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: +def rearm_escalation( + run_dir: Path, story_key: str | None = None, *, restore_patch: str | None = None +) -> str: """Re-arm an escalation-paused story so the next resume re-drives it. Flips the escalated task out of its terminal ESCALATED phase back to PENDING — which makes `_finish_inflight` reset the tree to the story's baseline and re-run it (clean rebuild) against the now-corrected frozen - spec. Deterministically sets that spec's status to `ready-for-dev` so the - dev session routes straight to implement, and strips the escalated - attempt's stale `## Auto Run Result` section so the re-drive cannot read - as terminal from its first save. Does NOT clear the pause; the caller - resumes the run separately. + spec. Strips the escalated attempt's stale `## Auto Run Result` section so + the re-drive cannot read as terminal from its first save, and sets the + spec's frontmatter status so step-01 routes to the right stage. Does NOT + clear the pause; the caller resumes the run separately. + + Two re-drive modes, selected by `restore_patch`: + + - **from-scratch** (default, ``restore_patch=None``): status → ``ready-for-dev`` + so the dev session re-implements from a clean baseline. Assigning None also + clears any stale latch from a prior restore attempt the human abandoned. + - **patch-restore** (BMAD-METHOD #2564, ``restore_patch`` set): the human + confirmed the escalated attempt's reading was correct. Status → ``in-review`` + so step-01 routes straight to step-04, and the path is latched onto the task + (`task.restore_patch`) so the engine re-applies the saved patch onto the + baseline before dispatching — the re-driven session resumes review on the + restored diff instead of re-implementing. The status is set here + deterministically; the resolve agent must NOT set it. Returns the re-armed story key. Raises RearmError when the run is not paused at the escalation stage or the target story is not escalated. @@ -543,11 +557,17 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: task.defer_reason = None task.rearmed = True # resume-time recovery notice describes a clean rebuild, # not a failed attempt (engine._finish_inflight clears it once the rebuild runs) + # Always (re)assign the latch: a None restore_patch clears a stale one left by + # a prior restore attempt the human then chose to redo from scratch. + task.restore_patch = restore_patch if task.spec_file: - # route /bmad-dev-auto to re-implement (decision table: ready-for-dev - # -> step-03); independent of the resolve agent having set it. - verify.set_frontmatter_status(Path(task.spec_file), "ready-for-dev") + # Route /bmad-dev-auto via the spec's frontmatter status (decision table): + # patch-restore -> in-review -> step-04 (resume review on the restored + # diff); from-scratch -> ready-for-dev -> step-03 (re-implement). Set here + # deterministically, independent of the resolve agent. + target_status = "in-review" if restore_patch else "ready-for-dev" + verify.set_frontmatter_status(Path(task.spec_file), target_status) # drop the stale `## Auto Run Result` section along with the status flip # (mirrors engine._reset_spec_for_repair): find_result_artifact keys on # that heading, so leaving it would let the re-driven session's first @@ -555,5 +575,5 @@ def rearm_escalation(run_dir: Path, story_key: str | None = None) -> str: devcontract.strip_auto_run_result(Path(task.spec_file)) save_state(run_dir, state) - Journal(run_dir).append("story-escalation-resolved", story_key=key) + Journal(run_dir).append("story-escalation-resolved", story_key=key, restore=bool(restore_patch)) return key diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index 1cb26d4..c59d47e 100644 --- a/src/bmad_loop/verify.py +++ b/src/bmad_loop/verify.py @@ -1207,6 +1207,15 @@ def finalize_commit(repo: Path, baseline: str | None, message: str) -> str | Non keeping the index (`reset --soft`), then commit the accumulated index. The working tree is never touched, so a failure leaves the chain intact. + Residual-artifacts note (BMAD-METHOD #2563): the skill now commits every file + of the reviewed diff and deliberately leaves unrelated `git status` residue + uncommitted (files outside the change's scope). The `add -A` here sweeps that + residue into the story commit too — an intentional divergence from the skill's + scoped commit. The loop must end each story on a clean tree because story + N+1's step-01 HALTs on a dirty tree, so the orchestrator squashes EVERYTHING + since baseline (skill commits + its own bookkeeping + any residue) into the one + story commit rather than leaving the tree dirty for the next story to trip on. + Returns the new HEAD sha, or None when there is nothing to finalize: no version control (`baseline` falsy or NO_VCS) or the tree already equals `baseline` (no skill commits and no bookkeeping delta).""" @@ -1238,6 +1247,25 @@ def finalize_commit(repo: Path, baseline: str | None, message: str) -> str | Non return rev_parse_head(repo) +def apply_patch(repo: Path, patch_path: Path) -> None: + """Apply a saved patch to `repo`'s working tree (`git apply`), raising on failure. + + The intent-gap patch-restore re-drive (BMAD-METHOD #2564) uses this to re-lay + the attempted change bmad-dev-auto saved before reverting: the patch is a diff + from the story's baseline, and the caller has just reset the tree to that same + baseline, so it applies cleanly by construction. New files in the patch are + created (they land untracked, matching how the original attempt sat before its + revert). A non-zero `git apply` — a conflict, a missing/corrupt patch, a tree + that has drifted from baseline — raises `GitError` with git's output; the caller + escalates rather than dispatch a session onto a half-applied tree. + """ + if not patch_path.is_file(): + raise GitError(f"restore patch not found: {patch_path}") + rc, out = _git(repo, "apply", str(patch_path)) + if rc != 0: + raise GitError(f"git apply {patch_path} failed: {out}") + + def commit_paths(repo: Path, message: str, paths: list[Path]) -> str | None: """Commit exactly `paths` (and nothing else), leaving any unrelated working or staged changes untouched. Unlike commit_story's `add -A`, this is safe to diff --git a/tests/test_cli.py b/tests/test_cli.py index 88fbc6e..59b7021 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -780,6 +780,94 @@ def test_resolve_rearm_only_skips_resume(tmp_path, monkeypatch, capsys): assert "resume when ready" in capsys.readouterr().out +def test_resolve_no_interactive_restore_patch_latches_in_review(tmp_path, monkeypatch, capsys): + from bmad_loop.journal import load_state + from bmad_loop.model import Phase + + spec = tmp_path / "spec.md" + spec.write_text("---\nstatus: blocked\n---\n", encoding="utf-8") + patch = tmp_path / "artifacts" / "attempt.patch" + patch.parent.mkdir(parents=True) + patch.write_text("diff", encoding="utf-8") + run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) + + monkeypatch.setattr(cli, "_resume_paused_run", lambda proj, rd: 0) + rc = cli.main( + [ + "resolve", + "--project", + str(tmp_path), + "r1", + "--no-interactive", + "--restore-patch", + str(patch), + "--resume", + ] + ) + assert rc == 0 + assert "restoring the attempted change" in capsys.readouterr().out + task = load_state(run_dir).tasks["s1"] + assert task.phase == Phase.PENDING + assert task.restore_patch == str(patch.resolve()) # validated + latched (absolute) + assert "in-review" in spec.read_text() # restore mode routes step-01 -> step-04 + + +def test_resolve_restore_patch_missing_file_rejected(tmp_path, monkeypatch, capsys): + from bmad_loop.journal import load_state + from bmad_loop.model import Phase + + spec = tmp_path / "spec.md" + spec.write_text("---\nstatus: blocked\n---\n", encoding="utf-8") + run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) + called: list = [] + monkeypatch.setattr(cli, "_resume_paused_run", lambda proj, rd: called.append(rd) or 0) + rc = cli.main( + [ + "resolve", + "--project", + str(tmp_path), + "r1", + "--no-interactive", + "--restore-patch", + str(tmp_path / "nope.patch"), + "--resume", + ] + ) + assert rc == 1 + assert "not a file under the project" in capsys.readouterr().err + assert called == [] # never resumed + assert load_state(run_dir).tasks["s1"].phase == Phase.ESCALATED # not re-armed + + +def test_resolve_interactive_restore_patch_from_resolution_json(tmp_path, monkeypatch): + from bmad_loop import resolve + from bmad_loop.journal import load_state + + spec = tmp_path / "spec.md" + spec.write_text("---\nstatus: blocked\n---\n", encoding="utf-8") + patch = tmp_path / "artifacts" / "attempt.patch" + patch.parent.mkdir(parents=True) + patch.write_text("diff", encoding="utf-8") + run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) + + def fake_session(adapter, project, rd, story_key, *, model=""): + # the resolve agent records a restore_patch in its output marker + marker = resolve.resolution_path(rd, story_key) + marker.parent.mkdir(parents=True, exist_ok=True) + marker.write_text(json.dumps({"restore_patch": str(patch)}), encoding="utf-8") + return True + + monkeypatch.setattr(cli, "_make_adapters", lambda *a, **k: {"dev": object()}) + monkeypatch.setattr(resolve, "build_context", lambda *a, **k: None) + monkeypatch.setattr(resolve, "run_session", fake_session) + monkeypatch.setattr(cli, "_resume_paused_run", lambda proj, rd: 0) + rc = cli.main(["resolve", "--project", str(tmp_path), "r1", "--resume"]) + assert rc == 0 + task = load_state(run_dir).tasks["s1"] + assert task.restore_patch == str(patch.resolve()) # picked up from resolution.json + assert "in-review" in spec.read_text() + + def test_sweep_command_parses_flags(): parser_args = [ "sweep", diff --git a/tests/test_engine.py b/tests/test_engine.py index e1cd435..13d4c0b 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -2067,6 +2067,164 @@ def halt_blocked(spec): assert read_frontmatter(sp)["status"] == "ready-for-dev" # re-drive will not HALT +# -------------------------------------------------- intent-gap patch-restore (#2564) + + +def _escalate_with_patch(project, story_key, patch_path): + """A dev effect that halts on an intent gap the way bmad-dev-auto #2564 does: + it saves the attempted change as a patch under the protected artifacts, reverts + the tree, and escalates blocked.""" + + def effect(spec): + repo = project.project + baseline = rev_parse_head(repo) + src = repo / "src.txt" + src.write_text("original\nattempted reading\n") # the attempted change + patch_path.parent.mkdir(parents=True, exist_ok=True) + patch_path.write_text(git(repo, "diff", "HEAD") + "\n", encoding="utf-8") # save it + src.write_text("original\n") # revert before halting (tree back at baseline) + sp = spec_path(project, story_key) + write_spec(sp, "blocked", baseline) + return SessionResult( + status="completed", + result_json={ + "workflow": "auto-dev", + "story_key": story_key, + "spec_file": str(sp), + "escalations": [ + { + "type": "blocked", + "severity": "CRITICAL", + "detail": f"intent gap; saved patch: {patch_path}", + } + ], + }, + ) + + return effect + + +def _restoring_dev_effect(project, story_key, seen): + """A re-driven dev effect that records what the tree looked like when it ran + (so a test can assert the restored diff was present) then finalizes to done.""" + + def effect(spec): + repo = project.project + seen.append((repo / "src.txt").read_text()) + baseline = rev_parse_head(repo) + sp = spec_path(project, story_key) + write_spec(sp, "done", baseline) + return SessionResult( + status="completed", + result_json={ + "workflow": "auto-dev", + "story_key": story_key, + "spec_file": str(sp), + "baseline_commit": baseline, + "escalations": [], + "followup_review_recommended": False, + }, + ) + + return effect + + +def test_restore_patch_applies_onto_baseline(project): + """_restore_patch re-lays the saved diff onto the baseline tree and journals + attempt-restored, leaving the phase untouched on success.""" + engine, _ = make_engine(project, []) + repo = project.project + task = StoryTask(story_key="1-1-a", epic=1) + task.baseline_commit = rev_parse_head(repo) + src = repo / "src.txt" + src.write_text("original\nattempted\n") + patch = project.implementation_artifacts / "attempt.patch" + patch.write_text(git(repo, "diff", "HEAD") + "\n", encoding="utf-8") + src.write_text("original\n") # tree at baseline + task.restore_patch = str(patch) + task.phase = Phase.DEV_RUNNING + + engine._restore_patch(task) + + assert src.read_text() == "original\nattempted\n" + assert task.phase == Phase.DEV_RUNNING # success does not advance the phase + assert "attempt-restored" in [e["kind"] for e in engine.journal.entries()] + + +def test_restore_patch_failure_escalates_without_dispatch(project): + """A patch that will not apply escalates (never dispatches onto a half-restored + tree): the task ends ESCALATED, attempt-restore-failed is journaled, and the + success marker is not.""" + engine, _ = make_engine(project, []) + task = StoryTask(story_key="1-1-a", epic=1) + task.baseline_commit = rev_parse_head(project.project) + patch = project.implementation_artifacts / "bad.patch" + patch.parent.mkdir(parents=True, exist_ok=True) + patch.write_text("--- a/src.txt\n+++ b/src.txt\n@@ -1 +1 @@\n-nope\n+x\n", encoding="utf-8") + task.restore_patch = str(patch) + task.phase = Phase.DEV_RUNNING + + with pytest.raises(RunPaused): + engine._restore_patch(task) + + assert task.phase == Phase.ESCALATED + kinds = [e["kind"] for e in engine.journal.entries()] + assert "attempt-restore-failed" in kinds + assert "attempt-restored" not in kinds + + +def test_intent_gap_restore_redrive_applies_patch_and_lands_done(project): + """End-to-end: a resolved escalation with a restore patch re-applies the + attempted change onto the baseline before the re-driven session runs, so the + session resumes on the restored diff and the story lands done; the latch clears + on commit.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + patch = project.implementation_artifacts / "attempt.patch" + engine, _ = make_engine(project, [_escalate_with_patch(project, "1-1-a", patch)]) + assert engine.run().escalated == 1 + + rearm_escalation(engine.run_dir, restore_patch=str(patch)) # human confirmed the reading + sp = spec_path(project, "1-1-a") + assert read_frontmatter(sp)["status"] == "in-review" # routes step-01 -> step-04 + assert load_state(engine.run_dir).tasks["1-1-a"].restore_patch == str(patch) + + seen: list[str] = [] + resumed, _ = resume_engine(project, engine, [_restoring_dev_effect(project, "1-1-a", seen)]) + summary = resumed.run() + + assert summary.done == 1 and not summary.paused + assert seen == ["original\nattempted reading\n"] # the session saw the RESTORED code + assert "attempt-restored" in [e["kind"] for e in resumed.journal.entries()] + task = load_state(resumed.run_dir).tasks["1-1-a"] + assert task.phase == Phase.DONE + assert task.restore_patch is None # latch cleared on commit + + +def test_intent_gap_restore_reapplies_after_mid_redrive_rollback(project): + """A non-fixable retry inside the restore re-drive resets to baseline (clearing + the restored code), so the patch is re-applied before the next dispatch; the + saved patch file under the protected artifacts survives the reset.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + patch = project.implementation_artifacts / "attempt.patch" + engine, _ = make_engine(project, [_escalate_with_patch(project, "1-1-a", patch)]) + assert engine.run().escalated == 1 + rearm_escalation(engine.run_dir, restore_patch=str(patch)) + + seen: list[str] = [] + resumed, _ = resume_engine( + project, + engine, + [SessionResult(status="stalled"), _restoring_dev_effect(project, "1-1-a", seen)], + ) + summary = resumed.run() + + assert summary.done == 1 and not summary.paused + assert seen == ["original\nattempted reading\n"] # the surviving retry ran on the restored tree + kinds = [e["kind"] for e in resumed.journal.entries()] + assert kinds.count("attempt-restored") == 2 # applied, rolled back on stall, re-applied + assert patch.is_file() # protected patch file survived the reset + + def test_dev_stall_retries_then_succeeds(project): write_sprint(project, {"1-1-a": "ready-for-dev"}) engine, adapter = make_engine( diff --git a/tests/test_install.py b/tests/test_install.py index a5d22a1..5d43cca 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -534,24 +534,43 @@ def test_provision_worktree_copies_base_skills_from_repo(tmp_path): def test_missing_base_skills_reports_absent_and_incomplete(tmp_path): claude = get_profile("claude") - # nothing installed → dev primitive + both inline review hunters reported + # nothing installed → dev primitive + all three inline review hunters reported # missing (the hunters are always required — bmad-dev-auto's step-04 invokes # them on every run, regardless of the orchestrator's follow-up review) problems = missing_base_skills(tmp_path, [claude.skill_tree]) - assert len(problems) == 3 + assert len(problems) == 4 assert all("install the BMad Method" in p for p in problems) # install everything → no problems _install_base_skills(tmp_path, claude.skill_tree) assert missing_base_skills(tmp_path, [claude.skill_tree]) == [] - # remove the dev primitive's marker → reported as incomplete + # remove the dev primitive's step-file marker → reported as incomplete (tmp_path / claude.skill_tree / "bmad-dev-auto" / "step-04-review.md").unlink() problems = missing_base_skills(tmp_path, [claude.skill_tree]) assert len(problems) == 1 assert "incomplete" in problems[0] assert "step-04-review.md" in problems[0] + # restore it, then drop customize.toml (the review-layer config marker, + # BMAD-METHOD #2535/#2550) → a pre-July bmm install is caught as incomplete + (tmp_path / claude.skill_tree / "bmad-dev-auto" / "step-04-review.md").write_text("x\n") + (tmp_path / claude.skill_tree / "bmad-dev-auto" / "customize.toml").unlink() + problems = missing_base_skills(tmp_path, [claude.skill_tree]) + assert len(problems) == 1 + assert "incomplete" in problems[0] + assert "customize.toml" in problems[0] + + # the newest review layer (verification-gap) reported by name when absent + _install_base_skills(tmp_path, claude.skill_tree) # re-complete everything + import shutil as _shutil + + _shutil.rmtree(tmp_path / claude.skill_tree / "bmad-review-verification-gap") + problems = missing_base_skills(tmp_path, [claude.skill_tree]) + assert len(problems) == 1 + assert "bmad-review-verification-gap" in problems[0] + assert "install the BMad Method" in problems[0] + def test_provision_worktree_seeds_gitignored_config(tmp_path): """A gitignored config present in the main repo is copied into the worktree diff --git a/tests/test_model.py b/tests/test_model.py index 7ce5d46..6aaca4d 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -84,6 +84,17 @@ def test_resolved_redrive_defaults_false_for_legacy_state(): assert StoryTask.from_dict(doc).resolved_redrive is False +def test_restore_patch_round_trips(): + task = StoryTask(story_key="1-1-a", epic=1, restore_patch="artifacts/attempt.patch") + assert StoryTask.from_dict(task.to_dict()).restore_patch == "artifacts/attempt.patch" + + +def test_restore_patch_defaults_none_for_legacy_state(): + doc = StoryTask(story_key="1-1-a", epic=1).to_dict() + del doc["restore_patch"] # state.json from before the field existed + assert StoryTask.from_dict(doc).restore_patch is None + + def test_stopped_round_trips(): state = _state(stopped=True) assert RunState.from_dict(state.to_dict()).stopped is True diff --git a/tests/test_runs.py b/tests/test_runs.py index 6d8db53..8ce2351 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -530,6 +530,69 @@ def test_delete_run(tmp_path): assert not run_dir.exists() +def _escalated_run(tmp_path, spec_text, *, restore_patch_stale=None): + from bmad_loop.model import PAUSE_ESCALATION, Phase, StoryTask + + spec = tmp_path / "spec.md" + spec.write_text(spec_text, encoding="utf-8") + task = StoryTask( + story_key="1-1-a", + epic=1, + phase=Phase.ESCALATED, + attempt=2, + spec_file=str(spec), + restore_patch=restore_patch_stale, + ) + run_dir = _make_state_run( + tmp_path, + "r1", + paused_reason="CRITICAL escalation", + paused_stage=PAUSE_ESCALATION, + paused_story_key="1-1-a", + tasks={"1-1-a": task}, + ) + return run_dir, spec + + +_SPEC_WITH_ARR = ( + "---\ntitle: t\nstatus: blocked\n---\n\n## Intent\n\nbody\n" + "\n## Auto Run Result\n\n- Status: blocked\n\nboom\n" +) + + +def test_rearm_restore_mode_sets_in_review_strips_arr_and_latches(tmp_path): + from bmad_loop.journal import Journal + from bmad_loop.model import Phase + + run_dir, spec = _escalated_run(tmp_path, _SPEC_WITH_ARR) + runs.rearm_escalation(run_dir, restore_patch="artifacts/attempt.patch") + + task = load_state(run_dir).tasks["1-1-a"] + assert task.phase == Phase.PENDING and task.attempt == 0 + assert task.restore_patch == "artifacts/attempt.patch" + text = spec.read_text() + assert "status: in-review" in text # in-review routes step-01 -> step-04 + assert "## Auto Run Result" not in text # stale terminal section stripped + entry = [e for e in Journal(run_dir).entries() if e["kind"] == "story-escalation-resolved"][-1] + assert entry["restore"] is True + + +def test_rearm_plain_mode_sets_ready_for_dev_and_clears_stale_latch(tmp_path): + from bmad_loop.journal import Journal + from bmad_loop.model import Phase + + # a stale latch from a prior restore attempt the human then chose to redo fresh + run_dir, spec = _escalated_run(tmp_path, _SPEC_WITH_ARR, restore_patch_stale="old.patch") + runs.rearm_escalation(run_dir) # no restore_patch => from-scratch + + task = load_state(run_dir).tasks["1-1-a"] + assert task.phase == Phase.PENDING + assert task.restore_patch is None # stale latch cleared + assert "status: ready-for-dev" in spec.read_text() + entry = [e for e in Journal(run_dir).entries() if e["kind"] == "story-escalation-resolved"][-1] + assert entry["restore"] is False + + def test_archive_run(tmp_path): run_dir = _make_state_run(tmp_path, "20260611-100000-aaaa") (run_dir / "journal.jsonl").write_text('{"kind":"x"}\n') diff --git a/tests/test_verify.py b/tests/test_verify.py index 815ffcb..9e6df43 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -700,6 +700,49 @@ def test_commit_paths_noop_when_unchanged(project): assert verify.commit_paths(project.project, "noop", [project.project.parent / "x"]) is None +def test_apply_patch_replays_saved_diff(project): + """A patch saved off the baseline re-applies cleanly onto that same baseline — + tracked edits AND new (untracked) files — reproducing the reverted attempt.""" + repo = project.project + baseline = verify.rev_parse_head(repo) + # the "attempt": edit a tracked file + add a new file, then capture the diff + (repo / "src.txt").write_text("original\nattempted change\n") + (repo / "new_module.py").write_text("print('hi')\n") + patch = project.implementation_artifacts / "attempt.patch" + # git diff HEAD includes new files with --binary-safe text; -N stages intent so + # untracked files appear in the diff (mirrors how the skill saves the attempt) + git(repo, "add", "-N", "new_module.py") + patch.write_text(git(repo, "diff", "HEAD") + "\n", encoding="utf-8") + # revert the attempt back to baseline (as the skill does before halting) + git(repo, "reset", "-q", "--hard", baseline) + (repo / "new_module.py").unlink(missing_ok=True) + assert (repo / "src.txt").read_text() == "original\n" + + verify.apply_patch(repo, patch) + + assert (repo / "src.txt").read_text() == "original\nattempted change\n" + assert (repo / "new_module.py").read_text() == "print('hi')\n" + + +def test_apply_patch_missing_file_raises(project): + with pytest.raises(verify.GitError, match="restore patch not found"): + verify.apply_patch(project.project, project.implementation_artifacts / "nope.patch") + + +def test_apply_patch_conflict_raises(project): + """A patch that does not apply against the current tree raises GitError with + git's output — the caller escalates rather than dispatch onto a broken tree.""" + repo = project.project + patch = project.implementation_artifacts / "bad.patch" + # a diff against content the tree does not have + patch.write_text( + "--- a/src.txt\n+++ b/src.txt\n@@ -1 +1 @@\n-something-else\n+patched\n", + encoding="utf-8", + ) + with pytest.raises(verify.GitError, match="git apply"): + verify.apply_patch(repo, patch) + + def test_read_frontmatter_tolerates_garbage(project): p = project.project / "x.md" p.write_text("no frontmatter here")