Skip to content

Commit 196344c

Browse files
committed
Un-export the turn-boundary predicate, cover it end to end instead
isRunSnapshotTurnBoundary had exactly one production caller in the same file, so its export existed only for a unit test. Make it module-private and replace that test with an end-to-end one that drives createRunSink, saveState, and loadState against a real temp session directory: turnsUsed incrementing per turn as read back off disk, 20 rapid back-to-back turns with no settling delay, and a late running write racing a done write to confirm status never resurrects.
1 parent 7a8dca4 commit 196344c

3 files changed

Lines changed: 142 additions & 15 deletions

File tree

src/tui/runner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ export function resolveExitCode(args: ResolveExitCodeArgs): number {
209209
// of turns already in the turns log. The terminal write on close still goes
210210
// through writeRunSnapshot directly with the real final status, so this
211211
// only needs to cover progress snapshots taken while the run is live.
212-
export function isRunSnapshotTurnBoundary(eventType: string): boolean {
212+
function isRunSnapshotTurnBoundary(eventType: string): boolean {
213213
return eventType === "inference.done";
214214
}
215215

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { mkdtempSync, rmSync } from "node:fs";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { EventEmitter } from "node:events";
5+
6+
import { describe, expect, test } from "bun:test";
7+
import type { ReactorEmittedEvent } from "@intx/inference";
8+
9+
import { generateSessionId } from "../../../src/session/index.js";
10+
import { createRunSink } from "../../../src/session/run-sink.js";
11+
import { saveState, loadState, type RunState } from "../../../src/session/state.js";
12+
13+
// End-to-end coverage for the run.json turn-boundary snapshot fix (CL-5534):
14+
// createRunSink, saveState, and loadState run for real against a temp
15+
// session directory — nothing mocked. This is what now covers
16+
// isRunSnapshotTurnBoundary's observable effect, since that predicate was
17+
// un-exported from src/tui/runner.ts as a testability-only surface with a
18+
// single production caller.
19+
20+
const noopHookManager = { dispatchPostTurn: () => undefined, getStatuses: () => [] };
21+
22+
function inferenceDone(): ReactorEmittedEvent {
23+
return {
24+
type: "inference.done",
25+
data: {
26+
turn: { content: [] },
27+
usage: {},
28+
source: "primary",
29+
},
30+
} as unknown as ReactorEmittedEvent;
31+
}
32+
33+
function baseState(overrides: Partial<RunState>, turnsUsed: number): RunState {
34+
return {
35+
status: "running",
36+
turnsUsed,
37+
task: "e2e run-state test",
38+
startedAt: Date.now(),
39+
model: "test-provider:test-model",
40+
mcpServers: [],
41+
...overrides,
42+
};
43+
}
44+
45+
describe("run.json turn-boundary snapshots — end to end", () => {
46+
test("turnsUsed increments and is readable off disk after every turn, and status settles to done", async () => {
47+
const cwd = mkdtempSync(join(tmpdir(), "corbits-run-state-cwd-"));
48+
const home = mkdtempSync(join(tmpdir(), "corbits-run-state-home-"));
49+
const sessionId = generateSessionId();
50+
try {
51+
const runSink = createRunSink({ emitter: new EventEmitter(), hookManager: noopHookManager });
52+
53+
const observed: number[] = [];
54+
for (let turn = 1; turn <= 4; turn++) {
55+
runSink.sink(inferenceDone());
56+
await saveState(cwd, sessionId, baseState({ status: "running" }, runSink.getTurnCount()), home);
57+
const onDisk = await loadState(cwd, sessionId, home);
58+
expect(onDisk).not.toBeNull();
59+
observed.push(onDisk!.turnsUsed);
60+
}
61+
62+
expect(observed).toEqual([1, 2, 3, 4]);
63+
64+
runSink.sink({ type: "reactor.done", data: {} } as unknown as ReactorEmittedEvent);
65+
await saveState(
66+
cwd,
67+
sessionId,
68+
baseState({ status: "done", finishedAt: Date.now() }, runSink.getTurnCount()),
69+
home,
70+
);
71+
const finalState = await loadState(cwd, sessionId, home);
72+
expect(finalState?.status).toBe("done");
73+
expect(finalState?.turnsUsed).toBe(4);
74+
} finally {
75+
rmSync(cwd, { recursive: true, force: true });
76+
rmSync(home, { recursive: true, force: true });
77+
}
78+
});
79+
80+
test("20 rapid back-to-back turns with no settling delay serialize without dropping a write", async () => {
81+
const cwd = mkdtempSync(join(tmpdir(), "corbits-run-state-cwd-"));
82+
const home = mkdtempSync(join(tmpdir(), "corbits-run-state-home-"));
83+
const sessionId = generateSessionId();
84+
try {
85+
const runSink = createRunSink({ emitter: new EventEmitter(), hookManager: noopHookManager });
86+
87+
// Fire all 20 turns and their snapshot writes back to back, with no
88+
// await between them — the per-session writeChains promise chain in
89+
// src/session/state.ts is what keeps these ordered rather than the
90+
// caller awaiting each one before starting the next.
91+
const writes: Promise<void>[] = [];
92+
for (let turn = 1; turn <= 20; turn++) {
93+
runSink.sink(inferenceDone());
94+
writes.push(saveState(cwd, sessionId, baseState({ status: "running" }, runSink.getTurnCount()), home));
95+
}
96+
await Promise.all(writes);
97+
98+
const finalState = await loadState(cwd, sessionId, home);
99+
expect(finalState?.turnsUsed).toBe(20);
100+
expect(runSink.getTurnCount()).toBe(20);
101+
} finally {
102+
rmSync(cwd, { recursive: true, force: true });
103+
rmSync(home, { recursive: true, force: true });
104+
}
105+
});
106+
107+
test("a late in-flight running write racing a done write never resurrects status to running", async () => {
108+
const cwd = mkdtempSync(join(tmpdir(), "corbits-run-state-cwd-"));
109+
const home = mkdtempSync(join(tmpdir(), "corbits-run-state-home-"));
110+
const sessionId = generateSessionId();
111+
try {
112+
const runSink = createRunSink({ emitter: new EventEmitter(), hookManager: noopHookManager });
113+
runSink.sink(inferenceDone());
114+
115+
// Issue a "running" progress snapshot but do not await it before
116+
// issuing the terminal "done" write right behind it — this models a
117+
// straggler turn-boundary snapshot racing the close-out write.
118+
const runningWrite = saveState(
119+
cwd,
120+
sessionId,
121+
baseState({ status: "running" }, runSink.getTurnCount()),
122+
home,
123+
);
124+
runSink.sink({ type: "reactor.done", data: {} } as unknown as ReactorEmittedEvent);
125+
const doneWrite = saveState(
126+
cwd,
127+
sessionId,
128+
baseState({ status: "done", finishedAt: Date.now() }, runSink.getTurnCount()),
129+
home,
130+
);
131+
132+
await Promise.all([runningWrite, doneWrite]);
133+
134+
const finalState = await loadState(cwd, sessionId, home);
135+
expect(finalState?.status).toBe("done");
136+
} finally {
137+
rmSync(cwd, { recursive: true, force: true });
138+
rmSync(home, { recursive: true, force: true });
139+
}
140+
});
141+
});

tests/unit/tui/runner.test.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { EventEmitter } from "node:events";
33
import {
44
createTUIEventEmitter,
55
getTUIRunSummaryStatus,
6-
isRunSnapshotTurnBoundary,
76
loadLocalSettingsWriteBase,
87
resumeTranscriptLoadErrorBlock,
98
} from "../../../src/tui/runner.js";
@@ -22,19 +21,6 @@ test("createTUIEventEmitter can emit and receive events", () => {
2221
expect(received.length).toBe(1);
2322
});
2423

25-
// Regression: run.json's turnsUsed must update every turn, not only once
26-
// at reactor shutdown. reactor.done fires exactly once, at agent shutdown,
27-
// so a live multi-turn interactive session never had its progress snapshot
28-
// re-fire until close — turnsUsed sat frozen at its resume-time value the
29-
// whole session (CL-5534). inference.done is the turn boundary every
30-
// reactor cycle guarantees, so that's what a mid-run snapshot must key off.
31-
test("isRunSnapshotTurnBoundary fires on inference.done, not reactor.done", () => {
32-
expect(isRunSnapshotTurnBoundary("inference.done")).toBe(true);
33-
expect(isRunSnapshotTurnBoundary("reactor.done")).toBe(false);
34-
expect(isRunSnapshotTurnBoundary("reactor.error")).toBe(false);
35-
expect(isRunSnapshotTurnBoundary("connector.reply")).toBe(false);
36-
});
37-
3824
test("getTUIRunSummaryStatus distinguishes done, failed, and cancelled runs", () => {
3925
expect(getTUIRunSummaryStatus(true, undefined)).toBe("done");
4026
expect(getTUIRunSummaryStatus(true, "network failed")).toBe("failed");

0 commit comments

Comments
 (0)