feat(sweep): extend intent-gap patch-restore re-drive to deferred-work bundles#93
Conversation
…k bundles (#75) An escalated sweep bundle can now be resolved with `bmad-loop resolve <run> --restore-patch <path>`, re-arming the bundle spec to `in-review` and re-applying the saved patch on the re-drive — the same recovery sprint/story runs got in #86. Bundles lack sprint-status entries and SweepEngine overrides `_loop` wholesale, so it never reached the base `_finish_inflight` re-drive wiring; two small deltas close the gap (the apply seam, latch-clear on commit, proof-of-work exclusion, rearm, and CLI resolve->resume were already inherited/pre-wired): - `_generic_bundle_prompt`: point a restore re-drive at the in-review bundle spec (task.spec_file) instead of the fresh intent.md, so step-01's spec-pointer intent check early-exits before its version-control sanity check HALTs blocked on the restored dirty tree (mirrors the sprint F1 fix, engine.py:2133-2141). - `_run_bundle` recovery: latch `resolved_redrive` and pass `cause="resolved"` for a re-armed task, mirroring `_finish_inflight` (engine.py:1151-1157). This is a correctness fix — without the latch a non-critical exhaustion mid-re-drive silently DEFERs the human-resolved escalation (escalation.py:110). It also repairs the pre-existing from-scratch sweep re-drive, which had the same bug. Tests: 4 engine cases in test_sweep.py (restore prompt, restore re-drive -> done + latch cleared + prompt-points-at-spec, pause-not-defer exhaustion for the restore and from-scratch paths) + an escalating-dev conftest helper; E2E scenario 8 drives the real sweep/resolve/resume binaries through tmux (the only coverage of the sweep-specific CLI resume path). All new tests fail without the fix. Full suite 1695 passed / 1 skipped; trunk clean.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughExtends the intent-gap patch-restore re-drive recovery mechanism from story runs to SweepEngine deferred-work bundles: bundle escalations can now be resolved with ChangesSweep patch-restore re-drive
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Filed the out-of-scope residual (triage.json reload failure orphaning a re-armed bundle) as #94. |
|
@CodeRabbit review |
|
Looks like you're out of credits for this review. Head to your account settings to top up and keep reviews running. https://app.augmentcode.com/account/subscription |
|
✅ Action performedReview finished.
|
Closes #75.
Extends the intent-gap patch-restore re-drive (built for sprint/story runs in #86) to
SweepEnginedeferred-work bundles. An escalated bundle can now be resolved withbmad-loop resolve <run> --restore-patch <path>, which re-arms the bundle spec toin-reviewand re-applies the saved patch on the re-drive, so the corrected session resumes review on the restored diff instead of reimplementing from scratch.Why this was small
Investigation showed almost the whole mechanism was already inherited or pre-wired for sweep — the apply seam (
_restore_patchinside the inherited_dev_phase), latch-clear on commit (_commit), the T4 proof-of-work exclusion (verify_dev_bundle),runs.rearm_escalation(run_type-agnostic), and the CLIresolve→resumerouting (_resume_paused_runbuildsSweepEngineforrun_type=="sweep"). The gap was thatSweepEngineoverrides_loopwholesale and hand-rolls recovery in_run_bundle, so it never reached the base_finish_inflightre-drive wiring.The two production changes (
src/bmad_loop/sweep.py)_generic_bundle_prompt— a restore re-drive now points at the in-review bundle spec (task.spec_file) instead of the freshintent.md, so upstream step-01's spec-pointer intent check early-exits before its version-control sanity check would HALTblockedon the restored dirty tree. Mirrors the sprint F1 fix (engine.py:2133-2141)._run_bundlerecovery — latchesresolved_redriveand passescause="resolved"for a re-armed task, mirroring_finish_inflight(engine.py:1151-1157). This is a correctness fix: without the latch, a non-critical exhaustion mid-re-drive silently DEFERs the human-resolved escalation (escalation.py:110→PAUSE if resolved_redrive else DEFER) — filing it as fresh deferred work and rolling back the correction. It also repairs the pre-existing from-scratch sweep re-drive, which had the same latent bug.Tests
tests/test_sweep.py) — restore-prompt unit; restore re-drive → done + both latches cleared + prompt-points-at-spec +attempt-restored; pause-not-defer exhaustion for both the restore and from-scratch paths. New escalating-dev helper inconftest.py.tests/test_stories_e2e.py) — drives the realsweep/resolve --restore-patch/resumebinaries through tmux (a triage branch added to the fake CLI; the bundle-dev session reuses the existing sprint branch). This is the only coverage of the sweep-specific CLI resume path.trunk checkclean.Residual (out of scope)
If
triage.jsonfails to reload on resume, a re-armed bundle can be orphaned by a fresh triage plan whose bundle names differ — pre-existing sweep fragility, not introduced here. Worth a follow-up issue.Summary by CodeRabbit
New Features
Bug Fixes