diff --git a/CHANGELOG.md b/CHANGELOG.md index fdba367..a77f01c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,15 @@ story `, 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 diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 0254022..984bbc6 100644 --- a/src/bmad_loop/engine.py +++ b/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) diff --git a/src/bmad_loop/recovery_flow.py b/src/bmad_loop/recovery_flow.py index 8cb601f..f76e833 100644 --- a/src/bmad_loop/recovery_flow.py +++ b/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) diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index 5f44929..c40def1 100644 --- a/src/bmad_loop/verify.py +++ b/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): diff --git a/src/bmad_loop/worktree_flow.py b/src/bmad_loop/worktree_flow.py index 6bd92b7..92c3a38 100644 --- a/src/bmad_loop/worktree_flow.py +++ b/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: diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 084c5dc..e85b488 100644 --- a/tests/test_cleanup.py +++ b/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" diff --git a/tests/test_engine.py b/tests/test_engine.py index e5d9307..a52b19c 100644 --- a/tests/test_engine.py +++ b/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 ): diff --git a/tests/test_engine_worktree.py b/tests/test_engine_worktree.py index 1ade3d7..b1c6f70 100644 --- a/tests/test_engine_worktree.py +++ b/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).""" diff --git a/tests/test_recovery_flow.py b/tests/test_recovery_flow.py index 46c0a12..9b8c407 100644 --- a/tests/test_recovery_flow.py +++ b/tests/test_recovery_flow.py @@ -264,10 +264,10 @@ def boom(*a, **k): def test_rollback_dirty_check_oserror_degrades_to_dirty(project, monkeypatch): - # #343: `_run_git` translates only a timeout, so a spawn-level OSError reaches - # this guard untyped. It must degrade exactly like the GitError above — this is - # the first git call on the rollback path, so an unguarded OSError here crashes - # before any preserve step can run. + # #343: spawn faults now arrive typed as GitSpawnError, but the guard keeps a + # plain-OSError net for any untyped fault out of the probe. It must degrade + # exactly like the GitError above — this is the first git call on the rollback + # path, so an unguarded fault here crashes before any preserve step can run. repo = project.project ws = Workspace.default(project) flow = _make_flow(workspace=ws, policy=_policy(rollback_on_failure=False)) @@ -687,9 +687,10 @@ def test_snapshot_failure_never_pauses_on_redrive(project, monkeypatch): def test_snapshot_oserror_degrades_into_the_typed_path(project, monkeypatch): - """`_run_git` translates only a timeout, so a spawn-level OSError (EMFILE/ENOMEM) - escapes untyped. Preservation is observation, not a repair write, so it degrades - into the same journal-and-decide path a GitError takes rather than crashing the + """`snapshot_worktree` can raise a plain OSError outright — ENOSPC/EMFILE from + its TemporaryDirectory — a filesystem fault the #343 spawn translation cannot + cover. Preservation is observation, not a repair write, so it degrades into + the same journal-and-decide path a GitError takes rather than crashing the run mid-rollback. Ablation target: narrow the `except` back to `verify.GitError` and this fails diff --git a/tests/test_verify.py b/tests/test_verify.py index 945c4b1..d81b565 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -122,6 +122,36 @@ def test_capture_diff_timeout_becomes_git_error(project, monkeypatch): verify.capture_diff(project.project, baseline) +@pytest.mark.parametrize( + "make_exc", + [ + lambda: FileNotFoundError(2, "No such file or directory"), + lambda: OSError(24, "Too many open files"), + lambda: OSError(12, "Cannot allocate memory"), + ], + ids=["enoent", "emfile", "enomem"], +) +def test_git_spawn_oserror_becomes_git_spawn_error(project, monkeypatch, make_exc): + """#343: a spawn-level OSError (EMFILE/ENOMEM, git gone from PATH) is raised + by `subprocess.run` before any return code exists, so left untranslated it + bypassed every `except GitError` guard and crashed the run. It must surface + as GitSpawnError — a GitError to every existing guard, a distinct type for + the callers that must tell an environment fault from git refusing. + + Ablation target: delete the `except OSError` arm in `_run_git` and this + fails with the raw OSError.""" + exc = make_exc() + + def failing_run(cmd, **kwargs): + raise exc + + monkeypatch.setattr(verify.subprocess, "run", failing_run) + with pytest.raises(verify.GitError, match="git rev-parse failed to spawn") as excinfo: + verify.rev_parse_head(project.project) + assert isinstance(excinfo.value, verify.GitSpawnError) + assert excinfo.value.__cause__ is exc # errno stays reachable for callers + + def test_configure_git_timeout_overrides_bound(project, monkeypatch): """The engine-applied `limits.git_timeout_s` value is what reaches subprocess.run; the module default stays GIT_TIMEOUT_S for standalone users.""" @@ -207,6 +237,32 @@ def test_verify_dev_happy(project): assert task.spec_file == str(sp) +def test_verify_dev_spawn_fault_escalates(project, monkeypatch): + """#343 acceptance, escalate class: with the OSError injected at + `subprocess.run` itself, the shared change-gate's `except GitError` guard + catches the translated GitSpawnError and escalates (CRITICAL, not + retryable) instead of crashing. Everything on the path before + `has_changes_since` is filesystem-only, so the blanket injection's first + git spawn is exactly the guarded call. + + Ablation target: delete the `except OSError` arm in `verify._run_git` and + this fails with the raw OSError.""" + write_sprint(project, {"1-1-a": "review"}) + task = make_task(project) + sp = spec_path(project, "1-1-a") + write_spec(sp, "in-review", task.baseline_commit) + (project.project / "src.txt").write_text("changed\n") + + def cannot_spawn(cmd, **kwargs): + raise OSError(12, "Cannot allocate memory") + + monkeypatch.setattr(verify.subprocess, "run", cannot_spawn) + out = verify.verify_dev(task, project, dev_result(sp)) + assert not out.ok and not out.retryable + assert out.severity == "CRITICAL" + assert "failed to spawn" in out.reason + + def test_verify_dev_status_is_case_insensitive(project): # A hand-edited spec with a stray-cased status must still pass the gate — # the spec template emits lowercase, but casing must never decide it. diff --git a/tests/test_verify_worktree.py b/tests/test_verify_worktree.py index c576709..bbcc13d 100644 --- a/tests/test_verify_worktree.py +++ b/tests/test_verify_worktree.py @@ -108,10 +108,10 @@ def boom(*a, **k): def test_worktree_prune_swallows_os_error(project, monkeypatch): - """`subprocess.run` can raise OSError outright (a spawn failure — EMFILE, - ENOMEM — happens before any return code exists), which #156's timeout - translation doesn't cover. prune's never-raise contract must hold for it - too — its callers invoke it from inside `except GitError` guards.""" + """Since #343 a spawn failure arrives typed as GitSpawnError (a GitError), + but prune's never-raise contract keeps its own plain-OSError net as the belt + for any untyped fault — its callers invoke it from inside `except GitError` + guards and lean on it never raising, whatever the cause.""" def boom(*a, **k): raise OSError("spawn failed") diff --git a/tests/test_worktree_flow.py b/tests/test_worktree_flow.py index 4ada5b0..e8b541c 100644 --- a/tests/test_worktree_flow.py +++ b/tests/test_worktree_flow.py @@ -281,6 +281,31 @@ def boom(*a, **k): assert not any(e.startswith("unit-") for e in flow.journal.events()) +def test_run_isolated_spawn_fault_pauses_instead_of_deferring(tmp_path): + """#343: 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. Pause instead; per-unit GitErrors + still take the defer path. + + Ablation target: delete the `except verify.GitSpawnError` arm in + `run_isolated` and this fails — the defer arm catches it and the phase + lands on DEFERRED.""" + + def boom(*a, **k): + raise verify.GitSpawnError("git worktree failed to spawn: [Errno 24] Too many open files") + + drove = [] + flow = _make_flow(tmp_path, open_unit_workspace=boom) + task = StoryTask(story_key="1-1", epic=1) + with pytest.raises(_Pause) as excinfo: + flow.run_isolated(task, lambda t: drove.append(t)) + assert task.phase == Phase.PENDING # not DEFERRED — nothing was burned + assert "worktree-open-failed" not in flow.journal.events() + assert flow.calls.pauses == [(excinfo.value.reason, "1-1")] + assert "cannot spawn git" in excinfo.value.reason + assert drove == [] # drive body never ran + + def test_escalate_unit_marks_escalated_notifies_and_pauses(tmp_path): flow = _make_flow( tmp_path, state=SimpleNamespace(target_branch="main", run_id="run-9", tasks={})