Make the crash write-chain race test deterministic

Firing 50 unawaited writes and hoping enough were still queued when
isCrashed() flipped was a coin flip in practice (5/13 passed with the
guard removed, when it should fail every time). A test-only write
gate in active-run.ts now lets the fixture park writes before they
reach the isCrashed() check and release them only after the crash
handler has flipped the flag, so the ordering is controlled instead
of hoped for. 25/25 passes with the guard in place; removing the
guard reliably lets the parked writes win the race again.

Author: TheGreatAxios

src/session/active-run.ts

@@ -49,3 +49,18 @@ export function markCrashed(): void {
 export function isCrashed(): boolean {
   return crashed;
 }
+
+// Test-only seam: lets an integration test hold a chained write open past the
+// moment markCrashed() fires, so it can deterministically prove a write still
+// queued in the chain sees isCrashed() before it fires — rather than hoping
+// real filesystem timing happens to interleave that way. No effect on
+// production callers, which never install a gate.
+let testWriteGate: Promise<void> | null = null;
+
+export function setTestWriteGate(gate: Promise<void> | null): void {
+  testWriteGate = gate;
+}
+
+export function getTestWriteGate(): Promise<void> | null {
+  return testWriteGate;
+}

src/session/state.ts

@@ -4,7 +4,7 @@ import { dirname, join } from "node:path";
 import { type } from "arktype";
 
 import { sessionDir } from "./index.js";
-import { isCrashed } from "./active-run.js";
+import { getTestWriteGate, isCrashed } from "./active-run.js";
 import { COMMAND_NAME } from "../branding.js";
 
 const ConnectedMcpServerSchema = type({
@@ -74,6 +74,10 @@ const writeChains = new Map<string, Promise<void>>();
 // dispatched to the kernel — that residual window is one atomicWrite call
 // wide (a small local JSON write), not the remaining lifetime of the process.
 async function atomicWriteUnlessCrashed(path: string, content: string): Promise<void> {
+  // No-op in production; lets a test hold this write open past the moment
+  // isCrashed() flips, so the check below is proven rather than assumed.
+  const gate = getTestWriteGate();
+  if (gate !== null) await gate;
   if (isCrashed()) return;
   await atomicWrite(path, content);
 }

tests/fixtures/crash-run/simulate-crash.ts

@@ -4,7 +4,7 @@
 // crash handlers), then throws asynchronously so it surfaces as a genuine
 // uncaughtException rather than a synchronous throw the caller could catch.
 import { installCrashHandlers } from "../../../src/index.js";
-import { setActiveRun } from "../../../src/session/active-run.js";
+import { setActiveRun, setTestWriteGate } from "../../../src/session/active-run.js";
 import { sessionDir } from "../../../src/session/index.js";
 import { saveState } from "../../../src/session/state.js";
 
@@ -31,23 +31,35 @@ installCrashHandlers();
 
 process.stdout.write(`${sessionDir(cwd, sessionId)}\n`);
 
-// Queue a burst of unawaited straggler snapshot writes (what
-// persistRunSnapshot does on every turn/model-switch/MCP-connect event) right
-// before crashing. Each is chained onto the previous one in state.ts's
-// per-session write queue, so most of these are still waiting their turn —
-// not yet dispatched to the kernel — at the moment the crash handler flips
-// the isCrashed() flag. Without that guard, one of these landing after
-// saveCrashState's rename() would resurrect status: "running".
-for (let i = 0; i < 50; i++) {
-  void saveState(cwd, sessionId, {
-    status: "running",
-    turnsUsed: i,
-    task,
-    startedAt,
-    model,
-  });
-}
+// Hold every write issued from here on at the gate, before it reaches
+// isCrashed(). This makes the race deterministic instead of hoping real
+// filesystem timing interleaves the right way: the two straggler writes
+// below are guaranteed to still be queued, not dispatched to the kernel,
+// when the crash handler flips isCrashed() — the exact scenario the guard
+// exists for.
+let releaseGate: () => void;
+const gate = new Promise<void>((resolve) => {
+  releaseGate = resolve;
+});
+setTestWriteGate(gate);
+
+// Two unawaited straggler snapshot writes, chained behind each other in
+// state.ts's per-session queue — what persistRunSnapshot fires on every
+// turn/model-switch/MCP-connect event. Both are parked at the gate.
+void saveState(cwd, sessionId, { status: "running", turnsUsed: 1, task, startedAt, model });
+void saveState(cwd, sessionId, { status: "running", turnsUsed: 2, task, startedAt, model });
 
+// Throws inside setImmediate so it surfaces as a real uncaughtException.
+// Node/Bun run the exception's own uncaughtException dispatch — including
+// handleFatal's synchronous markCrashed() call, which precedes its first
+// await — to completion before the event loop reaches the next queued
+// setImmediate callback. The second setImmediate below is therefore
+// guaranteed to run after isCrashed() has flipped to true, so releasing the
+// gate there always lets the two parked writes observe the flag rather than
+// racing it.
 setImmediate(() => {
   throw new Error("simulated crash");
 });
+setImmediate(() => {
+  releaseGate();
+});

tests/integration/crash-finalize.test.ts

@@ -35,10 +35,13 @@ describe("integration — crash finalizes run.json", () => {
       const raw = readFileSync(runJsonPath, "utf8");
       const state = JSON.parse(raw) as RunState;
 
-      // The fixture also fires 50 unawaited straggler "running" snapshot
-      // writes for the same session immediately before crashing. Without the
-      // isCrashed() guard in saveState (src/session/state.ts), one of those
-      // could win the rename() race and this would read back "running".
+      // The fixture also parks two unawaited straggler "running" snapshot
+      // writes behind a test-only gate (setTestWriteGate) that it releases
+      // only after the crash handler has flipped isCrashed(), guaranteeing
+      // both are still queued — not dispatched to the kernel — at that
+      // moment. Without the isCrashed() guard in saveState
+      // (src/session/state.ts), one of those would win the rename() race
+      // once released and this would read back "running".
       expect(state.status).toBe("crashed");
       expect(state.finishedAt).toBeGreaterThan(0);
       expect(state.error).toContain("simulated crash");