diff --git a/CHANGELOG.md b/CHANGELOG.md index 456513222..a55a12d0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ breaking changes may land in a minor release. invalidates Codex hook trust: Codex re-prompts at the next launch, and hooks silently do not fire until the new commands are accepted. Re-run `bmad-loop init` to migrate managed registrations. `validate` warns when a hook still points to another installation. +- Name an earlier attempt's parked work in the retry dev prompt (sprint, stories, sweep) + once Git confirms the ref still resolves on this task's baseline and a dev session + produced it; commits-only preservation is labelled, and nothing is replayed (#777). - Document the live-session removal guard's measured ceiling (#732): `delete`, `archive` and `clean` still remove a run directory when a listing omits a live session. Behavior unchanged; the psmux half is reported upstream (psmux/psmux#622), its retirement tracked in #754. ### Fixed diff --git a/docs/FEATURES.md b/docs/FEATURES.md index f34129209..ceee1f1ae 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -64,7 +64,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Bounded dev retries (default 2): verify-failures keep the tree and feed the failing output to the next session via `--feedback`; other failures roll back to baseline. - Silent dev/review sessions enter bounded stall recovery from launch: transport activity (pane output or parent/child OpenCode SSE) re-arms the grace, and a provable OpenCode `busy`/`retry` status protects active work from a nudge. Wake prompts are bounded attempts, not guaranteed recovery; if a dead multiplexer window rejects one, the loop degrades to its next liveness classification instead of escaping. None of these are completion signals — completion still requires Stop/idle evidence or process/window death, followed by deterministic artifact verification. -- An auto-rollback parks the attempt before it resets — commits above baseline on an `attempt-preserve/*` branch, the uncommitted tree (tracked edits + run-created untracked files) on a `refs/attempt-preserve-dirty/*` snapshot — and **refuses the reset if it could not** (#340): the run pauses with rescue instructions naming the tree, rather than discarding work the safety net failed to capture. Ordinary resolved re-drive preservation is best-effort and proceeds after journaling a fault; restoring a changed snapshot-backed spec is the exception, because replacing the only unparked child copy is unsafe. A configured external artifact cannot enter a Git recovery ref, so that case pauses for manual adoption. `scm.preserve_keep` (default 20) bounds retention of both ref families. +- An auto-rollback parks the attempt before it resets — commits above baseline on an `attempt-preserve/*` branch, the uncommitted tree (tracked edits + run-created untracked files) on a `refs/attempt-preserve-dirty/*` snapshot — and **refuses the reset if it could not** (#340): the run pauses with rescue instructions naming the tree, rather than discarding work the safety net failed to capture. Ordinary resolved re-drive preservation is best-effort and proceeds after journaling a fault; restoring a changed snapshot-backed spec is the exception, because replacing the only unparked child copy is unsafe. A configured external artifact cannot enter a Git recovery ref, so that case pauses for manual adoption. `scm.preserve_keep` (default 20) bounds retention of both ref families. A fresh-baseline dev retry after such a rollback is told where that work is parked (#777) — the ref, a `git log`/`git diff` against the baseline, and that the work is unverified and every gate must pass fresh — but only when Git confirms the ref still resolves, carries this run's name and descends from this task's baseline, and a dev session of the rolled-back attempt was dispatched — recorded, or cut off mid-session by a hard stop or crash. A pruned ref, a resolve re-drive's reset, or a sweep bundle that adopted another bundle's ids gets no paragraph; commits-only preservation says so. Nothing is replayed onto the tree. - Plateau-defer: when review won't converge the story is skipped, the spec stashed into the run dir, deferred-work preserved, and the run continues. The defer notification names where the attempt survives — in place, the recovery ref plus the `git merge --ff-only` line that restores it (flagged commits-only when the uncommitted snapshot could not be captured); isolated, the kept-failed unit branch plus any earlier attempt's ref, named rather than offered as a merge. That ref is projected as `preserve_ref` in `status`/`--json`; the unit branch never is (#333). When the recovery itself pauses the run, the defer record still lands first, pointing at the manual-recovery notice instead of a ref (#342). - Stories owing human-only external actions park at `awaiting-operator` instead of lying (#335). A story owing something no agent can do (buy a domain, publish a DNS record, grant an API key) **commits** everything an agent can, records what is owed in its spec's `operator_actions:` frontmatter, and parks. The board moves forward, the run continues, and nothing is rolled back — a park is a success that commits, so there is no stash and no recovery ref. It clears the deterministic gates that still apply (spec/board pair, your verify commands, a non-empty action list) and skips two: the review loop, and the dev gate's proof-of-work — a park's whole output can legitimately be the spec and the board (#676). Proof-of-work is skipped only when verification observes both a valid parked spec and `park_asserted: true` in the synthesized dev result. That strict boolean is minted only from the current session's last genuine, non-fenced `## Auto Run Result` marker reporting `awaiting-operator`; frontmatter-only fallback, orchestrator-repaired markers, legacy results, and malformed values fail closed onto the ordinary diff requirement. This prevents previous-run, out-of-band, and re-armed specs from inheriting waiver authority through retained frontmatter or `operator_actions:` while preserving crash and fixable-retry result replay. Nothing else narrows: the status pair, action list, workflow tag, baseline match and board sync all still select on the observed parked state, so an unasserted park that did real work passes as before. A park that clears the artifact gate with the waiver is journaled as `park-proof-of-work-skipped`; `zero_diff` reports whether the waived gate would have found non-excluded residue (`true` means none, `false` means some, `null` means the probe could not answer). The record means only that this attempt cleared the artifact gate with proof-of-work waived; later verify commands, review verification and repair, pre-commit workflows, or the commit may still reject it. The committed half is the later `story-awaiting-operator` event. Parking is notify-only and never halts the run; `[operator] enabled = false` restores the old two-outcome behavior, where such a story could only be `done` or `blocked`. - A successful stories-mode plan halt is likewise journaled at the artifact-gate boundary as `plan-halt-proof-of-work-skipped`. Its `zero_diff` uses the same tri-state projection (`true` = the waived gate found no non-excluded residue, `false` = it found changes, `null` = Git could not answer), including the stories manifest/spec and orchestrator-written exclusions the live gate would have used. The record is emitted only after the `ready-for-dev` artifact verification passes and never substitutes for the independent `result.json` `plan_halt: true` marker that authorizes the halt; an absent marker or an earlier gate failure produces no waiver record. diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 893d53240..fa4cc758b 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -868,7 +868,8 @@ def __init__( # active workspace; `escalate` routes an intent-gap restore failure through # the engine's escalation; `escalation_pause` raises RunPaused for it # (injected so recovery_flow need not import engine — that would reintroduce - # a runtime<->engine cycle). + # a runtime<->engine cycle); `dev_attempt_dispatched` is the preserve-ref + # provenance probe (#777). self._recovery_flow = RecoveryFlow( paths=self.paths, policy=self.policy, @@ -880,6 +881,7 @@ def __init__( save=self._save, escalate=self._escalate, escalation_pause=self._escalation_pause, + dev_attempt_dispatched=self._dev_attempt_dispatched, ) def _escalation_pause( @@ -1548,6 +1550,9 @@ def _first(epic: int | None): def _protected_relpaths(self) -> tuple[str, ...]: return self._recovery_flow.protected_relpaths() + def _retry_preserve_notice(self, task: StoryTask) -> str: + return self._recovery_flow.retry_preserve_notice(task) + def _rollback_or_pause(self, task: StoryTask, *, cause: str = "stopped") -> None: self._recovery_flow.rollback_or_pause(task, cause=cause) @@ -2172,6 +2177,18 @@ def _current_dev_session_index(self, task: StoryTask) -> int | None: return index return None + def _dev_attempt_dispatched(self, task: StoryTask) -> bool: + """Whether a dev session of the task's current attempt was dispatched — + the provenance a rollback stamps on the ref it parks (#777, + ``StoryTask.preserve_from_attempt``). A recorded session proves it, but a + session is recorded only once it returns: a hard stop or host death + mid-session leaves none, and the restart arm then parks that session's + tree. A durable ``DEV_RUNNING`` covers that case — it is saved after + ``attempt`` is bumped and before the launch, and a resolve re-drive's + reset runs from the ``PENDING`` that ``runs.rearm_escalation`` leaves + (under a bumped ``generation``, so no current-attempt record either).""" + return task.phase == Phase.DEV_RUNNING or self._current_dev_session_index(task) is not None + def _current_review_session_index(self, task: StoryTask) -> int | None: """Index of the newest review record for the current cycle.""" task_id = _session_task_id(task.story_key, "review", task.review_cycle, task.generation) @@ -7084,6 +7101,13 @@ def _generic_dev_prompt(self, task: StoryTask, feedback: Path | None) -> str: f"the working tree after an intent-gap resolution; review it " f"against the amended spec." ) + after_sentence + # The two fresh-baseline legs below may follow a rolled-back attempt: + # point at its parked work in a paragraph of its own (#777). It sits + # after the park clause, which stays last on the invocation line; these + # legs carry no feedback-file pointer for its backticks to be read as. + preserved = self._retry_preserve_notice(task) + after_sentence += f"\n\n{preserved}" if preserved else "" + after_key += f"\n\n{preserved}" if preserved else "" # The attempt binding was resolved in the active workspace immediately # before DEV_RUNNING became durable. A retained `spec_file` alone may # name a discarded unit worktree, so it cannot authorize this route or diff --git a/src/bmad_loop/model.py b/src/bmad_loop/model.py index e5a15f69d..04bbf8cea 100644 --- a/src/bmad_loop/model.py +++ b/src/bmad_loop/model.py @@ -416,7 +416,8 @@ class StoryTask: # writer wins, so one `git merge --ff-only ` recovers the whole attempt # — unless `preserve_partial` is set). Set by RecoveryFlow, cleared at the top # of every auto-rollback so it can never name a *previous* attempt's ref; read - # by `_defer` (notification) and projected into `status`. None = the last + # by `_defer` (notification), projected into `status`, and — once verified + # against git — named in the retry dev prompt (#777). None = the last # auto-rollback parked nothing (no commits above baseline and a clean or # uncapturable tree, or the ref failed to take). Isolation-INDEPENDENT: a unit # worktree's own dev-retry rollback parks on the same shared refs, so a @@ -435,6 +436,21 @@ class StoryTask: # cannot tell those apart, which is why this is recorded rather than derived. # Cleared with `preserve_ref`. Survives the resume serialization round-trip. preserve_partial: bool = False + # provenance of `preserve_ref`, the one input the retry dev prompt's pointer + # at it cannot get from git (#777, `recovery_flow.retry_preserve_paragraph`): + # True when the auto-rollback that cleared the previous ref ran after a dev + # session of the current attempt was dispatched — a recorded session, or a + # durable DEV_RUNNING that a hard stop or crash mid-session left without one + # (`Engine._dev_attempt_dispatched`) — so whatever it parks is that attempt's + # work. False when neither holds — a resolve re-drive resets a tree no + # dispatched attempt produced — and set back to False by `SweepEngine. + # _reset_superseded_bundle_state`, which keeps a superseded bundle's ref + # (clearing the name would orphan the work) that shares this task's run, key + # and baseline. Positive evidence + # only: a state.json written before this field loads False, so an unproven ref + # is never offered. Reset with `preserve_ref`. Survives the resume + # serialization round-trip. + preserve_from_attempt: bool = False # set by runs.rearm_escalation: this task was re-armed out of ESCALATED for a # clean rebuild against the corrected spec (not a failed attempt). Lets the # resume-time manual-recovery notice describe the real cause; cleared once the @@ -571,6 +587,7 @@ def to_dict(self) -> dict[str, Any]: "defer_reason": self.defer_reason, "preserve_ref": self.preserve_ref, "preserve_partial": self.preserve_partial, + "preserve_from_attempt": self.preserve_from_attempt, "rearmed": self.rearmed, "resolved_redrive": self.resolved_redrive, "plan_checkpoint_pending": self.plan_checkpoint_pending, @@ -814,6 +831,7 @@ def from_dict(cls, d: dict[str, Any]) -> "StoryTask": defer_reason=d.get("defer_reason"), preserve_ref=d.get("preserve_ref"), preserve_partial=bool(d.get("preserve_partial", False)), + preserve_from_attempt=bool(d.get("preserve_from_attempt", False)), rearmed=bool(d.get("rearmed", False)), resolved_redrive=bool(d.get("resolved_redrive", False)), plan_checkpoint_pending=bool(d.get("plan_checkpoint_pending", False)), diff --git a/src/bmad_loop/recovery_flow.py b/src/bmad_loop/recovery_flow.py index 2d0b684c9..6e00c3a7b 100644 --- a/src/bmad_loop/recovery_flow.py +++ b/src/bmad_loop/recovery_flow.py @@ -17,6 +17,7 @@ import errno import os +import re import stat from pathlib import Path from typing import TYPE_CHECKING, Callable, NoReturn @@ -58,6 +59,85 @@ def attempt_preserve_ref_name(run_id: str, tip: str) -> str: return f"attempt-preserve/{safe_ref_segment(run_id)}-{tip[:8]}" +def retry_preserve_paragraph(repo: Path, task: StoryTask, run_id: str) -> str: + """The retry dev prompt's pointer at an earlier attempt's parked work (#777), + or "" when the evidence does not support one. Informational only: the + orchestrator never replays, merges or cherry-picks the ref, and the paragraph + never asks the session to. + + Shared by every dev-prompt builder (``Engine._generic_dev_prompt``, + ``StoriesEngine._stories_dev_prompt``, ``SweepEngine._generic_bundle_prompt``) + through ``Engine._retry_preserve_notice``; each appends it, as its own + paragraph, to its fresh-baseline legs only — a repair leg keeps the rejected + attempt's tree, and a patch-restore leg has already laid that attempt back + onto it. ``task.preserve_ref`` is set only by an auto-rollback of this task, + so a set ref already means this dispatch follows a rolled-back attempt. + + Offered only when git confirms the claim, through the verify chokepoint: + + - the name is one this run's rollback mints — ``refs/attempt-preserve-dirty/ + --[-rN]`` with ```` this task's baseline, + or the ``attempt-preserve/-`` commits branch whose ```` is + the commit it still resolves to; + - it resolves to a commit (``rev-parse --verify ^{commit}``) — a ref the + run-start retention pruned before a resume does not; + - that commit descends from, and differs from, ``task.baseline_commit`` — so + the offered ``git diff`` is this task's work over this task's tree. A + baseline re-stamped past the work (another unit merged first) fails it. + + ``preserve_from_attempt`` must be set: git can show that the ref is this + run's and sits on this baseline, but not that a dispatched attempt produced + it. A resolve re-drive parks a tree no attempt wrote, and a sweep bundle + replacement keeps the superseded bundle's ref on the same key and baseline; + both leave the flag False. + + Says "an earlier attempt", never "the previous" one: a clean rollback keeps + an older attempt's ref (see ``rollback_or_pause``), and ``task.attempt`` is + re-armed to 0 by a resolve, so neither the ref nor the counter proves the + work is the immediately preceding attempt's. ``preserve_partial`` narrows the + claim to the commits alone. A failed check omits the paragraph and leaves the + ref and task untouched: the ref may still be the only copy of that work.""" + ref = task.preserve_ref + baseline = task.baseline_commit + if not ref or not baseline or not task.preserve_from_attempt: + return "" + slug = re.escape(safe_ref_segment(run_id)) + dirty = re.fullmatch(rf"refs/attempt-preserve-dirty/{slug}-([0-9a-f]{{8}})-\d+(?:-r\d+)?", ref) + commits = re.fullmatch(rf"attempt-preserve/{slug}-([0-9a-f]{{8}})", ref) + if dirty is not None: + if dirty.group(1) != baseline[:8]: + return "" + refname = ref + elif commits is not None: + # Fully qualified so neither the check nor the offered commands can fall + # back to a same-named tag or remote ref. + refname = f"refs/heads/{ref}" + else: + return "" + try: + tip = verify.rev_parse_revision(repo, refname) + except (verify.GitError, OSError): + return "" + if commits is not None and tip[:8] != commits.group(1): + return "" + if tip == baseline or not verify.is_ancestor(repo, baseline, tip): + return "" + if task.preserve_partial: + held = ( + f"only its commits were preserved, at `{refname}` — its uncommitted " + f"changes were not captured there" + ) + else: + held = f"its work is preserved at `{refname}`" + return ( + f"An earlier attempt at this work was rolled back; {held}. Inspect it with " + f"`git log --oneline {baseline}..{refname}` and `git diff {baseline} {refname}`. " + f"That work is unverified and has not been applied to this working tree: " + f"judge anything you take from it against the spec, and every gate must " + f"pass fresh on this attempt." + ) + + class _OwnedSpecAuthorityError(RuntimeError): """A previously canonical owned spec lost trustworthy restore authority.""" @@ -89,7 +169,9 @@ class RecoveryFlow: ``escalation_pause`` raises the engine's ``RunPaused`` (injected so this module need not import ``engine`` — that would reintroduce a runtime<->engine import cycle). ``workspace_get`` reads the engine's live (worktree-swappable) - active workspace.""" + active workspace. ``dev_attempt_dispatched`` answers whether a dev session of + the task's current attempt was dispatched — the provenance a rollback stamps + on the ref it parks (``StoryTask.preserve_from_attempt``).""" def __init__( self, @@ -104,6 +186,7 @@ def __init__( save: Callable[[], None], escalate: Callable[[StoryTask, str], None], escalation_pause: Callable[..., NoReturn], + dev_attempt_dispatched: Callable[[StoryTask], bool], ) -> None: self.paths = paths self.policy = policy @@ -119,6 +202,7 @@ def __init__( self._save = save self._escalate = escalate self._pause = escalation_pause + self._dev_attempt_dispatched = dev_attempt_dispatched def protected_relpaths(self) -> tuple[str, ...]: """Repo-relative posix paths of the BMAD artifact folders. These are @@ -1207,6 +1291,9 @@ def rollback_or_pause(self, task: StoryTask, *, cause: str = "stopped") -> None: # ref is then the only place the story's work survives. task.preserve_ref = None task.preserve_partial = False + # Provenance for whatever this rollback parks (#777): the retry + # prompt names the ref only when a dev session of this attempt ran. + task.preserve_from_attempt = self._dev_attempt_dispatched(task) self.journal.append( "rollback-auto", story_key=task.story_key, @@ -1481,6 +1568,11 @@ def restore_patch(self, task: StoryTask) -> None: 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 retry_preserve_notice(self, task: StoryTask) -> str: + """:func:`retry_preserve_paragraph` against the active workspace — recovery + refs are shared by every worktree of the repository — and this run.""" + return retry_preserve_paragraph(self._workspace_get().root, task, self.state.run_id) + 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/* diff --git a/src/bmad_loop/stories_engine.py b/src/bmad_loop/stories_engine.py index 2a904161d..2561b25c0 100644 --- a/src/bmad_loop/stories_engine.py +++ b/src/bmad_loop/stories_engine.py @@ -388,6 +388,8 @@ def _stories_dev_prompt(self, task: StoryTask, feedback: Path | None) -> str: (the primitive is disk-resolved — see ``Engine._dev_skill``) + (plan-halt leg) `` Halt after planning.`` + (when ``invoke_dev_with`` non-empty) a newline then its verbatim text. + + (on a retry with verified parked work) a blank line then + ``recovery_flow.retry_preserve_paragraph`` (#777). The folder is always project-relative (kills the absolute-path concern; the contract allows an absolute one but we never emit it). ``invoke_dev_with`` @@ -415,6 +417,11 @@ def _stories_dev_prompt(self, task: StoryTask, feedback: Path | None) -> str: prompt += " Halt after planning." if entry is not None and entry.invoke_dev_with: prompt += "\n" + entry.invoke_dev_with + # A retry after a rolled-back attempt names its verified parked work (#777), + # after the planner's verbatim text so that channel stays untouched. + preserved = self._retry_preserve_notice(task) + if preserved: + prompt += "\n\n" + preserved return prompt def _entry_for(self, task: StoryTask) -> stories.StoryEntry | None: diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index 9bbc4a33e..49de6cdac 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -2958,6 +2958,10 @@ def _reset_superseded_bundle_state(self, task: StoryTask) -> None: no-op once set, so a survivor would also refuse the replacement bundle's own spec on escalation. - ``restore_patch`` -- the diff of the superseded bundle's attempt. + - ``preserve_from_attempt`` -- the claim that ``preserve_ref`` holds THIS + task's attempt. The ref itself stays (below); the flag goes so the retry + dev prompt does not offer the superseded bundle's work as this one's + (#777). Git cannot tell the two apart: same run, key and baseline. - ``attempt`` + ``review_cycle`` + ``followup_reviews_spent`` -- reset the retry and review counters; clear the associated ``defer_reason`` and advance ``generation`` for fresh session ids. These operations follow @@ -2992,7 +2996,7 @@ def _reset_superseded_bundle_state(self, task: StoryTask) -> None: reset that got us here. - ``preserve_ref`` / ``preserve_partial`` -- a ref to a rolled-back worktree that still exists on disk; clearing the name would orphan it - rather than release it. + rather than release it. Its provenance flag is cleared instead (above). - the ``baseline_*`` pair and ``worktree_path`` / ``branch`` -- mount and rollback anchors owned by the reset, not by either bundle. - ``dispatched_spec_file`` / ``dispatched_spec_snapshot`` -- ``Sweep`` @@ -3035,6 +3039,7 @@ def _reset_superseded_bundle_state(self, task: StoryTask) -> None: task.artifact_publication_complete = False task.spec_file = None task.restore_patch = None + task.preserve_from_attempt = False task.attempt = 0 task.review_cycle = 0 task.followup_reviews_spent = 0 @@ -7426,12 +7431,15 @@ def _generic_bundle_prompt(self, task: StoryTask, feedback: Path | None) -> str: f"Do NOT edit the deferred-work ledger; the orchestrator records " f"resolution.{artifact_only_guidance}" ) + # A retry after a rolled-back attempt names its verified parked work + # (#777); a superseded bundle's ref is suppressed by the shared builder. + preserved = self._retry_preserve_notice(task) return ( f"/{self._dev_skill()} Implement the deferred-work bundle described in " f"`{bundle_ref}` — it carries the intent and the verbatim ledger " f"entries to resolve. Do NOT edit the deferred-work ledger; the " f"orchestrator records resolution.{artifact_only_guidance}" - ) + ) + (f"\n\n{preserved}" if preserved else "") self._reset_spec_for_repair(task) spec_ref = task.spec_file or bundle_ref return ( diff --git a/tests/test_engine.py b/tests/test_engine.py index d321328e7..c5eec0bd6 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -9324,6 +9324,91 @@ def test_rollback_preserves_uncommitted_attempt_worktree(project): assert git(repo, "show", f"{ref}:new_test.txt") == "uncommitted new file" +def _dirty_timeout_effect(paths, text: str): + """A dev session that times out mid-work, leaving an uncommitted edit that + the non-fixable retry's rollback parks under a worktree snapshot.""" + + def effect(spec): + (paths.project / "src.txt").write_text(text) + return SessionResult(status="timeout") + + return effect + + +def test_retry_dev_prompt_names_the_earlier_attempts_parked_work(project): + """#777: attempt 1 times out with work on the tree; the retry rolls it back + and parks it. Attempt 2's prompt names that verified snapshot — and only + attempt 2's: the first dispatch has nothing earlier to point at.""" + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [ + _dirty_timeout_effect(project, "half-built attempt 1\n"), + dev_effect(project, "1-1-a", followup_review=False), + ], + ) + + assert engine.run().done == 1 + + (entry,) = [e for e in engine.journal.entries() if e["kind"] == "attempt-worktree-preserved"] + ref = entry["ref"] + first, second = [s.prompt for s in adapter.sessions if s.role == "dev"] + assert "earlier attempt" not in first + base = engine.state.tasks["1-1-a"].baseline_commit + assert ( + "\n\nAn earlier attempt at this work was rolled back; its work is preserved at " in second + ) + assert f"`git diff {base} {ref}`" in second + assert "every gate must pass fresh on this attempt" in second + assert "half-built attempt 1" in git(project.project, "show", f"{ref}:src.txt") + + +def test_retry_dev_prompt_names_work_parked_after_a_mid_session_crash(project): + """#777: a host death mid-session records no session, so the resume's restart + arm parks the dirty tree with only the durable DEV_RUNNING to attribute it. + The re-dispatched attempt's prompt still names that work. + + Ablation: drop the DEV_RUNNING arm of `Engine._dev_attempt_dispatched` and + the paragraph disappears from the resumed prompt.""" + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + + def crash_mid_session(spec): + (project.project / "src.txt").write_text("half-built before the crash\n") + raise RuntimeError("host died mid-session") + + engine, _ = make_engine(project, [crash_mid_session]) + assert engine.run().crashed + crashed = load_state(engine.run_dir).tasks["1-1-a"] + assert crashed.phase == Phase.DEV_RUNNING and crashed.sessions == [] + + resumed, adapter = resume_engine( + project, engine, [dev_effect(project, "1-1-a", followup_review=False)] + ) + assert resumed.run().done == 1 + + kinds = [e["kind"] for e in resumed.journal.entries()] + assert "resume-restart" in kinds + (entry,) = [e for e in resumed.journal.entries() if e["kind"] == "attempt-worktree-preserved"] + ref = entry["ref"] + (prompt,) = [s.prompt for s in adapter.sessions if s.role == "dev"] + assert f"its work is preserved at `{ref}`" in prompt + assert "half-built before the crash" in git(project.project, "show", f"{ref}:src.txt") + + +def test_retry_dev_prompt_has_no_preserve_paragraph_when_nothing_was_parked(project): + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + engine, adapter = make_engine( + project, + [SessionResult(status="timeout"), dev_effect(project, "1-1-a", followup_review=False)], + ) + + assert engine.run().done == 1 + + assert engine.state.tasks["1-1-a"].preserve_ref is None + second = [s.prompt for s in adapter.sessions if s.role == "dev"][1] + assert "earlier attempt" not in second + + def test_rollback_preserves_distinct_refs_across_repeated_dirty_rollbacks(project): """Two dirty rollbacks against the SAME baseline_commit (mimicking the dev retry loop, where baseline_commit is fixed) must each park their uncommitted work under diff --git a/tests/test_recovery_flow.py b/tests/test_recovery_flow.py index 52b88e3ef..9d9c92b82 100644 --- a/tests/test_recovery_flow.py +++ b/tests/test_recovery_flow.py @@ -11,6 +11,7 @@ import os import socket import stat +import subprocess import sys from pathlib import Path, PureWindowsPath from types import SimpleNamespace @@ -1262,6 +1263,7 @@ def _make_flow( state=None, journal: _RecordingJournal | None = None, run_dir: Path | None = None, + dev_attempt_dispatched: bool = True, ): """Build a RecoveryFlow wired to recording stubs. The returned flow carries a ``.calls`` namespace tallying the injected callbacks for assertions. ``paths`` @@ -1295,6 +1297,7 @@ def _pause(reason, story_key="", *, cause=None): save=_save, escalate=_escalate, escalation_pause=_pause, + dev_attempt_dispatched=lambda task: dev_attempt_dispatched, ) flow.calls = calls return flow @@ -4328,3 +4331,225 @@ def boom(repo, patch): assert task.phase == Phase.DEV_VERIFY assert flow.calls.escalates assert "attempt-restore-failed" in flow.journal.events() + + +# --------------------------------------------------------------------------- +# retry dev prompt: the pointer at an earlier attempt's parked work (#777) + + +def _dirty_rollback(flow: RecoveryFlow, task: StoryTask, repo: Path, text: str) -> str: + """Leave one tracked edit on the tree and roll it back; return the snapshot ref.""" + (repo / "src.txt").write_text(text) + flow.rollback_or_pause(task) + assert git(repo, "status", "--porcelain") == "" + return flow.journal.entries[-1][1]["ref"] + + +def test_retry_preserve_notice_names_a_verified_worktree_snapshot(project): + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + + ref = _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + + assert ref.startswith("refs/attempt-preserve-dirty/run-1-") and task.preserve_ref == ref + notice = flow.retry_preserve_notice(task) + base = task.baseline_commit + assert notice.startswith("An earlier attempt at this work was rolled back; its work is ") + assert f"preserved at `{ref}`" in notice + assert f"`git log --oneline {base}..{ref}`" in notice + assert f"`git diff {base} {ref}`" in notice + assert "unverified and has not been applied to this working tree" in notice + assert "every gate must pass fresh on this attempt" in notice + # informational only: never a replay instruction, never a provenance overclaim + for word in ("cherry-pick", "merge", "previous attempt", "only its commits"): + assert word not in notice + # the offered command really shows the parked attempt against this baseline + assert "attempt 1 edit" in git(repo, "diff", base, ref) + + +def test_retry_preserve_notice_absent_without_a_preserved_ref(project): + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + + flow.rollback_or_pause(task) # nothing to park: the attempt left no trace + + assert flow.journal.events() == ["rollback-skipped-clean"] + assert task.preserve_ref is None + assert flow.retry_preserve_notice(task) == "" + + +def test_retry_preserve_notice_labels_commits_only_preservation(project, monkeypatch): + """The worktree snapshot failed, so `preserve_ref` names the commits branch + alone (`preserve_partial`). The notice must narrow its claim to the commits. + + Ablation: drop the `preserve_partial` branch of the wording and this reddens + on both the label and the absent whole-attempt claim.""" + repo = project.project + task = _task(repo) + task.attempt = 1 + (repo / "src.txt").write_text("committed attempt work\n") + git(repo, "commit", "-qam", "attempt commit") + head = rev_parse_head(repo) + + def snapshot_fails(*args, **kwargs): + raise GitError("commit-tree failed") + + monkeypatch.setattr(verify, "snapshot_worktree", snapshot_fails) + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + + flow.rollback_or_pause(task) + + assert rev_parse_head(repo) == task.baseline_commit + assert task.preserve_partial is True + assert task.preserve_ref == recovery_flow.attempt_preserve_ref_name("run-1", head) + notice = flow.retry_preserve_notice(task) + qualified = f"refs/heads/{task.preserve_ref}" + assert ( + f"only its commits were preserved, at `{qualified}` — its uncommitted " + "changes were not captured there" in notice + ) + assert "its work is preserved" not in notice + assert f"`git diff {task.baseline_commit} {qualified}`" in notice + + +def test_retry_preserve_notice_clean_rollback_keeps_the_earlier_attempts_ref(project): + """A later attempt that leaves a clean tree parks nothing, so its rollback + keeps the older attempt's ref — the notice must call that work an EARLIER + attempt's, never the previous one's.""" + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + first = _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + + task.attempt = 2 + flow.rollback_or_pause(task) # attempt 2 left the tree clean + + assert flow.journal.events()[-1] == "rollback-skipped-clean" + assert task.preserve_ref == first + notice = flow.retry_preserve_notice(task) + assert notice.startswith("An earlier attempt at this work was rolled back") + assert f"`{first}`" in notice + assert "previous attempt" not in notice + + +def test_retry_preserve_notice_follows_the_ref_a_later_rollback_parks(project): + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + first = _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + + task.attempt = 2 + second = _dirty_rollback(flow, task, repo, "attempt 2 edit\n") + + assert second != first and task.preserve_ref == second + notice = flow.retry_preserve_notice(task) + assert f"`{second}`" in notice + assert first not in notice.replace(second, "") # the replaced ref is not offered + + +def test_retry_preserve_notice_withheld_when_no_dev_session_produced_the_ref(project): + """A rollback with no dispatched dev session for the current attempt (a + resolve re-drive's reset) still parks the tree — but nothing proves an + attempt wrote it, so the notice must not claim one did. The rollback replaces + the earlier, attributable ref's provenance along with the ref.""" + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + assert flow.retry_preserve_notice(task) + + unattributed = _make_flow( + workspace=Workspace.default(project), + policy=_policy(rollback_on_failure=True), + dev_attempt_dispatched=False, + ) + ref = _dirty_rollback(unattributed, task, repo, "re-drive residue\n") + + assert task.preserve_ref == ref and task.preserve_from_attempt is False + assert unattributed.retry_preserve_notice(task) == "" + assert git(repo, "rev-parse", "--verify", ref) # suppressed, not deleted + + +def test_retry_preserve_notice_omitted_when_retention_pruned_the_ref(project): + """Run-start retention (`scm.preserve_keep`) can delete the task's ref before + a resume re-dispatches it. The stale name must yield no notice and no crash, + and the task record keeps the name — nothing is cleared to hide guidance. + + Ablation: skip the resolve/ancestry probes and this reddens by offering a + `git diff` against a ref that no longer exists.""" + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), + policy=_policy(rollback_on_failure=True, preserve_keep=1), + ) + ref = _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + # a newer snapshot (another story's rollback) outranks it under keep=1 + newer = subprocess.run( + ["git", "-C", str(repo), "commit-tree", "-p", "HEAD", "-m", "newer", "HEAD^{tree}"], + capture_output=True, + text=True, + check=True, + env={**os.environ, "GIT_COMMITTER_DATE": "@4102444800 +0000"}, + ).stdout.strip() + git(repo, "update-ref", "refs/attempt-preserve-dirty/run-1-other-1", newer) + + flow.prune_preserve_refs() + + assert "attempt-preserve-dirty-pruned" in flow.journal.events() + assert not verify.ref_exists(repo, ref) + assert flow.retry_preserve_notice(task) == "" + assert task.preserve_ref == ref + + +@pytest.mark.parametrize("stale", ["baseline-moved", "foreign-run", "tip-moved"]) +def test_retry_preserve_notice_refuses_a_ref_this_task_cannot_own(project, stale): + """The ref must be this run's, on this task's baseline, and still where the + rollback parked it; otherwise `git diff ` would show something + other than this task's earlier attempt.""" + repo = project.project + task = _task(repo) + task.attempt = 1 + flow = _make_flow( + workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) + ) + if stale == "tip-moved": + (repo / "src.txt").write_text("committed attempt work\n") + git(repo, "commit", "-qam", "attempt commit") + flow.rollback_or_pause(task) + assert task.preserve_ref and task.preserve_ref.startswith("attempt-preserve/") + assert flow.retry_preserve_notice(task) + git(repo, "branch", "-f", task.preserve_ref, task.baseline_commit) + else: + _dirty_rollback(flow, task, repo, "attempt 1 edit\n") + assert flow.retry_preserve_notice(task) + if stale == "baseline-moved": + # another unit merged first: the re-stamped baseline is past the work + (repo / "other.txt").write_text("someone else's merge\n") + git(repo, "add", "other.txt") + git(repo, "commit", "-qm", "unrelated merge") + task.baseline_commit = rev_parse_head(repo) + else: + flow.state = SimpleNamespace(run_id="run-2") + + assert flow.retry_preserve_notice(task) == "" diff --git a/tests/test_stories_engine.py b/tests/test_stories_engine.py index d6cf52c06..40cbd4f20 100644 --- a/tests/test_stories_engine.py +++ b/tests/test_stories_engine.py @@ -2377,3 +2377,32 @@ def test_stories_harvest_alone_is_not_proof_of_work(project): assert "no changes" in decisions[0]["reason"] assert _kinds(engine.journal, "spec-deferrals-harvested")[0]["dw_ids"] == ["DW-1"] assert summary.done == 1 + + +# ------------------------------------ retry prompt names parked work (#777) + + +def test_stories_retry_prompt_names_the_earlier_attempts_parked_work(project): + """Stories mode builds its own dev prompt, and gets the same shared paragraph + on a retry after a parking rollback — appended after the planner's verbatim + `invoke_dev_with` text, which stays untouched. The first dispatch carries none.""" + + def dirty_timeout(spec) -> SessionResult: + (Path(spec.cwd) / "src.txt").write_text("half-built attempt 1\n") + return SessionResult(status="timeout") + + setup_stories(project, [entry("1", invoke_dev_with="Planner note.")]) + engine, adapter = make_engine(project, [dirty_timeout, stories_dev_effect()]) + + assert engine.run().done == 1 + + (parked,) = [e for e in engine.journal.entries() if e["kind"] == "attempt-worktree-preserved"] + first, second = [s.prompt for s in adapter.sessions if s.role == "dev"] + lead = "/bmad-dev-auto Spec folder: _bmad-output/epic-1. Story id: 1.\nPlanner note." + assert first == lead + base = engine.state.tasks["1"].baseline_commit + assert second.startswith( + f"{lead}\n\nAn earlier attempt at this work was rolled back; its work is " + f"preserved at `{parked['ref']}`." + ) + assert f"`git diff {base} {parked['ref']}`" in second diff --git a/tests/test_sweep.py b/tests/test_sweep.py index 136f75395..fcf8c47a9 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -50,13 +50,14 @@ from bmad_loop.adapters.base import SessionResult from bmad_loop.adapters.mock import MockAdapter from bmad_loop.bmadconfig import ProjectPaths -from bmad_loop.engine import RunPaused +from bmad_loop.engine import RunPaused, _session_task_id from bmad_loop.journal import Journal, load_state, save_state from bmad_loop.model import ( PAUSE_ESCALATION, PAUSE_STORY_GATE, Phase, RunState, + SessionRecord, StoryTask, TokenUsage, VerifyOutcome, @@ -25955,6 +25956,50 @@ def test_run_bundle_clears_superseded_bundle_state_on_divergent_adoption( assert "spec-superseded-dw-1.md" not in prompt +@pytest.mark.parametrize("adopted", [["DW-1"], ["DW-2"]], ids=["same-bundle", "replacement"]) +def test_bundle_retry_prompt_never_offers_a_superseded_bundles_parked_work( + project, monkeypatch, adopted +): + """#777: a bundle retry names the earlier attempt's verified parked work. A + divergent adoption keeps the superseded bundle's ref (it is still that work's + only copy) on the same key and baseline, so git cannot tell the bundles apart — + the reset's provenance clear is what stops the replacement's prompt claiming it. + The same bundle re-dispatched keeps its pointer. + + Ablation: delete `task.preserve_from_attempt = False` from + `_reset_superseded_bundle_state` and the replacement case reddens.""" + write_ledger(project, {"DW-1": "open", "DW-2": "open"}) + engine, _ = make_sweep(project, []) + repo = project.project + task = _bundle_task(engine, "dw-fix", ["DW-1"], phase=Phase.DEV_RUNNING) + task.baseline_commit = verify.rev_parse_head(repo) + task.baseline_untracked = [] + task.attempt = 1 + task.record_session( + SessionRecord( + task_id=_session_task_id(task.story_key, "dev", task.attempt, task.generation), + role="dev", + status="timeout", + ) + ) + (repo / "src.txt").write_text("bundle DW-1 attempt\n") + engine._rollback_or_pause(task) + ref = task.preserve_ref + assert ref and ref.startswith("refs/attempt-preserve-dirty/") + assert f"preserved at `{ref}`" in engine._generic_bundle_prompt(task, None) + _stub_run_story(engine, monkeypatch) + + engine._run_bundle(Bundle(name="fix", dw_ids=tuple(adopted), intent="next"), 1) + + prompt = engine._generic_bundle_prompt(task, None) + assert task.preserve_ref == ref # the ref itself is never cleared... + assert verify.ref_exists(repo, ref) # ...nor deleted + if adopted == ["DW-1"]: + assert f"preserved at `{ref}`" in prompt + else: + assert "earlier attempt" not in prompt and ref not in prompt + + @pytest.mark.parametrize( "persisted", [["DW-1", "DW-2"], ["DW-2", "DW-1"]], ids=["same-order", "reordered"] )