_run_git translates only TimeoutExpired, so a spawn-level OSError bypasses every except GitError guard

[pbean]

verify._run_git is the sole git spawn point, and it translates exactly one failure into GitError:

    try:
        return subprocess.run(cmd, capture_output=True, text=True, timeout=_git_timeout_s, env=...)
    except subprocess.TimeoutExpired as exc:
        raise GitError(f"git {cmd[3]} timed out after {_git_timeout_s}s in {repo}") from exc

An OSError — EMFILE, ENOMEM, ENOENT on the git binary — is not translated. It escapes subprocess.run untyped, sails past every except verify.GitError guard in the codebase, and reaches the engine's catch-all (engine.py:441), which converts it to state.crashed + a crash.txt.

OSError also reaches these guards without any spawn at all: verify.snapshot_worktree opens a tempfile.TemporaryDirectory() (verify.py:461) before its first git child, so an ENOSPC/EMFILE there raises straight out of a function whose contract says GitError.

This is already known and defended case by case: verify.is_ancestor and verify.worktree_prune catch (GitError, OSError), with comments acknowledging the gap. But the defense is per-site and by reviewer memory.

Scale

$ grep -rnE "^\s*except [(]?[a-zA-Z_.]*GitError" src/bmad_loop/ | wc -l
27
$ grep -rnE "^\s*except \([a-zA-Z_.]*GitError, OSError)" src/bmad_loop/ | wc -l
8

19 of 27 guards are OSError-blind (was 22 of 26 before #341). Each is a latent crash under memory or fd pressure — and pressure is precisely when the recovery paths that own most of these guards are running.

#341 closed recovery_flow.py completely; it is now uniformly OSError-safe. The remaining 19 sit in workspace.py (7), worktree_flow.py (4), runs.py (2), engine.py (2), and one each in verify.py, tui/app.py, decisions.py, cli.py.

The worked example: recovery_flow.rollback_or_pause — now fixed in #341

#341 widened three guards on this path, then found four more frames upstream of all of them, on the way into the same method. All four are fixed in #341 and are recorded here only as the evidence for what the repo-wide default costs:

Site	Guard before	Failure before
attempt_dirty (dirty check)	except verify.GitError	OSError crashed
commits_above (range enumeration)	none	OSError and GitError crashed
rev_parse_head (ref tip)	none	OSError and GitError crashed
preserve_commits (ref write)	except verify.GitError	OSError crashed

The two unguarded calls were the sharper finding, and they are the reason this issue is worth acting on rather than closing: they had no try at all, so a plain GitError — a git timeout, which _run_git does translate and which every sibling on that path already treats as routine — crashed the rollback outright. The OSError audit is what surfaced them; the bug it surfaced was not an OSError bug.

That is the argument for a chokepoint answer. A per-site audit motivated by OSError found a GitError hole by accident, four frames from where anyone was looking.

Possible shapes

• Translate at the chokepoint. Wrap subprocess.run in _run_git so OSError becomes GitError (or a GitSpawnError(GitError) subclass) the same way TimeoutExpired does. Every existing guard then works as written, and the 19 sites need no edit. Biggest behavior change, and it converts some genuine "the machine is broken" signals into ordinary git failures — needs a look at whether any caller should keep crashing. Note this does not cover the non-spawn OSError at verify.py:461.
• Audit and widen all 19. Mechanical, no semantic change, but permanently load-bearing on reviewer memory and re-rots with every new guard.
• Chokepoint translation plus a lint rule banning bare except GitError where an OSError is still reachable. Belt and braces.

I lean toward the first: it is the only one that makes the default correct rather than the exception. GitError is currently an empty Exception subclass, so a GitSpawnError subclass would let the few callers that genuinely want to distinguish "git said no" from "the machine is out of file descriptors" do so, which nothing can do today.

Whichever shape wins, the worked example above is the argument for acting: a guard gap that a chokepoint fix would have closed for free went unnoticed through three review rounds.

Acceptance

A decision on shape. If the chokepoint translates, the tests should cover at least one caller per behavioural class (observation-degrades vs repair-raises) with the OSError injected at subprocess.run, and each needs an ablation — narrow the translation back and confirm the test fails.

Not in scope

recovery_flow.py — #341 closed all seven of its guards (the three it started with, the three upstream frames, and the restore-patch apply). This issue is the other eight modules and the chokepoint default.

---

Correction (2026-07-27). The original text of this issue justified chokepoint translation with a "correlated failure chain" argument — that #341's three rounds each found the same defect one frame further along, and that widening the first guard alone "bought nothing" because the same EMFILE would break the next call. That argument was wrong and has been removed.

Traced against the code: a genuinely process-wide, persistent OSError never reaches any of the three frames #341 patched. It crashes at recovery_flow.py:151, upstream, on the way in. The argument was self-defeating — the more persistent the fault, the less those guards matter. And widening the first guard alone does buy something: it fully handles a fault that onsets at the snapshot, which is the realistic case, since the likeliest OSError out of snapshot_worktree is ENOSPC from its TemporaryDirectory — a filesystem fault git spawns do not share.

The systemic gap this issue describes is real; the chain narrative was not. The motivation is now stated as the unguarded upstream frames, which are demonstrable.

[coderabbitai]

🔗 Related PRs

#113 - fix: degrade spec read faults instead of crashing the run [merged]
#174 - fix(verify): git timeouts become GitError; rollback dirty check degrades instead of crashing the run (#156) [merged]
#182 - fix(worktree): degrade post-merge teardown failures instead of crashing the run (#139) [merged]
#237 - fix(verify): force LC_ALL=C at the git chokepoint so rollback stops matching English (#236) [merged]
#341 - fix(recovery): refuse the rollback reset when the snapshot fails (#340) [open]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[pbean]

Validation record before implementation (PR #344). The systemic gap is confirmed and the chokepoint shape adopted; three corrections to this issue's text for the record:

1. The TemporaryDirectory is at verify.py:467, not 461 — and it is not before the first git child. rev_parse_head at line 466 spawns git rev-parse HEAD first, so the OSError-escape window in snapshot_worktree opens at a git spawn, then the TemporaryDirectory. The function's own docstring repeated the "before any git child is spawned" claim; corrected in the PR. The substantive point stands: the TemporaryDirectory OSError is untranslatable at any git chokepoint, so its callers' (GitError, OSError) guards stay.
2. True inventory is 28 guards / 9 widened / 19 blind, not 27/8/19. verify.py:156 (is_ancestor) guards except (OSError, GitError) — the reversed tuple order escapes both greps in this issue. The blind count of 19 is coincidentally correct, as is the module distribution.
3. One blind site had a deliberate, commented relationship with the escaping OSError: engine.py:1788's BaseException arm names "a raw OSError on spawn escapes _run_git as itself" as a case it restores the deferred-close ledger against. Its purpose is ledger protection, not crash preservation, so translation completes what it compensated for — the spawn class now takes the escalate arm (same disposition as a git timeout there); non-spawn OSErrors still restore and crash.

Resolution: chokepoint translation via GitSpawnError(GitError) (precedent: PrunePreserveError), plus two site-level decisions — a spawn fault opening a unit worktree pauses the run rather than deferring the whole queue (#194 doctrine), and merge_local's reconcile guard widens to (GitError, OSError) for clean_incoming_collisions' non-spawn FS ops, the one hole among the 19 the chokepoint cannot close. The lint-rule option was declined: no clean way to express "bare except GitError where an OSError is reachable", and the chokepoint removes the need. Acceptance met: one caller per behavioural class tested with the OSError injected at subprocess.run, all four new arms/guards ablation-verified.