fix(rearm): exclude an abandoned restore's residue from the new baseline

[pbean]

Closes #90

Origin: the PR #86 final review, which traced this chain link-by-link. #86's restore latch supplies the untracked payload; #78's re-arm baseline machinery supplies the absorption. Tracked hunks were always safe — this is untracked-files-only.

The bug

A restore re-drive escalates after the patch applied (session error / CRITICAL / exhausted-mid-redrive / pre-commit veto / commit-fail — all PAUSE without a rollback). The human re-resolves and re-arms from scratch, and rearm_escalation refreshes task.baseline_untracked from the current tree — capturing the applied patch's new files as "pre-existing". Every later rollback then preserves them, and finalize_commit's add -A sweeps the abandoned attempt into the corrected story's commit. The only remaining defense was step-01's prose dirty-tree check: a probabilistic LLM judgment, exactly the guarantee class the restore design refuses elsewhere.

The fix

rearm_escalation captures old_latch / old_baseline before the unconditional latch overwrite, then between the spec block and the snapshot refresh subtracts the stale patch's creations from the refreshed baseline_untracked.

• New verify.patch_new_files(patch_path) — text-parses the saved patch (--- /dev/null in a diff --git block marks a creation). The caller has moved the tree on, so git apply --numstat can't be trusted to still apply. Deletions are never returned, and every ambiguous entry (quoted paths, renames, header-lookalike hunk content) is skipped rather than guessed: under-reporting degrades to today's behavior, over-reporting gets a user's file deleted.
• No git apply -R, per the issue's alternative. Re-arm already advances the baseline to HEAD and the re-drive's safe_rollback cleans uncommitted tracked edits; apply -R would fail on any drift and misbehaves on the committed variant. Nothing is deleted inside rearm — the existing reset machinery removes the now-unblessed paths at re-drive time.
• Runs whenever a latch was set, including a re-arm re-latching the same patch — which also fixes the re-restore double-apply (git apply would hit "already exists").
• Committed variant is warn-only by design. Commits the escalated session left below the refreshed baseline sit in the same range as the resolve session's own blessed commits; a mechanical reversal would claw back the human's resolution. stale-restore-commits names the shas and the human classifies.
• Best-effort: a missing/unreadable patch journals stale-restore-unparseable and degrades to the pre-fix snapshot. It must never raise RearmError — a deleted patch file cannot be allowed to wedge resolve.

New journal events stale-restore-excluded / -unparseable / -commits, each echoed as one stderr line from cmd_resolve (the commits warning is the only notice the human ever gets).

_DIFF_ABSENT hoists git's /dev/null diff token to one acked constant; verify.py joins the portability guard's PATH_ALLOW for it, since git spells the absent side /dev/null on Windows too.

Verification

• Full suite green (1940 passed, 1 skipped); full trunk check clean.
• tests/test_runs.py::_escalated_run gains git_project=True — a real repo, so the snapshot refresh actually runs (in a bare tmp_path its best-effort except swallowed every git call and the refresh silently no-opped). Five re-arm tests: residue excluded, re-latch still excludes, missing patch degrades loudly and still warns about commits, no latch → no events, commits above the old baseline → stale-restore-commits. Seven patch_new_files unit tests, plus a CLI test that the echo fires for this re-arm's events only.
• The three positive re-arm tests were confirmed to fail without the runs.py change.
• Driven end-to-end against the real bmad-loop resolve CLI on a seeded escalation: newfile.txt dropped from the refreshed snapshot while human.txt and the latched patch survive; a follow-up safe_rollback at the re-drive's baseline then removes newfile.txt and keeps human.txt.

Summary by CodeRabbit

• Bug Fixes
  • Re-arming a resumed restore now avoids pulling abandoned new files into the refreshed baseline, reducing unexpected file conflicts.
  • The resolve flow now shows clearer warnings when a previous restore patch can’t be read or when earlier restore-related commits are detected.
  • Restore behavior is more consistent across repeated retries and missing patch files.

[coderabbitai]

Warning

Review limit reached

@pbean, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 27 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 21fcbf67-5404-43bc-8a8a-f54663696949

📥 Commits

Reviewing files that changed from the base of the PR and between 6a59590 and 0c70428.

📒 Files selected for processing (8)

• CHANGELOG.md
• src/bmad_loop/cli.py
• src/bmad_loop/runs.py
• src/bmad_loop/verify.py
• tests/test_cli.py
• tests/test_portability_guard.py
• tests/test_runs.py
• tests/test_verify.py

Walkthrough

Adds a patch-diff parser (patch_new_files) that identifies files created by a saved restore patch, uses it in rearm_escalation to compute and exclude abandoned "stale restore" residue from the refreshed baseline snapshot, journals related warning events, and surfaces them via CLI stderr output during resolve.

Changes

Stale Restore Residue Exclusion

Layer / File(s)	Summary
Patch new-files parser src/bmad_loop/verify.py, tests/test_verify.py, tests/test_portability_guard.py	Adds _DIFF_ABSENT constant and patch_new_files() to parse unified diffs and extract created file paths, skipping ambiguous/quoted headers; tested against creation, modification, deletion, mixed, false-positive body text, quoted-path, and missing-file cases; portability guard allowlist updated for verify.py.
Rearm baseline residue exclusion src/bmad_loop/runs.py, tests/test_runs.py	rearm_escalation now captures prior restore latch/baseline, computes stale residue via new _stale_restore_residue() helper (using patch_new_files and commits_above), excludes residue from baseline_untracked, journals warning-only stale-restore-* events on unreadable patches or commit activity, and never fails the re-arm; covered by new tests for exclusion, re-latching, missing patch, no-latch, and commits-above scenarios.
CLI stale-restore echo src/bmad_loop/cli.py, tests/test_cli.py, CHANGELOG.md	Adds _echo_stale_restore() to read new journal entries after rearm_escalation and print targeted stderr messages; wired into cmd_resolve around the re-arm call; new regression test and changelog entry documenting the fix (closes #90).

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related issues

• Abandoned patch-restore attempt: untracked patch files become 'pre-existing' at the next re-arm and are swept into the corrected story's commit #90: The PR implements the abandoned patch-restore residue exclusion and stale-restore journaling described in this issue, and its changelog entry closes it.
• Patch-restore cleanup batch: centralize latch preconditions, one path normalizer, test-fixture dedup #91: Both touch runs.rearm_escalation and restore-patch latch handling, indicating overlapping work in the same code path.

Possibly related PRs

• bmad-code-org/bmad-loop#78: Both modify rearm_escalation's baseline refresh logic for baseline_untracked.
• bmad-code-org/bmad-loop#86: Both extend the patch-restore/re-arm flow across runs.py and verify.py.

Poem

A patch abandoned, files astray,
No more shall they sneak into the fray!
I sniff each diff with twitching nose,
"/dev/null" tells me what it shows.
Baseline swept clean, residue gone—
This bunny hops on, the fix is strong! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.90% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main fix: excluding abandoned restore residue from the refreshed baseline during re-arm.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rearm-stale-restore-residue-90

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/bmad_loop/verify.py`:
- Around line 1554-1561: The created-path handling in verify.py is only
stripping the b/ prefix, so mnemonic prefixes like w/ and 2/ still end up
recorded in new_files. Update the path normalization in the branch that
processes the +++ header to strip all known new-side prefixes before adding the
path, using the existing _DIFF_ABSENT check and the target parsing logic around
creating/new_files.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 686c9edd-3adb-4703-98f1-02aeb59c6bec

📥 Commits

Reviewing files that changed from the base of the PR and between 7113694 and 6a59590.

📒 Files selected for processing (8)

• CHANGELOG.md
• src/bmad_loop/cli.py
• src/bmad_loop/runs.py
• src/bmad_loop/verify.py
• tests/test_cli.py
• tests/test_portability_guard.py
• tests/test_runs.py
• tests/test_verify.py