diff --git a/src/session/active-run.ts b/src/session/active-run.ts index db817bf1e..9d0bc9fc0 100644 --- a/src/session/active-run.ts +++ b/src/session/active-run.ts @@ -84,6 +84,14 @@ export function isCrashed(): boolean { return crashed; } +// Test-only seam: the crashed flag is process-global and sticky by design, so +// a test that flips it must restore the not-crashed state — otherwise every +// later saveState chain in the test process would no-op. Follows the +// setTestWriteGate precedent below. +export function resetCrashedForTests(): void { + crashed = false; +} + // Test-only seam: lets an integration test hold a chained write open past // markCrashed(), so it can deterministically prove a write still queued in // the chain sees isCrashed() before it fires — rather than hoping filesystem diff --git a/src/session/state.test.ts b/src/session/state.test.ts index 7bb8e6fc0..f12ce947e 100644 --- a/src/session/state.test.ts +++ b/src/session/state.test.ts @@ -21,8 +21,10 @@ await withMockedModule( }), ); -const { finalizeRunState, loadState, saveState } = await import("./state.js"); -const { getActiveRun, setActiveRun } = await import("./active-run.js"); +const { finalizeRunState, loadState, saveCrashState, saveState } = + await import("./state.js"); +const { getActiveRun, markCrashed, resetCrashedForTests, setActiveRun } = + await import("./active-run.js"); type RunState = import("./state.js").RunState; let cwd = ""; @@ -105,3 +107,34 @@ test("saveState calls for different sessions do not block each other", async () expect(a).toMatchObject({ kind: "ok", state: { task: "a" } }); expect(b).toMatchObject({ kind: "ok", state: { task: "b" } }); }); + +test("saveCrashState fails closed without the crash fence and lands nothing", async () => { + const sessionId = "sess-gated"; + await expect( + saveCrashState(cwd, sessionId, state({ status: "crashed" }), home), + ).rejects.toThrow("not in crashed state"); + expect(await loadState(cwd, sessionId, home)).toMatchObject({ + kind: "missing", + }); +}); + +test("saveCrashState lands the terminal record once crashed", async () => { + const sessionId = "sess-crash-lands"; + markCrashed(); + try { + await saveCrashState( + cwd, + sessionId, + state({ status: "crashed", finishedAt: 7, error: "boom" }), + home, + ); + } finally { + // The flag is process-global and sticky: restore it so later tests in + // this process keep their saveState chains live. + resetCrashedForTests(); + } + expect(await loadState(cwd, sessionId, home)).toMatchObject({ + kind: "ok", + state: { status: "crashed", finishedAt: 7, error: "boom" }, + }); +}); diff --git a/src/session/state.ts b/src/session/state.ts index 9b35fdaf6..c1060d5fe 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -205,9 +205,11 @@ export async function finalizeRunState( } // Crash-time terminal write that bypasses writeChains: awaiting a hung queued -// write would block the crash handler's process.exit. Callers must call -// markCrashed() (src/session/active-run.ts) first so queued snapshot writes -// step aside instead of racing this rename(). +// write would block the crash handler's process.exit. The crash fence is +// enforced here, not left to caller discipline: both production callers +// (process-handlers.ts handleFatal and the signal handlers) flip it via +// markCrashed() before finalizing, and any other caller fails closed instead +// of landing a terminal record that queued snapshot writes may then clobber. // // Second terminal path, separate on purpose: only index.ts's process-level // uncaughtException/unhandledRejection and signal handlers reach it (a crash @@ -219,6 +221,11 @@ export async function saveCrashState( state: RunState, home?: string, ): Promise { + if (!isCrashed()) { + throw new Error( + `saveCrashState refused for session ${sessionId}: not in crashed state (call markCrashed() first)`, + ); + } const path = statePath(cwd, sessionId, home); await atomicWrite(path, JSON.stringify(state, null, 2)); }