diff --git a/CHANGELOG.md b/CHANGELOG.md index 06981de..182c04c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,21 @@ breaking changes may land in a minor release. `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. +### Changed + +- **The patch-restore seam is now one validator, one path normalizer, and one exclusion site.** + `runs.validate_restore_latch` holds every latch precondition (sentinel wedge, spec-less escalation, + worktree isolation) — the worktree check lived only in the CLI, so `rearm_escalation` called + programmatically could latch a patch the re-drive can never honor; it now rejects it too. + `verify.resolve_restore_path` replaces four copies of the maybe-relative→absolute join, and the + shared verify gate derives the restore-patch proof-of-work exclusion from the task instead of + threading it in from three call sites. The resolve context's `restore_supported` signal is now the + validator's verdict too, so the agent never negotiates a restore for a sentinel-wedged or spec-less + escalation either. Otherwise behavior-neutral. (closes #91) +- **Test helper fidelity.** `make_engine` seeds the launching scope (`max_stories`, `story_filter`, + `epic_filter`) on `RunState` like `cmd_run` does, so resume tests no longer silently ran uncapped; + the three `_escalated_run` fixtures collapse into one parameterized conftest builder. (closes #84) + ### Fixed - **An abandoned patch-restore no longer smuggles its files into the corrected story's commit.** diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 3683d02..83ea91a 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -811,6 +811,7 @@ def _resolve_restore_patch( story_key: str, args: argparse.Namespace, pol, + state, task, ) -> tuple[str | None, str | None]: """Determine the intent-gap patch-restore latch (BMAD-METHOD #2564) for a re-arm. @@ -825,7 +826,12 @@ def _resolve_restore_patch( outside the trusted roots, the run can't restore in place, or the restore input itself is corrupt (unreadable resolution.json, empty/non-string value) — the caller aborts strictly rather than silently re-driving from scratch - when a restore was (or may have been) asked for.""" + when a restore was (or may have been) asked for. + + The run-state preconditions come from ``runs.validate_restore_latch``, shared + verbatim with ``runs.rearm_escalation``; only the CLI-side halves live here — + path resolution against ``--project`` and trusted-roots containment, which need + the loaded bmad config.""" raw = getattr(args, "restore_patch", None) if raw is not None and not raw.strip(): # `--restore-patch ""` is a classic unset-shell-var slip. Treating it as @@ -859,27 +865,21 @@ def _resolve_restore_patch( raw = val if not raw: return None, None - # Restore is an in-place-only recovery: a worktree-isolation re-drive discards - # the unit's worktree (engine._finish_inflight — taking a patch saved inside it - # along) and re-mounts a fresh one, so the re-apply could only fail on a - # destroyed patch file. Reject up front instead of latching a patch that can - # never restore. Checked against BOTH the recorded run state - # (task.worktree_path — how the escalated unit actually executed) and the live - # policy (how the resume will execute), so a policy edit between escalation - # and resolve can't skew the guard. - if pol.scm.isolation == "worktree" or getattr(task, "worktree_path", ""): - return None, ( - "restore patch is unsupported for worktree-isolation runs (the re-drive " - "discards and re-mounts the unit's worktree, so an in-place restore has " - "nothing durable to land on) — re-arm from scratch instead: drop " - "--restore-patch, or if the resolve agent recorded the restore in " - "resolution.json, re-run with --no-interactive (which ignores that " - "marker) instead of repeating the agent session" - ) - patch = Path(raw) - if not patch.is_absolute(): - patch = project / patch - patch = patch.resolve() + # The state-side preconditions (sentinel wedge, spec-less escalation, worktree + # isolation) are the same set rearm_escalation enforces — run them here so an + # unhonorable restore aborts BEFORE the interactive resolve session rather than + # after. The live policy's isolation mode is the one input run state can't + # carry, so pass it: a policy edit between escalation and resolve can't skew + # the guard. + err = runs.validate_restore_latch( + state, task, story_key, worktree_isolation=pol.scm.isolation == "worktree" + ) + if err is not None: + return None, err + # `.resolve()` on top of the shared normalizer: this is the one consumer that + # feeds a containment check (spec_within_roots), which needs `..`/symlinks + # collapsed. The resolved absolute path is what gets latched. + patch = verify.resolve_restore_path(raw, project).resolve() # Same trusted-roots shape as the frontmatter reconcile's spec_within_roots: # bmad-dev-auto saves the patch under implementation_artifacts, and artifact # dirs configured OUTSIDE the project tree are a supported layout — a bare @@ -984,7 +984,9 @@ def cmd_resolve(args: argparse.Namespace) -> int: # can't be honored so it never negotiates one. restore_patch: str | None = None if args.restore_patch is not None: - restore_patch, err = _resolve_restore_patch(project, run_dir, story_key, args, pol, task) + restore_patch, err = _resolve_restore_patch( + project, run_dir, story_key, args, pol, state, task + ) if err is not None: print(err, file=sys.stderr) return 1 @@ -1014,7 +1016,9 @@ def cmd_resolve(args: argparse.Namespace) -> int: # resolution.json restore latch: only exists after the session ran, so this # arm of the validation cannot be hoisted above it. if args.restore_patch is None: - restore_patch, err = _resolve_restore_patch(project, run_dir, story_key, args, pol, task) + restore_patch, err = _resolve_restore_patch( + project, run_dir, story_key, args, pol, state, task + ) if err is not None: print(err, file=sys.stderr) return 1 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 46279d7..d13536d 100644 --- a/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md +++ b/src/bmad_loop/data/skills/bmad-loop-resolve/SKILL.md @@ -122,7 +122,9 @@ implemented. **First check `restore_supported` in the context file.** When it is `false` (worktree-isolation runs: the re-drive discards and re-mounts the unit's -worktree, so an in-place restore can never land), **never offer the restore +worktree, so an in-place restore can never land; an escalation with no recorded +spec: a restored patch has no review to resume; a pre-planning sentinel wedge: +there is no attempted implementation to restore), **never offer the restore option and never record `restore_patch`** — the orchestrator would reject the resolution and this whole session's negotiation would be wasted. The patch is still available as evidence. diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 22ec1d1..342717a 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -905,9 +905,7 @@ def _restore_patch(self, task: StoryTask) -> None: 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 + patch = verify.resolve_restore_path(task.restore_patch, self.workspace.root) try: verify.apply_patch(self.workspace.root, patch) except verify.GitError as e: @@ -917,8 +915,11 @@ def _restore_patch(self, task: StoryTask) -> None: 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 + # Call-site invariant: `_dev_phase` advances the task to DEV_RUNNING + # immediately before dispatch, and this runs on that path only — so the + # step to DEV_VERIFY is unconditional. It is required because `_escalate` + # cannot transition out of DEV_RUNNING directly. + advance(task, Phase.DEV_VERIFY) 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) diff --git a/src/bmad_loop/resolve.py b/src/bmad_loop/resolve.py index 64ee7df..f0f1c8a 100644 --- a/src/bmad_loop/resolve.py +++ b/src/bmad_loop/resolve.py @@ -23,6 +23,7 @@ from .adapters.base import SessionSpec from .model import RunState from .platform_util import safe_segment +from .runs import validate_restore_latch RESOLVE_DIR = "resolve" @@ -96,6 +97,16 @@ def _gather_escalations(run_dir: Path, state: RunState, story_key: str) -> list[ def build_context(state: RunState, run_dir: Path, story_key: str, *, isolation: str = "") -> Path: """Write resolve//context.json for the resolve skill to read.""" task = state.tasks.get(story_key) + # Patch-restore availability (#2564): the shared `validate_restore_latch` + # verdict, not a local copy of one leg. Any of them — worktree isolation (the + # re-drive discards and re-mounts the unit's worktree), a spec-less escalation, + # a pre-planning sentinel wedge — means the orchestrator would reject a + # `restore_patch` after the session; told to the agent up front so it never + # negotiates a restore it can't honor. + restore_supported = task is not None and ( + validate_restore_latch(state, task, story_key, worktree_isolation=isolation == "worktree") + is None + ) context = { "story_key": story_key, "run_id": state.run_id, @@ -106,11 +117,7 @@ def build_context(state: RunState, run_dir: Path, story_key: str, *, isolation: # as_posix so the context contract is the same string on every OS (the # path is consumed by the agent, and Python/tools accept '/' on Windows). "resolution_path": resolution_path(run_dir, story_key).as_posix(), - # Patch-restore availability (#2564): a worktree-isolation re-drive - # discards and re-mounts the unit's worktree, so an in-place restore can - # never land — the orchestrator rejects a `restore_patch` up front. Told - # to the agent here so it never negotiates a restore it can't honor. - "restore_supported": isolation != "worktree" and not (task.worktree_path if task else ""), + "restore_supported": restore_supported, } # Stories mode: hand the resolver the manifest intent (the story entry) and a # sentinel indicator, so it sees WHAT the story is meant to do and WHETHER the diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 65ab45e..3fd8065 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -13,7 +13,7 @@ from . import devcontract, verify from .adapters.multiplexer import get_multiplexer from .journal import STATE_FILE, Journal, load_state, save_state -from .model import PAUSE_ESCALATION, Phase +from .model import PAUSE_ESCALATION, Phase, RunState, StoryTask from .platform_util import atomic_replace from .process_host import get_process_host @@ -506,6 +506,66 @@ class RearmError(Exception): """The run/story is not in a re-armable escalation state.""" +def validate_restore_latch( + state: RunState, task: StoryTask, story_key: str, *, worktree_isolation: bool = False +) -> str | None: + """Every precondition an intent-gap patch-restore latch (BMAD-METHOD #2564) must + satisfy, in one place. Returns an operator-facing error string, or None to latch. + + The single seam for both entry points: `rearm_escalation` (which performs the + latch, and is also reachable programmatically — a TUI restore, a future caller) + and `cli._resolve_restore_patch` (which fails fast *before* the interactive + resolve session, so an unhonorable restore doesn't cost an agent conversation). + Splitting these let a non-CLI caller bypass the worktree half; keeping them here + means a caller cannot latch a patch the engine could never honor. + + The CLI knows one thing this cannot: the *live* policy's isolation mode, which + may have been edited between escalation and resolve. It passes that as + `worktree_isolation`; the recorded `task.worktree_path` (how the unit actually + executed) is checked here either way, so both entry points reject a + worktree-isolation restore and the CLI additionally catches a policy flip. + + Path resolution and trusted-roots containment stay CLI-side: they need + `--project` and the loaded bmad config, neither of which run state carries. + """ + # A sentinel-wedged story escalated BEFORE planning — there is no attempted + # implementation to restore, and its re-arm re-dispatches a planning leg. + # Keyed on the recorded detection verdict (task.sentinel_kind), not the on-disk + # basename, mirroring rearm_escalation's sentinel-clear branch. + if state.source == "stories" and task.sentinel_kind: + return ( + f"story {story_key} is wedged on a pre-planning {task.sentinel_kind} sentinel — " + "there is no attempted implementation to restore, and the re-drive starts " + "at planning. Re-run resolve without a restore patch for a clean re-plan." + ) + # Same seam, broader shape: a restore only works through the spec's in-review + # flip, so an escalation with NO recorded spec (an ambiguous two-file wedge, an + # unknown --story selector, a session that died before naming one) has no + # routing target — the latch would stick, the flip would be skipped, and the + # engine would lay the patch onto the tree before a planning leg. + if not task.spec_file: + return ( + f"story {story_key} has no recorded spec file, so a restored patch has no " + "review to resume (the re-drive starts at planning). Re-run resolve " + "without a restore patch for a from-scratch re-drive." + ) + # Restore is an in-place-only recovery: a worktree-isolation re-drive discards + # the unit's worktree (engine._finish_inflight — taking a patch saved inside it + # along) and re-mounts a fresh one, so the re-apply could only fail on a + # destroyed patch file. Reject up front instead of latching a patch that can + # never restore. + if worktree_isolation or task.worktree_path: + return ( + "restore patch is unsupported for worktree-isolation runs (the re-drive " + "discards and re-mounts the unit's worktree, so an in-place restore has " + "nothing durable to land on) — re-arm from scratch instead: drop " + "--restore-patch, or if the resolve agent recorded the restore in " + "resolution.json, re-run with --no-interactive (which ignores that " + "marker) instead of repeating the agent session" + ) + return None + + def rearm_escalation( run_dir: Path, story_key: str | None = None, *, restore_patch: str | None = None ) -> str: @@ -545,13 +605,11 @@ def rearm_escalation( Instead preserve a copy under `{run_dir}/sentinels/`, journal `sentinel-cleared` with the blocking condition, and delete it, so the re-dispatch resolves to a clean PENDING and re-plans from scratch (leg 1 again for a spec_checkpoint id). - A patch-restore re-arm is rejected for a sentinel-wedged story: a sentinel is a - pre-planning halt, so there is no attempted implementation to restore and the - re-dispatch is a planning leg — laying implementation onto the tree before a - planning session is never safe. - 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. + Returns the re-armed story key. Raises RearmError when the run is not paused at + the escalation stage, the target story is not escalated, or a supplied + `restore_patch` fails `validate_restore_latch` (the shared precondition set — + sentinel wedge, spec-less escalation, worktree isolation). """ state = load_state(run_dir) if state.paused_stage != PAUSE_ESCALATION: @@ -567,30 +625,15 @@ def rearm_escalation( raise RearmError(f"run {run_dir.name} has no task for story {key}") if task.phase != Phase.ESCALATED: raise RearmError(f"story {key} is not escalated (phase: {task.phase})") - # T1 guard (stories x patch-restore): a sentinel-wedged story escalated BEFORE - # planning — there is no attempted implementation to restore, and its re-arm - # re-dispatches a planning leg. Keyed on the recorded detection verdict - # (task.sentinel_kind), not the on-disk basename, mirroring the sentinel-clear - # branch below. Rejected here, before any task mutation, so the escalation - # stays armed for a corrected resolve. - if restore_patch and state.source == "stories" and task.sentinel_kind: - raise RearmError( - f"story {key} is wedged on a pre-planning {task.sentinel_kind} sentinel — " - "there is no attempted implementation to restore, and the re-drive starts " - "at planning. Re-run resolve without a restore patch for a clean re-plan." - ) - # Same seam, broader shape: a restore only works through the spec's in-review - # flip below, so an escalation with NO recorded spec (an ambiguous two-file - # wedge, an unknown --story selector, a session that died before naming one) - # has no routing target — the latch would stick, the flip would be skipped, - # and the engine would lay the patch onto the tree before a planning leg. - # Rejected before any mutation, like the sentinel guard above. - if restore_patch and not task.spec_file: - raise RearmError( - f"story {key} has no recorded spec file, so a restored patch has no " - "review to resume (the re-drive starts at planning). Re-run resolve " - "without a restore patch for a from-scratch re-drive." - ) + # Patch-restore preconditions (T1 guard + spec-less wedge + worktree isolation), + # rejected here before any task mutation so the escalation stays armed for a + # corrected resolve. `cli._resolve_restore_patch` runs the same validator ahead + # of the interactive session; this call is what makes a programmatic caller + # (TUI restore parity, scripts) unable to bypass it. + if restore_patch: + err = validate_restore_latch(state, task, key) + if err is not None: + raise RearmError(err) journal = Journal(run_dir) # Read before the unconditional overwrite below: they describe the restore @@ -752,9 +795,7 @@ def _stale_restore_residue( """ if not old_latch: return set() - patch_path = Path(old_latch) - if not patch_path.is_absolute(): - patch_path = repo / patch_path + patch_path = verify.resolve_restore_path(old_latch, repo) residue: set[str] = set() try: diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index a89c1b1..16c7228 100644 --- a/src/bmad_loop/verify.py +++ b/src/bmad_loop/verify.py @@ -1048,8 +1048,7 @@ def verify_dev_exclude_relpaths( status flip on the session's own spec count as real work.""" candidates: list[Path] = [paths.sprint_status, spec_path] if restore_patch: - p = Path(restore_patch) - candidates.append(p if p.is_absolute() else paths.project / p) + candidates.append(resolve_restore_path(restore_patch, paths.project)) out: list[str] = [] project = paths.project.resolve() for path in candidates: @@ -1096,15 +1095,23 @@ def _verify_shared_gates( paths: ProjectPaths, *, expected_status: str, - proof_exclude: tuple[str, ...] | None, + extra_exclude: tuple[str, ...] | None, ) -> VerifyOutcome | None: """The workflow-tag, expected-status, baseline-match, and proof-of-work gates shared verbatim by :func:`verify_dev`, :func:`verify_dev_bundle`, and :func:`verify_dev_stories` — factored out so the sprint-mode and stories-mode gates can't silently drift. Reads frontmatter once (callers must not re-read it). Returns a failing :class:`VerifyOutcome`, or ``None`` when every gate - passes and the caller may run its mode-specific tail. ``proof_exclude=None`` - skips the proof-of-work gate (a plan-halt leg produced only its own spec).""" + passes and the caller may run its mode-specific tail. + + The proof-of-work exclude is derived here from the `task` this gate already + receives (`verify_dev_exclude_relpaths`, which needs the latched restore patch); + ``extra_exclude`` carries only what a mode adds on top — ``()`` for sprint and + bundle, the story record + manifest for stories. Threading the restore patch in + from three call sites instead left a default-None foot-gun for a future fourth + mode, which would silently let a restore re-drive pass proof-of-work on the + patch file's mere presence. ``extra_exclude=None`` still skips the gate outright + (a plan-halt leg produced only its own spec).""" workflow = rj.get("workflow") if workflow != DEV_WORKFLOW: return VerifyOutcome.retry( @@ -1132,12 +1139,13 @@ def _verify_shared_gates( f"orchestrator-recorded baseline {task.baseline_commit[:12]}" ) - if proof_exclude is not None and task.baseline_commit: + if extra_exclude is not None and task.baseline_commit: + exclude = verify_dev_exclude_relpaths(paths, spec_path, task.restore_patch) + extra_exclude try: if not has_changes_since( paths.project, task.baseline_commit, - exclude=proof_exclude, + exclude=exclude, baseline_untracked=task.baseline_untracked, ): return VerifyOutcome.retry("no changes in worktree since baseline commit") @@ -1178,7 +1186,7 @@ def verify_dev( task, paths, expected_status="in-review" if review_enabled else "done", - proof_exclude=verify_dev_exclude_relpaths(paths, spec_path, task.restore_patch), + extra_exclude=(), ) if gate is not None: return gate @@ -1221,7 +1229,7 @@ def verify_dev_bundle( task, paths, expected_status="in-review" if review_enabled else "done", - proof_exclude=verify_dev_exclude_relpaths(paths, spec_path, task.restore_patch), + extra_exclude=(), ) if gate is not None: return gate @@ -1319,24 +1327,18 @@ def verify_dev_stories( expected = "in-review" if review_enabled else "done" # A plan-halt leg produced only its own spec (the plan), which proof-of-work - # already excludes; skip it (proof_exclude=None) and record the plan spec. - # Otherwise proof-of-work uses the file-granular exclude (verify_dev_exclude_ - # relpaths, matching verify_dev post-#79: only the session's own spec + the - # sprint-status ledger) plus the spec folder's stories/ subdir + stories.yaml — - # NOT the whole-folder artifact_relpaths, so a story whose entire authorized - # scope is ledger/spec reconciliation doesn't register as a false "no changes". + # already excludes; skip it (extra_exclude=None) and record the plan spec. + # Otherwise stories mode adds the spec folder's stories/ subdir + stories.yaml + # on top of the gate's own file-granular exclude — NOT the whole-folder + # artifact_relpaths, so a story whose entire authorized scope is ledger/spec + # reconciliation doesn't register as a false "no changes". gate = _verify_shared_gates( spec_path, rj, task, paths, expected_status=expected, - proof_exclude=( - None - if plan_halt - else verify_dev_exclude_relpaths(paths, spec_path, task.restore_patch) - + _stories_relpaths(paths.project, spec_folder) - ), + extra_exclude=(None if plan_halt else _stories_relpaths(paths.project, spec_folder)), ) if gate is not None: return gate @@ -1528,6 +1530,27 @@ def finalize_commit(repo: Path, baseline: str | None, message: str) -> str | Non return rev_parse_head(repo) +def resolve_restore_path(raw: str, root: Path) -> Path: + """The latched intent-gap patch (`StoryTask.restore_patch`) as a concrete path: + absolute values pass through, relative ones are anchored on `root`. + + `model.StoryTask.restore_patch` documents the field as repo-relative-or-absolute, + and every consumer must resolve it against the base it actually reads the tree + from — the engine's live workspace root (the unit worktree under isolation), + `paths.project` for the proof-of-work exclude, the CLI's `--project`. Hence the + caller-supplied `root` rather than one baked-in base. + + In practice `cli._resolve_restore_patch` always latches an already-`.resolve()`d + absolute path, so the relative branch is exercised only by a hand-written state + file or a future non-CLI latcher; it is kept because the field's contract + promises it. Deliberately does NOT `.resolve()` the result — callers that need + symlink/`..` normalization (path containment checks) do it themselves, and the + apply/exclude paths match the pre-existing behavior byte-for-byte without it. + """ + p = Path(raw) + return p if p.is_absolute() else root / p + + def apply_patch(repo: Path, patch_path: Path) -> None: """Apply a saved patch to `repo`'s working tree (`git apply`), raising on failure. diff --git a/tests/conftest.py b/tests/conftest.py index bd3efc7..c7a9dd9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,7 @@ import shutil import subprocess import sys +from dataclasses import dataclass from pathlib import Path import pytest @@ -13,6 +14,8 @@ from bmad_loop.adapters.base import SessionResult, SessionSpec from bmad_loop.bmadconfig import ProjectPaths +from bmad_loop.journal import save_state +from bmad_loop.model import PAUSE_ESCALATION, Phase, RunState, SessionRecord, StoryTask from bmad_loop.verify import rev_parse_head # The suite reads/writes UTF-8 files (specs, journals, JSON, reports). Windows' @@ -155,19 +158,51 @@ def install_bmad_config(paths: ProjectPaths) -> None: ) +def _write_skill_stubs(skills: Path, catalog: dict) -> None: + """Stub every skill in `catalog` (an install.py {skill: marker_files} map) under + `skills`. Reading the catalog instead of restating it means a newly required + skill or marker file fails the scaffolds loudly rather than drifting.""" + for skill, markers in catalog.items(): + d = skills / skill + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text(f"# {skill}\n", encoding="utf-8") + for marker in markers: + (d / marker).write_text("x\n", encoding="utf-8") + + +def install_dev_base_skills(root: Path, tree: str = ".claude/skills", *, folder_id: bool) -> Path: + """Lay down stubs of the upstream skills the orchestrator drives on every dev run + (`install.DEV_BASE_SKILLS`: bmad-dev-auto + the review hunters) under + ``root/tree``, so the run-start preflight (`install.missing_base_skills`) passes. + + ``folder_id`` also writes bmad-dev-auto's step-01 carrying the dispatch marker + `install.missing_stories_support` content-probes for — stories mode needs a newer + bmad-dev-auto than file existence alone can prove. Returns the skills tree root.""" + from bmad_loop.install import ( + DEV_BASE_SKILLS, + STORIES_PROBE_FILE, + STORIES_PROBE_SKILL, + STORIES_PROBE_TEXT, + ) + + skills = Path(root) / tree + _write_skill_stubs(skills, DEV_BASE_SKILLS) + if folder_id: + (skills / STORIES_PROBE_SKILL / STORIES_PROBE_FILE).write_text( + f"This is a **{STORIES_PROBE_TEXT}** router.\n", encoding="utf-8" + ) + return skills + + def install_base_skills(paths: ProjectPaths, trees=(".claude/skills", ".agents/skills")) -> None: - """Lay down stubs of the non-bundled upstream skills the orchestrator drives - (bmad-dev-auto + the review hunters) so the run-start preflight - (`install.missing_base_skills`) passes.""" + """Stub every non-bundled upstream skill (`install.BASE_SKILLS` — a superset of + DEV_BASE_SKILLS that also covers what a worktree mount must copy) in each of a + sandbox project's active CLI skill trees. Sprint mode drives any bmad-dev-auto, + so no folder+id probe is written.""" from bmad_loop.install import BASE_SKILLS for tree in trees: - for skill, markers in BASE_SKILLS.items(): - d = paths.project / tree / skill - d.mkdir(parents=True, exist_ok=True) - (d / "SKILL.md").write_text(f"# {skill}\n", encoding="utf-8") - for marker in markers: - (d / marker).write_text("x\n", encoding="utf-8") + _write_skill_stubs(paths.project / tree, BASE_SKILLS) def write_sprint(paths: ProjectPaths, statuses: dict[str, str]) -> None: @@ -212,6 +247,8 @@ def dev_effect( final_status: str = "done", followup_review: bool = True, prose_status: str | None = None, + seen: list[str] | None = None, + write_src: bool = True, ): """Simulate a successful bmad-dev-auto session: it self-finalizes the spec (no in-review handoff — always straight to ``done``) but never touches the @@ -223,12 +260,21 @@ def dev_effect( ``review.trigger = "recommended"``; set False to exercise the skip path. ``prose_status`` appends a terminal ``## Auto Run Result`` block with that Status line — pair it with a non-terminal ``final_status`` to reproduce the - skill leaving frontmatter behind its prose (the reconcile path).""" + skill leaving frontmatter behind its prose (the reconcile path). + + ``seen``, when given, collects `src.txt` as the session found it on entry — the + patch-restore tests assert the re-driven session ran against the RESTORED diff. + ``write_src=False`` then keeps the session from appending its own line, so what + lands in the tree is exactly what the restore laid down (the applied patch is + the session's proof of work; a second edit would muddy the assertion).""" def effect(spec: SessionSpec) -> SessionResult: baseline = rev_parse_head(paths.project) source = paths.project / "src.txt" - source.write_text(source.read_text() + f"change for {story_key}\n") + if seen is not None: + seen.append(source.read_text()) + if write_src: + source.write_text(source.read_text() + f"change for {story_key}\n") sp = spec_path(paths, story_key) write_spec(sp, final_status, baseline, prose_status=prose_status) # deliberately NO set_sprint: the dev skill does not write sprint-status @@ -466,3 +512,90 @@ def effect(spec: SessionSpec) -> SessionResult: ) return effect + + +# --------------------------------------------------------- escalated-run scaffolds + + +@dataclass +class EscalatedRun: + """What `escalated_run` built, so each caller can unpack only what it asserts on.""" + + run_dir: Path + state: RunState + task: StoryTask + + +def escalated_run( + project: Path, + run_id: str = "r1", + *, + story_key: str = "s1", + epic: int = 1, + attempt: int = 1, + review_cycle: int = 0, + baseline_commit: str | None = None, + started_at: str = "now", + paused_reason: str = "CRITICAL escalation", + source: str = "sprint-status", + spec_file: str | None = None, + restore_patch: str | None = None, + sentinel_kind: str = "", + worktree_path: str = "", + with_session: bool = False, + git_project: bool = False, +) -> EscalatedRun: + """A saved RunState paused at a CRITICAL escalation, with one ESCALATED task — + the shared shape behind test_runs / test_resolve / test_cli, whose three local + copies had drifted into different defaults, different return tuples, and one + unique kwarg each. Parameterized as a superset rather than lowest-common- + denominator: every field a caller relied on is still reachable, so no test's + fixture-specific assertion is weakened by the dedup. + + ``with_session`` appends the completed review SessionRecord the resolve-context + builder reads. ``git_project`` makes ``state.project`` a REAL repo (spec files + already written are committed, run state is gitignored) so `rearm_escalation`'s + baseline snapshot refresh actually runs and `baseline_commit` defaults to HEAD — + in a bare tmp_path its best-effort `except` swallows every git call and the + refresh silently no-ops. + """ + project = Path(project) + if git_project: + (project / ".gitignore").write_text(".bmad-loop/\n") # keep run state out of the snapshot + git(project, "init", "-q", "-b", "main") + git(project, "config", "user.email", "test@test") + git(project, "config", "user.name", "test") + git(project, "add", "-A") + git(project, "commit", "-q", "-m", "initial") + if baseline_commit is None: + baseline_commit = git(project, "rev-parse", "HEAD") + + task = StoryTask( + story_key=story_key, + epic=epic, + phase=Phase.ESCALATED, + attempt=attempt, + review_cycle=review_cycle, + baseline_commit=baseline_commit, + spec_file=spec_file, + restore_patch=restore_patch, + sentinel_kind=sentinel_kind, + worktree_path=worktree_path, + ) + if with_session: + task.sessions.append( + SessionRecord(task_id=f"{story_key}-review-1", role="review", status="completed") + ) + state = RunState( + run_id=run_id, + project=str(project), + started_at=started_at, + paused_reason=paused_reason, + paused_stage=PAUSE_ESCALATION, + paused_story_key=story_key, + tasks={story_key: task}, + source=source, + ) + run_dir = project / ".bmad-loop" / "runs" / run_id + save_state(run_dir, state) + return EscalatedRun(run_dir=run_dir, state=state, task=task) diff --git a/tests/test_cli.py b/tests/test_cli.py index 0ca5667..7107576 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5,7 +5,7 @@ import pytest import yaml -from conftest import install_bmad_config, write_sprint +from conftest import escalated_run, install_bmad_config, write_sprint from bmad_loop import cli from bmad_loop import policy as policy_mod @@ -824,24 +824,11 @@ def _write_bmad_config(project, impl="{project-root}/artifacts"): def _escalated_run(project, run_id="r1", *, story="s1", spec_file=None, worktree_path=""): - from bmad_loop.model import Phase, StoryTask - - task = StoryTask( - story_key=story, - epic=1, - phase=Phase.ESCALATED, - attempt=1, - spec_file=spec_file, - worktree_path=worktree_path, - ) - return _make_run_with_state( - project, - run_id, - paused_reason="CRITICAL escalation", - paused_stage="escalation", - paused_story_key=story, - tasks={story: task}, - ) + """conftest's builder with this module's shape: only the run_dir comes back (the + CLI tests drive the real `resolve` command and re-load state from disk).""" + return escalated_run( + project, run_id, story_key=story, spec_file=spec_file, worktree_path=worktree_path + ).run_dir def test_resolve_no_such_run(tmp_path, capsys): diff --git a/tests/test_engine.py b/tests/test_engine.py index 80639ce..428d143 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -2339,28 +2339,10 @@ def effect(spec): 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 + """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) and leaves `src.txt` exactly as the + restore laid it down — the applied patch IS this session's proof of work.""" + return dev_effect(project, story_key, followup_review=False, seen=seen, write_src=False) def test_restore_patch_applies_onto_baseline(project): diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 7e4b023..f56557b 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -4,7 +4,7 @@ import pytest import yaml -from conftest import git +from conftest import escalated_run, git from bmad_loop import devcontract, resolve, runs, verify from bmad_loop.journal import load_state, save_state @@ -12,8 +12,6 @@ PAUSE_ESCALATION, Phase, RunState, - SessionRecord, - StoryTask, ) from bmad_loop.platform_util import safe_segment @@ -40,36 +38,26 @@ def _escalated_run( with_session=True, source="sprint-status", sentinel_kind="", + worktree_path="", ): - task = StoryTask( + """conftest's builder with this module's shape: a review-cycle-1 task carrying a + completed review session (what `build_context` reads), returning the full triple.""" + run = escalated_run( + project, + run_id, story_key="6-4-cli-list-command", epic=6, - phase=Phase.ESCALATED, - attempt=1, review_cycle=1, baseline_commit="abc123", - spec_file=spec_file, - sentinel_kind=sentinel_kind, - ) - if with_session: - task.sessions.append( - SessionRecord( - task_id="6-4-cli-list-command-review-1", role="review", status="completed" - ) - ) - state = RunState( - run_id=run_id, - project=str(project), started_at="2026-06-13T11:14:29", paused_reason="CRITICAL escalation from review session: names not unique", - paused_stage=PAUSE_ESCALATION, - paused_story_key="6-4-cli-list-command", - tasks={task.story_key: task}, + spec_file=spec_file, + with_session=with_session, source=source, + sentinel_kind=sentinel_kind, + worktree_path=worktree_path, ) - run_dir = project / ".bmad-loop" / "runs" / run_id - save_state(run_dir, state) - return run_dir, state, task + return run.run_dir, run.state, run.task # ----------------------------------------------------------- set_frontmatter_status @@ -160,10 +148,12 @@ def test_build_context_no_session_files(tmp_path): def test_build_context_restore_supported_signal(tmp_path): - """The agent must know up front when a patch-restore can't be honored - (worktree isolation / a worktree-executed task), so it never negotiates a - restore the orchestrator will reject after the session.""" - run_dir, state, task = _escalated_run(tmp_path, with_session=False) + """The agent must know up front when a patch-restore can't be honored, so it + never negotiates a restore the orchestrator will reject after the session. The + flag is `validate_restore_latch`'s verdict — every leg (worktree isolation / a + worktree-executed task, a spec-less escalation, a pre-planning sentinel wedge), + not a worktree-only copy that drifts from the validator (#91).""" + run_dir, state, task = _escalated_run(tmp_path, spec_file="/abs/spec.md", with_session=False) key = "6-4-cli-list-command" path = resolve.build_context(state, run_dir, key) @@ -176,6 +166,17 @@ def test_build_context_restore_supported_signal(tmp_path): path = resolve.build_context(state, run_dir, key) assert json.loads(path.read_text(encoding="utf-8"))["restore_supported"] is False + task.worktree_path = "" + task.spec_file = None # spec-less escalation: a restored patch has no review to resume + path = resolve.build_context(state, run_dir, key) + assert json.loads(path.read_text(encoding="utf-8"))["restore_supported"] is False + + task.spec_file = "/abs/spec.md" + state.source = "stories" + task.sentinel_kind = "missing-prd" # pre-planning wedge: nothing attempted to restore + path = resolve.build_context(state, run_dir, key) + assert json.loads(path.read_text(encoding="utf-8"))["restore_supported"] is False + def test_build_context_sanitizes_dirty_story_key(tmp_path): """A story key with Windows-illegal chars lands in a sanitized directory, @@ -438,6 +439,40 @@ def test_rearm_rejects_restore_patch_without_a_spec_file(tmp_path): assert load_state(run_dir).tasks["6-4-cli-list-command"].phase == Phase.PENDING +def test_rearm_rejects_restore_patch_for_a_worktree_executed_task(tmp_path): + """#91: the worktree guard used to live ONLY in `cli._resolve_restore_patch`, so a + programmatic caller (TUI restore parity, a script) could latch a patch the + re-drive can never honor — it discards the unit's worktree and re-mounts a fresh + one. `validate_restore_latch` is now the single seam, enforced here too.""" + spec = tmp_path / "spec.md" + spec.write_text(SPEC, encoding="utf-8") + run_dir, _, _ = _escalated_run( + tmp_path, spec_file=str(spec), worktree_path=str(tmp_path / "wt" / "s1") + ) + + with pytest.raises(runs.RearmError, match="worktree-isolation"): + runs.rearm_escalation(run_dir, restore_patch="artifacts/attempt.patch") + + task = load_state(run_dir).tasks["6-4-cli-list-command"] + assert task.phase == Phase.ESCALATED # nothing mutated; still armed for a re-resolve + assert task.restore_patch is None + # a from-scratch re-arm of the same task is unaffected — the guard is latch-only + assert runs.rearm_escalation(run_dir) == "6-4-cli-list-command" + + +def test_validate_restore_latch_passes_a_clean_in_place_escalation(tmp_path): + """The happy leg of the shared validator, and the one input run state cannot + carry: `worktree_isolation` (the LIVE policy, which the CLI passes) rejects even + a task that executed in place, so a policy edit between escalation and resolve + cannot skew the guard.""" + _, state, task = _escalated_run(tmp_path, spec_file="/abs/spec.md") + key = "6-4-cli-list-command" + + assert runs.validate_restore_latch(state, task, key) is None + err = runs.validate_restore_latch(state, task, key, worktree_isolation=True) + assert err is not None and "worktree-isolation" in err + + def test_rearm_restore_patch_on_a_real_stories_spec_is_allowed(tmp_path): """The T1 guard keys on the recorded sentinel verdict, not on stories mode: a review-stage intent gap on a REAL stories spec (sentinel_kind unset) is a diff --git a/tests/test_runs.py b/tests/test_runs.py index cef6346..58a2d4b 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -7,7 +7,7 @@ import tarfile import pytest -from conftest import git +from conftest import escalated_run, git from bmad_loop import runs from bmad_loop.adapters import tmux_base @@ -532,40 +532,21 @@ def test_delete_run(tmp_path): def _escalated_run(tmp_path, spec_text, *, restore_patch_stale=None, git_project=False): - """`git_project=True` makes `state.project` a real repo with the spec committed, - so rearm's baseline snapshot refresh actually runs — in a bare tmp_path its - best-effort `except` swallows every git call and the refresh silently no-ops.""" - from bmad_loop.model import PAUSE_ESCALATION, Phase, StoryTask - + """conftest's builder with this module's shape: the spec is written first (so + `git_project=True` commits it), and only `(run_dir, spec)` comes back.""" spec = tmp_path / "spec.md" spec.write_text(spec_text, encoding="utf-8") - baseline = None - if git_project: - (tmp_path / ".gitignore").write_text(".bmad-loop/\n") # keep run state out of the snapshot - git(tmp_path, "init", "-q", "-b", "main") - git(tmp_path, "config", "user.email", "test@test") - git(tmp_path, "config", "user.name", "test") - git(tmp_path, "add", "-A") - git(tmp_path, "commit", "-q", "-m", "initial") - baseline = git(tmp_path, "rev-parse", "HEAD") - task = StoryTask( + run = escalated_run( + tmp_path, + "r1", story_key="1-1-a", - epic=1, - phase=Phase.ESCALATED, attempt=2, + started_at="2026-06-11T10:00:00", spec_file=str(spec), restore_patch=restore_patch_stale, - baseline_commit=baseline, - ) - 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}, + git_project=git_project, ) - return run_dir, spec + return run.run_dir, spec _SPEC_WITH_ARR = ( diff --git a/tests/test_stories_e2e.py b/tests/test_stories_e2e.py index ee4d230..9f41656 100644 --- a/tests/test_stories_e2e.py +++ b/tests/test_stories_e2e.py @@ -40,6 +40,7 @@ import pytest import yaml +from conftest import install_dev_base_skills HAVE_TMUX = sys.platform != "win32" and shutil.which("tmux") is not None pytestmark = pytest.mark.skipif(not HAVE_TMUX, reason="stories E2E needs real tmux") @@ -210,23 +211,7 @@ def _scaffold(root: Path, entries: list[dict]) -> None: (root / "_bmad-output" / sub).mkdir(parents=True, exist_ok=True) (root / "_bmad-output" / sub / ".keep").write_text("", encoding="utf-8") - # base skills (DEV_BASE_SKILLS) + the folder+id dispatch content probe - skills = root / ".claude" / "skills" - dev = skills / "bmad-dev-auto" - dev.mkdir(parents=True) - (dev / "SKILL.md").write_text("# bmad-dev-auto\n", encoding="utf-8") - (dev / "step-04-review.md").write_text("x\n", encoding="utf-8") - (dev / "customize.toml").write_text("# review layers\n", encoding="utf-8") - (dev / "step-01-clarify-and-route.md").write_text( - "This is a **folder+id dispatch** router.\n", encoding="utf-8" - ) - for hunter in ( - "bmad-review-adversarial-general", - "bmad-review-edge-case-hunter", - "bmad-review-verification-gap", - ): - (skills / hunter).mkdir(parents=True) - (skills / hunter / "SKILL.md").write_text(f"# {hunter}\n", encoding="utf-8") + install_dev_base_skills(root, folder_id=True) # tree matches PROFILE_TOML's skill_tree folder = root / SPEC_FOLDER (folder / "stories").mkdir(parents=True) @@ -280,22 +265,7 @@ def _scaffold_sprint(root: Path, story_key: str) -> None: (root / "_bmad-output" / sub / ".keep").write_text("", encoding="utf-8") # the SAME new folder+id-capable skill stub the stories scaffold installs - skills = root / ".claude" / "skills" - dev = skills / "bmad-dev-auto" - dev.mkdir(parents=True) - (dev / "SKILL.md").write_text("# bmad-dev-auto\n", encoding="utf-8") - (dev / "step-04-review.md").write_text("x\n", encoding="utf-8") - (dev / "customize.toml").write_text("# review layers\n", encoding="utf-8") - (dev / "step-01-clarify-and-route.md").write_text( - "This is a **folder+id dispatch** router.\n", encoding="utf-8" - ) - for hunter in ( - "bmad-review-adversarial-general", - "bmad-review-edge-case-hunter", - "bmad-review-verification-gap", - ): - (skills / hunter).mkdir(parents=True) - (skills / hunter / "SKILL.md").write_text(f"# {hunter}\n", encoding="utf-8") + install_dev_base_skills(root, folder_id=True) # tree matches PROFILE_TOML's skill_tree sprint = { "generated": "01-06-2026 10:00", @@ -355,22 +325,7 @@ def _scaffold_sweep(root: Path) -> None: (root / "_bmad-output" / sub / ".keep").write_text("", encoding="utf-8") # the SAME folder+id-capable skill stubs the other scaffolds install - skills = root / ".claude" / "skills" - dev = skills / "bmad-dev-auto" - dev.mkdir(parents=True) - (dev / "SKILL.md").write_text("# bmad-dev-auto\n", encoding="utf-8") - (dev / "step-04-review.md").write_text("x\n", encoding="utf-8") - (dev / "customize.toml").write_text("# review layers\n", encoding="utf-8") - (dev / "step-01-clarify-and-route.md").write_text( - "This is a **folder+id dispatch** router.\n", encoding="utf-8" - ) - for hunter in ( - "bmad-review-adversarial-general", - "bmad-review-edge-case-hunter", - "bmad-review-verification-gap", - ): - (skills / hunter).mkdir(parents=True) - (skills / hunter / "SKILL.md").write_text(f"# {hunter}\n", encoding="utf-8") + install_dev_base_skills(root, folder_id=True) # tree matches PROFILE_TOML's skill_tree # canonical DW-format ledger (no legacy content → migration is skipped) (impl / "deferred-work.md").write_text( diff --git a/tests/test_stories_engine.py b/tests/test_stories_engine.py index 86efc2e..d698e9d 100644 --- a/tests/test_stories_engine.py +++ b/tests/test_stories_engine.py @@ -11,7 +11,7 @@ from bmad_loop.adapters.base import SessionResult from bmad_loop.adapters.mock import MockAdapter -from bmad_loop.journal import Journal, load_state +from bmad_loop.journal import Journal, load_state, save_state from bmad_loop.model import ( PAUSE_ESCALATION, PAUSE_PLAN_CHECKPOINT, @@ -136,9 +136,20 @@ def effect(spec) -> SessionResult: def make_engine(project, script, *, policy=None, spec_folder=SPEC_FOLDER, **kwargs): + """Mirrors `cli.cmd_run`: the launching scope (`--max-stories`, `--story`, `--epic`) + is persisted on RunState as well as handed to the engine, because `resume` rebuilds + the cap and filters from run state alone (cli._resume_paused_run). Seeding only the + constructor made every resume test silently run uncapped/unfiltered (#84).""" run_dir = project.project / ".bmad-loop" / "runs" / "test-run" adapter = MockAdapter(script, usage_per_session=TokenUsage(input_tokens=10, output_tokens=5)) - state = RunState(run_id="test-run", project=str(project.project), started_at="now") + state = RunState( + run_id="test-run", + project=str(project.project), + started_at="now", + max_stories=kwargs.get("max_stories"), + story_filter=kwargs.get("story_filter"), + epic_filter=kwargs.get("epic_filter"), + ) engine = StoriesEngine( paths=project, policy=policy or _stories_policy(), @@ -1004,7 +1015,6 @@ def test_max_stories_survives_a_checkpoint_pause_resume(project): exceeded).""" setup_stories(project, [entry("1", done_checkpoint=True), entry("2"), entry("3")]) engine, _ = make_engine(project, [stories_dev_effect()], max_stories=2) - engine.state.max_stories = 2 # cli.py persists this on RunState so resume rebuilds the cap # run 1: story 1 dispatched + committed, then pauses at its done_checkpoint # (dispatched=1 < cap=2, more stories pending → not skip-if-last) @@ -1024,6 +1034,31 @@ def test_max_stories_survives_a_checkpoint_pause_resume(project): assert _kinds(resumed.journal, "max-stories-reached") +def test_make_engine_persists_the_launching_scope_for_resume(project): + """#84: the launching scope must survive the state round-trip with no per-test + seeding — a `make_engine` that configured only the constructor let every resume + test above silently run uncapped/unfiltered, masking exactly the durability + regressions they exist to catch.""" + setup_stories(project, [entry("1"), entry("2")]) + engine, _ = make_engine(project, [], max_stories=2, story_filter="1", epic_filter=7) + save_state(engine.run_dir, engine.state) # the engine's own _save, ahead of a run + + reloaded = load_state(engine.run_dir) # what `resume` actually reads back + assert reloaded.max_stories == 2 + assert reloaded.story_filter == "1" + assert reloaded.epic_filter == 7 + + resumed, _ = resume_engine(project, engine, []) # no manual seeding anywhere + assert resumed.state.max_stories == 2 + # construction must not clobber the durable scope: StoriesEngine nulls the + # story_filter/epic_filter *constructor kwargs* (a flat list has no E-S refs), + # and the base Engine parks them on itself — never back onto RunState. + assert resumed.state.story_filter == "1" + assert resumed.state.epic_filter == 7 + # `--story` instead drives StoriesEngine's own id filter, scanned at pick time + assert resumed._story_id_filter == "1" + + def test_sentinel_detected_journaled_at_pick(project): """MINOR-6: a fixed-slug sentinel found by the pick-time read-back journals a distinct sentinel-detected event carrying its recorded blocking condition, not diff --git a/tests/test_verify.py b/tests/test_verify.py index 92571dd..e0ed7e8 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -1300,6 +1300,45 @@ def test_verify_dev_latched_restore_patch_is_not_proof_of_work(project): assert not out.ok and "no changes" in out.reason +def test_verify_dev_stories_latched_restore_patch_is_not_proof_of_work(project): + """The same T4 exclusion, reached through the stories gate. `_verify_shared_gates` + derives it from `task.restore_patch` for every mode, so a mode that forgets to + thread the latch through can no longer regress silently (#91).""" + spec_folder = project.planning_artifacts / "epic-a" + task = make_stories_task(project, "1") + sp = write_story(spec_folder, "1", "x", "done", "NO_VCS") + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", "baseline") + task.baseline_commit = verify.rev_parse_head(project.project) + patch = project.implementation_artifacts / "attempt.patch" + patch.write_text("stale attempt diff\n", encoding="utf-8") # untracked residue + + # control: unlatched, the residue passes for real session work + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert out.ok + + task.restore_patch = str(patch) + out = verify.verify_dev_stories( + task, project, dev_result(sp), spec_folder=spec_folder, review_enabled=False + ) + assert not out.ok and "no changes" in out.reason + + +def test_resolve_restore_path_joins_only_relative_latches(tmp_path): + """The one normalizer behind engine/verify/cli/runs (#91): absolute latches pass + through untouched, relative ones anchor on the caller's root, and nothing is + `.resolve()`d (the CLI adds that itself, for its containment check).""" + absolute = tmp_path / "sub" / "attempt.patch" + assert verify.resolve_restore_path(str(absolute), tmp_path / "other") == absolute + assert verify.resolve_restore_path("art/attempt.patch", tmp_path) == tmp_path / "art" / ( + "attempt.patch" + ) + # no normalization: the `..` survives, so a containment check still sees it + assert ".." in str(verify.resolve_restore_path("../escape.patch", tmp_path)) + + def test_verify_dev_baseline_era_untracked_is_not_proof_of_work(project): """#88: the from-scratch case the T4 latch exclusion cannot reach. After an intent-gap halt the saved patch is untracked residue under the protected