Skip to content

Commit 607f59f

Browse files
committed
Tie active-run liveness to a single 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.
1 parent 11d4c0e commit 607f59f

5 files changed

Lines changed: 59 additions & 12 deletions

File tree

src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export async function handleFatal(kind: CrashKind, error: unknown): Promise<void
148148
// would block process.exit indefinitely, defeating this handler's one job.
149149
async function finalizeActiveRunOnCrash(error: unknown): Promise<void> {
150150
const run = getActiveRun();
151-
if (run === null || !run.active) return;
151+
if (run === null) return;
152152
const message = error instanceof Error ? error.message : String(error);
153153
try {
154154
await saveCrashState(run.cwd, run.sessionId, {

src/session/active-run.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@
99
// crash path has the exact failure mode primeCrashReporting (src/crash/
1010
// report.ts) exists to avoid for git: a stalled disk or network mount would
1111
// block process.exit forever.
12+
//
13+
// Liveness has exactly one representation: presence of this handle in the
14+
// module-level slot (see getActiveRun below). There is no separate "active"
15+
// flag on the handle itself — a second field would just be a copy of the
16+
// same fact, free to drift from the slot it's meant to describe.
1217
export type RunStateHandle = {
1318
sessionId: string;
1419
cwd: string;
15-
active: boolean;
1620
task: string;
1721
startedAt: number;
1822
model?: string;

src/session/state.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ afterAll(() => {
2929
mock.module("node:fs/promises", () => realFs);
3030
});
3131

32-
const { loadState, saveState } = await import("./state.js");
32+
const { finalizeRunState, loadState, saveState } = await import("./state.js");
33+
const { getActiveRun, setActiveRun } = await import("./active-run.js");
3334
type RunState = Awaited<ReturnType<typeof loadState>>;
3435

3536
let cwd = "";
@@ -72,6 +73,19 @@ test("a straggler snapshot started before a terminal write does not overwrite it
7273
expect(final?.finishedAt).toBe(999);
7374
});
7475

76+
test("a persisted terminal status agrees with the active-run handle without a second call site", async () => {
77+
const sessionId = "sess-terminal";
78+
setActiveRun({ sessionId, cwd, task: "task", startedAt: 1 });
79+
80+
await finalizeRunState(cwd, sessionId, state({ status: "done", finishedAt: 1000 }), home);
81+
82+
const persisted = await loadState(cwd, sessionId, home);
83+
expect(persisted?.status).toBe("done");
84+
// The only liveness representation left is presence in the active-run
85+
// slot -- a terminal RunState.status must leave nothing there to read.
86+
expect(getActiveRun()).toBeNull();
87+
});
88+
7589
test("saveState calls for different sessions do not block each other", async () => {
7690
await Promise.all([
7791
saveState(cwd, "session-a", state({ task: "a" }), home),

src/session/state.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { dirname, join } from "node:path";
44
import { type } from "arktype";
55

66
import { sessionDir } from "./index.js";
7-
import { getTestWriteGate, isCrashed } from "./active-run.js";
7+
import { clearActiveRun, getTestWriteGate, isCrashed } from "./active-run.js";
88
import { COMMAND_NAME } from "../branding.js";
99

1010
const ConnectedMcpServerSchema = type({
@@ -108,6 +108,21 @@ export async function saveState(
108108
return write;
109109
}
110110

111+
// Single write path for a terminal RunState: pairs the on-disk status with
112+
// clearing the in-memory active-run handle (active-run.ts) so the two facts
113+
// are set together instead of at two independent call sites that could drift.
114+
// Callers writing a non-terminal ("running") snapshot should call saveState
115+
// directly — clearing the active-run handle on a running snapshot would be
116+
// wrong, not merely redundant.
117+
export async function finalizeRunState(
118+
cwd: string,
119+
sessionId: string,
120+
state: RunState,
121+
home?: string,
122+
): Promise<void> {
123+
await saveState(cwd, sessionId, state, home);
124+
clearActiveRun();
125+
}
111126

112127
// Crash-time terminal write. Deliberately bypasses writeChains: a hung or
113128
// still-pending write for this session (possibly the very write mid-flight

src/tui/runner.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ import {
174174
import { createRunSink } from "../session/run-sink.js";
175175
import { generateSessionId, initSessionDir, renameSession, sessionContextDir, sessionDir } from "../session/index.js";
176176
import { resolveSessionLabel, truncateSessionLabel } from "../session/session-label.js";
177-
import { loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js";
177+
import { finalizeRunState, loadState, saveState, type ConnectedMcpServer, type RunState } from "../session/state.js";
178178
import { setActiveRun, clearActiveRun, type RunStateHandle } from "../session/active-run.js";
179179
import { openInBrowser } from "../auth/oauth/browser.js";
180180
import { pickSession } from "./pick-session.js";
@@ -503,7 +503,6 @@ export async function runTUI(initialConfig: Config): Promise<number> {
503503
const activeRunHandle: RunStateHandle = {
504504
sessionId,
505505
cwd: config.cwd,
506-
active: true,
507506
task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)",
508507
startedAt,
509508
model: `${config.providerName}:${config.model}`,
@@ -536,7 +535,12 @@ export async function runTUI(initialConfig: Config): Promise<number> {
536535
const finalizeOnCrash = async (err: unknown): Promise<void> => {
537536
if (finalized) return;
538537
finalized = true;
539-
activeRunHandle.active = false;
538+
// Clear the active-run handle up front, before the awaits below, so a
539+
// second crash mid-flush can't see this run as still live and race the
540+
// finalize write issued here. finalizeRunState (state.ts) would otherwise
541+
// do this itself, but only after saveState resolves — too late for that
542+
// guard, so it's done here and finalizeRunState's own clear becomes a
543+
// no-op repeat of the same fact rather than a second independent write.
540544
clearActiveRun();
541545
await flushPartialOnCrash().catch((flushErr: unknown) => {
542546
// Best-effort only — still attempt saveState below. Log so a flush
@@ -546,7 +550,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
546550
process.stderr.write(`${COMMAND_NAME}: crash finalize partial flush failed: ${flushMessage}\n`);
547551
});
548552
const message = err instanceof Error ? err.message : String(err);
549-
await saveState(config.cwd, sessionId, {
553+
await finalizeRunState(config.cwd, sessionId, {
550554
status: "failed",
551555
turnsUsed: 0,
552556
task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)",
@@ -1445,15 +1449,24 @@ export async function runTUI(initialConfig: Config): Promise<number> {
14451449
activeRunHandle.task = task;
14461450
activeRunHandle.startedAt = startedAt;
14471451
activeRunHandle.model = model;
1448-
await saveState(config.cwd, sessionId, {
1452+
const state: RunState = {
14491453
status,
14501454
turnsUsed: runSink.getTurnCount(),
14511455
task,
14521456
startedAt,
14531457
model,
14541458
mcpServers: connectedMcpServers,
14551459
...extra,
1456-
});
1460+
};
1461+
// "running" is the only non-terminal status writeRunSnapshot ever
1462+
// receives (progress snapshots); anything else closes the run out, so
1463+
// the active-run handle is cleared in the same call as the disk write
1464+
// rather than by a separate statement at each terminal call site.
1465+
if (status === "running") {
1466+
await saveState(config.cwd, sessionId, state);
1467+
} else {
1468+
await finalizeRunState(config.cwd, sessionId, state);
1469+
}
14571470
};
14581471

14591472
// Progress snapshots are fired unsequenced (model switch, MCP connect, turn
@@ -2375,8 +2388,9 @@ export async function runTUI(initialConfig: Config): Promise<number> {
23752388
// finished run (finishedAt set) can be left reading as still in progress.
23762389
const persistedStatus: RunState["status"] = summaryStatus;
23772390
finalized = true;
2378-
activeRunHandle.active = false;
2379-
clearActiveRun();
2391+
// writeRunSnapshot clears the active-run handle itself for a terminal
2392+
// status (via finalizeRunState in state.ts), pairing the on-disk write
2393+
// with the in-memory one instead of setting them at two call sites.
23802394
await writeRunSnapshot(persistedStatus, {
23812395
finishedAt,
23822396
...(sinkError !== undefined ? { error: sinkError } : {}),

0 commit comments

Comments
 (0)