diff --git a/CHANGELOG.md b/CHANGELOG.md index a5974903b..330712588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,9 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ### Fixed +- Crash and signal finalizers preserve `turnsUsed` from the in-memory active-run + handle instead of writing `0`, so a signaled or crashed session keeps the + turn count already persisted by mid-run snapshots. - Occupancy takes one dry-episode shot when the parent settles idle even if the live fleet 1→0 edge was never observed. - Dry-fleet transcript and `/status` report the outcome tally only diff --git a/src/exec/runner.ts b/src/exec/runner.ts index b064f7198..85270a054 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -70,7 +70,11 @@ import { sessionContextDir, sessionDir, } from "../session/index.js"; -import { setActiveRun } from "../session/active-run.js"; +import { + setActiveRun, + syncRunStateHandle, + type RunStateHandle, +} from "../session/active-run.js"; import { setActiveDisposeHost, clearActiveDisposeHost, @@ -377,6 +381,14 @@ export async function runExec(config: Config): Promise { let providerFailureObserved = false; let providerError: InferenceErrorLike | undefined; let result: ExecResult | undefined; + const activeRunHandle: RunStateHandle = { + sessionId, + cwd: config.cwd, + task, + startedAt, + turnsUsed: 0, + model: `${config.providerName}:${config.model}`, + }; const persist = async ( status: "running" | "done" | "failed" | "cancelled", @@ -384,12 +396,20 @@ export async function runExec(config: Config): Promise { ): Promise => { if (finalized && status === "running") return; if (status !== "running") finalized = true; + const model = `${config.providerName}:${config.model}`; + const nextTurnsUsed = runSink?.getTurnCount() ?? turnsUsed; + syncRunStateHandle(activeRunHandle, { + turnsUsed: nextTurnsUsed, + task, + startedAt, + model, + }); const snapshot = { status, - turnsUsed: runSink?.getTurnCount() ?? turnsUsed, + turnsUsed: nextTurnsUsed, task, startedAt, - model: `${config.providerName}:${config.model}`, + model, mcpServers: connectedMcp, ...(status !== "running" ? { finishedAt: Date.now() } : {}), ...(extra?.error !== undefined ? { error: extra.error } : {}), @@ -398,13 +418,7 @@ export async function runExec(config: Config): Promise { status === "running" ? saveState(config.cwd, sessionId, snapshot).then(() => { if (!finalized) { - setActiveRun({ - sessionId, - cwd: config.cwd, - task, - startedAt, - model: `${config.providerName}:${config.model}`, - }); + setActiveRun(activeRunHandle); } }) : finalizeRunState(config.cwd, sessionId, snapshot); diff --git a/src/index.ts b/src/index.ts index f0730e788..03d2d31b2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -207,8 +207,8 @@ export async function handleFatal( // (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 the in-flight // runner (TUI or exec), 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 +// enough (task, startedAt, model, turnsUsed) 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 @@ -222,7 +222,7 @@ async function finalizeActiveRunOnCrash(error: unknown): Promise { try { await saveCrashState(run.cwd, run.sessionId, { status: "crashed", - turnsUsed: 0, + turnsUsed: run.turnsUsed, task: run.task, startedAt: run.startedAt, finishedAt: Date.now(), @@ -267,7 +267,7 @@ async function finalizeActiveRunOnSignal( try { await saveCrashState(run.cwd, run.sessionId, { status: "failed", - turnsUsed: 0, + turnsUsed: run.turnsUsed, task: run.task, startedAt: run.startedAt, finishedAt: Date.now(), diff --git a/src/session/active-run.test.ts b/src/session/active-run.test.ts new file mode 100644 index 000000000..e400a7a47 --- /dev/null +++ b/src/session/active-run.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, test } from "bun:test"; + +import { + clearActiveRun, + getActiveRun, + setActiveRun, + syncRunStateHandle, + type RunStateHandle, +} from "./active-run.js"; + +describe("syncRunStateHandle", () => { + test("updates turnsUsed and identity fields on the live handle", () => { + clearActiveRun(); + const handle: RunStateHandle = { + sessionId: "sess", + cwd: "/tmp", + task: "old", + startedAt: 1, + turnsUsed: 0, + model: "provider:old", + }; + setActiveRun(handle); + + syncRunStateHandle(handle, { + turnsUsed: 7, + task: "new", + startedAt: 42, + model: "provider:new", + }); + + expect(getActiveRun()).toBe(handle); + expect(handle.turnsUsed).toBe(7); + expect(handle.task).toBe("new"); + expect(handle.startedAt).toBe(42); + expect(handle.model).toBe("provider:new"); + clearActiveRun(); + }); +}); diff --git a/src/session/active-run.ts b/src/session/active-run.ts index 7bff94ed1..2bd3fb2be 100644 --- a/src/session/active-run.ts +++ b/src/session/active-run.ts @@ -4,8 +4,8 @@ // consulted from the crash path and from signal handlers: a run that never // crashes and is never signaled 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 +// Carries enough of the live run state (task, startedAt, model, turnsUsed) 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 @@ -20,9 +20,30 @@ export interface RunStateHandle { cwd: string; task: string; startedAt: number; + turnsUsed: number; model?: string; } +// Keep the crash/signal handle in step with every persisted snapshot so a +// terminal write never falls back to turnsUsed: 0 when the live run has +// already advanced past that. +export function syncRunStateHandle( + handle: RunStateHandle, + snapshot: { + turnsUsed: number; + task: string; + startedAt: number; + model?: string; + }, +): void { + handle.turnsUsed = snapshot.turnsUsed; + handle.task = snapshot.task; + handle.startedAt = snapshot.startedAt; + if (snapshot.model !== undefined) { + handle.model = snapshot.model; + } +} + let activeRun: RunStateHandle | null = null; export function setActiveRun(handle: RunStateHandle): void { diff --git a/src/session/state.test.ts b/src/session/state.test.ts index e577324b3..da8ea8990 100644 --- a/src/session/state.test.ts +++ b/src/session/state.test.ts @@ -80,7 +80,7 @@ test("a straggler snapshot started before a terminal write does not overwrite it 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 }); + setActiveRun({ sessionId, cwd, task: "task", startedAt: 1, turnsUsed: 0 }); await finalizeRunState( cwd, diff --git a/src/tui/run-snapshot-kind.test.ts b/src/tui/run-snapshot-kind.test.ts index c63f06652..1f781ffa2 100644 --- a/src/tui/run-snapshot-kind.test.ts +++ b/src/tui/run-snapshot-kind.test.ts @@ -69,7 +69,13 @@ describe("a snapshot write dispatched by kind", () => { }); test("a rotation still records the outgoing session but leaves the run crash-coverable", async () => { - setActiveRun({ sessionId: "old", cwd, task: "task", startedAt: 1 }); + setActiveRun({ + sessionId: "old", + cwd, + task: "task", + startedAt: 1, + turnsUsed: 0, + }); await write( "old", @@ -87,7 +93,13 @@ describe("a snapshot write dispatched by kind", () => { }); test("the run-ending write records the session and disarms the handle", async () => { - setActiveRun({ sessionId: "last", cwd, task: "task", startedAt: 1 }); + setActiveRun({ + sessionId: "last", + cwd, + task: "task", + startedAt: 1, + turnsUsed: 0, + }); await write( "last", diff --git a/src/tui/runner/exit.test.ts b/src/tui/runner/exit.test.ts index f2749f152..4054e72ed 100644 --- a/src/tui/runner/exit.test.ts +++ b/src/tui/runner/exit.test.ts @@ -49,7 +49,7 @@ function stubQuit(args: { getToolCallCount: () => 0, }, crashGuard: { markFinalized: () => undefined, isFinalized: () => false }, - activeRunHandle: { task: "", startedAt: 0, model: "" }, + activeRunHandle: { task: "", startedAt: 0, turnsUsed: 0, model: "" }, hookManager: { dispatchPostRun: async () => undefined }, liveSessionMode: "orchestrator", } as unknown as RunnerServices; diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index b7877c615..7e80c8751 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -29,6 +29,7 @@ import { truncateSessionLabel, } from "../../session/session-label.js"; import { clearActiveDisposeHost } from "../../session/active-host.js"; +import { syncRunStateHandle } from "../../session/active-run.js"; import { getValidCodexToken } from "../../auth/codex/session.js"; import { getValidXaiToken } from "../../auth/xai/session.js"; import { suppressProviderFailurePresentation } from "../provider/failure-attempt.js"; @@ -185,12 +186,16 @@ function createRunPersistence(state: RunnerState, services: RunnerServices) { const model = `${state.liveSource.id}:${state.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. - services.activeRunHandle.task = task; - services.activeRunHandle.startedAt = state.startedAt; - services.activeRunHandle.model = model; + const turnsUsed = services.runSink.getTurnCount(); + syncRunStateHandle(services.activeRunHandle, { + turnsUsed, + task, + startedAt: state.startedAt, + model, + }); const persisted: RunState = { status, - turnsUsed: services.runSink.getTurnCount(), + turnsUsed, task, startedAt: state.startedAt, model, @@ -586,11 +591,25 @@ export async function createRunLifecycle( "session-rotation", ); state.sessionId = generateSessionId(); - // Repointed, not cleared: the process lives on, so the crash handler - // must keep finding this handle and close out the *new* session. - services.activeRunHandle.sessionId = state.sessionId; state.startedAt = Date.now(); state.runTaskTitle = state.config.task; + const rotatedBundle = services.buildSessionSources(); + // Repointed, not cleared: the process lives on, so the crash handler + // must keep finding this handle and close out the *new* session. The + // fields it copies reseed with the repoint — a crash inside + // initSessionDir/buildAgent below would otherwise stamp the outgoing + // session's turnsUsed (and task, startedAt, model) onto a session + // that has run zero turns. + services.activeRunHandle.sessionId = state.sessionId; + syncRunStateHandle(services.activeRunHandle, { + turnsUsed: 0, + task: + state.runTaskTitle.trim().length > 0 + ? state.runTaskTitle.trim() + : "(conversation)", + startedAt: state.startedAt, + model: `${rotatedBundle.selected.id}:${rotatedBundle.selected.model}`, + }); services.emitter.emit( "session.title", state.runTaskTitle.trim().length > 0 @@ -599,7 +618,6 @@ export async function createRunLifecycle( ); state.workdir = sessionContextDir(state.config.cwd, state.sessionId); await initSessionDir(state.config.cwd, state.sessionId); - const rotatedBundle = services.buildSessionSources(); state.liveSources = rotatedBundle.sources; state.liveDefaultSource = rotatedBundle.defaultSource; state.liveSource = rotatedBundle.selected; diff --git a/src/tui/session-start.test.ts b/src/tui/session-start.test.ts index 8e36e0fa7..c223c4910 100644 --- a/src/tui/session-start.test.ts +++ b/src/tui/session-start.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js"; +import { setActiveRun, clearActiveRun } from "../session/active-run.js"; import type { RunState } from "../session/state.js"; describe("createTUICrashGuard", () => { @@ -20,6 +21,15 @@ describe("createTUICrashGuard", () => { }, }), async () => { + clearActiveRun(); + setActiveRun({ + sessionId: "live-session", + cwd: "/live-cwd", + task: "live task", + startedAt: 99, + turnsUsed: 4, + model: "live-provider:live-model", + }); const { createTUICrashGuard } = await import("./session-start.js"); const guard = createTUICrashGuard(() => ({ cwd: "/boot-cwd", @@ -64,6 +74,8 @@ describe("createTUICrashGuard", () => { expect(captured[0]?.state.startedAt).toBe(99); expect(captured[0]?.state.error).toBe("boom"); expect(captured[0]?.state.model).toBe("live-provider:live-model"); + expect(captured[0]?.state.turnsUsed).toBe(4); + clearActiveRun(); }, ); }); diff --git a/src/tui/session-start.ts b/src/tui/session-start.ts index 3140c2434..6c80b76f6 100644 --- a/src/tui/session-start.ts +++ b/src/tui/session-start.ts @@ -15,6 +15,7 @@ import type { Telemetry } from "../telemetry/index.js"; import { clearActiveDisposeHost } from "../session/active-host.js"; import { clearActiveRun, + getActiveRun, setActiveRun, type RunStateHandle, } from "../session/active-run.js"; @@ -111,6 +112,7 @@ export function createTUICrashGuard( // 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. + const turnsUsed = getActiveRun()?.turnsUsed ?? 0; clearActiveRun(); clearActiveDisposeHost(); await flushPartialOnCrash().catch((flushErr: unknown) => { @@ -129,7 +131,7 @@ export function createTUICrashGuard( const message = err instanceof Error ? err.message : String(err); await finalizeRunState(live.cwd, live.sessionId, { status: "failed", - turnsUsed: 0, + turnsUsed, task: live.runTaskTitle.trim().length > 0 ? live.runTaskTitle.trim() @@ -284,6 +286,7 @@ export async function prepareTUISession( task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)", startedAt, + turnsUsed: resumeSeed.turnsUsed, model: `${config.providerName}:${config.model}`, }; setActiveRun(activeRunHandle); diff --git a/tests/fixtures/crash-run/simulate-crash.ts b/tests/fixtures/crash-run/simulate-crash.ts index e48fdb362..8ec7b9ffb 100644 --- a/tests/fixtures/crash-run/simulate-crash.ts +++ b/tests/fixtures/crash-run/simulate-crash.ts @@ -7,6 +7,7 @@ import { installCrashHandlers } from "../../../src/index.js"; import { setActiveRun, setTestWriteGate, + syncRunStateHandle, } from "../../../src/session/active-run.js"; import { sessionDir } from "../../../src/session/index.js"; import { finalizeRunState, saveState } from "../../../src/session/state.js"; @@ -34,7 +35,14 @@ await saveState(cwd, sessionId, { // 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 }; +const activeRunHandle = { + sessionId, + cwd, + task, + startedAt, + turnsUsed: 3, + model, +}; setActiveRun(activeRunHandle); installCrashHandlers(); @@ -63,6 +71,15 @@ if (rotatedSessionId !== undefined) { await saveState(cwd, sessionId, rotationState); } activeRunHandle.sessionId = rotatedSessionId; + // Matches runner.ts reseeding the handle's snapshot fields at repoint: the + // new session has run zero turns, so a crash before its first persist must + // carry 0, not the outgoing session's count. + syncRunStateHandle(activeRunHandle, { + turnsUsed: 0, + task, + startedAt, + model, + }); activeSessionId = rotatedSessionId; } diff --git a/tests/fixtures/crash-run/simulate-exec-signal.ts b/tests/fixtures/crash-run/simulate-exec-signal.ts index 84e496ce3..820aadd88 100644 --- a/tests/fixtures/crash-run/simulate-exec-signal.ts +++ b/tests/fixtures/crash-run/simulate-exec-signal.ts @@ -18,6 +18,7 @@ if (sessionId === undefined) { const task = "headless exec signal task"; const runDir = sessionDir(cwd, sessionId); const runJsonPath = join(runDir, "run.json"); +const turnsUsed = Number(process.env["SIGNAL_TEST_TURNS_USED"] ?? "5"); async function waitForRunningRunJson(): Promise { for (;;) { @@ -46,6 +47,8 @@ await withMockedModuleDuring( async () => { const { installSignalHandlers } = await import("../../../src/index.js"); const { runExec } = await import("../../../src/exec/runner.js"); + const { getActiveRun, syncRunStateHandle } = + await import("../../../src/session/active-run.js"); installSignalHandlers(); const config = { command: "exec", @@ -62,6 +65,18 @@ await withMockedModuleDuring( } as unknown as Config; void runExec(config); await waitForRunningRunJson(); + const handle = getActiveRun(); + if (handle === null) { + throw new Error("runExec did not register an active-run handle"); + } + // Advance the live counter the way a mid-run snapshot would, without + // reading run.json on the signal path under test. + syncRunStateHandle(handle, { + turnsUsed, + task: handle.task, + startedAt: handle.startedAt, + ...(handle.model !== undefined ? { model: handle.model } : {}), + }); process.stdout.write(`${runDir}\n`); await new Promise(() => undefined); }, diff --git a/tests/fixtures/crash-run/simulate-run-end-crash.ts b/tests/fixtures/crash-run/simulate-run-end-crash.ts index 87237af1a..9d4cfcc80 100644 --- a/tests/fixtures/crash-run/simulate-run-end-crash.ts +++ b/tests/fixtures/crash-run/simulate-run-end-crash.ts @@ -29,7 +29,7 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, task, startedAt, model }); +setActiveRun({ sessionId, cwd, task, startedAt, turnsUsed: 3, model }); installCrashHandlers(); process.stdout.write(`${sessionDir(cwd, sessionId)}\n`); diff --git a/tests/fixtures/crash-run/simulate-signal.ts b/tests/fixtures/crash-run/simulate-signal.ts index 386fd2818..12aeb70cd 100644 --- a/tests/fixtures/crash-run/simulate-signal.ts +++ b/tests/fixtures/crash-run/simulate-signal.ts @@ -36,7 +36,7 @@ await saveState(cwd, sessionId, { model, }); -setActiveRun({ sessionId, cwd, task, startedAt, model }); +setActiveRun({ sessionId, cwd, task, startedAt, turnsUsed: 3, model }); installSignalHandlers(); let releaseGate: () => void; diff --git a/tests/integration/crash-finalize.test.ts b/tests/integration/crash-finalize.test.ts index 48ee1cbd4..c428c627e 100644 --- a/tests/integration/crash-finalize.test.ts +++ b/tests/integration/crash-finalize.test.ts @@ -53,6 +53,7 @@ describe("integration — crash finalizes run.json", () => { expect(state.error).toContain("simulated crash"); expect(state.task).toBe("simulated crash task"); expect(state.model).toBe("test-provider:test-model"); + expect(state.turnsUsed).toBe(3); } finally { rmSync(cwd, { recursive: true, force: true }); rmSync(home, { recursive: true, force: true }); @@ -97,6 +98,7 @@ describe("integration — crash finalizes run.json", () => { readFileSync(outgoingRunJsonPath, "utf8"), ) as RunState; expect(outgoingState.status).toBe("done"); + expect(outgoingState.turnsUsed).toBe(3); const rotatedRunJsonPath = join(stdout.trim(), "run.json"); const rotatedState = JSON.parse( @@ -108,6 +110,7 @@ describe("integration — crash finalizes run.json", () => { expect(rotatedState.status).toBe("crashed"); expect(rotatedState.finishedAt).toBeGreaterThan(0); expect(rotatedState.error).toContain("simulated crash"); + expect(rotatedState.turnsUsed).toBe(0); } finally { rmSync(cwd, { recursive: true, force: true }); rmSync(home, { recursive: true, force: true }); diff --git a/tests/integration/exec-signal-finalize.test.ts b/tests/integration/exec-signal-finalize.test.ts index 83f3d69d7..587c12bc9 100644 --- a/tests/integration/exec-signal-finalize.test.ts +++ b/tests/integration/exec-signal-finalize.test.ts @@ -72,6 +72,7 @@ describe("integration — signaled exec process finalizes run.json", () => { expect(state.finishedAt).toBeGreaterThan(0); expect(state.error).toBe(`terminated by ${signal}`); expect(state.task).toBe("headless exec signal task"); + expect(state.turnsUsed).toBe(5); } finally { rmSync(cwd, { recursive: true, force: true }); rmSync(home, { recursive: true, force: true }); diff --git a/tests/integration/signal-finalize.test.ts b/tests/integration/signal-finalize.test.ts index f0d4f85a5..c317793ad 100644 --- a/tests/integration/signal-finalize.test.ts +++ b/tests/integration/signal-finalize.test.ts @@ -74,6 +74,7 @@ describe("integration — signal finalizes run.json", () => { expect(state.finishedAt).toBeGreaterThan(0); expect(state.error).toBe(`terminated by ${signal}`); expect(state.task).toBe("simulated signal task"); + expect(state.turnsUsed).toBe(3); } finally { rmSync(cwd, { recursive: true, force: true }); rmSync(home, { recursive: true, force: true });