fix(verify): translate spawn OSError to GitSpawnError in _run_git (#343)

CHANGELOG.md

@@ -164,6 +164,15 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories
 
 ### Fixed
 
+- **Spawn-level `OSError` is translated at the git chokepoint (#343).** `_run_git` translated only
+  a timeout, so an EMFILE/ENOMEM/ENOENT out of `subprocess.run` bypassed all 19 OSError-blind
+  `except GitError` guards and crashed the run — under exactly the resource pressure the recovery
+  paths owning those guards exist for. Spawn faults now raise `GitSpawnError` (a `GitError`), so
+  every guard holds as written; the errno stays on `__cause__`. A spawn fault while opening a unit
+  worktree now pauses the run instead of marching the queue into DEFERRED, and an FS or spawn
+  fault during merge-target reconciliation keeps the unit branch and escalates instead of
+  crashing — naming the fault rather than claiming stray uncommitted files to clean.
+
 - **`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

src/bmad_loop/engine.py

@@ -1788,9 +1788,11 @@ def _finalize_commit_phase(self, task: StoryTask) -> None:
         except BaseException:
             # A failed commit is not the only way out of this window: the signal
             # handler `_run` installed raises RunStopped from wherever the main
-            # thread is standing, and a raw OSError on spawn escapes `_run_git`
-            # as itself. Either leaves the ledger flipped for a commit that does
-            # not exist. Restore, then re-raise untouched.
+            # thread is standing, and an OSError raised outside `_run_git` (an
+            # FS fault; the spawn class arrives as GitSpawnError since #343 and
+            # takes the arm above) passes through as itself. Either leaves the
+            # ledger flipped for a commit that does not exist. Restore, then
+            # re-raise untouched.
             self._restore_deferred_closes(task, snapshot)
             raise
         advance(task, Phase.DONE)

src/bmad_loop/recovery_flow.py

@@ -256,10 +256,10 @@ def restore_patch(self, task: StoryTask) -> None:
         try:
             verify.apply_patch(workspace.root, patch)
         except (verify.GitError, OSError) as e:
-            # OSError joins GitError for the reason the rollback guards do (#343):
-            # `_run_git` translates only a timeout, and the patch file is read from
-            # disk here, so an ENOENT/EACCES/ENOSPC arrives untyped. Crashing would
-            # skip the escalation this branch exists to perform and leave the tree
+            # OSError joins GitError because the patch file is read from disk
+            # here, so an ENOENT/EACCES/ENOSPC arrives untyped — a non-spawn FS
+            # fault the #343 chokepoint cannot translate. Crashing would skip
+            # the escalation this branch exists to perform and leave the tree
             # half-restored with no attention file.
             self.journal.append(
                 "attempt-restore-failed",
@@ -405,9 +405,10 @@ def preserve_attempt_worktree(self, task: StoryTask, *, allow_pause: bool) -> No
         same terms: with ``allow_pause`` (a plain rollback) pause for manual
         recovery rather than reset; on a re-drive (``allow_pause=False``) the
         caller's contract forbids pausing, so journal and let the human-directed
-        reset proceed. Both now guard ``(GitError, OSError)`` too — ``_run_git``
-        translates only a timeout, so a spawn-level fault would otherwise crash the
-        rollback rather than refuse it (#343).
+        reset proceed. Both now guard ``(GitError, OSError)`` too: spawn faults
+        arrive typed as ``GitSpawnError`` since #343, but ``snapshot_worktree``'s
+        ``TemporaryDirectory`` can still raise a plain ``OSError`` (ENOSPC),
+        which would otherwise crash the rollback rather than refuse it.
 
         The refusal is gated on :meth:`_reset_would_destroy`, so a capture failure
         over a tree with nothing left to lose (commits already parked, nothing
@@ -432,9 +433,10 @@ def preserve_attempt_worktree(self, task: StoryTask, *, allow_pause: bool) -> No
                 workspace.root, ref, baseline_untracked=task.baseline_untracked
             )
         except (verify.GitError, OSError) as exc:
-            # OSError alongside GitError: `_run_git` only translates a timeout, so a
-            # spawn-level EMFILE/ENOMEM escapes untyped (the sibling guards at
-            # verify.is_ancestor / worktree_prune catch both). Uncaught it crashed the
+            # OSError alongside GitError: spawn faults arrive typed as GitSpawnError
+            # since #343, but `snapshot_worktree`'s `TemporaryDirectory` can raise a
+            # plain OSError (ENOSPC) — a non-spawn FS fault the chokepoint cannot
+            # translate, so this arm stays load-bearing. Uncaught it crashed the
             # run here — after the commits ref, before the reset — which is the safe
             # outcome reached the loudest possible way. Preservation is observation,
             # so it degrades into the decision below; `safe_reset` is the repair
@@ -489,11 +491,12 @@ def _reset_would_destroy(self, task: StoryTask) -> bool:
         un-determinable probe reads as work-at-risk, mirroring the dirty check's
         own git-fault doctrine (#156).
 
-        Catches ``OSError`` for the same reason the caller does, and it matters more
-        here: this runs immediately after a snapshot fault, against the same git
-        binary, so the EMFILE/ENOMEM that broke the capture is likely to break the
-        probe too. Guarding only `GitError` would undo the broadening one frame up
-        and crash the rollback anyway."""
+        Catches ``OSError`` for the same reason the caller does: spawn faults
+        arrive as ``GitSpawnError`` since #343, but this runs immediately after a
+        snapshot fault — often an ENOSPC out of ``snapshot_worktree``'s
+        ``TemporaryDirectory`` — and a filesystem this broken can fail the probe
+        in non-spawn ways too. Guarding only `GitError` would undo the broadening
+        one frame up and crash the rollback anyway."""
         workspace = self._workspace_get()
         try:
             head = verify.rev_parse_head(workspace.root)

src/bmad_loop/verify.py

@@ -69,14 +69,24 @@ class GitError(Exception):
     pass
 
 
+class GitSpawnError(GitError):
+    """The git child could not be spawned at all (an OSError out of
+    `subprocess.run` — EMFILE, ENOMEM, ENOENT on the git binary). A GitError so
+    every existing guard treats it like any other git failure; a distinct type
+    so the rare caller can tell "git said no" from "the machine is broken"
+    (#343). The underlying errno stays reachable via ``exc.__cause__.errno``."""
+
+
 def _run_git(
     cmd: list[str], repo: Path, *, env: dict[str, str] | None = None
 ) -> subprocess.CompletedProcess[str]:
-    """Sole spawn point for git subprocesses. A timeout is raised by
-    `subprocess.run` *before* any return code exists, so left uncaught it would
-    bypass every `except GitError` guard and crash the run (#156); translating
-    it here puts timeouts in the same taxonomy as any other git failure —
-    observation guards degrade, unguarded paths fail typed.
+    """Sole spawn point for git subprocesses. Two failures are raised by
+    `subprocess.run` *before* any return code exists — a timeout (#156) and a
+    spawn-level OSError (#343) — so left uncaught either would bypass every
+    `except GitError` guard and crash the run. Both are translated here into
+    the GitError taxonomy — observation guards degrade, unguarded paths fail
+    typed — with the spawn class marked as `GitSpawnError` for the callers
+    that must distinguish an environment fault from git refusing.
 
     Every git child runs with `LC_ALL=C` so messages stay stable English: the one
     place that inspects git *text* rather than a return code — `safe_rollback`'s
@@ -94,6 +104,8 @@ def _run_git(
         )
     except subprocess.TimeoutExpired as exc:
         raise GitError(f"git {cmd[3]} timed out after {_git_timeout_s}s in {repo}") from exc
+    except OSError as exc:
+        raise GitSpawnError(f"git {cmd[3]} failed to spawn in {repo}: {exc}") from exc
 
 
 def _git(repo: Path, *args: str) -> tuple[int, str]:
@@ -459,10 +471,10 @@ def snapshot_worktree(
     reset past work it could not park (only a re-drive, whose caller contract
     forbids pausing, still journals and lets the human-directed reset run).
 
-    Not every failure here is a :class:`GitError`: ``_run_git`` translates only a
-    timeout, so a spawn-level ``OSError`` escapes untyped, and the
-    ``TemporaryDirectory`` below can raise one outright (ENOSPC/EMFILE) before any
-    git child is spawned. Callers guard ``(GitError, OSError)`` (#343)."""
+    Not every failure here is a :class:`GitError`: spawn faults arrive typed as
+    :class:`GitSpawnError` since #343, but the ``TemporaryDirectory`` below can
+    raise a plain ``OSError`` outright (ENOSPC/EMFILE) — a filesystem fault no
+    git chokepoint can translate. Callers keep guarding ``(GitError, OSError)``."""
     head = rev_parse_head(repo)
     with tempfile.TemporaryDirectory() as td:
         env = {**os.environ, "GIT_INDEX_FILE": str(Path(td) / "index")}
@@ -706,13 +718,12 @@ def worktree_remove(repo: Path, path: Path, force: bool = False) -> None:
 def worktree_prune(repo: Path) -> None:
     """Drop administrative entries for worktrees whose directories are gone.
     Best-effort housekeeping — never raises. The return code is already ignored,
-    but since #156 `_git` can *raise* GitError on a timeout, which would bypass
-    this never-raise contract (and the teardown degrade paths that lean on it —
-    close_unit_workspace / discard_worktree call prune from inside their own
-    GitError guards). `subprocess.run` can also raise OSError outright (a spawn
-    failure — EMFILE, ENOMEM — happens before any return code exists), which the
-    #156 translation doesn't cover. Swallow both here so the contract holds at
-    its source."""
+    but `_git` can *raise* — GitError on a timeout (#156) or GitSpawnError on a
+    spawn fault (#343) — which would bypass this never-raise contract (and the
+    teardown degrade paths that lean on it — close_unit_workspace /
+    discard_worktree call prune from inside their own GitError guards). The
+    OSError in the net predates the #343 translation and stays as the belt for
+    any non-spawn fault: the contract holds at its source."""
     try:
         _git(repo, "worktree", "prune")
     except (GitError, OSError):

src/bmad_loop/worktree_flow.py

@@ -428,6 +428,15 @@ def run_isolated(self, task: StoryTask, drive: Callable[[StoryTask], None]) -> N
                 self.policy.scm.branch_per,
                 self.run_dir,
             )
+        except verify.GitSpawnError as e:
+            # a spawn fault is machine-wide, not this unit's: deferring would
+            # march the whole queue into DEFERRED one notification at a time
+            # and end the run "finished" over a broken environment (#194/#343).
+            self._pause(
+                f"cannot spawn git while opening a worktree for {task.story_key}: {e}",
+                task.story_key,
+                cause=e,
+            )
         except verify.GitError as e:
             # could not mount a worktree (e.g. branch_per=run with a kept-failed
             # unit still holding the shared branch). Defer this unit rather than
@@ -554,13 +563,27 @@ def merge_local(self, task: StoryTask, unit: UnitWorkspace) -> None:
         # falls outside this branch's path set — that may be real operator work.
         try:
             cleaned = verify.clean_incoming_collisions(repo, target, unit.branch)
-        except verify.GitError as e:
-            reason = (
-                f"merge of {unit.branch} into {target} blocked: the target checkout has "
-                f"uncommitted changes that are not part of this branch (likely a Unity "
-                f"Editor wrote into the main project) — clean them, then "
-                f"`bmad-loop resume {self.state.run_id}`. {e}"
-            )
+        except (verify.GitError, OSError) as e:
+            # OSError joins GitError because clean_incoming_collisions mutates the
+            # checkout directly (unlink/iterdir/rmdir) — a non-spawn FS fault the
+            # #343 chokepoint cannot translate. Crashing here would strand a DONE
+            # unit mid-merge; the keep-branch escalation is the point of this guard.
+            if isinstance(e, (verify.GitSpawnError, OSError)):
+                # environment fault (spawn failure or direct-FS error) — there may
+                # be no stray files at all, so no "clean them" guidance: the inner
+                # error is the diagnosis.
+                reason = (
+                    f"merge of {unit.branch} into {target} blocked: could not "
+                    f"reconcile the target checkout ({e}) — fix the underlying "
+                    f"fault, then `bmad-loop resume {self.state.run_id}`"
+                )
+            else:
+                reason = (
+                    f"merge of {unit.branch} into {target} blocked: the target checkout has "
+                    f"uncommitted changes that are not part of this branch (likely a Unity "
+                    f"Editor wrote into the main project) — clean them, then "
+                    f"`bmad-loop resume {self.state.run_id}`. {e}"
+                )
             self.keep_branch_and_escalate(task, unit, reason)  # always raises RunPaused
             return
         if cleaned:

tests/test_cleanup.py

@@ -94,6 +94,30 @@ def test_reconcile_orphan_worktrees_dry_run(project):
     assert wt.exists()  # dry run removes nothing
 
 
+def test_reconcile_orphan_worktrees_spawn_fault_degrades_to_noop(project, monkeypatch):
+    """#343 acceptance, observation-degrades class: a spawn-level OSError out of
+    `git worktree list` arrives typed as GitSpawnError and reads as "no orphans"
+    — reclaim is housekeeping, so a broken environment degrades it to a no-op
+    rather than crashing the caller. Injected at `subprocess.run` itself to
+    prove the whole chain: the chokepoint translation is what lets the
+    `except GitError` guard hold.
+
+    Ablation target: delete the `except OSError` arm in `verify._run_git` and
+    this fails with the raw OSError."""
+    repo = project.project
+    run_dir = repo / ".bmad-loop" / "runs" / "20260101-000000-aaaa"
+    wt = run_dir / "worktrees" / "unit"
+    wt.parent.mkdir(parents=True)
+    verify.worktree_add(repo, wt, "feat", "main")
+
+    def cannot_spawn(cmd, **kwargs):
+        raise OSError(24, "Too many open files")
+
+    monkeypatch.setattr(verify.subprocess, "run", cannot_spawn)
+    assert runs.reconcile_orphan_worktrees(repo, run_dir) == []
+    assert wt.exists()  # nothing was reclaimed, nothing was rmtree'd
+
+
 def test_reconcile_stale_worktrees_finished_only(project):
     repo = project.project
     fin = repo / ".bmad-loop" / "runs" / "20260101-000000-aaaa"

tests/test_engine.py

@@ -2404,9 +2404,9 @@ def sigterm_mid_commit(*a, **kw):
 
 
 def test_closes_deferred_rolls_back_when_the_commit_raises_a_non_git_error(project, monkeypatch):
-    """`_run_git` translates only TimeoutExpired, so a raw OSError from the spawn
-    itself (a fork failure, git gone from PATH) reaches the commit boundary as
-    itself and slips past the GitError arm."""
+    """An OSError raised *outside* `_run_git` — an FS fault, here the patched
+    callee itself — is not the spawn class #343 translates, so it reaches the
+    commit boundary as itself and slips past the GitError arm."""
     engine = _closes_deferred_run(project, ["DW-1"])
     before = project.deferred_work.read_bytes()
 
@@ -2422,6 +2422,33 @@ def cannot_spawn(*a, **kw):
     assert project.deferred_work.read_bytes() == before
 
 
+def test_closes_deferred_restores_and_escalates_when_the_commit_spawn_fails(project, monkeypatch):
+    """#343: a spawn fault during finalize arrives typed as GitSpawnError (a
+    GitError), so the commit window restores the ledger and escalates — the
+    same disposition as a git timeout there — instead of crashing the run. The
+    raw-OSError crash above remains for the non-spawn class only.
+
+    Ablation target: delete the `except verify.GitError` arm in
+    `_finalize_commit_phase` and this fails — the fault falls through to the
+    BaseException arm and the run crashes."""
+    engine = _closes_deferred_run(project, ["DW-1"])
+    before = project.deferred_work.read_bytes()
+
+    def cannot_spawn(*a, **kw):
+        raise verify.GitSpawnError("git add failed to spawn: [Errno 12] Cannot allocate memory")
+
+    monkeypatch.setattr(verify, "finalize_commit", cannot_spawn)
+
+    summary = engine.run()
+
+    assert summary.paused and summary.escalated == 1 and not summary.crashed
+    assert load_state(engine.run_dir).tasks["1-1-a"].phase == Phase.ESCALATED
+    reasons = [e["reason"] for e in engine.journal.entries() if e["kind"] == "story-escalated"]
+    assert len(reasons) == 1 and reasons[0].startswith("commit failed:")
+    assert _ledger_entries(project)["DW-1"].open
+    assert project.deferred_work.read_bytes() == before  # byte-identical restore
+
+
 def test_closes_deferred_rolls_back_when_the_journal_fails_after_the_ledger_write(
     project, monkeypatch
 ):

tests/test_engine_worktree.py

@@ -10,6 +10,7 @@
 
 import shutil
 
+import pytest
 from conftest import (
     _OK,
     _exists_run,
@@ -978,6 +979,53 @@ def test_merge_stray_dirt_escalates_with_clear_message(project):
     assert "merge-target-cleaned" not in journal_kinds(engine)
 
 
+@pytest.mark.parametrize(
+    "make_exc",
+    [
+        lambda: OSError(13, "Permission denied"),
+        lambda: verify.GitSpawnError("git status failed to spawn: Permission denied"),
+    ],
+    ids=["fs-oserror", "git-spawn"],
+)
+def test_merge_env_fault_during_target_clean_keeps_branch_and_escalates(
+    project, monkeypatch, make_exc
+):
+    """#343: `clean_incoming_collisions` mutates the checkout directly
+    (unlink/rmdir), so a non-spawn FS fault arrives as a plain OSError no
+    chokepoint can translate — and its git reads can raise a typed
+    GitSpawnError. The guard must treat both like any other reconcile
+    failure: keep the branch and escalate rather than crash a DONE unit
+    mid-merge — and the escalation must name the environment fault, not
+    claim stray uncommitted files that may not exist.
+
+    Ablation targets: narrow the guard in `merge_local` back to
+    `verify.GitError` and the fs-oserror case fails — the OSError crashes the
+    run. Revert the reason branch to the unconditional stray-files text and
+    both cases fail on the message assertions."""
+    commit_sprint(project, {"1-1-a": "ready-for-dev"})
+    engine, _ = make_engine(
+        project,
+        [wt_dev_effect(project, "1-1-a"), wt_review_effect(project, "1-1-a", clean=True)],
+    )
+    exc = make_exc()
+
+    def env_fault(*a, **kw):
+        raise exc
+
+    monkeypatch.setattr(verify, "clean_incoming_collisions", env_fault)
+    summary = engine.run()
+
+    assert summary.paused and summary.escalated == 1 and not summary.crashed
+    assert engine.state.tasks["1-1-a"].phase == Phase.ESCALATED
+    reason = engine.state.paused_reason or ""
+    assert "Permission denied" in reason
+    # environment fault, not the stray-files refusal: no "clean them" guidance
+    assert "could not reconcile" in reason
+    assert "clean them" not in reason
+    # branch kept for manual merge — the unit's work is not stranded
+    assert branch_exists(project.project, "bmad-loop/test-run/1-1-a")
+
+
 def test_spec_file_serialized_relative_to_worktree():
     """A worktree task persists spec_file relative to its worktree so a kept run's
     state stays portable (no dangling absolute path into a pruned worktree)."""