Split out of #338 review (see this thread). #338 fixed the claim; this issue decides whether the policy should change.
Current behavior
RecoveryFlow.rollback_or_pause runs: clear preserve_ref → preserve_attempt_commits → preserve_attempt_worktree → safe_reset.
preserve_attempt_worktree is best-effort by contract: on verify.GitError it journals attempt-worktree-preserve-failed and returns. safe_reset at the end of the arm is unconditional and outside any try, so it runs anyway — git reset --hard <baseline> plus deletion of run-created untracked files. Those are exactly the two categories the failed snapshot existed to capture.
verify.snapshot_worktree raises GitError at seven points (read-tree, add -u, add -- <untracked>, write-tree, rev-parse HEAD^{tree}, commit-tree, update-ref), and _run_git converts a GIT_TIMEOUT_S (120s) timeout into one. A slow write-tree on a large dirty tree, a full disk, or an unreadable file during git add all land there.
Net: the attempt's uncommitted work is destroyed, and the only record is one journal line.
What #338 already did
StoryTask.preserve_partial is latched in that except arm, and the defer notice downgrades its claim to committed-work-only instead of offering a merge --ff-only that implies the whole attempt is recoverable. That makes the operator-facing message honest. It does not save the work.
The question
Should a worktree-snapshot failure pause the way a commits-preserve failure does (preserve_attempt_commits calls pause_for_manual_recovery(..., preserve_failed=True) when allow_pause)?
Arguments against, which is why this was not done in #338:
Arguments for: the failure mode is silent data loss on work the operator never chose to discard, and the asymmetry with the commits path is hard to justify from the outside.
Possible middle ground worth evaluating: skip only the untracked-file deletion half of safe_reset when the snapshot failed, keeping the tracked reset. That bounds the loss without introducing a pause.
Acceptance
A decision, recorded in preserve_attempt_worktree's docstring either way. If the behavior changes, it needs a test at the seam plus an ablation, and the #161 pause-free invariant must be shown intact.
Split out of #338 review (see this thread). #338 fixed the claim; this issue decides whether the policy should change.
Current behavior
RecoveryFlow.rollback_or_pauseruns: clearpreserve_ref→preserve_attempt_commits→preserve_attempt_worktree→safe_reset.preserve_attempt_worktreeis best-effort by contract: onverify.GitErrorit journalsattempt-worktree-preserve-failedand returns.safe_resetat the end of the arm is unconditional and outside anytry, so it runs anyway —git reset --hard <baseline>plus deletion of run-created untracked files. Those are exactly the two categories the failed snapshot existed to capture.verify.snapshot_worktreeraisesGitErrorat seven points (read-tree,add -u,add -- <untracked>,write-tree,rev-parse HEAD^{tree},commit-tree,update-ref), and_run_gitconverts aGIT_TIMEOUT_S(120s) timeout into one. A slowwrite-treeon a large dirty tree, a full disk, or an unreadable file duringgit addall land there.Net: the attempt's uncommitted work is destroyed, and the only record is one journal line.
What #338 already did
StoryTask.preserve_partialis latched in thatexceptarm, and the defer notice downgrades its claim to committed-work-only instead of offering amerge --ff-onlythat implies the whole attempt is recoverable. That makes the operator-facing message honest. It does not save the work.The question
Should a worktree-snapshot failure pause the way a commits-preserve failure does (
preserve_attempt_commitscallspause_for_manual_recovery(..., preserve_failed=True)whenallow_pause)?Arguments against, which is why this was not done in #338:
preserve_attempt_worktree's docstring states the best-effort contract explicitly: "a capture failure is journaled but never blocks the … reset — the recovery ref is a safety net, not a gate."allow_pause=not redrive), so any new pause would need the same carve-out.Arguments for: the failure mode is silent data loss on work the operator never chose to discard, and the asymmetry with the commits path is hard to justify from the outside.
Possible middle ground worth evaluating: skip only the untracked-file deletion half of
safe_resetwhen the snapshot failed, keeping the tracked reset. That bounds the loss without introducing a pause.Acceptance
A decision, recorded in
preserve_attempt_worktree's docstring either way. If the behavior changes, it needs a test at the seam plus an ablation, and the #161 pause-free invariant must be shown intact.