From 7ce603e09184688bf7dab42094d6ef372e52db34 Mon Sep 17 00:00:00 2001 From: pbean Date: Sat, 20 Jun 2026 21:35:45 -0700 Subject: [PATCH 1/2] fix(rollback): never blanket-clean the tree; gate auto-rollback behind opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed in-place attempt previously ran `git reset --hard` + a blanket `git clean -fd` over the whole checkout, sparing only `.automator/` and two artifact subdirs. That could delete a project's `_bmad-output/` and any other untracked files (the cause of the reported notey data loss). The orchestrator now never runs a blanket `git clean`. New `verify.safe_rollback` reverts the attempt's tracked changes to baseline and removes only the untracked files THIS run created (diffed against a baseline-time snapshot, `StoryTask.baseline_untracked`); pre-existing untracked files and the whole `_bmad-output/` are preserved. The output folder is now exposed wholesale via `ProjectPaths.output_folder`. Auto-rollback is gated behind `[scm] rollback_on_failure` (default off). Off: the engine never touches the tree — it pauses with bold manual-recovery instructions (back up untracked files -> git reset --hard -> restore). On: the safe rollback above runs and warns. Sweep migration uses a non-pausing `_safe_reset` for internal ledger restore. Worktree isolation sidesteps the path entirely. Co-Authored-By: Claude Opus 4.8 --- src/automator/bmadconfig.py | 11 ++++ src/automator/data/settings/core.toml | 6 ++ src/automator/engine.py | 84 +++++++++++++++++++++++---- src/automator/model.py | 11 ++++ src/automator/policy.py | 13 +++++ src/automator/sweep.py | 7 ++- src/automator/verify.py | 59 ++++++++++++++++--- tests/test_engine.py | 69 +++++++++++++++++++++- tests/test_hook_bus.py | 3 +- tests/test_plugin_workflows.py | 9 ++- tests/test_sweep.py | 10 +++- tests/test_verify.py | 73 ++++++++++++++++++++--- 12 files changed, 319 insertions(+), 36 deletions(-) diff --git a/src/automator/bmadconfig.py b/src/automator/bmadconfig.py index 133d6a58c..14d4df6d0 100644 --- a/src/automator/bmadconfig.py +++ b/src/automator/bmadconfig.py @@ -17,11 +17,18 @@ class ProjectPaths: project: Path implementation_artifacts: Path planning_artifacts: Path + # the BMAD output root (parent of the artifact dirs, holds project-context, + # test-artifacts, story-automator, …). Protected wholesale on rollback so a + # failed attempt never deletes generated BMAD output. Defaults to + # {project-root}/_bmad-output when the config omits `output_folder`. + output_folder: Path = field(default=None) # type: ignore[assignment] # the git root code/git work happens against; defaults to `project`. Phase 1 # foundation for worktree isolation — see ProjectPaths.rebased and Workspace. repo_root: Path = field(default=None) # type: ignore[assignment] def __post_init__(self) -> None: + if self.output_folder is None: + object.__setattr__(self, "output_folder", (self.project / "_bmad-output").resolve()) if self.repo_root is None: object.__setattr__(self, "repo_root", self.project) @@ -51,6 +58,7 @@ def rebase(p: Path) -> Path: project=new_root, implementation_artifacts=rebase(self.implementation_artifacts), planning_artifacts=rebase(self.planning_artifacts), + output_folder=rebase(self.output_folder), repo_root=new_root, ) @@ -77,9 +85,12 @@ def load_paths(project: Path) -> ProjectPaths: ) repo_root_raw = doc.get("repo_root") repo_root = _resolve(str(repo_root_raw), project) if repo_root_raw else project + out_raw = doc.get("output_folder") + output_folder = _resolve(str(out_raw), project) if out_raw else (project / "_bmad-output") return ProjectPaths( project=project, implementation_artifacts=_resolve(str(impl), project), planning_artifacts=_resolve(str(plan), project), + output_folder=output_folder.resolve(), repo_root=repo_root, ) diff --git a/src/automator/data/settings/core.toml b/src/automator/data/settings/core.toml index a69e35dab..bf3be5691 100644 --- a/src/automator/data/settings/core.toml +++ b/src/automator/data/settings/core.toml @@ -195,6 +195,12 @@ kind = "switch" default_ref = "ScmPolicy.keep_failed" description = "worktree mode: keep a failed unit's worktree + branch mounted for inspection" [[section.field]] +key = "rollback_on_failure" +kind = "switch" +default_ref = "ScmPolicy.rollback_on_failure" +label = "auto-rollback failed attempts" +description = "⚠ in-place mode (isolation=none): when ON, a failed attempt's tracked changes are auto-reverted and the untracked files this run created are deleted (its uncommitted work is lost). When OFF (default), the orchestrator never touches your tree — it pauses with manual recovery steps. Prefer isolation=worktree to keep failures off your main checkout." +[[section.field]] key = "seed_adapter_defaults" kind = "switch" default_ref = "ScmPolicy.seed_adapter_defaults" diff --git a/src/automator/engine.py b/src/automator/engine.py index f02ebd53a..7dfc42cec 100644 --- a/src/automator/engine.py +++ b/src/automator/engine.py @@ -636,20 +636,79 @@ def _pick_next(self): continue return story - def _reset_to(self, baseline: str) -> None: - """Roll back code changes, preserving run state and BMAD artifacts - (sprint-status etc. may be untracked in young projects — `git clean` - must never eat them).""" + def _rollback_or_pause(self, task: StoryTask) -> None: + """Recover from a failed in-place attempt. + + With ``scm.rollback_on_failure`` OFF (default) the orchestrator never + touches the working tree: it emits a bold manual-recovery notice and + pauses the run (stop-and-wait), so nothing proceeds on a half-finished + tree. With it ON, it does the safest possible automatic rollback — + revert the attempt's tracked changes to baseline and delete only the + untracked files this run created (the whole BMAD output folder and every + pre-existing untracked file are preserved; there is no blanket + ``git clean``).""" + if not self.policy.scm.rollback_on_failure: + self._pause_for_manual_recovery(task, task.baseline_commit or "") + return # unreachable: _pause_for_manual_recovery always raises + self.journal.append( + "rollback-auto", + story_key=task.story_key, + baseline=task.baseline_commit or "", + note="reverting tracked changes + run-created untracked files", + ) + self._safe_reset(task) + + def _safe_reset(self, task: StoryTask) -> None: + """Revert tracked changes to the task baseline and remove only the + untracked files this run created — never a blanket `git clean`. Used by + the gated rollback (when enabled) and by internal ledger recovery (sweep + migration), which restores the orchestrator's own state and must not + pause.""" keep = [".automator"] - for artifact_dir in ( + for protected in ( + self.workspace.paths.output_folder, self.workspace.paths.implementation_artifacts, self.workspace.paths.planning_artifacts, ): try: - keep.append(str(artifact_dir.relative_to(self.workspace.root))) + keep.append(str(protected.relative_to(self.workspace.root))) except ValueError: - pass # artifacts configured outside the repo; nothing to protect - verify.reset_hard(self.workspace.root, baseline, keep=tuple(keep)) + pass # configured outside the repo; nothing to protect here + verify.safe_rollback( + self.workspace.root, + task.baseline_commit or "", + baseline_untracked=task.baseline_untracked, + keep=tuple(keep), + ) + + def _pause_for_manual_recovery(self, task: StoryTask, baseline: str) -> None: + """OFF path: leave the tree untouched, surface bold manual-recovery + instructions, and pause the run. Always raises RunPaused.""" + short = baseline[:12] or "the run's baseline commit" + notice = ( + "**ACTION REQUIRED — manual rollback needed**\n" + f"Story **{task.story_key}** failed and auto-rollback is OFF, so the " + "working tree was left exactly as-is for you to inspect.\n" + "To discard this attempt yourself:\n" + " 1. **BACK UP any untracked files you want to keep** — the reset " + "below deletes uncommitted work.\n" + f" 2. `git reset --hard {short}` then review/remove leftover " + "untracked files.\n" + " 3. **Restore the files you backed up in step 1.**\n" + f"Then run `bmad-auto resume {self.state.run_id}`. To let the " + "orchestrator do a safe automatic rollback next time, enable " + "`[scm] rollback_on_failure` (it discards the attempt's uncommitted " + "work but never deletes pre-existing untracked files)." + ) + self.journal.append("rollback-manual-required", story_key=task.story_key, baseline=baseline) + gates.notify( + self.policy, + self.run_dir, + f"ACTION REQUIRED: manual rollback for {task.story_key}", + notice, + ) + self._save() + raise RunPaused(notice, PAUSE_ESCALATION, task.story_key) def _finish_inflight(self) -> None: """Complete or roll back tasks interrupted by a pause or crash.""" @@ -681,7 +740,7 @@ def _finish_inflight(self) -> None: task.worktree_path = "" task.branch = "" elif task.baseline_commit: - self._reset_to(task.baseline_commit) + self._rollback_or_pause(task) task.phase = Phase.PENDING # deliberate reset, not a normal transition self._save() self._run_story(task) @@ -914,6 +973,9 @@ def _dev_phase(self, task: StoryTask) -> bool: if self._vetoed(self._emit("pre_dev_phase", task), task): return False task.baseline_commit = verify.rev_parse_head(self.workspace.root) + # snapshot untracked files now so a later rollback removes only what THIS + # attempt creates, never files the user already had on disk. + task.baseline_untracked = sorted(verify.untracked_files(self.workspace.root)) feedback: Path | None = None while True: task.attempt += 1 @@ -962,7 +1024,7 @@ def _dev_phase(self, task: StoryTask) -> bool: feedback = self._write_feedback(task, decision.reason) else: feedback = None - self._reset_to(task.baseline_commit) + self._rollback_or_pause(task) continue if decision.action == Action.DEFER: self._defer(task, decision.reason) @@ -1291,7 +1353,7 @@ def _defer(self, task: StoryTask, reason: str) -> None: snapshot = ( deferred_work.read_text(encoding="utf-8") if deferred_work.is_file() else None ) - self._reset_to(task.baseline_commit) + self._rollback_or_pause(task) # reset reverts tracked deferred-work.md edits; restore review-found # defer entries — they are real knowledge worth keeping if snapshot is not None: diff --git a/src/automator/model.py b/src/automator/model.py index 5d711e1cf..eaba82feb 100644 --- a/src/automator/model.py +++ b/src/automator/model.py @@ -123,6 +123,11 @@ class StoryTask: attempt: int = 0 review_cycle: int = 0 baseline_commit: str | None = None + # untracked, non-ignored paths present at baseline capture (repo-relative + # posix). On rollback only paths NOT in this set are removed, so files the + # user already had on disk are never deleted. None = pre-upgrade run (no + # snapshot); rollback then removes no untracked files at all. + baseline_untracked: list[str] | None = None spec_file: str | None = None commit_sha: str | None = None defer_reason: str | None = None @@ -155,6 +160,7 @@ def to_dict(self) -> dict[str, Any]: "attempt": self.attempt, "review_cycle": self.review_cycle, "baseline_commit": self.baseline_commit, + "baseline_untracked": self.baseline_untracked, "spec_file": self._serialized_spec_file(), "commit_sha": self.commit_sha, "defer_reason": self.defer_reason, @@ -187,6 +193,11 @@ def from_dict(cls, d: dict[str, Any]) -> "StoryTask": attempt=int(d.get("attempt", 0)), review_cycle=int(d.get("review_cycle", 0)), baseline_commit=d.get("baseline_commit"), + baseline_untracked=( + [str(p) for p in d["baseline_untracked"]] + if d.get("baseline_untracked") is not None + else None + ), spec_file=d.get("spec_file"), commit_sha=d.get("commit_sha"), defer_reason=d.get("defer_reason"), diff --git a/src/automator/policy.py b/src/automator/policy.py index 72a3b7be2..58459f7ff 100644 --- a/src/automator/policy.py +++ b/src/automator/policy.py @@ -159,6 +159,17 @@ class ScmPolicy: merge_strategy: str = "merge" # ff | merge | squash delete_branch: bool = True # delete the unit branch after a successful merge keep_failed: bool = True # keep a failed unit's worktree+branch for inspection + # rollback_on_failure governs in-place (isolation = "none") recovery after a + # failed attempt / rejected review. Default OFF: the orchestrator never + # touches the working tree — it pauses the run with manual recovery + # instructions, so a half-finished attempt is left for you to inspect. ON: + # the orchestrator auto-reverts the attempt's tracked changes and removes the + # untracked files THIS run created (never a blanket `git clean`; pre-existing + # untracked files and the whole _bmad-output/ are preserved) — convenient but + # it discards the attempt's uncommitted work, so a warning is journalled when + # it fires. Worktree isolation sidesteps this entirely (failed work stays in + # its worktree), so this knob only matters for isolation = "none". + rollback_on_failure: bool = False # failed_diff_max_mb caps the per-file size (MB) of untracked files captured # into a kept-failed unit's forensic changes.patch, so a stray build dir or # huge log can't blow it up; oversized files are skipped with a labelled @@ -414,6 +425,7 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: merge_strategy=str(scm_d.get("merge_strategy", ScmPolicy.merge_strategy)), delete_branch=bool(scm_d.get("delete_branch", ScmPolicy.delete_branch)), keep_failed=bool(scm_d.get("keep_failed", ScmPolicy.keep_failed)), + rollback_on_failure=bool(scm_d.get("rollback_on_failure", ScmPolicy.rollback_on_failure)), failed_diff_max_mb=int(scm_d.get("failed_diff_max_mb", ScmPolicy.failed_diff_max_mb)), failed_diff_unlimited=bool( scm_d.get("failed_diff_unlimited", ScmPolicy.failed_diff_unlimited) @@ -600,6 +612,7 @@ def _fold_deprecated_engine( merge_strategy = "merge" # ff | merge | squash (worktree mode merges the unit branch into target locally) delete_branch = true # delete the unit branch after a successful merge keep_failed = true # keep a failed unit's worktree+branch for inspection +rollback_on_failure = false # in-place (isolation="none") recovery after a failed attempt. false = never touch the tree; pause with manual recovery steps. true = auto-revert the attempt's tracked changes + remove only the untracked files this run created (WARNING: discards the attempt's uncommitted work; never a blanket git clean). Prefer isolation="worktree" to avoid touching your main checkout. failed_diff_max_mb = 5 # per-file size cap (MB) for untracked files in a kept-failed unit's changes.patch; oversized files are skipped with a marker failed_diff_unlimited = false # true = capture the failed-unit diff with no size cap (may produce very large patches; warns when active) # commit_message_template: when set, the commit message dev sessions use for a diff --git a/src/automator/sweep.py b/src/automator/sweep.py index 50de09a7d..38a26a71c 100644 --- a/src/automator/sweep.py +++ b/src/automator/sweep.py @@ -538,7 +538,7 @@ def _run_bundle(self, bundle: Bundle, cycle: int) -> None: task.worktree_path = "" task.branch = "" elif task.baseline_commit: - self._reset_to(task.baseline_commit) + self._rollback_or_pause(task) task.phase = Phase.PENDING # deliberate reset, not a normal transition dirname = bundle.name if cycle == 1 else f"c{cycle}-{bundle.name}" task.bundle_file = str(self._write_intent(bundle, dirname)) @@ -567,11 +567,12 @@ def _ensure_migration(self, text: str) -> None: if task.phase == Phase.ESCALATED: task.attempt = 0 # the human resumed deliberately; fresh budget if task.baseline_commit and not verify.worktree_clean(self.workspace.root): - self._reset_to(task.baseline_commit) # a session died mid-rewrite + self._safe_reset(task) # a session died mid-rewrite; restore our ledger text = ledger.read_text(encoding="utf-8") if ledger.is_file() else "" task.phase = Phase.PENDING # deliberate reset, not a normal transition if not task.baseline_commit: task.baseline_commit = verify.rev_parse_head(self.workspace.root) + task.baseline_untracked = sorted(verify.untracked_files(self.workspace.root)) legacy = deferredwork.parse_legacy(text) pre_canonical = {e.id: e.status for e in deferredwork.parse_ledger(text)} @@ -640,7 +641,7 @@ def _ensure_migration(self, text: str) -> None: # never re-prompt over a half-broken rewrite; the baseline reset # covers tracked files, the explicit write covers an untracked # ledger that `git reset` cannot restore - self._reset_to(task.baseline_commit) + self._safe_reset(task) ledger.parent.mkdir(parents=True, exist_ok=True) ledger.write_text(text, encoding="utf-8") if task.attempt >= self.policy.sweep.max_migration_attempts: diff --git a/src/automator/verify.py b/src/automator/verify.py index 91c71c12b..d9e986bc5 100644 --- a/src/automator/verify.py +++ b/src/automator/verify.py @@ -121,16 +121,61 @@ def has_changes_since(repo: Path, baseline: str) -> bool: return rc == 0 and out != "" -def reset_hard(repo: Path, baseline: str, keep: tuple[str, ...] = (".automator",)) -> None: +def untracked_files(repo: Path) -> set[str]: + """Untracked, non-ignored paths (repo-relative posix), mirroring what a + plain `git clean -fd` (no -x) treats as removable. Ignored files are + excluded, so they are never rollback candidates.""" + rc, out = _git(repo, "ls-files", "--others", "--exclude-standard") + if rc != 0: + raise GitError(f"git ls-files --others failed in {repo}: {out}") + return {line.strip() for line in out.splitlines() if line.strip()} + + +def safe_rollback( + repo: Path, + baseline: str, + *, + baseline_untracked: list[str] | None, + keep: tuple[str, ...] = (".automator",), +) -> None: + """Undo a failed attempt WITHOUT a blanket `git clean`. + + Reverts tracked changes to `baseline` (the dev attempt's commits/edits), + then removes only untracked files that appeared since `baseline` — i.e. + files this run created. Untracked files already present at baseline, every + ignored file, and anything under a `keep` dir are preserved. The orchestrator + therefore never runs `git clean -fd`, so it can't eat a user's pre-existing + untracked work. `baseline_untracked` is the snapshot taken when the baseline + was captured; None (a pre-upgrade run with no snapshot) removes nothing. + """ rc, out = _git(repo, "reset", "--hard", baseline) if rc != 0: raise GitError(f"git reset --hard {baseline} failed: {out}") - clean_args = ["clean", "-fd"] - for path in keep: - clean_args += ["-e", path] - rc, out = _git(repo, *clean_args) - if rc != 0: - raise GitError(f"git clean failed: {out}") + if baseline_untracked is None: + return # no snapshot to diff against: never delete untracked files + created = untracked_files(repo) - set(baseline_untracked) + repo = repo.resolve() + keep_roots = [(repo / k).resolve() for k in keep] + for rel in sorted(created): + path = (repo / rel).resolve() + if any(path == root or path.is_relative_to(root) for root in keep_roots): + continue + try: + path.unlink(missing_ok=True) + except OSError: + continue + _prune_empty_parents(path.parent, repo) + + +def _prune_empty_parents(start: Path, repo: Path) -> None: + """Remove now-empty directories from `start` up to (not including) `repo`.""" + d = start.resolve() + while d != repo and d.is_relative_to(repo): + try: + d.rmdir() # succeeds only when empty + except OSError: + break + d = d.parent # -------------------------------------------------------------------------- diff --git a/tests/test_engine.py b/tests/test_engine.py index 6784489da..29a98ff05 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -24,6 +24,7 @@ LimitsPolicy, NotifyPolicy, Policy, + ScmPolicy, StageAdapterPolicy, SweepPolicy, VerifyPolicy, @@ -39,7 +40,15 @@ def make_engine(project, script, policy=None, **kwargs) -> tuple[Engine, MockAda state = RunState(run_id="test-run", project=str(project.project), started_at="now") engine = Engine( paths=project, - policy=policy or Policy(gates=GatesPolicy(mode="none"), notify=QUIET), + policy=policy + or Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + # in-place tests exercise the retry/defer continuation path, which + # needs auto-rollback on; the OFF (pause) default is covered by its + # own tests. + scm=ScmPolicy(rollback_on_failure=True), + ), adapter=adapter, run_dir=run_dir, journal=Journal(run_dir), @@ -341,6 +350,64 @@ def reviewing_with_defer(spec): assert "DW-1: pre-existing flaky retry" in project.deferred_work.read_text() +def test_rollback_off_pauses_with_manual_notice(project): + """Production default (rollback_on_failure=False): a would-be defer reset + never touches the tree — it pauses with bold manual-recovery instructions.""" + from automator.model import PAUSE_ESCALATION + + write_sprint(project, {"1-1-a": "ready-for-dev"}) + policy = Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + scm=ScmPolicy(rollback_on_failure=False), + ) + engine, _ = make_engine( + project, + [dev_effect(project, "1-1-a")] + + [review_effect(project, "1-1-a", clean=False, patched=1) for _ in range(3)], + policy=policy, + ) + precious = project.project / "keep-me.txt" + precious.write_text("precious\n") # an untracked file the user wants kept + summary = engine.run() + + assert summary.paused + state = load_state(engine.run_dir) + assert state.paused_stage == PAUSE_ESCALATION + reason = state.paused_reason.lower() + assert "manual rollback" in reason and "back up" in reason + # the orchestrator left the tree exactly as-is — no reset, nothing deleted + assert not worktree_clean(project.project) + assert precious.read_text() == "precious\n" + kinds = [e["kind"] for e in engine.journal.entries()] + assert "rollback-manual-required" in kinds + + +def test_rollback_on_preserves_preexisting_untracked(project): + """With rollback_on_failure=True the auto-rollback reverts tracked changes + but never deletes untracked files that predate the attempt.""" + write_sprint(project, {"1-1-a": "ready-for-dev"}) + policy = Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + scm=ScmPolicy(rollback_on_failure=True), + ) + engine, _ = make_engine( + project, + [dev_effect(project, "1-1-a")] + + [review_effect(project, "1-1-a", clean=False, patched=1) for _ in range(3)], + policy=policy, + ) + precious = project.project / "user-notes.txt" + precious.write_text("keep me\n") # untracked, present before baseline capture + summary = engine.run() + + assert summary.deferred == 1 and not summary.paused + task = engine.state.tasks["1-1-a"] + assert rev_parse_head(project.project) == task.baseline_commit # tracked reverted + assert precious.read_text() == "keep me\n" # pre-existing untracked survives + + def test_dev_stall_retries_then_succeeds(project): write_sprint(project, {"1-1-a": "ready-for-dev"}) engine, adapter = make_engine( diff --git a/tests/test_hook_bus.py b/tests/test_hook_bus.py index 7f67f07fc..a75667dd6 100644 --- a/tests/test_hook_bus.py +++ b/tests/test_hook_bus.py @@ -31,7 +31,7 @@ ) from automator.plugins.bus import _HookError, _run_subprocess from automator.plugins.model import HookSpec, LoadedPlugin -from automator.policy import GatesPolicy, LimitsPolicy, NotifyPolicy, Policy +from automator.policy import GatesPolicy, LimitsPolicy, NotifyPolicy, Policy, ScmPolicy QUIET = NotifyPolicy(desktop=False, file=True) @@ -361,6 +361,7 @@ def on_pre_dev_session(self, c): # noqa: ANN001 gates=GatesPolicy(mode="none"), notify=QUIET, limits=LimitsPolicy(max_dev_attempts=2), + scm=ScmPolicy(rollback_on_failure=True), # exercise retry/defer continuation ) # no adapter calls happen (every session is vetoed before launch) engine, _ = make_engine(project, one_story(project), registry_of(py_plugin(P, "sv")), policy) diff --git a/tests/test_plugin_workflows.py b/tests/test_plugin_workflows.py index 66b476fc1..49eaf1329 100644 --- a/tests/test_plugin_workflows.py +++ b/tests/test_plugin_workflows.py @@ -33,7 +33,7 @@ PythonSpec, WorkflowSpec, ) -from automator.policy import GatesPolicy, NotifyPolicy, PluginsPolicy, Policy +from automator.policy import GatesPolicy, NotifyPolicy, PluginsPolicy, Policy, ScmPolicy QUIET = NotifyPolicy(desktop=False, file=True) EXAMPLE_DIR = Path(__file__).resolve().parents[1] / "examples" / "plugins" / "guardrails" @@ -64,7 +64,12 @@ def make_engine(project, script, registry=None, policy=None, **kw): state = RunState(run_id="wf-run", project=str(project.project), started_at="now") engine = Engine( paths=project, - policy=policy or Policy(gates=GatesPolicy(mode="none"), notify=QUIET), + policy=policy + or Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + scm=ScmPolicy(rollback_on_failure=True), + ), adapter=adapter, run_dir=run_dir, journal=Journal(run_dir), diff --git a/tests/test_sweep.py b/tests/test_sweep.py index 1f932a6e6..21b9ce2e3 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -24,6 +24,7 @@ NotifyPolicy, Policy, ReviewPolicy, + ScmPolicy, SweepPolicy, ) from automator.sweep import DecisionPrompter, SweepEngine, validate_migration, validate_triage @@ -53,7 +54,12 @@ def make_sweep(project, script, policy=None, answers=(), prompting=False, **kwar prompter = DecisionPrompter(input_fn=lambda _: next(inputs), print_fn=lambda _line: None) engine = SweepEngine( paths=project, - policy=policy or Policy(gates=GatesPolicy(mode="none"), notify=QUIET), + policy=policy + or Policy( + gates=GatesPolicy(mode="none"), + notify=QUIET, + scm=ScmPolicy(rollback_on_failure=True), + ), adapter=adapter, run_dir=run_dir, journal=Journal(run_dir), @@ -312,7 +318,6 @@ def test_sweep_worktree_bundle_merges_to_target(project): closes land on the target branch and the worktree is cleaned up.""" from conftest import _spec_baseline, write_spec - from automator.policy import ScmPolicy from automator.verify import branch_exists, rev_parse_head, worktree_list write_ledger(project, {"DW-1": "open"}) # committed → visible in the worktree @@ -1040,6 +1045,7 @@ def test_repeat_failed_bundle_not_rebuilt(project): notify=QUIET, sweep=SweepPolicy(repeat=True), limits=LimitsPolicy(max_review_cycles=1, max_dev_attempts=1), + scm=ScmPolicy(rollback_on_failure=True), # exercise defer-and-continue ) engine, adapter = make_sweep( project, diff --git a/tests/test_verify.py b/tests/test_verify.py index 29f1e37e7..86b2bac0f 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -204,18 +204,73 @@ def test_verify_review_bundle_missing_entry_fails(project): assert not out.ok and out.fixable and "DW-2" in out.reason -def test_reset_hard_keeps_automator_dir(project): - baseline = verify.rev_parse_head(project.project) - (project.project / "src.txt").write_text("dirty\n") - (project.project / "junk.txt").write_text("untracked\n") - keep = project.project / ".automator" / "runs" / "r1" +def test_safe_rollback_reverts_tracked_and_removes_run_created(project): + repo = project.project + baseline = verify.rev_parse_head(repo) + snap = sorted(verify.untracked_files(repo)) # snapshot before the attempt + (repo / "src.txt").write_text("dirty\n") # tracked edit + (repo / "junk.txt").write_text("run-created\n") # untracked, created now + keep = repo / ".automator" / "runs" / "r1" keep.mkdir(parents=True) (keep / "state.json").write_text("{}") - verify.reset_hard(project.project, baseline) - assert (project.project / "src.txt").read_text() == "original\n" - assert not (project.project / "junk.txt").exists() - assert (keep / "state.json").exists() + verify.safe_rollback(repo, baseline, baseline_untracked=snap, keep=(".automator",)) + assert (repo / "src.txt").read_text() == "original\n" # tracked reverted + assert not (repo / "junk.txt").exists() # run-created removed + assert (keep / "state.json").exists() # .automator preserved + + +def test_safe_rollback_preserves_preexisting_untracked(project): + repo = project.project + (repo / "_bmad-output").mkdir(exist_ok=True) + (repo / "_bmad-output" / "project-context.md").write_text("keep me\n") + (repo / ".design-build").mkdir() + (repo / ".design-build" / "x").write_text("keep me too\n") + baseline = verify.rev_parse_head(repo) + snap = sorted(verify.untracked_files(repo)) # includes the two files above + (repo / "junk.txt").write_text("run-created\n") + + verify.safe_rollback(repo, baseline, baseline_untracked=snap, keep=(".automator",)) + assert (repo / "_bmad-output" / "project-context.md").read_text() == "keep me\n" + assert (repo / ".design-build" / "x").read_text() == "keep me too\n" + assert not (repo / "junk.txt").exists() # only run-created file removed + + +def test_safe_rollback_keep_dir_protects_run_created(project): + repo = project.project + baseline = verify.rev_parse_head(repo) + snap = sorted(verify.untracked_files(repo)) + out = repo / "_bmad-output" + out.mkdir(exist_ok=True) + (out / "fresh-artifact.md").write_text("generated this run\n") # run-created + + verify.safe_rollback( + repo, baseline, baseline_untracked=snap, keep=(".automator", "_bmad-output") + ) + assert (out / "fresh-artifact.md").exists() # protected by keep even though new + + +def test_safe_rollback_none_snapshot_removes_nothing(project): + repo = project.project + baseline = verify.rev_parse_head(repo) + (repo / "src.txt").write_text("dirty\n") + (repo / "junk.txt").write_text("untracked\n") + + verify.safe_rollback(repo, baseline, baseline_untracked=None, keep=(".automator",)) + assert (repo / "src.txt").read_text() == "original\n" # tracked still reverted + assert (repo / "junk.txt").exists() # no snapshot => never delete untracked + + +def test_safe_rollback_prunes_emptied_dirs(project): + repo = project.project + baseline = verify.rev_parse_head(repo) + snap = sorted(verify.untracked_files(repo)) + nested = repo / "tmpdir" / "sub" + nested.mkdir(parents=True) + (nested / "f.txt").write_text("x\n") + + verify.safe_rollback(repo, baseline, baseline_untracked=snap, keep=(".automator",)) + assert not (repo / "tmpdir").exists() # emptied parent dirs pruned def test_worktree_clean_ignores_policy_file(project): From 804dd772f128b2261c6dab34c1382845a13f04ee Mon Sep 17 00:00:00 2001 From: pbean Date: Sat, 20 Jun 2026 21:37:07 -0700 Subject: [PATCH 2/2] =?UTF-8?q?chore(release):=200.6.0=20=E2=80=94=20Rollb?= =?UTF-8?q?ack=20no=20longer=20wipes=20non-automator=20files.=20A=20failed?= =?UTF-8?q?=20in-pl=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude-plugin/marketplace.json | 2 +- CHANGELOG.md | 20 +++++++++++++++++++ module.yaml | 2 +- pyproject.toml | 2 +- src/automator/__init__.py | 2 +- .../skills/bmad-auto-setup/assets/module.yaml | 2 +- uv.lock | 2 +- 7 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index c278568ea..00b416d3f 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -12,7 +12,7 @@ "name": "bauto", "source": "./src/automator/data/skills", "description": "Automation-mode skills driven by the bmad-auto orchestrator: unattended dev (bmad-auto-dev), adversarial review (bmad-auto-review), and deferred-work sweep triage (bmad-auto-sweep)", - "version": "0.5.1", + "version": "0.6.0", "author": { "name": "pinkyd" }, diff --git a/CHANGELOG.md b/CHANGELOG.md index f217be1f9..a7ba6aa69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to `bmad-auto` are documented here. The format is based on [Semantic Versioning](https://semver.org/spec/v2.0.0.html). While the project is pre-1.0, breaking changes may land in a minor release. +## [0.6.0] — 2026-06-20 + +### Fixed + +- **Rollback no longer wipes non-automator files.** A failed in-place attempt previously ran + `git reset --hard` + a blanket `git clean -fd` over the whole checkout, which could delete a + project's `_bmad-output/` and any other untracked files (only `.automator/` and two artifact + subdirs were spared). The orchestrator now never runs a blanket `git clean`: it reverts the + attempt's tracked changes and removes only the untracked files **that run created**, preserving + pre-existing untracked files and the entire `_bmad-output/` tree. + +### Changed + +- **Auto-rollback is now opt-in (`[scm] rollback_on_failure`, default off).** With it off the + orchestrator never touches your working tree on a failed attempt — it pauses the run with bold + manual-recovery instructions (back up untracked files → `git reset --hard ` → restore). + Turn it on for the safe automatic rollback above (it discards the attempt's uncommitted work, so + it warns when it fires). Worktree isolation (`scm.isolation = "worktree"`) sidesteps this entirely. + ## [0.5.1] — 2026-06-20 ### Added @@ -396,6 +415,7 @@ enforced in CI. implementation phase, driven by a Python control loop with hook-based session transport and resumable on-disk run state. +[0.6.0]: https://github.com/bmad-code-org/bmad-auto/releases/tag/v0.6.0 [0.5.1]: https://github.com/bmad-code-org/bmad-auto/releases/tag/v0.5.1 [0.5.0]: https://github.com/bmad-code-org/bmad-auto/releases/tag/v0.5.0 [0.4.4]: https://github.com/bmad-code-org/bmad-auto/releases/tag/v0.4.4 diff --git a/module.yaml b/module.yaml index 3c191c98d..e9bb0fedd 100644 --- a/module.yaml +++ b/module.yaml @@ -1,7 +1,7 @@ code: bauto name: BMAD Auto Skills description: "Automation-mode skills driven by the bmad-auto orchestrator: unattended dev (bmad-auto-dev), adversarial review (bmad-auto-review), and deferred-work sweep triage (bmad-auto-sweep)" -module_version: 0.5.1 +module_version: 0.6.0 default_selected: false module_greeting: > BMAD Auto installed — both the four automation skills and the diff --git a/pyproject.toml b/pyproject.toml index ac8d60e29..539f7ce42 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "bmad-auto" -version = "0.5.1" +version = "0.6.0" description = "Deterministic ralph-loop orchestrator for the BMAD implementation phase" readme = "README.md" license = "MIT" diff --git a/src/automator/__init__.py b/src/automator/__init__.py index 3fef7f6f3..884f30d96 100644 --- a/src/automator/__init__.py +++ b/src/automator/__init__.py @@ -6,4 +6,4 @@ spec files, and the per-run directory under .automator/runs/. """ -__version__ = "0.5.1" +__version__ = "0.6.0" diff --git a/src/automator/data/skills/bmad-auto-setup/assets/module.yaml b/src/automator/data/skills/bmad-auto-setup/assets/module.yaml index 3c191c98d..e9bb0fedd 100644 --- a/src/automator/data/skills/bmad-auto-setup/assets/module.yaml +++ b/src/automator/data/skills/bmad-auto-setup/assets/module.yaml @@ -1,7 +1,7 @@ code: bauto name: BMAD Auto Skills description: "Automation-mode skills driven by the bmad-auto orchestrator: unattended dev (bmad-auto-dev), adversarial review (bmad-auto-review), and deferred-work sweep triage (bmad-auto-sweep)" -module_version: 0.5.1 +module_version: 0.6.0 default_selected: false module_greeting: > BMAD Auto installed — both the four automation skills and the diff --git a/uv.lock b/uv.lock index 41c0bab90..e81630625 100644 --- a/uv.lock +++ b/uv.lock @@ -4,7 +4,7 @@ requires-python = ">=3.11" [[package]] name = "bmad-auto" -version = "0.5.1" +version = "0.6.0" source = { editable = "." } dependencies = [ { name = "pyyaml" },