Merge crash-time run.json finalize

Author: TheGreatAxios

src/index.ts

@@ -1,6 +1,8 @@
 import { getLogger } from "@intx/log";
 import { LOG_NAMESPACE_ROOT } from "./branding.js";
 import { primeCrashReporting, writeCrashReport, type CrashKind } from "./crash/report.js";
+import { getActiveRun, markCrashed } from "./session/active-run.js";
+import { saveCrashState } from "./session/state.js";
 import { loadConfig } from "./config/index.js";
 import { ensureTelemetrySettings, globalSettingsPath } from "./config/settings.js";
 import { installFileLogSink } from "./logging/sink.js";
@@ -113,32 +115,77 @@ export async function main(argv: readonly string[]): Promise<number> {
   });
 }
 
-async function handleFatal(kind: CrashKind, error: unknown): Promise<void> {
+// Exported so an integration test can register these process-level handlers
+// and inject a crash without spawning the full TUI stack.
+export async function handleFatal(kind: CrashKind, error: unknown): Promise<void> {
+  // Flip this before any awaits below so any snapshot write still queued
+  // behind another one in state.ts's per-session chain sees it and steps
+  // aside the moment it's next in line, rather than racing saveCrashState's
+  // rename() below. See markCrashed's doc comment for the residual window
+  // this cannot close.
+  markCrashed();
   process.stderr.write(`${kind}: ${error instanceof Error ? (error.stack ?? error.message) : String(error)}\n`);
   const file = await writeCrashReport(kind, error);
   if (file !== null) {
     process.stderr.write(`crash report written to ${file}\n`);
   } else {
     process.stderr.write("failed to write crash report\n");
   }
+  await finalizeActiveRunOnCrash(error);
   process.exit(1);
 }
 
-if (import.meta.main) {
-  // OpenTUI installs a process-global uncaughtException/unhandledRejection
-  // handler that only logs (opentui/core's Renderer.handleError), which
-  // suppresses Bun's default print-and-exit. Combined with raw-mode stdin
-  // holding the event loop open, an escaped throw would otherwise hang the
-  // process forever with the terminal still in the alternate screen. Node
-  // invokes every registered listener for the event regardless of order, so
-  // these still run and terminate the process even though OpenTUI's own
-  // listener never exits or rethrows.
+// A crash reaching here escaped without ever hitting runTUI's own try/catch
+// (e.g. a throw inside a fire-and-forget `void` call), so run.json was never
+// closed out. getActiveRun surfaces the in-flight session set by runTUI, with
+// enough (task, startedAt, model) carried on the handle itself that no read
+// of run.json is needed — a readFile here would be exactly the kind of
+// unbounded crash-path I/O primeCrashReporting (src/crash/report.ts) exists
+// to avoid for git: a stalled disk or network mount would block process.exit
+// forever. The write itself goes through saveCrashState, which bypasses the
+// per-session write chain in state.ts on purpose — chaining behind a write
+// that never settles (possibly the very write that triggered this crash)
+// would block process.exit indefinitely, defeating this handler's one job.
+async function finalizeActiveRunOnCrash(error: unknown): Promise<void> {
+  const run = getActiveRun();
+  if (run === null || !run.active) return;
+  const message = error instanceof Error ? error.message : String(error);
+  try {
+    await saveCrashState(run.cwd, run.sessionId, {
+      status: "crashed",
+      turnsUsed: 0,
+      task: run.task,
+      startedAt: run.startedAt,
+      finishedAt: Date.now(),
+      error: message,
+      ...(run.model !== undefined ? { model: run.model } : {}),
+    });
+  } catch (saveErr: unknown) {
+    process.stderr.write(
+      `failed to finalize run state after crash: ${saveErr instanceof Error ? saveErr.message : String(saveErr)}\n`,
+    );
+  }
+}
+
+// OpenTUI installs a process-global uncaughtException/unhandledRejection
+// handler that only logs (opentui/core's Renderer.handleError), which
+// suppresses Bun's default print-and-exit. Combined with raw-mode stdin
+// holding the event loop open, an escaped throw would otherwise hang the
+// process forever with the terminal still in the alternate screen. Node
+// invokes every registered listener for the event regardless of order, so
+// these still run and terminate the process even though OpenTUI's own
+// listener never exits or rethrows.
+export function installCrashHandlers(): void {
   process.on("uncaughtException", (err) => {
     void handleFatal("uncaughtException", err);
   });
   process.on("unhandledRejection", (reason) => {
     void handleFatal("unhandledRejection", reason);
   });
+}
+
+if (import.meta.main) {
+  installCrashHandlers();
 
   let code: number;
   try {

src/session/active-run.ts

@@ -0,0 +1,66 @@
+// A module-level slot the top-level uncaughtException/unhandledRejection
+// handler (src/index.ts) can reach even though persistRunSnapshot is a
+// closure local to runTUI. Only ever consulted from the crash path: a run
+// that never crashes never has this read.
+//
+// Carries enough of the live run state (task, startedAt, model) that the
+// crash handler can build a full RunState record itself. It must not read
+// run.json back off disk to fill these in — an unbounded readFile on the
+// 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.
+export type RunStateHandle = {
+  sessionId: string;
+  cwd: string;
+  active: boolean;
+  task: string;
+  startedAt: number;
+  model?: string;
+};
+
+let activeRun: RunStateHandle | null = null;
+
+export function setActiveRun(handle: RunStateHandle): void {
+  activeRun = handle;
+}
+
+export function clearActiveRun(): void {
+  activeRun = null;
+}
+
+export function getActiveRun(): RunStateHandle | null {
+  return activeRun;
+}
+
+// Set once, by the crash handler, immediately before it writes the terminal
+// "crashed" record. saveState (src/session/state.ts) reads this synchronously
+// right before each queued write actually fires, so any snapshot write still
+// waiting behind another one in its per-session chain sees the flag and
+// no-ops instead of firing after (and clobbering) the crash write. It cannot
+// stop a write whose writeFile/rename has already been dispatched to the
+// kernel at the moment the flag flips — that window is one atomicWrite call
+// wide, not the full remaining lifetime of the process.
+let crashed = false;
+
+export function markCrashed(): void {
+  crashed = true;
+}
+
+export function isCrashed(): boolean {
+  return crashed;
+}
+
+// Test-only seam: lets an integration test hold a chained write open past the
+// moment markCrashed() fires, so it can deterministically prove a write still
+// queued in the chain sees isCrashed() before it fires — rather than hoping
+// real filesystem timing happens to interleave that way. No effect on
+// production callers, which never install a gate.
+let testWriteGate: Promise<void> | null = null;
+
+export function setTestWriteGate(gate: Promise<void> | null): void {
+  testWriteGate = gate;
+}
+
+export function getTestWriteGate(): Promise<void> | null {
+  return testWriteGate;
+}

src/session/state.ts

@@ -4,6 +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 { COMMAND_NAME } from "../branding.js";
 
 const ConnectedMcpServerSchema = type({
@@ -14,7 +15,7 @@ const ConnectedMcpServerSchema = type({
 export type ConnectedMcpServer = typeof ConnectedMcpServerSchema.infer;
 
 const RunStateSchema = type({
-  status: "'running' | 'done' | 'failed' | 'cancelled'",
+  status: "'running' | 'done' | 'failed' | 'cancelled' | 'crashed'",
   turnsUsed: "number",
   task: "string",
   startedAt: "number",
@@ -65,6 +66,22 @@ export function warnUnreadableState(path: string, reason: string): void {
 // only ever address one file per session.
 const writeChains = new Map<string, Promise<void>>();
 
+// Checked right before a chained write actually fires (not at saveState()
+// call time) so a snapshot write still queued behind another one, at the
+// moment the crash handler flips this flag, sees it and no-ops instead of
+// landing after (and clobbering) the crash write issued via saveCrashState.
+// This cannot recall a write whose writeFile/rename has already been
+// dispatched to the kernel — that residual window is one atomicWrite call
+// wide (a small local JSON write), not the remaining lifetime of the process.
+async function atomicWriteUnlessCrashed(path: string, content: string): Promise<void> {
+  // No-op in production; lets a test hold this write open past the moment
+  // isCrashed() flips, so the check below is proven rather than assumed.
+  const gate = getTestWriteGate();
+  if (gate !== null) await gate;
+  if (isCrashed()) return;
+  await atomicWrite(path, content);
+}
+
 export async function saveState(
   cwd: string,
   sessionId: string,
@@ -75,8 +92,8 @@ export async function saveState(
   const content = JSON.stringify(state, null, 2);
   const previous = writeChains.get(sessionId) ?? Promise.resolve();
   const write = previous.then(
-    () => atomicWrite(path, content),
-    () => atomicWrite(path, content),
+    () => atomicWriteUnlessCrashed(path, content),
+    () => atomicWriteUnlessCrashed(path, content),
   );
   // Swallow the error in the chain tail (not in `write`, which still rejects
   // for this caller) so one failed save doesn't permanently wedge later
@@ -92,6 +109,23 @@ export async function saveState(
 }
 
 
+// Crash-time terminal write. Deliberately bypasses writeChains: a hung or
+// still-pending write for this session (possibly the very write mid-flight
+// when the process crashed) must never be awaited here, or a queued write
+// that never settles would block the crash handler's process.exit forever.
+// 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().
+export async function saveCrashState(
+  cwd: string,
+  sessionId: string,
+  state: RunState,
+  home?: string,
+): Promise<void> {
+  const path = statePath(cwd, sessionId, home);
+  await atomicWrite(path, JSON.stringify(state, null, 2));
+}
+
 // Returns the parsed state, or the arktype error summary when the shape is
 // invalid, so callers can surface a specific reason rather than "invalid shape".
 function parseRunState(data: unknown): RunState | { error: string } {

src/tui/runner.ts

@@ -163,6 +163,7 @@ 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 { setActiveRun, clearActiveRun, type RunStateHandle } from "../session/active-run.js";
 import { openInBrowser } from "../auth/oauth/browser.js";
 import { pickSession } from "./pick-session.js";
 import { RESUME_TRANSCRIPT_BLOCK_LIMIT, turnsToContentBlocks } from "./turns-to-blocks.js";
@@ -470,6 +471,22 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     mcpServers: resumeSeed.mcpServers,
   });
 
+  // Registered the moment a run starts so the top-level uncaughtException /
+  // unhandledRejection handler in index.ts (which cannot see any local state
+  // in this function) can finalize run.json for crashes that escape without
+  // ever reaching this function's own try/catch — e.g. a throw inside a
+  // fire-and-forget `void` call. Cleared wherever `finalized` below flips
+  // true, since those paths already write a terminal run.json themselves.
+  const activeRunHandle: RunStateHandle = {
+    sessionId,
+    cwd: config.cwd,
+    active: true,
+    task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)",
+    startedAt,
+    model: `${config.providerName}:${config.model}`,
+  };
+  setActiveRun(activeRunHandle);
+
   // Crash guard: if anything from setup onward throws all the way out of
   // runTUI instead of reaching the normal finalize block, this still closes
   // out run.json so status and finishedAt never disagree. Declared before the
@@ -496,6 +513,8 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   const finalizeOnCrash = async (err: unknown): Promise<void> => {
     if (finalized) return;
     finalized = true;
+    activeRunHandle.active = false;
+    clearActiveRun();
     await flushPartialOnCrash().catch((flushErr: unknown) => {
       // Best-effort only — still attempt saveState below. Log so a flush
       // failure is not invisible when diagnosing a crash exit.
@@ -1375,12 +1394,19 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     status: RunState["status"],
     extra?: Pick<RunState, "finishedAt" | "error">,
   ): Promise<void> => {
+    const task = runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)";
+    const model = `${liveSource.id}:${liveSource.model}`;
+    // Kept in step with every persisted snapshot so the crash handler's copy
+    // (activeRunHandle, read by index.ts) never lags what's actually on disk.
+    activeRunHandle.task = task;
+    activeRunHandle.startedAt = startedAt;
+    activeRunHandle.model = model;
     await saveState(config.cwd, sessionId, {
       status,
       turnsUsed: runSink.getTurnCount(),
-      task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)",
+      task,
       startedAt,
-      model: `${liveSource.id}:${liveSource.model}`,
+      model,
       mcpServers: connectedMcpServers,
       ...extra,
     });
@@ -1640,6 +1666,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
         });
         await persistRunSnapshot("done", { finishedAt: Date.now() });
         sessionId = generateSessionId();
+        activeRunHandle.sessionId = sessionId;
         startedAt = Date.now();
         runTaskTitle = config.task;
         emitter.emit("session.title", runTaskTitle.trim().length > 0 ? truncateSessionLabel(runTaskTitle) : "Untitled session");
@@ -2254,6 +2281,8 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   // finished run (finishedAt set) can be left reading as still in progress.
   const persistedStatus: RunState["status"] = summaryStatus;
   finalized = true;
+  activeRunHandle.active = false;
+  clearActiveRun();
   await writeRunSnapshot(persistedStatus, {
     finishedAt,
     ...(sinkError !== undefined ? { error: sinkError } : {}),

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

@@ -0,0 +1,65 @@
+// Spawned as a subprocess by tests/integration/crash-finalize.test.ts. Mimics
+// what runTUI does at startup (register the active run, write the initial
+// "running" run.json) and what index.ts does at process entry (install the
+// crash handlers), then throws asynchronously so it surfaces as a genuine
+// uncaughtException rather than a synchronous throw the caller could catch.
+import { installCrashHandlers } from "../../../src/index.js";
+import { setActiveRun, setTestWriteGate } from "../../../src/session/active-run.js";
+import { sessionDir } from "../../../src/session/index.js";
+import { saveState } from "../../../src/session/state.js";
+
+const cwd = process.cwd();
+const sessionId = process.env["CRASH_TEST_SESSION_ID"];
+if (sessionId === undefined) {
+  throw new Error("CRASH_TEST_SESSION_ID must be set");
+}
+
+const startedAt = Date.now();
+const task = "simulated crash task";
+const model = "test-provider:test-model";
+
+await saveState(cwd, sessionId, {
+  status: "running",
+  turnsUsed: 3,
+  task,
+  startedAt,
+  model,
+});
+
+setActiveRun({ sessionId, cwd, active: true, task, startedAt, model });
+installCrashHandlers();
+
+process.stdout.write(`${sessionDir(cwd, sessionId)}\n`);
+
+// Hold every write issued from here on at the gate, before it reaches
+// isCrashed(). This makes the race deterministic instead of hoping real
+// filesystem timing interleaves the right way: the two straggler writes
+// below are guaranteed to still be queued, not dispatched to the kernel,
+// when the crash handler flips isCrashed() — the exact scenario the guard
+// exists for.
+let releaseGate: () => void;
+const gate = new Promise<void>((resolve) => {
+  releaseGate = resolve;
+});
+setTestWriteGate(gate);
+
+// Two unawaited straggler snapshot writes, chained behind each other in
+// state.ts's per-session queue — what persistRunSnapshot fires on every
+// turn/model-switch/MCP-connect event. Both are parked at the gate.
+void saveState(cwd, sessionId, { status: "running", turnsUsed: 1, task, startedAt, model });
+void saveState(cwd, sessionId, { status: "running", turnsUsed: 2, task, startedAt, model });
+
+// Throws inside setImmediate so it surfaces as a real uncaughtException.
+// Node/Bun run the exception's own uncaughtException dispatch — including
+// handleFatal's synchronous markCrashed() call, which precedes its first
+// await — to completion before the event loop reaches the next queued
+// setImmediate callback. The second setImmediate below is therefore
+// guaranteed to run after isCrashed() has flipped to true, so releasing the
+// gate there always lets the two parked writes observe the flag rather than
+// racing it.
+setImmediate(() => {
+  throw new Error("simulated crash");
+});
+setImmediate(() => {
+  releaseGate();
+});