diff --git a/src/index.ts b/src/index.ts index 35106d3d6..95cb27df9 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, { @@ -209,10 +209,12 @@ 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 || !run.active) return; + if (run === null) return; try { await saveCrashState(run.cwd, run.sessionId, { status: "failed", @@ -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/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..850ae2a9d 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,29 @@ 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. +// +// 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 { + clearActiveRun(); + await saveState(cwd, sessionId, state, home); +} // Crash-time terminal write. Deliberately bypasses writeChains: a hung or // still-pending write for this session (possibly the very write mid-flight @@ -116,6 +139,15 @@ export async function saveState( // 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, 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 2158e6cef..70e8d8a8e 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"; @@ -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", @@ -503,7 +521,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 +553,15 @@ 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. 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. 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) => { @@ -547,7 +572,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)", @@ -1360,6 +1385,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}`; @@ -1368,7 +1394,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,20 +1402,30 @@ export async function runTUI(initialConfig: Config): Promise { model, mcpServers: connectedMcpServers, ...extra, - }); + }; + 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 @@ -1628,8 +1664,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; @@ -2296,13 +2334,22 @@ 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(); + // 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, { - finishedAt, - ...(sinkError !== undefined ? { error: sinkError } : {}), - }); + 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..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, active: true, 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/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/fixtures/crash-run/simulate-signal.ts b/tests/fixtures/crash-run/simulate-signal.ts index ce660292e..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"; @@ -26,11 +36,30 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, active: true, task, startedAt, model }); +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/crash-finalize.test.ts b/tests/integration/crash-finalize.test.ts index 5e85432b9..0454b1f5d 100644 --- a/tests/integration/crash-finalize.test.ts +++ b/tests/integration/crash-finalize.test.ts @@ -4,11 +4,12 @@ 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"; 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 () => { @@ -53,4 +54,90 @@ 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); + + 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); }); 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;