fix(recovery): refuse the rollback reset when the snapshot fails (#340)

CHANGELOG.md

@@ -101,6 +101,21 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories
 
 ### Changed
 
+- **A failed worktree snapshot now blocks the rollback reset (#340).** The two preserve steps were
+  asymmetric: an auto-rollback refused to reset past commits it could not park, but a failed
+  _uncommitted_-work snapshot was journaled and the `reset --hard` ran anyway — destroying the
+  tracked edits and run-created untracked files the snapshot existed to capture. That protected the
+  more recoverable half; orphaned commits stay reachable by reflog until `gc`, while a discarded
+  uncommitted edit is gone. Both paths now refuse on the same terms, pausing with rescue
+  instructions that name the tree (the mounted worktree when there is one) and offer a git-free
+  copy-out, since the fault that broke the snapshot may still be breaking git.
+
+  Gated on there being something left to lose: a capture failure over a tree whose content was all
+  committed still resets, so a git fault can't halt an unattended run over a harmless reset. A
+  resolved re-drive stays pause-free by contract — it journals, resets, and flags the park
+  commits-only (#338). Inert under the shipped defaults, where the snapshot is reached only on a
+  re-drive; it bites `scm.rollback_on_failure = true` and in-worktree dev retries.
+
 - **A review that revokes the sprint sign-off now escalates instead of burning review cycles
   (#334).** The orchestrator advances `sprint-status` to `done` at dev time; a review session that
   judges the story unfinished and writes the board back to an earlier stage contradicts that — and
@@ -149,6 +164,30 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories
 
 ### Fixed
 
+- **`safe_rollback` no longer swallows a failed `git stash create`.** The empty snapshot silently
+  disabled the whole `preserve` restore, so the hard reset reverted exactly the paths the caller
+  asked to keep — a resolved re-drive's corrected spec — with no error anywhere. It now raises
+  before the reset, and only when a restore was actually requested, so the no-`preserve` callers
+  (both sweep sites) degrade as before.
+
+- **A spawn-level `OSError` during the attempt snapshot no longer crashes the run.** `_run_git`
+  translates only a timeout, so an EMFILE/ENOMEM from `subprocess.run` escaped untyped out of the
+  middle of a rollback. Preservation is observation rather than a repair write, so it now degrades
+  into the same journal-and-decide path a `GitError` takes, keeping the errno as the breadcrumb;
+  `safe_reset` still raises.
+
+- **A git fault while counting the attempt's commits no longer crashes the rollback (#343).**
+  `preserve_attempt_commits` enumerated the range above baseline with no guard at all, so an
+  ordinary git _timeout_ — already translated and treated as routine everywhere else — took the
+  run down mid-rollback; a spawn-level `OSError` did the same, there and at the ref write. An
+  un-determinable range now reads as "there may be work above baseline" and refuses the reset
+  (pausing for manual recovery, or journalling on a re-drive), never the clean-tree early return.
+  Journals `attempt-preserve-enumerate-failed`, distinct from `attempt-preserve-failed`, so a
+  post-mortem can tell "could not count the work" from "counted it but could not park it". The
+  rollback's dirty check degrades on an `OSError` for the same reason, and a restore-patch apply
+  that fails with one now escalates instead of crashing past its own escalation — closing every
+  such guard in the recovery collaborator.
+
 - **The orchestrator's ledger writers no longer inject lines from a multiline value (#305).** Found
   by [@Haven2026](https://github.com/Haven2026) in #274. The deferred-work ledger is line-oriented,
   but `deferredwork`'s mutators interpolated their arguments verbatim: a resolution note of

docs/FEATURES.md

@@ -59,7 +59,8 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se
 ### Failure handling & resilience
 
 - 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.
-- Plateau-defer: when review won't converge, the story is skipped, the spec stashed into the run dir, deferred-work preserved, the run continues. The defer notification names where the attempt survives — in place, the recovery ref the rollback parked it on plus the `git merge --ff-only` line that restores it, flagged as commits-only when the uncommitted snapshot could not be captured; isolated, the kept-failed unit branch plus any earlier rolled-back attempt's ref (named, not offered as a merge — it is not fast-forwardable there). That recovery ref is projected as `preserve_ref` in `status` and `--json`; the unit branch never is (#333).
+- 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. A resolved re-drive is pause-free by contract, so there it journals and proceeds. `scm.preserve_keep` (default 20) bounds retention of both ref families.
+- Plateau-defer: when review won't converge, the story is skipped, the spec stashed into the run dir, deferred-work preserved, the run continues. The defer notification names where the attempt survives — in place, the recovery ref the rollback parked it on plus the `git merge --ff-only` line that restores it, flagged as commits-only when the uncommitted snapshot could not be captured (only reachable on a re-drive since #340 — a plain rollback refuses the reset instead); isolated, the kept-failed unit branch plus any earlier rolled-back attempt's ref (named, not offered as a merge — it is not fast-forwardable there). That recovery ref is projected as `preserve_ref` in `status` and `--json`; the unit branch never is (#333).
 - Typed escalations: `CRITICAL` pauses the run + notifies (desktop + `ATTENTION` file); `PREFERENCE` is journaled and continues.
 - Environment faults pause without burning budget (#194): a session whose coding CLI never reached the API — a deterministic verify command whose _environment_ is broken (`sh` reports `rc 126/127`; `cmd` has no such convention, so on Windows a missing tool is caught by its `is not recognized` message or by resolving the command's leading token, and a command naming a file `cmd` cannot execute — a `.sh`, anything outside `PATHEXT` — is a fault rather than the silent `rc 0` pass it used to be, #302), **or** a dev/review/fix/workflow/sweep session whose pane log matches the profile's `env_fault_patterns` (an `API Error … Connection refused`-class transport failure that idled out the session clock) — pauses the run with the matched evidence instead of charging the attempt/cycle and deferring the story as if its code were broken. Re-arm (or a sweep's escalation-resume) restores the budget, so a resume after the outage clears re-drives from a clean slate. Patterns are per-profile (seeded only for `claude`; extend/disable via a project profile overlay).
 - CRITICAL resolution: `bmad-loop resolve <run-id>` opens an interactive resolve agent seeded with the escalation + frozen spec; you disambiguate, it re-arms the story (`escalated → pending`, spec reset to `ready-for-dev`) and resumes. `--no-interactive` skips to re-arm if you fixed the spec yourself.

src/bmad_loop/engine.py

@@ -765,14 +765,22 @@ def _prune_preserve_refs(self) -> None:
     def _preserve_attempt_commits(self, task: StoryTask, *, allow_pause: bool) -> None:
         self._recovery_flow.preserve_attempt_commits(task, allow_pause=allow_pause)
 
-    def _preserve_attempt_worktree(self, task: StoryTask) -> None:
-        self._recovery_flow.preserve_attempt_worktree(task)
+    def _preserve_attempt_worktree(self, task: StoryTask, *, allow_pause: bool) -> None:
+        self._recovery_flow.preserve_attempt_worktree(task, allow_pause=allow_pause)
 
     def _pause_for_manual_recovery(
-        self, task: StoryTask, baseline: str, *, preserve_failed: bool = False
+        self,
+        task: StoryTask,
+        baseline: str,
+        *,
+        preserve_failed: bool = False,
+        snapshot_failed: bool = False,
     ) -> None:
         self._recovery_flow.pause_for_manual_recovery(
-            task, baseline, preserve_failed=preserve_failed
+            task,
+            baseline,
+            preserve_failed=preserve_failed,
+            snapshot_failed=snapshot_failed,
         )
 
     def _finish_inflight(self) -> None: