From ff4348f00b6fb091169eac00e27d6bb73d6ce2aa Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 13:25:53 +1000 Subject: [PATCH 1/5] test: finished runs are skipped by reading only the file's tail Two cases fail: a large finished run and small finished runs are still read whole by listUnfinishedRuns. Issue: #49 --- .../src/factory/ledger/listRuns.test.ts | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/apps/server/src/factory/ledger/listRuns.test.ts b/apps/server/src/factory/ledger/listRuns.test.ts index eb563d8a4063..88e325855c13 100644 --- a/apps/server/src/factory/ledger/listRuns.test.ts +++ b/apps/server/src/factory/ledger/listRuns.test.ts @@ -2,7 +2,7 @@ import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; -import { afterEach, describe, expect, it } from "vite-plus/test"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import type { State } from "../domain/lifecycle.ts"; import { makeCause } from "./events.ts"; @@ -11,6 +11,12 @@ import { holdsSlot, listUnfinishedRuns } from "./listRuns.ts"; import { RUNS_DIR, runIdFileName, type RunId } from "./runId.ts"; import { T0, ticking, useTempDirs } from "./testSupport.ts"; +// Records every full-file read; `node:fs` namespaces are frozen, so the module itself is wrapped. +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readFileSync: vi.fn(actual.readFileSync) }; +}); + const tempDir = useTempDirs(); const handles: Array = []; @@ -163,6 +169,79 @@ describe("listUnfinishedRuns: whether a session may be running", () => { }); }); +const line = (event: string, data: unknown, ms = T0) => + `${JSON.stringify({ ts: "2026-10-04T10:13:16.123Z", ms, event, data })}\n`; +const stateLine = (to: string) => line("state_changed", { from: "ready", to }); +const finishedLine = line("finished", { outcome: "stuck" }); + +const writeRun = (dataDir: string, id: RunId, content: string) => { + NodeFS.mkdirSync(NodePath.join(dataDir, RUNS_DIR), { recursive: true }); + const path = NodePath.join(dataDir, RUNS_DIR, runIdFileName(id)); + NodeFS.writeFileSync(path, content); + return path; +}; + +const fullReads = () => vi.mocked(NodeFS.readFileSync).mock.calls.map(([path]) => String(path)); + +describe("listUnfinishedRuns: finished runs are skipped by their tail", () => { + it("never reads a large finished run whole, and still lists an unfinished sibling", () => { + const dataDir = tempDir(); + const finished = writeRun( + dataDir, + rid(1), + stateLine("triage") + + line("step_changed", { step: "x".repeat(200) }).repeat(1000) + + finishedLine, + ); + expect(NodeFS.statSync(finished).size).toBeGreaterThan(64 * 1024); + const open = writeRun(dataDir, rid(2), stateLine("building")); + vi.mocked(NodeFS.readFileSync).mockClear(); + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(2), state: "building", unreadable: false, confirmed: false, session: false }, + ]); + expect(fullReads()).toContain(open); + expect(fullReads()).not.toContain(finished); + }); + + it("skips a small finished run, also after a trailing blank line or CRLF", () => { + const dataDir = tempDir(); + const small = writeRun(dataDir, rid(1), stateLine("triage") + finishedLine); + const blank = writeRun(dataDir, rid(2), stateLine("triage") + finishedLine + "\n\n"); + const crlf = writeRun(dataDir, rid(3), stateLine("triage") + finishedLine + "\r\n"); + vi.mocked(NodeFS.readFileSync).mockClear(); + expect(listUnfinishedRuns(dataDir)).toEqual([]); + for (const path of [small, blank, crlf]) expect(fullReads()).not.toContain(path); + }); + + it("reads a file whose last line is cut off in full and lists it as unfinished", () => { + const dataDir = tempDir(); + writeRun(dataDir, rid(1), stateLine("building") + finishedLine.slice(0, 40)); + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(1), state: "building", unreadable: false, confirmed: false, session: false }, + ]); + }); + + it("reads a file whose last complete line is not JSON in full and lists it as unfinished", () => { + const dataDir = tempDir(); + writeRun(dataDir, rid(1), stateLine("building") + "garbage\n"); + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(1), state: "building", unreadable: false, confirmed: false, session: false }, + ]); + }); + + it("a finished line longer than the tail is still skipped, after a full read", () => { + const dataDir = tempDir(); + const long = writeRun( + dataDir, + rid(1), + stateLine("triage") + line("finished", { outcome: "stuck", pad: "y".repeat(20_000) }), + ); + vi.mocked(NodeFS.readFileSync).mockClear(); + expect(listUnfinishedRuns(dataDir)).toEqual([]); + expect(fullReads()).toContain(long); + }); +}); + describe("holdsSlot", () => { const run = (state: State | null, confirmed: boolean, unreadable = false) => ({ id: rid(1), From a264b7b5c85cb56bb8e078ac2970d8508a00e2d3 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 13:26:23 +1000 Subject: [PATCH 2/5] feat: listUnfinishedRuns skips a finished run by its tail A run whose last complete line is a finished event is skipped after one bounded read from the end of its file. A cut-off or invalid last line, or a finished line longer than the tail, falls back to the full read. Issue: #49 --- apps/server/src/factory/ledger/listRuns.ts | 8 ++-- apps/server/src/factory/ledger/read.ts | 56 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/apps/server/src/factory/ledger/listRuns.ts b/apps/server/src/factory/ledger/listRuns.ts index cf260579b2e6..3a0aa09124bc 100644 --- a/apps/server/src/factory/ledger/listRuns.ts +++ b/apps/server/src/factory/ledger/listRuns.ts @@ -5,8 +5,8 @@ import * as NodePath from "node:path"; import type { State } from "../domain/lifecycle.ts"; import { readProgress } from "../runner/progress.ts"; import { errnoCode, LedgerIoError } from "./errors.ts"; -import { readRun } from "./read.ts"; -import { asciiLower, RUNS_DIR, runIdFromFileName, type RunId } from "./runId.ts"; +import { lastLineIsFinished, readRun } from "./read.ts"; +import { asciiLower, RUNS_DIR, runIdFromFileName, runPath, type RunId } from "./runId.ts"; /** One unfinished run as the pass sees it: its id, its last state, and whether its file could be read. */ export interface RunInfo { @@ -37,7 +37,8 @@ export const holdsSlot = (run: RunInfo): boolean => run.unreadable || (run.confirmed && run.state !== "pr-open"); /** - * Every run under `/runs/` that is not finished, oldest first by the first line's time; with + * Every run under `/runs/` that is not finished (a file whose last line is `finished` is + * skipped by reading only its tail), oldest first by the first line's time; with * `only`, the runs of that one issue, and no other file is read. Files that are not named like a * run are ignored, and a missing runs directory is an empty list. A run file that cannot be read * is still listed, flagged `unreadable`, so one bad file does not hide the others. A run with no @@ -68,6 +69,7 @@ export const listUnfinishedRuns = ( let firstMs: number | null = null; let info: RunInfo; try { + if (lastLineIsFinished(runPath(dataDir, id))) continue; const record = readRun(dataDir, id); firstMs = record.entries[0]?.ms ?? null; const seen = readProgress(record.entries); diff --git a/apps/server/src/factory/ledger/read.ts b/apps/server/src/factory/ledger/read.ts index d92e8a0221b0..50d70b22ec41 100644 --- a/apps/server/src/factory/ledger/read.ts +++ b/apps/server/src/factory/ledger/read.ts @@ -152,3 +152,59 @@ export const readRun = (dataDir: string, id: RunId): RunRecord => { const bytes = readRunBytes(runPath(dataDir, id)); return { id, ...parseLines(bytes) }; }; + +/** How much of a file's end `lastLineIsFinished` reads; a `finished` line is a few hundred bytes. */ +const TAIL_BYTES = 8192; + +/** + * Whether the last complete line of a run file is a valid `finished` event, read from at most + * `TAIL_BYTES` at the end of the file. `finish` writes that line last, so a run for which this is + * true is over without parsing the rest. Answers false for anything it cannot be sure of: a cut-off + * last line, a last line that is not a valid entry, or one longer than the tail. Never fails on + * content; IO errors map as `readRun` maps them. + */ +export const lastLineIsFinished = (path: string): boolean => { + requireRegularFile(path); + let tail: Buffer; + let fromStart: boolean; + try { + const fd = NodeFS.openSync(path, NodeFS.constants.O_RDONLY | NodeFS.constants.O_NOFOLLOW); + try { + const size = NodeFS.fstatSync(fd).size; + const offset = Math.max(0, size - TAIL_BYTES); + tail = Buffer.alloc(size - offset); + let filled = 0; + while (filled < tail.length) { + const n = NodeFS.readSync(fd, tail, filled, tail.length - filled, offset + filled); + if (n === 0) break; + filled += n; + } + tail = tail.subarray(0, filled); + fromStart = offset === 0; + } finally { + NodeFS.closeSync(fd); + } + } catch (error) { + if (errnoCode(error) === "ENOENT") throw new LedgerNotFoundError({ path }); + throw new LedgerIoError({ path, cause: error }); + } + + // Bytes after the last newline are a write that was cut off, as `parseLines` reads them. + let end = tail.lastIndexOf(NEWLINE) + 1; + if (end === 0 || !isBlankLine(tail.subarray(end))) return false; + // Step back over blank lines to the last line that holds something. + for (;;) { + const lineEnd = end - 1; + const start = tail.lastIndexOf(NEWLINE, lineEnd - 1) + 1; + const line = tail.subarray(start, lineEnd); + if (isBlankLine(line)) { + if (start === 0) return false; + end = start; + continue; + } + // A line that starts the chunk of a longer file may have begun before it. + if (start === 0 && !fromStart) return false; + const object = decodeLine(line); + return object !== null && lineHead(object)?.event === "finished"; + } +}; From 7d264025ca3cfa9b61b5a4ac5bfa9f990438fdf9 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 13:31:35 +1000 Subject: [PATCH 3/5] test: a run file of only blank lines is listed as unfinished A run file holding only blank lines, or a large file whose last 8192 bytes are all blank, must be listed as unfinished exactly as readRun reads it (state null, or the state of its first line). Red: before the fix the run hung node (the vitest worker printed no result and had to be killed after the 60s alarm did not stop it). Issue: #49 --- .../src/factory/ledger/listRuns.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/apps/server/src/factory/ledger/listRuns.test.ts b/apps/server/src/factory/ledger/listRuns.test.ts index 88e325855c13..b29c21d3d2c7 100644 --- a/apps/server/src/factory/ledger/listRuns.test.ts +++ b/apps/server/src/factory/ledger/listRuns.test.ts @@ -240,6 +240,27 @@ describe("listUnfinishedRuns: finished runs are skipped by their tail", () => { expect(listUnfinishedRuns(dataDir)).toEqual([]); expect(fullReads()).toContain(long); }); + + it("lists a run file of only blank lines as unfinished, as readRun reads it", () => { + const dataDir = tempDir(); + writeRun(dataDir, rid(1), "\n"); + writeRun(dataDir, rid(2), "\n\n"); + writeRun(dataDir, rid(3), "\n\r\n"); + const unfinished = { state: null, unreadable: false, confirmed: false, session: false }; + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(1), ...unfinished }, + { id: rid(2), ...unfinished }, + { id: rid(3), ...unfinished }, + ]); + }); + + it("lists a large run file whose whole tail is blank lines, with its state", () => { + const dataDir = tempDir(); + writeRun(dataDir, rid(1), stateLine("building") + "\n".repeat(9000)); + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(1), state: "building", unreadable: false, confirmed: false, session: false }, + ]); + }); }); describe("holdsSlot", () => { From baee5a7daf5d94d2c06f48eac6841533f7b87bb8 Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 13:31:46 +1000 Subject: [PATCH 4/5] fix: lastLineIsFinished stops at a blank line that starts the tail The blank-line step-back called lastIndexOf with a byteOffset of -1 once it reached the first byte, which Buffer reads as counting from the end, so the walk looped forever. With no earlier line it now returns false and the file is read in full. Issue: #49 --- apps/server/src/factory/ledger/read.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/server/src/factory/ledger/read.ts b/apps/server/src/factory/ledger/read.ts index 50d70b22ec41..8dd0c574745b 100644 --- a/apps/server/src/factory/ledger/read.ts +++ b/apps/server/src/factory/ledger/read.ts @@ -195,7 +195,8 @@ export const lastLineIsFinished = (path: string): boolean => { // Step back over blank lines to the last line that holds something. for (;;) { const lineEnd = end - 1; - const start = tail.lastIndexOf(NEWLINE, lineEnd - 1) + 1; + // A negative offset would count from the end of the buffer, so the first line has no earlier one. + const start = lineEnd === 0 ? 0 : tail.lastIndexOf(NEWLINE, lineEnd - 1) + 1; const line = tail.subarray(start, lineEnd); if (isBlankLine(line)) { if (start === 0) return false; From cd28e906c0eb67239ab5190af48e68d6b9c9996f Mon Sep 17 00:00:00 2001 From: Tom Kaczocha Date: Fri, 16 Oct 2026 13:42:56 +1000 Subject: [PATCH 5/5] test: a finished-named invalid line and a cut-off fragment after finished are read in full Both tests pin existing behavior, so there was no red phase. They kill two mutation survivors: a last line naming finished that is not a valid entry, and a cut-off fragment after a complete finished line, must each force a full read. Issue: #49 --- .../src/factory/ledger/listRuns.test.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/apps/server/src/factory/ledger/listRuns.test.ts b/apps/server/src/factory/ledger/listRuns.test.ts index b29c21d3d2c7..e1cdfdd46298 100644 --- a/apps/server/src/factory/ledger/listRuns.test.ts +++ b/apps/server/src/factory/ledger/listRuns.test.ts @@ -221,6 +221,32 @@ describe("listUnfinishedRuns: finished runs are skipped by their tail", () => { ]); }); + it("reads a file in full when its last line names finished but is not a valid entry", () => { + const dataDir = tempDir(); + const path = writeRun( + dataDir, + rid(1), + stateLine("building") + '{"event":"finished","data":{}}\n', + ); + vi.mocked(NodeFS.readFileSync).mockClear(); + expect(listUnfinishedRuns(dataDir)).toEqual([ + { id: rid(1), state: "building", unreadable: false, confirmed: false, session: false }, + ]); + expect(fullReads()).toContain(path); + }); + + it("reads a file in full when a cut-off fragment follows the finished line", () => { + const dataDir = tempDir(); + const path = writeRun( + dataDir, + rid(1), + stateLine("triage") + finishedLine + finishedLine.slice(0, 40), + ); + vi.mocked(NodeFS.readFileSync).mockClear(); + expect(listUnfinishedRuns(dataDir)).toEqual([]); + expect(fullReads()).toContain(path); + }); + it("reads a file whose last complete line is not JSON in full and lists it as unfinished", () => { const dataDir = tempDir(); writeRun(dataDir, rid(1), stateLine("building") + "garbage\n");