From c0d7a35f5545fcf929c2eec6f92714bafc81822a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 16 Oct 2026 18:19:19 -0700 Subject: [PATCH 1/2] feat(session): gate saveCrashState on crashed state --- src/session/active-run.ts | 8 +++ src/session/optimized-context-store.test.ts | 61 +++++++++++++++++++++ src/session/state.test.ts | 37 ++++++++++++- src/session/state.ts | 13 ++++- 4 files changed, 114 insertions(+), 5 deletions(-) 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/optimized-context-store.test.ts b/src/session/optimized-context-store.test.ts index 8c78df32c..1f0657ad5 100644 --- a/src/session/optimized-context-store.test.ts +++ b/src/session/optimized-context-store.test.ts @@ -2268,3 +2268,64 @@ describe("createOptimizedContextStore metadata ownership (CL-10667)", () => { expect((await resumed.load()).tokenUsage).toEqual(USAGE_TWO); }); }); + +describe("live-turns generation cache staleness (CL-10757)", () => { + test("in-process append invalidates the cache: load serves fresh turns", async () => { + const dir = tempDir(); + const store = await createOptimizedContextStore(dir); + // The reactor extends its live turns array in place, so the store detects + // the append by reference: keep the first turn object stable. + const first = turn("one"); + await store.writeTurns([first]); + // Populates the generation cache. + expect(turnTexts((await store.load()).turns)).toEqual(["one"]); + + await store.writeTurns([first, turn("two")]); + expect(turnTexts((await store.load()).turns)).toEqual(["one", "two"]); + }); + + test("a committed rewrite is never served stale from the cache", async () => { + const dir = tempDir(); + const store = await createOptimizedContextStore(dir); + await store.writeTurns([turn("keep-a"), turn("drop-me")]); + await store.writeMetadata(EMPTY_CHECKPOINT_METADATA); + await store.commit({ message: "published original" }); + // Populates the generation cache. + expect(turnTexts((await store.load()).turns)).toEqual([ + "keep-a", + "drop-me", + ]); + + // A rewrite stages off the live generation until commit, then the commit + // lands it on disk: the post-commit load must serve it, not the cache. + await store.writeTurns([turn("[Compacted prior context]"), turn("keep-b")]); + await store.commit({ message: "publish compact" }); + expect(turnTexts((await store.load()).turns)).toEqual([ + "[Compacted prior context]", + "keep-b", + ]); + }); + + test("a foreign write between loads is never served stale from the cache", async () => { + const dir = tempDir(); + const first = await createOptimizedContextStore(dir); + await first.writeTurns([turn("one")]); + // Populates the first generation's cache. + expect(turnTexts((await first.load()).turns)).toEqual(["one"]); + + // A second live generation on the same dir (longer body so the segment + // stat changes even at coarse mtime granularity). + const second = await createOptimizedContextStore(dir); + await second.writeTurns([turn("one"), turn("two-with-a-longer-body")]); + expect(turnTexts((await second.load()).turns)).toEqual([ + "one", + "two-with-a-longer-body", + ]); + + // The first generation's cached parse must not shadow the foreign write. + expect(turnTexts((await first.load()).turns)).toEqual([ + "one", + "two-with-a-longer-body", + ]); + }); +}); 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..c2def0d82 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 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)); } From c4fd0977db9156a85dcbdb2c758b2f732bd8a803 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 16 Oct 2026 18:28:56 -0700 Subject: [PATCH 2/2] test(session): drop unrelated cache tests from the crash fence --- src/session/optimized-context-store.test.ts | 61 --------------------- src/session/state.ts | 2 +- 2 files changed, 1 insertion(+), 62 deletions(-) diff --git a/src/session/optimized-context-store.test.ts b/src/session/optimized-context-store.test.ts index 1f0657ad5..8c78df32c 100644 --- a/src/session/optimized-context-store.test.ts +++ b/src/session/optimized-context-store.test.ts @@ -2268,64 +2268,3 @@ describe("createOptimizedContextStore metadata ownership (CL-10667)", () => { expect((await resumed.load()).tokenUsage).toEqual(USAGE_TWO); }); }); - -describe("live-turns generation cache staleness (CL-10757)", () => { - test("in-process append invalidates the cache: load serves fresh turns", async () => { - const dir = tempDir(); - const store = await createOptimizedContextStore(dir); - // The reactor extends its live turns array in place, so the store detects - // the append by reference: keep the first turn object stable. - const first = turn("one"); - await store.writeTurns([first]); - // Populates the generation cache. - expect(turnTexts((await store.load()).turns)).toEqual(["one"]); - - await store.writeTurns([first, turn("two")]); - expect(turnTexts((await store.load()).turns)).toEqual(["one", "two"]); - }); - - test("a committed rewrite is never served stale from the cache", async () => { - const dir = tempDir(); - const store = await createOptimizedContextStore(dir); - await store.writeTurns([turn("keep-a"), turn("drop-me")]); - await store.writeMetadata(EMPTY_CHECKPOINT_METADATA); - await store.commit({ message: "published original" }); - // Populates the generation cache. - expect(turnTexts((await store.load()).turns)).toEqual([ - "keep-a", - "drop-me", - ]); - - // A rewrite stages off the live generation until commit, then the commit - // lands it on disk: the post-commit load must serve it, not the cache. - await store.writeTurns([turn("[Compacted prior context]"), turn("keep-b")]); - await store.commit({ message: "publish compact" }); - expect(turnTexts((await store.load()).turns)).toEqual([ - "[Compacted prior context]", - "keep-b", - ]); - }); - - test("a foreign write between loads is never served stale from the cache", async () => { - const dir = tempDir(); - const first = await createOptimizedContextStore(dir); - await first.writeTurns([turn("one")]); - // Populates the first generation's cache. - expect(turnTexts((await first.load()).turns)).toEqual(["one"]); - - // A second live generation on the same dir (longer body so the segment - // stat changes even at coarse mtime granularity). - const second = await createOptimizedContextStore(dir); - await second.writeTurns([turn("one"), turn("two-with-a-longer-body")]); - expect(turnTexts((await second.load()).turns)).toEqual([ - "one", - "two-with-a-longer-body", - ]); - - // The first generation's cached parse must not shadow the foreign write. - expect(turnTexts((await first.load()).turns)).toEqual([ - "one", - "two-with-a-longer-body", - ]); - }); -}); diff --git a/src/session/state.ts b/src/session/state.ts index c2def0d82..c1060d5fe 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -209,7 +209,7 @@ export async function finalizeRunState( // 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 queued snapshot writes may then clobber. +// 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