feat(resolve): intent-gap patch-restore re-drive + inline-review-layer preflight#76
feat(resolve): intent-gap patch-restore re-drive + inline-review-layer preflight#76pbean wants to merge 1 commit into
Conversation
…r preflight Integrate the July 4-6 upstream bmad-dev-auto changes into bmad-loop. Phase A - provisioning + doc alignment: - install: require bmad-review-verification-gap (BMAD-METHOD #2550) and a bmad-dev-auto customize.toml marker in the base-skill preflight; the three inline step-04 review hunters are now all bmm prerequisites - devcontract: refresh the in-review reconcile note (restore path re-arms TO in-review pre-session; reconcile semantics unchanged) - verify: document finalize_commit's intentional residual-artifact squash (#2563) - docs: README skill table, FEATURES review-layer + resolve notes, CHANGELOG Phase B - intent-gap patch-restore re-drive (BMAD-METHOD #2564): A review-stage intent-gap halt now saves the reverted attempt as a patch file. When the attempted reading was correct, bmad-loop resolve re-arms the spec to in-review and re-applies that patch onto the baseline before the re-drive, so the session resumes review (step-04) on the restored diff instead of re-implementing from scratch. - model.StoryTask.restore_patch latch; verify.apply_patch; runs.rearm_escalation restore branch (in-review vs ready-for-dev + restore journal field) - engine: Engine._restore_patch applied in the dev loop gated on feedback-is-None (escalates rather than dispatch on apply failure; cleared on commit) - cli: --restore-patch flag + strict validation + resolution.json passthrough - resolve.read_resolution; bmad-loop-resolve SKILL.md guidance Sweep-workflow extension tracked in #75. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughThis PR adds an intent-gap patch-restore recovery flow: escalated stories can latch a saved patch ( ChangesIntent-gap patch-restore and preflight expansion
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Human
participant CLI as bmad-loop CLI (resolve)
participant Runs as runs.rearm_escalation
participant Engine
participant Verify as verify.apply_patch
Human->>CLI: resolve --restore-patch PATH / resolution.json
CLI->>CLI: _resolve_restore_patch (validate path)
CLI->>Runs: rearm_escalation(restore_patch)
Runs->>Runs: latch task.restore_patch, set spec status "in-review"
Runs-->>CLI: story_key
CLI-->>Human: re-armed (restoring attempted change)
Engine->>Engine: _dev_phase (feedback is None)
Engine->>Engine: _restore_patch(task)
Engine->>Verify: apply_patch(repo, patch_path)
alt patch applies
Verify-->>Engine: success
Engine->>Engine: dispatch dev session on restored tree
Engine->>Engine: commit, clear restore_patch latch
else patch fails
Verify-->>Engine: GitError
Engine->>Engine: journal attempt-restore-failed, raise RunPaused
end
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🤖 Augment PR SummarySummary: Integrates July 4–6 upstream Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| patch = Path(raw) | ||
| if not patch.is_absolute(): | ||
| patch = project / patch | ||
| patch = patch.resolve() |
There was a problem hiding this comment.
src/bmad_loop/cli.py:627 — Path.resolve() can raise in some filesystem edge cases (e.g., symlink loops or permission errors); since --restore-patch is user-supplied input, an uncaught exception here would crash bmad-loop resolve instead of producing the intended validation failure.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if not patch.is_absolute(): | ||
| patch = self.workspace.root / patch | ||
| try: | ||
| verify.apply_patch(self.workspace.root, patch) |
There was a problem hiding this comment.
src/bmad_loop/engine.py:892 — If git apply fails, it can leave the working tree partially modified; escalating immediately without restoring a known baseline could leave the paused run in a confusing half-applied state for subsequent recovery/debugging.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
Integrates the July 4–6 upstream
bmad-dev-autochanges into bmad-loop. Two phases; the rest of that upstream batch needed no orchestrator work (blocking-condition renames, checkbox/template tweaks, and customize.toml plumbing are all skill-internal or already carried by worktree provisioning).Phase A — provisioning + doc alignment
bmad-loop validate/ run-start require the three review huntersbmad-dev-auto's step-04 invokes —bmad-review-adversarial-general,bmad-review-edge-case-hunter, and the newbmad-review-verification-gap(BMAD-METHOD#2550) — plus acustomize.tomlmarker onbmad-dev-auto(the review-layer config from #2535/#2550). A pre-July bmm install missing any is reported with remediation instead of failing mid-run.devcontractin-reviewreconcile note and documentedverify.finalize_commit's intentional residual-artifact squash (#2563).Phase B — intent-gap patch-restore re-drive (BMAD-METHOD#2564)
A review-stage
intent gaphalt now saves the reverted attempt as a patch file. When the human confirms the attempted reading was actually correct,bmad-loop resolvere-arms the spec toin-review(notready-for-dev), latches the patch, and the engine re-applies it onto the baseline after every reset of the re-drive — so the re-driven session resumes review on the restored diff instead of re-implementing from scratch.model.StoryTask.restore_patch(persisted, legacy-defaulting);verify.apply_patch;runs.rearm_escalation(..., restore_patch=)with thein-reviewvsready-for-devbranch and arestorejournal field.Engine._restore_patchis applied inside the_dev_phasedispatch loop, gated onfeedback is None(fresh attempt or non-fixable rollback — never a fixable-feedback retry that kept the tree). On apply failure it escalates rather than dispatch onto a half-restored tree; the latch clears on commit._rollback_or_pauseso the patch always lands after the cleanbaseline_untrackedsnapshot — otherwise a mid-re-drive reset would preserve the patch's own new files and then collide with the re-apply. The plan explicitly left the exact seam to the implementation.cli:--restore-patch <path>flag (hand-driven) + strict path validation +resolution.jsonpassthrough for the interactive agent;resolve.read_resolution;bmad-loop-resolveSKILL.md guidance for the "attempted reading was correct" option.Scope was limited to story runs; the sweep-workflow equivalent is tracked in #75.
Testing
apply_patchsuccess/conflict/missing, restore-vs-plain re-arm + journal, patch-applied-before-dispatch, mid-re-drive re-apply + protected-patch survival, apply-failure-escalates, latch-cleared-on-commit, and the CLI flag / resolution.json / missing-file paths.trunk check(full, unfiltered): clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation