Finalize run.json when the process crashes - #355
Merged
Conversation
TheGreatAxios
force-pushed
the
cl-5574-finalize-run-on-crash
branch
from
August 7, 2026 07:32
08e4cc2 to
9c4f070
Compare
Process-level uncaughtException/unhandledRejection handlers wrote a crash report but left run.json stuck at status: running, so crashed sessions kept reappearing in the resume picker as in-progress. runTUI now registers a narrow handle (session id, cwd, active flag) in a module-level slot the moment a run starts, clearing it on any finalize path it already owns. The top-level crash handler reads that slot and writes status: crashed plus finishedAt through a new saveCrashState that bypasses the per-session write chain entirely, so a write that never settles can't block process.exit.
The crash handler awaited loadState (a plain readFile) to recover task/startedAt/model before writing, the same unbounded-I/O hazard primeCrashReporting exists to avoid for git. active-run.ts now carries those fields directly, updated by runTUI wherever it already tracks them, so the handler needs no read. Bypassing writeChains for the crash write also reopened the exact race CL-5567 closed: an in-flight progress snapshot for the same session could still land after the crash write and resurrect status: running. saveState now checks a synchronous isCrashed() flag right before each queued write fires, so anything still waiting in the chain when the crash handler marks the process crashed steps aside instead of racing it.
Firing 50 unawaited writes and hoping enough were still queued when isCrashed() flipped was a coin flip in practice (5/13 passed with the guard removed, when it should fail every time). A test-only write gate in active-run.ts now lets the fixture park writes before they reach the isCrashed() check and release them only after the crash handler has flipped the flag, so the ordering is controlled instead of hoped for. 25/25 passes with the guard in place; removing the guard reliably lets the parked writes win the race again.
TheGreatAxios
force-pushed
the
cl-5574-finalize-run-on-crash
branch
from
August 7, 2026 07:49
9c4f070 to
13584dd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RunStateHandle(session id, cwd, active flag, task, startedAt, model) fromrunTUI's setup into a module-level slot (src/session/active-run.ts), registered the moment a run starts, kept in sync whereverrunTUIalready tracks those fields, and cleared on every path that already finalizesrun.jsonitself.handleFatalinsrc/index.tsreads that slot and, beforeprocess.exit, best-effort writesstatus: "crashed",finishedAt, and the error message through a newsaveCrashStateinsrc/session/state.ts— built entirely from the handle, with no disk read.loadStatehere would be a plainreadFilewith no timeout, the exact unbounded crash-path I/OprimeCrashReportingexists to avoid for git.saveCrashStatebypasses the per-sessionwriteChainsmap (a hung queued write must never blockprocess.exit), but a bypass alone can't stop an in-flight progress snapshot'srename()from landing after it and resurrectingstatus: "running".handleFatalnow callsmarkCrashed()first, andsaveStatechecks a synchronousisCrashed()flag immediately before each queued write actually fires, so anything still waiting in the chain at that moment steps aside. This can't recall a write whosewriteFile/renameis already dispatched to the kernel — that residual window is oneatomicWritecall wide, not the process's remaining lifetime."crashed"is a new terminalRunStatestatus, excluded fromisResumableByDefault(like"done"/"failed"), so a crashed session no longer appears as an incomplete resumable run in the picker.installCrashHandlersis factored out ofsrc/index.ts's entry block so an integration test can register the real crash handlers without spinning up the TUI.Verification
bun run typecheckbun run buildbun run test— 3984 pass, 0 failtests/integration/crash-finalize.test.tsspawns a subprocess fixture that registers the active run and crash handlers, then parks two unawaited straggler "running" snapshot writes behind a test-only write gate (setTestWriteGateinactive-run.ts) that it only releases after the crash handler has flippedisCrashed()— guaranteeing both writes are still queued, not dispatched to the kernel, at that exact moment, rather than hoping real filesystem timing lines up. Asserts the resultingrun.jsonhasstatus: "crashed", afinishedAt, the error message, the righttask/model(recovered from the handle, no read), and is excluded byisResumableByDefault.if (isCrashed()) return;reliably lets a parked write win the race again (9/15 failures observed in that state — still probabilistic without the fix, as expected, since that's a real race).Closes CL-5574