From d27abdafb0412ec0882a9d4bf1baecc20f9db22d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:03:51 -0700 Subject: [PATCH 01/10] Tie active-run liveness to one write instead of two independent ones RunStateHandle carried its own active boolean alongside RunState.status on disk, set at two separate call sites in src/tui/runner.ts (finalizeOnCrash and the normal completion path). Both sites happened to null the handle immediately after flipping the flag, so the boolean was always true whenever the handle existed, making it a redundant copy of the same fact rather than independent state -- deleted in favor of "the handle is null" as the sole liveness signal (src/session/active-run.ts). Terminal RunState writes now go through finalizeRunState (src/session/state.ts), which persists the record and clears the active-run handle in one call instead of leaving each terminal call site to remember both. --- src/index.ts | 4 ++-- src/session/active-run.ts | 6 +++++- src/session/state.test.ts | 16 +++++++++++++++- src/session/state.ts | 17 ++++++++++++++++- src/tui/runner.ts | 31 ++++++++++++++++++++++--------- 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/index.ts b/src/index.ts index 35106d3d6..bd8a27f2f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -170,7 +170,7 @@ export async function handleFatal(kind: CrashKind, error: unknown): Promise { const run = getActiveRun(); - if (run === null || !run.active) return; + if (run === null) return; const message = error instanceof Error ? error.message : String(error); try { await saveCrashState(run.cwd, run.sessionId, { @@ -212,7 +212,7 @@ export function installCrashHandlers(): void { // crash report is written for it. async function finalizeActiveRunOnSignal(signal: NodeJS.Signals): Promise { const run = getActiveRun(); - if (run === null || !run.active) return; + if (run === null) return; try { await saveCrashState(run.cwd, run.sessionId, { status: "failed", diff --git a/src/session/active-run.ts b/src/session/active-run.ts index 749dd4162..3a907a7c7 100644 --- a/src/session/active-run.ts +++ b/src/session/active-run.ts @@ -9,10 +9,14 @@ // crash path has the exact failure mode primeCrashReporting (src/crash/ // report.ts) exists to avoid for git: a stalled disk or network mount would // block process.exit forever. +// +// Liveness has exactly one representation: presence of this handle in the +// module-level slot (see getActiveRun below). There is no separate "active" +// flag on the handle itself — a second field would just be a copy of the +// same fact, free to drift from the slot it's meant to describe. export type RunStateHandle = { sessionId: string; cwd: string; - active: boolean; task: string; startedAt: number; model?: string; diff --git a/src/session/state.test.ts b/src/session/state.test.ts index 8c2479aa9..c0c5797e2 100644 --- a/src/session/state.test.ts +++ b/src/session/state.test.ts @@ -29,7 +29,8 @@ afterAll(() => { mock.module("node:fs/promises", () => realFs); }); -const { loadState, saveState } = await import("./state.js"); +const { finalizeRunState, loadState, saveState } = await import("./state.js"); +const { getActiveRun, setActiveRun } = await import("./active-run.js"); type RunState = Awaited>; let cwd = ""; @@ -72,6 +73,19 @@ test("a straggler snapshot started before a terminal write does not overwrite it expect(final?.finishedAt).toBe(999); }); +test("a persisted terminal status agrees with the active-run handle without a second call site", async () => { + const sessionId = "sess-terminal"; + setActiveRun({ sessionId, cwd, task: "task", startedAt: 1 }); + + await finalizeRunState(cwd, sessionId, state({ status: "done", finishedAt: 1000 }), home); + + const persisted = await loadState(cwd, sessionId, home); + expect(persisted?.status).toBe("done"); + // The only liveness representation left is presence in the active-run + // slot -- a terminal RunState.status must leave nothing there to read. + expect(getActiveRun()).toBeNull(); +}); + test("saveState calls for different sessions do not block each other", async () => { await Promise.all([ saveState(cwd, "session-a", state({ task: "a" }), home), diff --git a/src/session/state.ts b/src/session/state.ts index 545be6a82..4c6675da8 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -4,7 +4,7 @@ import { dirname, join } from "node:path"; import { type } from "arktype"; import { sessionDir } from "./index.js"; -import { getTestWriteGate, isCrashed } from "./active-run.js"; +import { clearActiveRun, getTestWriteGate, isCrashed } from "./active-run.js"; import { COMMAND_NAME } from "../branding.js"; const ConnectedMcpServerSchema = type({ @@ -108,6 +108,21 @@ export async function saveState( return write; } +// Single write path for a terminal RunState: pairs the on-disk status with +// clearing the in-memory active-run handle (active-run.ts) so the two facts +// are set together instead of at two independent call sites that could drift. +// Callers writing a non-terminal ("running") snapshot should call saveState +// directly — clearing the active-run handle on a running snapshot would be +// wrong, not merely redundant. +export async function finalizeRunState( + cwd: string, + sessionId: string, + state: RunState, + home?: string, +): Promise { + await saveState(cwd, sessionId, state, home); + clearActiveRun(); +} // Crash-time terminal write. Deliberately bypasses writeChains: a hung or // still-pending write for this session (possibly the very write mid-flight diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 2158e6cef..b451f2655 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -166,7 +166,7 @@ import { import { createRunSink } from "../session/run-sink.js"; import { generateSessionId, initSessionDir, renameSession, sessionContextDir, sessionDir } from "../session/index.js"; import { resolveSessionLabel, truncateSessionLabel } from "../session/session-label.js"; -import { loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js"; +import { finalizeRunState, loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js"; import { setActiveRun, clearActiveRun, type RunStateHandle } from "../session/active-run.js"; import { setActiveDisposeHost, clearActiveDisposeHost } from "../session/active-host.js"; import { openInBrowser } from "../auth/oauth/browser.js"; @@ -503,7 +503,6 @@ export async function runTUI(initialConfig: Config): Promise { const activeRunHandle: RunStateHandle = { sessionId, cwd: config.cwd, - active: true, task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)", startedAt, model: `${config.providerName}:${config.model}`, @@ -536,7 +535,12 @@ export async function runTUI(initialConfig: Config): Promise { const finalizeOnCrash = async (err: unknown): Promise => { if (finalized) return; finalized = true; - activeRunHandle.active = false; + // Clear the active-run handle up front, before the awaits below, so a + // second crash mid-flush can't see this run as still live and race the + // finalize write issued here. finalizeRunState (state.ts) would otherwise + // do this itself, but only after saveState resolves — too late for that + // guard, so it's done here and finalizeRunState's own clear becomes a + // no-op repeat of the same fact rather than a second independent write. clearActiveRun(); clearActiveDisposeHost(); await flushPartialOnCrash().catch((flushErr: unknown) => { @@ -547,7 +551,7 @@ export async function runTUI(initialConfig: Config): Promise { process.stderr.write(`${COMMAND_NAME}: crash finalize partial flush failed: ${flushMessage}\n`); }); const message = err instanceof Error ? err.message : String(err); - await saveState(config.cwd, sessionId, { + await finalizeRunState(config.cwd, sessionId, { status: "failed", turnsUsed: 0, task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)", @@ -1368,7 +1372,7 @@ export async function runTUI(initialConfig: Config): Promise { activeRunHandle.task = task; activeRunHandle.startedAt = startedAt; activeRunHandle.model = model; - await saveState(config.cwd, sessionId, { + const state: RunState = { status, turnsUsed: runSink.getTurnCount(), task, @@ -1376,7 +1380,16 @@ export async function runTUI(initialConfig: Config): Promise { model, mcpServers: connectedMcpServers, ...extra, - }); + }; + // "running" is the only non-terminal status writeRunSnapshot ever + // receives (progress snapshots); anything else closes the run out, so + // the active-run handle is cleared in the same call as the disk write + // rather than by a separate statement at each terminal call site. + if (status === "running") { + await saveState(config.cwd, sessionId, state); + } else { + await finalizeRunState(config.cwd, sessionId, state); + } }; // Progress snapshots are fired unsequenced (model switch, MCP connect, turn @@ -2296,9 +2309,9 @@ export async function runTUI(initialConfig: Config): Promise { // finished run (finishedAt set) can be left reading as still in progress. const persistedStatus: RunState["status"] = summaryStatus; finalized = true; - activeRunHandle.active = false; - clearActiveRun(); - clearActiveDisposeHost(); + // writeRunSnapshot clears the active-run handle itself for a terminal + // status (via finalizeRunState in state.ts), pairing the on-disk write + // with the in-memory one instead of setting them at two call sites. await writeRunSnapshot(persistedStatus, { finishedAt, ...(sinkError !== undefined ? { error: sinkError } : {}), From a0a6f510037cf18bbf80489838d7877d8075c444 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 13:02:04 -0700 Subject: [PATCH 02/10] Keep a rotated session crash-coverable instead of inferring it Routing every non-"running" run.json write through finalizeRunState made the terminal status itself the signal that the run had ended, and so cleared the active-run handle. Session rotation (/clear, /new) breaks that equivalence: it persists a terminal "done" for the outgoing session while the process keeps running under a fresh session id. Clearing liveness there left getActiveRun() null for the rest of the process, so a crash after the first rotation never wrote a terminal record and the session read as "running" forever. Why a write happens is now explicit (SnapshotKind: progress, session-rotation, run-end) instead of inferred from what it writes. Only run-end clears the handle; persistRunSnapshot cannot request it, since everything routed there happens while the process is still alive. Also drops the deleted `active` field from the crash fixture, which sits outside tsconfig's include and so escaped typecheck. --- src/tui/run-snapshot-kind.test.ts | 76 ++++++++++++++++++++++ src/tui/runner.ts | 60 ++++++++++++----- tests/fixtures/crash-run/simulate-crash.ts | 2 +- 3 files changed, 120 insertions(+), 18 deletions(-) create mode 100644 src/tui/run-snapshot-kind.test.ts diff --git a/src/tui/run-snapshot-kind.test.ts b/src/tui/run-snapshot-kind.test.ts new file mode 100644 index 000000000..d6abf5702 --- /dev/null +++ b/src/tui/run-snapshot-kind.test.ts @@ -0,0 +1,76 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; + +import { clearActiveRun, getActiveRun, setActiveRun } from "../session/active-run.js"; +import { finalizeRunState, loadState, saveState, type RunState } from "../session/state.js"; +import { clearsActiveRun, type SnapshotKind } from "./runner.js"; + +describe("clearsActiveRun", () => { + test("only the run-ending write clears the active-run handle", () => { + expect(clearsActiveRun("run-end")).toBe(true); + expect(clearsActiveRun("progress")).toBe(false); + // The regression this pins: a /clear or /new rotation persists a + // terminal "done" for the outgoing session, but the process lives on. + // Clearing liveness here leaves every later session uncovered by the + // crash handler, so a crash after the first rotation never writes a + // terminal record and the session reads as "running" forever. + expect(clearsActiveRun("session-rotation")).toBe(false); + }); +}); + +describe("a snapshot write dispatched by kind", () => { + let cwd = ""; + let home = ""; + + // Mirrors writeRunSnapshot's dispatch in runner.ts so the rule above is + // exercised against the real state writers, not just asserted in isolation. + const write = async (sessionId: string, state: RunState, kind: SnapshotKind): Promise => { + if (clearsActiveRun(kind)) { + await finalizeRunState(cwd, sessionId, state, home); + return; + } + await saveState(cwd, sessionId, state, home); + }; + + const runState = (over: Partial): RunState => ({ + status: "running", + turnsUsed: 0, + task: "task", + startedAt: 1, + ...over, + }); + + beforeEach(() => { + cwd = mkdtempSync(join(tmpdir(), "snapshot-kind-cwd-")); + home = mkdtempSync(join(tmpdir(), "snapshot-kind-home-")); + }); + + afterEach(() => { + clearActiveRun(); + rmSync(cwd, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + }); + + test("a rotation still records the outgoing session but leaves the run crash-coverable", async () => { + setActiveRun({ sessionId: "old", cwd, task: "task", startedAt: 1 }); + + await write("old", runState({ status: "done", finishedAt: 10 }), "session-rotation"); + + expect((await loadState(cwd, "old", home))?.status).toBe("done"); + // The rotated-in session is repointed on the same handle, so the handle + // must survive the write for the crash handler to have anything to close. + expect(getActiveRun()).not.toBeNull(); + }); + + test("the run-ending write records the session and disarms the handle", async () => { + setActiveRun({ sessionId: "last", cwd, task: "task", startedAt: 1 }); + + await write("last", runState({ status: "done", finishedAt: 20 }), "run-end"); + + expect((await loadState(cwd, "last", home))?.status).toBe("done"); + expect(getActiveRun()).toBeNull(); + }); +}); diff --git a/src/tui/runner.ts b/src/tui/runner.ts index b451f2655..43a547693 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -241,6 +241,24 @@ export function resolveResumeSeed(pickedState: RunState | null): ResumeSeed { }; } +/** + * Why a run.json snapshot is being written. Only "run-end" ends the run + * itself and so clears the active-run handle that the crash handler in + * index.ts reads. + * + * RunState.status cannot stand in for this. A /clear or /new rotation + * persists a terminal "done" for the outgoing session while the process + * keeps running under a fresh session id, so inferring "the run is over" + * from a non-"running" status disarms crash finalization for everything + * after the first rotation -- the session that dies then never gets its + * terminal record and reads as "running" forever. + */ +export type SnapshotKind = "progress" | "session-rotation" | "run-end"; + +export function clearsActiveRun(kind: SnapshotKind): boolean { + return kind === "run-end"; +} + const GRANT_SCOPE_LABEL: Record = { session: "This session", project: "This project", @@ -1364,6 +1382,7 @@ export async function runTUI(initialConfig: Config): Promise { const writeRunSnapshot = async ( status: RunState["status"], extra?: Pick, + kind: SnapshotKind = "progress", ): Promise => { const task = runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)"; const model = `${liveSource.id}:${liveSource.model}`; @@ -1381,28 +1400,29 @@ export async function runTUI(initialConfig: Config): Promise { mcpServers: connectedMcpServers, ...extra, }; - // "running" is the only non-terminal status writeRunSnapshot ever - // receives (progress snapshots); anything else closes the run out, so - // the active-run handle is cleared in the same call as the disk write - // rather than by a separate statement at each terminal call site. - if (status === "running") { - await saveState(config.cwd, sessionId, state); - } else { + if (clearsActiveRun(kind)) { await finalizeRunState(config.cwd, sessionId, state); + } else { + await saveState(config.cwd, sessionId, state); } }; // Progress snapshots are fired unsequenced (model switch, MCP connect, turn // completion), so a straggler could otherwise land after the terminal write // and resurrect status "running" — atomicWrite is last-rename-wins. Once the - // run is finalized, drop them; the terminal paths write through + // run is finalized, drop them; the run-ending path writes through // writeRunSnapshot directly. + // + // Never a "run-end" write: everything routed here happens while the process + // is still alive and must stay crash-coverable, including the rotation + // "done" that closes out a session on /clear or /new. const persistRunSnapshot = async ( status: RunState["status"], extra?: Pick, + kind: Exclude = "progress", ): Promise => { if (finalized) return; - await writeRunSnapshot(status, extra); + await writeRunSnapshot(status, extra, kind); }; // Cycles persist to the context store only on inference.done; the recorder @@ -1641,8 +1661,10 @@ export async function runTUI(initialConfig: Config): Promise { error: err instanceof Error ? err.message : String(err), }); }); - await persistRunSnapshot("done", { finishedAt: Date.now() }); + await persistRunSnapshot("done", { finishedAt: Date.now() }, "session-rotation"); sessionId = generateSessionId(); + // Repointed, not cleared: the process lives on, so the crash handler + // must keep finding this handle and close out the *new* session. activeRunHandle.sessionId = sessionId; startedAt = Date.now(); runTaskTitle = config.task; @@ -2309,13 +2331,17 @@ export async function runTUI(initialConfig: Config): Promise { // finished run (finishedAt set) can be left reading as still in progress. const persistedStatus: RunState["status"] = summaryStatus; finalized = true; - // writeRunSnapshot clears the active-run handle itself for a terminal - // status (via finalizeRunState in state.ts), pairing the on-disk write - // with the in-memory one instead of setting them at two call sites. - await writeRunSnapshot(persistedStatus, { - finishedAt, - ...(sinkError !== undefined ? { error: sinkError } : {}), - }); + // The run itself is over here, so this write clears the active-run handle + // (via finalizeRunState in state.ts) in the same call, rather than pairing + // the on-disk write with a separate in-memory statement at this call site. + await writeRunSnapshot( + persistedStatus, + { + finishedAt, + ...(sinkError !== undefined ? { error: sinkError } : {}), + }, + "run-end", + ); const runSummary = createRunSummary({ task: runTaskTitle.length > 0 ? runTaskTitle : config.task, status: summaryStatus, diff --git a/tests/fixtures/crash-run/simulate-crash.ts b/tests/fixtures/crash-run/simulate-crash.ts index 00ecb52b9..de4b2cdf3 100644 --- a/tests/fixtures/crash-run/simulate-crash.ts +++ b/tests/fixtures/crash-run/simulate-crash.ts @@ -26,7 +26,7 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, active: true, task, startedAt, model }); +setActiveRun({ sessionId, cwd, task, startedAt, model }); installCrashHandlers(); process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); From 7675e1a54a38c6cd9c467ab4f2410077da95cfce Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 14:39:29 -0700 Subject: [PATCH 03/10] Add a regression test for a crash after session rotation The existing rotation tests covered clearsActiveRun as a pure function and confirmed a handle survives a rotation write, but nothing exercised a crash actually occurring afterward. Extend the crash fixture to optionally rotate the session first, through the same clearsActiveRun dispatch runner.ts uses, then crash and assert the crashed record lands under the post-rotation session. --- tests/fixtures/crash-run/simulate-crash.ts | 44 +++++++++++++++++--- tests/integration/crash-finalize.test.ts | 48 +++++++++++++++++++++- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/tests/fixtures/crash-run/simulate-crash.ts b/tests/fixtures/crash-run/simulate-crash.ts index de4b2cdf3..2888b17f5 100644 --- a/tests/fixtures/crash-run/simulate-crash.ts +++ b/tests/fixtures/crash-run/simulate-crash.ts @@ -6,7 +6,8 @@ import { installCrashHandlers } from "../../../src/index.js"; import { setActiveRun, setTestWriteGate } from "../../../src/session/active-run.js"; import { sessionDir } from "../../../src/session/index.js"; -import { saveState } from "../../../src/session/state.js"; +import { finalizeRunState, saveState } from "../../../src/session/state.js"; +import { clearsActiveRun } from "../../../src/tui/runner.js"; const cwd = process.cwd(); const sessionId = process.env["CRASH_TEST_SESSION_ID"]; @@ -26,10 +27,43 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, task, startedAt, model }); +// A single handle object, mutated in place on rotation below rather than +// replaced — matching runner.ts's activeRunHandle, so a rotation that (on +// buggy code) clears the module-level slot behind this object is not +// papered over by re-registering a fresh handle afterward. +const activeRunHandle = { sessionId, cwd, task, startedAt, model }; +setActiveRun(activeRunHandle); installCrashHandlers(); -process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); +// Optional: mimic a session rotation (/clear, /new) before the crash. Routes +// the outgoing session's terminal "done" write through the same +// clearsActiveRun("session-rotation") dispatch writeRunSnapshot uses in +// runner.ts, so this fixture exercises the real production decision of +// whether a rotation write clears the active-run handle, rather than +// asserting the desired behavior directly. Then repoints the handle at the +// new session id, matching runner.ts reassigning activeRunHandle.sessionId +// in place rather than replacing the handle. +const rotatedSessionId = process.env["CRASH_TEST_ROTATED_SESSION_ID"]; +let activeSessionId = sessionId; +if (rotatedSessionId !== undefined) { + const rotationState = { + status: "done" as const, + turnsUsed: 3, + task, + startedAt, + finishedAt: Date.now(), + model, + }; + if (clearsActiveRun("session-rotation")) { + await finalizeRunState(cwd, sessionId, rotationState); + } else { + await saveState(cwd, sessionId, rotationState); + } + activeRunHandle.sessionId = rotatedSessionId; + activeSessionId = rotatedSessionId; +} + +process.stdout.write(`${sessionDir(cwd, activeSessionId)}\n`); // Hold every write issued from here on at the gate, before it reaches // isCrashed(). This makes the race deterministic instead of hoping real @@ -46,8 +80,8 @@ 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 }); +void saveState(cwd, activeSessionId, { status: "running", turnsUsed: 1, task, startedAt, model }); +void saveState(cwd, activeSessionId, { 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 diff --git a/tests/integration/crash-finalize.test.ts b/tests/integration/crash-finalize.test.ts index 5e85432b9..5132f3818 100644 --- a/tests/integration/crash-finalize.test.ts +++ b/tests/integration/crash-finalize.test.ts @@ -4,7 +4,7 @@ import { join } from "node:path"; import { describe, expect, test } from "bun:test"; -import { generateSessionId } from "../../src/session/index.js"; +import { generateSessionId, sessionDir } from "../../src/session/index.js"; import type { RunState } from "../../src/session/state.js"; import { isResumableByDefault } from "../../src/tui/pick-session.js"; @@ -53,4 +53,50 @@ describe("integration — crash finalizes run.json", () => { rmSync(home, { recursive: true, force: true }); } }, 15_000); + + test("a crash after session rotation still writes crashed for the new session", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-crash-cwd-")); + const home = mkdtempSync(join(tmpdir(), "corbits-crash-home-")); + const sessionId = generateSessionId(); + const rotatedSessionId = generateSessionId(); + + try { + const proc = Bun.spawn(["bun", "run", FIXTURE], { + cwd, + env: { + ...process.env, + HOME: home, + CRASH_TEST_SESSION_ID: sessionId, + CRASH_TEST_ROTATED_SESSION_ID: rotatedSessionId, + }, + stdout: "pipe", + stderr: "pipe", + }); + + const exitCode = await proc.exited; + const stdout = await new Response(proc.stdout).text(); + const stderr = await new Response(proc.stderr).text(); + + expect(exitCode).toBe(1); + expect(stderr).toContain("uncaughtException: Error: simulated crash"); + + // The bug this pins: the outgoing session's terminal "done" write must + // not clear the active-run handle, or the crash below finds it null + // and never writes a crashed record for the session actually running + // at the time of the crash. + const outgoingRunJsonPath = join(sessionDir(cwd, sessionId, home), "run.json"); + const outgoingState = JSON.parse(readFileSync(outgoingRunJsonPath, "utf8")) as RunState; + expect(outgoingState.status).toBe("done"); + + const rotatedRunJsonPath = join(stdout.trim(), "run.json"); + const rotatedState = JSON.parse(readFileSync(rotatedRunJsonPath, "utf8")) as RunState; + expect(rotatedRunJsonPath).toBe(join(sessionDir(cwd, rotatedSessionId, home), "run.json")); + expect(rotatedState.status).toBe("crashed"); + expect(rotatedState.finishedAt).toBeGreaterThan(0); + expect(rotatedState.error).toContain("simulated crash"); + } finally { + rmSync(cwd, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, 15_000); }); From 6e1f9a0c787cd2a286fbabb3d544c3c1cb573af0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 14:39:34 -0700 Subject: [PATCH 04/10] Explain why finalizeOnCrash clears the active-run handle early The prior comment only noted that finalizeRunState's own clear becomes a no-op repeat, which reads as an argument for deleting the early call rather than keeping it. It is not redundant: index.ts installs its own crash listeners that read the handle directly and would otherwise race a competing write during the awaits here. --- src/tui/runner.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 43a547693..9766473eb 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -553,12 +553,18 @@ export async function runTUI(initialConfig: Config): Promise { const finalizeOnCrash = async (err: unknown): Promise => { if (finalized) return; finalized = true; - // Clear the active-run handle up front, before the awaits below, so a - // second crash mid-flush can't see this run as still live and race the - // finalize write issued here. finalizeRunState (state.ts) would otherwise - // do this itself, but only after saveState resolves — too late for that - // guard, so it's done here and finalizeRunState's own clear becomes a - // no-op repeat of the same fact rather than a second independent write. + // Clear the active-run handle up front, before the awaits below. This + // handler isn't the only reader of the handle: index.ts installs its own + // uncaughtException/unhandledRejection listeners that call getActiveRun() + // directly and, if it's still set, write a competing "crashed" record via + // saveCrashState. An escaped throw during flushPartialOnCrash or the + // finalizeRunState await below would otherwise reach that listener while + // the handle still reads as live, racing its write against the "failed" + // write in progress here. finalizeRunState (state.ts) also clears the + // handle, but only after its own saveState resolves — too late to close + // that window, so the clear is duplicated here. finalizeRunState is left + // unchanged: its other callers (normal completion, session rotation) rely + // on it being the one that clears the handle. clearActiveRun(); clearActiveDisposeHost(); await flushPartialOnCrash().catch((flushErr: unknown) => { From 3084accb1fbfb3f075395273fddc8618ee2291c0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 14:39:39 -0700 Subject: [PATCH 05/10] Explain why saveCrashState stays a separate write path Document that this is a second terminal write path alongside finalizeRunState, kept apart because its only callers are the process-level crash and signal handlers in index.ts, which cannot afford to wait on saveState's per-session write chain during exit. --- src/session/state.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/session/state.ts b/src/session/state.ts index 4c6675da8..9f578ebdf 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -131,6 +131,15 @@ export async function finalizeRunState( // Callers must call markCrashed() (src/session/active-run.ts) before this, so // any snapshot write still queued behind another one in the chain steps // aside instead of racing this write's rename(). +// +// This is a second terminal write path alongside finalizeRunState, and stays +// separate on purpose: its only callers are index.ts's process-level +// uncaughtException/unhandledRejection and signal handlers, reached when a +// crash escapes runTUI's own try/catch entirely. finalizeRunState routes +// through saveState's per-session write chain so writes apply in call order; +// that chain is exactly what a crash exit cannot afford to wait on, since +// process.exit must happen deterministically and a stuck earlier write +// (possibly the one that caused the crash) would otherwise hang it. export async function saveCrashState( cwd: string, sessionId: string, From 61b509d74f32125ff08011ff01948e5cd5fec550 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 17:42:11 -0700 Subject: [PATCH 06/10] Clear the active-run handle before the terminal write, not after finalizeRunState cleared the handle only after its saveState await resolved, so a signal or uncaught exception landing during that write still saw a live run and raced a competing "crashed" write against the terminal write in progress. That reopened the crash-after-rotation race on every ordinary run-end write, not just the crash path's own. Clearing before the await closes it: by the time anything else can observe the handle, it already reads as gone. --- src/session/state.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/session/state.ts b/src/session/state.ts index 9f578ebdf..850ae2a9d 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -114,14 +114,22 @@ export async function saveState( // Callers writing a non-terminal ("running") snapshot should call saveState // directly — clearing the active-run handle on a running snapshot would be // wrong, not merely redundant. +// +// The clear happens before the saveState await, not after: this run is +// closing out regardless of whether the write below succeeds, and a signal +// or uncaught exception landing during that await must see the handle +// already gone, or it races a second "crashed" write (src/index.ts's process +// handlers, via saveCrashState) against the terminal write in flight here. +// Clearing after the await leaves that exact window open on every terminal +// write, not only the crash path's own. export async function finalizeRunState( cwd: string, sessionId: string, state: RunState, home?: string, ): Promise { - await saveState(cwd, sessionId, state, home); clearActiveRun(); + await saveState(cwd, sessionId, state, home); } // Crash-time terminal write. Deliberately bypasses writeChains: a hung or From a0b0571908740926b63ce95cb1378e591cf82a06 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 17:42:18 -0700 Subject: [PATCH 07/10] Restore the dispose-host clear on the normal run-end path Deleted along with the standalone active-run clear when that call site was folded into finalizeRunState's write, but the dispose host has no on-disk write to piggyback on, so it never got a replacement. Every normal run was leaving the dispose host pointing at a torn-down closure that a later signal could still invoke. Restored to mirror finalizeOnCrash, matching active-host.ts's stated contract of clearing on either path. --- src/tui/runner.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/tui/runner.ts b/src/tui/runner.ts index 9766473eb..70e8d8a8e 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -557,14 +557,11 @@ export async function runTUI(initialConfig: Config): Promise { // handler isn't the only reader of the handle: index.ts installs its own // uncaughtException/unhandledRejection listeners that call getActiveRun() // directly and, if it's still set, write a competing "crashed" record via - // saveCrashState. An escaped throw during flushPartialOnCrash or the - // finalizeRunState await below would otherwise reach that listener while - // the handle still reads as live, racing its write against the "failed" - // write in progress here. finalizeRunState (state.ts) also clears the - // handle, but only after its own saveState resolves — too late to close - // that window, so the clear is duplicated here. finalizeRunState is left - // unchanged: its other callers (normal completion, session rotation) rely - // on it being the one that clears the handle. + // saveCrashState. finalizeRunState (state.ts) also clears the handle + // before its own saveState await, but only once it's called below — an + // escaped throw during the flushPartialOnCrash await just above would + // still reach that listener with the handle live, so it's cleared here + // too to close that earlier window. clearActiveRun(); clearActiveDisposeHost(); await flushPartialOnCrash().catch((flushErr: unknown) => { @@ -2340,6 +2337,11 @@ export async function runTUI(initialConfig: Config): Promise { // The run itself is over here, so this write clears the active-run handle // (via finalizeRunState in state.ts) in the same call, rather than pairing // the on-disk write with a separate in-memory statement at this call site. + // The dispose host has no on-disk counterpart to piggyback on, so it still + // needs its own clear here, mirroring finalizeOnCrash — otherwise a signal + // arriving after this normal exit would find a handle pointing at a + // torn-down closure. + clearActiveDisposeHost(); await writeRunSnapshot( persistedStatus, { From fe23e577687a35da826812b7c7b72f8d41cb5d1b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 17:42:23 -0700 Subject: [PATCH 08/10] Stop passing the deleted active field in the signal fixture RunStateHandle dropped its own active flag in favor of handle presence as the sole liveness signal, but this fixture still set it. tsconfig excludes tests/ from typecheck, so nothing caught the stale field and the setter silently ignored the excess property. --- tests/fixtures/crash-run/simulate-signal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/crash-run/simulate-signal.ts b/tests/fixtures/crash-run/simulate-signal.ts index ce660292e..fbfb67b88 100644 --- a/tests/fixtures/crash-run/simulate-signal.ts +++ b/tests/fixtures/crash-run/simulate-signal.ts @@ -26,7 +26,7 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, active: true, task, startedAt, model }); +setActiveRun({ sessionId, cwd, task, startedAt, model }); installSignalHandlers(); process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); From dfc7cd424c7522c8eb253b23e0b63f246908120d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 17:42:28 -0700 Subject: [PATCH 09/10] Add a regression test for a crash during the run-end write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing crash-after-rotation test only covers a crash arriving before any terminal write starts, so it stayed green even with the run-end handle-clearing regression present. This test parks the run-end write mid-flight and fires an unrelated uncaughtException during that window, asserting the outcome is never "crashed" — pinning the fix that moved finalizeRunState's clear ahead of its own write. --- .../crash-run/simulate-run-end-crash.ts | 61 +++++++++++++++++++ tests/integration/crash-finalize.test.ts | 41 +++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 tests/fixtures/crash-run/simulate-run-end-crash.ts diff --git a/tests/fixtures/crash-run/simulate-run-end-crash.ts b/tests/fixtures/crash-run/simulate-run-end-crash.ts new file mode 100644 index 000000000..6adec7932 --- /dev/null +++ b/tests/fixtures/crash-run/simulate-run-end-crash.ts @@ -0,0 +1,61 @@ +// Spawned as a subprocess by tests/integration/crash-finalize.test.ts. Mimics +// the run-end write (writeRunSnapshot's "done" call through finalizeRunState +// in state.ts) landing mid-flight when an unrelated uncaughtException fires, +// rather than simulate-crash.ts's scenario of a crash escaping before any +// terminal write is issued at all. +import { installCrashHandlers } from "../../../src/index.js"; +import { setActiveRun, setTestWriteGate } from "../../../src/session/active-run.js"; +import { sessionDir } from "../../../src/session/index.js"; +import { finalizeRunState, saveState } from "../../../src/session/state.js"; + +const cwd = process.cwd(); +const sessionId = process.env["RUN_END_TEST_SESSION_ID"]; +if (sessionId === undefined) { + throw new Error("RUN_END_TEST_SESSION_ID must be set"); +} + +const startedAt = Date.now(); +const task = "simulated run-end task"; +const model = "test-provider:test-model"; + +await saveState(cwd, sessionId, { + status: "running", + turnsUsed: 3, + task, + startedAt, + model, +}); + +setActiveRun({ sessionId, cwd, task, startedAt, model }); +installCrashHandlers(); + +process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); + +// Held open for the rest of the process's life — saveCrashState (the crash +// path) bypasses this gate entirely via a raw atomicWrite, so parking the +// run-end write here forever is enough to simulate "the run-end snapshot +// write is still in flight" without needing to release it: whether the +// process observes "done" or "crashed" is decided before this write would +// ever land. +setTestWriteGate(new Promise(() => {})); + +// Fire the run-end write the same way writeRunSnapshot does for a terminal +// status, but don't await it — runner.ts doesn't either from the crash +// handler's point of view, since the crash below arrives asynchronously. +void finalizeRunState(cwd, sessionId, { + status: "done", + turnsUsed: 3, + task, + startedAt, + finishedAt: Date.now(), + model, +}); + +// Runs after the synchronous portion of finalizeRunState above (its +// clearActiveRun call, if placed before the saveState await) has already +// executed, since setImmediate always waits for the current synchronous +// script to finish. This is the window the bug reopened: an unrelated +// exception landing while the run-end write is still in flight. +setImmediate(() => { + throw new Error("simulated crash during run-end write"); +}); diff --git a/tests/integration/crash-finalize.test.ts b/tests/integration/crash-finalize.test.ts index 5132f3818..0454b1f5d 100644 --- a/tests/integration/crash-finalize.test.ts +++ b/tests/integration/crash-finalize.test.ts @@ -9,6 +9,7 @@ import type { RunState } from "../../src/session/state.js"; import { isResumableByDefault } from "../../src/tui/pick-session.js"; const FIXTURE = join(import.meta.dirname, "../fixtures/crash-run/simulate-crash.ts"); +const RUN_END_FIXTURE = join(import.meta.dirname, "../fixtures/crash-run/simulate-run-end-crash.ts"); describe("integration — crash finalizes run.json", () => { test("uncaughtException writes status: crashed with finishedAt, racing in-flight snapshot writes", async () => { @@ -99,4 +100,44 @@ describe("integration — crash finalizes run.json", () => { rmSync(home, { recursive: true, force: true }); } }, 15_000); + + test("an unrelated crash while the run-end write is in flight does not report crashed", async () => { + const cwd = mkdtempSync(join(tmpdir(), "corbits-crash-cwd-")); + const home = mkdtempSync(join(tmpdir(), "corbits-crash-home-")); + const sessionId = generateSessionId(); + + try { + const proc = Bun.spawn(["bun", "run", RUN_END_FIXTURE], { + cwd, + env: { ...process.env, HOME: home, RUN_END_TEST_SESSION_ID: sessionId }, + stdout: "pipe", + stderr: "pipe", + }); + + const exitCode = await proc.exited; + const stdout = await new Response(proc.stdout).text(); + const stderr = await new Response(proc.stderr).text(); + + expect(exitCode).toBe(1); + expect(stderr).toContain("uncaughtException: Error: simulated crash during run-end write"); + + // The bug this pins: finalizeRunState used to clear the active-run + // handle only after its own saveState write resolved. With the + // run-end write parked mid-flight (this fixture's gate never + // releases), the handle stayed live for the entire window, so the + // crash handler saw a live run and wrote a "crashed" record via + // saveCrashState — which bypasses the gate — clobbering what should + // have been a clean finish. Clearing the handle before the await + // closes that window: the crash handler finds no active run and + // writes nothing, so the last write to land is the one from the + // initial saveState above ("running"), never "crashed". + const runJsonPath = join(stdout.trim(), "run.json"); + const raw = readFileSync(runJsonPath, "utf8"); + const state = JSON.parse(raw) as RunState; + expect(state.status).not.toBe("crashed"); + } finally { + rmSync(cwd, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); + } + }, 15_000); }); From 5aa65e218223a1a09f267073963380c81720f572 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 23:09:36 -0700 Subject: [PATCH 10/10] Fence signal finalization the same way crash finalization is fenced saveCrashState requires markCrashed() first so chained saveState renames cannot clobber the terminal write. handleFatal already did this for uncaughtException; SIGINT/SIGTERM/SIGHUP called saveCrashState without the fence. Flip the flag before the signal write, and park stragglers in the signal fixture so the integration test pins it. --- src/index.ts | 7 ++++- tests/fixtures/crash-run/simulate-signal.ts | 31 ++++++++++++++++++++- tests/integration/signal-finalize.test.ts | 4 +++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index bd8a27f2f..95cb27df9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -209,7 +209,9 @@ export function installCrashHandlers(): void { // Mirrors finalizeActiveRunOnCrash but is not itself a crash — a signal is a // clean, externally-requested termination (operator, shell, orchestrator), // so the run is left "failed" (interrupted) rather than "crashed", and no -// crash report is written for it. +// crash report is written for it. Callers must markCrashed() before this so +// chained saveState renames cannot clobber the terminal write (same contract +// as the uncaughtException path). async function finalizeActiveRunOnSignal(signal: NodeJS.Signals): Promise { const run = getActiveRun(); if (run === null) return; @@ -270,6 +272,9 @@ export function installSignalHandlers(): void { `host dispose failed handling ${signal}: ${disposeErr instanceof Error ? disposeErr.message : String(disposeErr)}\n`, ); } + // Same fence as handleFatal: any snapshot still queued in writeChains must + // see isCrashed and step aside before saveCrashState renames run.json. + markCrashed(); void finalizeActiveRunOnSignal(signal).finally(() => { process.exit(128 + SIGNAL_EXIT_NUMBER[signal]); }); diff --git a/tests/fixtures/crash-run/simulate-signal.ts b/tests/fixtures/crash-run/simulate-signal.ts index fbfb67b88..b81c6a0f2 100644 --- a/tests/fixtures/crash-run/simulate-signal.ts +++ b/tests/fixtures/crash-run/simulate-signal.ts @@ -3,8 +3,18 @@ // initial "running" run.json) and what index.ts does at process entry // (install the signal handlers), then waits to receive a real signal sent by // the test from outside the process. +// +// Also parks two unawaited straggler snapshot writes behind setTestWriteGate, +// released only after the signal handler has flipped isCrashed via +// markCrashed(). Without that fence, a chained "running" rename can clobber +// the signal's terminal "failed" write — the same race the crash path already +// fences. import { installSignalHandlers } from "../../../src/index.js"; -import { setActiveRun } from "../../../src/session/active-run.js"; +import { + isCrashed, + setActiveRun, + setTestWriteGate, +} from "../../../src/session/active-run.js"; import { sessionDir } from "../../../src/session/index.js"; import { saveState } from "../../../src/session/state.js"; @@ -29,8 +39,27 @@ await saveState(cwd, sessionId, { setActiveRun({ sessionId, cwd, task, startedAt, model }); installSignalHandlers(); +let releaseGate: () => void; +const gate = new Promise((resolve) => { + releaseGate = resolve; +}); +setTestWriteGate(gate); +void saveState(cwd, sessionId, { status: "running", turnsUsed: 1, task, startedAt, model }); +void saveState(cwd, sessionId, { status: "running", turnsUsed: 2, task, startedAt, model }); + process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); process.stdout.write("ready\n"); +// After the parent sends a real signal, installSignalHandlers flips +// isCrashed() before saveCrashState. Releasing the gate then lets the two +// parked writes observe the flag rather than racing the terminal rename. +const poll = setInterval(() => { + if (isCrashed()) { + clearInterval(poll); + releaseGate(); + } +}, 10); +if (typeof poll.unref === "function") poll.unref(); + // Keep the event loop alive until the test sends a signal. setInterval(() => {}, 60_000); diff --git a/tests/integration/signal-finalize.test.ts b/tests/integration/signal-finalize.test.ts index 270707c19..f3efd06c3 100644 --- a/tests/integration/signal-finalize.test.ts +++ b/tests/integration/signal-finalize.test.ts @@ -51,6 +51,10 @@ describe("integration — signal finalizes run.json", () => { expect(exitCode).toBe(expectedExitCode); + // The fixture parks two unawaited straggler "running" snapshot writes + // behind setTestWriteGate and releases them only after markCrashed() + // flips. Without that fence on the signal path, one of those renames + // can last-write-win over status: "failed". const runJsonPath = join(runDir, "run.json"); const raw = readFileSync(runJsonPath, "utf8"); const state = JSON.parse(raw) as RunState;