From d0c8eefe1ea78c82daabb1c60b41097d556a456c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 10:23:20 +0000 Subject: [PATCH 01/21] chore: bump overrides to clear high-severity audit advisories aube audit --audit-level high (run by CI) reports 8 high advisories in transitive dev dependencies: brace-expansion (<5.0.9), js-yaml (<4.3.1), nanoid (<3.3.18), and postcss (<=8.5.17). Bump the existing overrides block to the fixed versions; each package resolves to a single version in the tree, so the bumps are mechanical. --- aube-lock.yaml | 49 ++++++++++++++++++++++++++----------------------- package.json | 5 ++++- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/aube-lock.yaml b/aube-lock.yaml index e16de45..8be2bd9 100644 --- a/aube-lock.yaml +++ b/aube-lock.yaml @@ -5,8 +5,11 @@ settings: excludeLinksFromLockfile: false overrides: - brace-expansion: 5.0.6 + brace-expansion: 5.0.9 esbuild: 0.28.1 + js-yaml: 4.3.1 + nanoid: 3.3.18 + postcss: 8.5.18 vite: 8.0.16 ws: 8.21.0 @@ -101,7 +104,7 @@ time: balanced-match@4.0.4: 2026-02-22T11:38:25.951Z before-after-hook@2.2.3: 2022-10-04T00:19:26.570Z boolbase@1.0.0: 2014-02-15T14:44:50.620Z - brace-expansion@5.0.6: 2026-05-08T05:41:43.205Z + brace-expansion@5.0.9: 2026-07-30T10:00:32.762Z camelcase-keys@6.2.2: 2020-04-03T03:51:03.816Z camelcase@5.3.1: 2019-04-03T13:34:32.701Z chai@6.2.2: 2025-12-22T21:26:03.989Z @@ -181,7 +184,7 @@ time: is-obj@2.0.0: 2019-04-19T15:37:37.870Z is-plain-obj@1.1.0: 2015-11-05T09:31:58.189Z js-tokens@4.0.0: 2018-01-28T11:58:58.170Z - js-yaml@4.2.0: 2026-05-31T22:17:13.783Z + js-yaml@4.3.1: 2026-07-31T17:39:51.183Z jsep@1.4.0: 2024-11-05T14:49:55.640Z json-parse-even-better-errors@2.3.1: 2020-09-02T16:37:58.371Z json-stringify-safe@5.0.1: 2015-05-19T01:42:09.719Z @@ -212,7 +215,7 @@ time: minipass@7.1.3: 2026-02-19T00:34:33.886Z modify-values@1.0.1: 2018-03-23T07:35:47.388Z ms@2.1.3: 2020-12-08T13:54:35.223Z - nanoid@3.3.12: 2026-04-30T22:04:14.515Z + nanoid@3.3.18: 2026-08-07T16:41:05.696Z neo-async@2.6.2: 2020-07-09T18:23:53.065Z node-addon-api@7.1.1: 2024-07-12T10:15:07.595Z node-addon-api@8.7.0: 2026-03-26T01:10:33.995Z @@ -243,7 +246,7 @@ time: picomatch@4.0.4: 2026-03-23T20:39:47.960Z playwright-core@1.60.0: 2026-05-11T19:09:40.047Z playwright@1.60.0: 2026-05-11T19:09:33.114Z - postcss@8.5.15: 2026-05-19T09:51:29.843Z + postcss@8.5.18: 2026-07-12T20:38:40.936Z quick-lru@4.0.1: 2019-05-29T17:21:30.565Z react-reconciler@0.33.0: 2025-10-01T21:39:00.081Z react@19.2.7: 2026-06-01T18:00:48.323Z @@ -895,9 +898,9 @@ packages: boolbase@1.0.0: resolution: {integrity: sha512-JZOSA7Mo9sNGB8+UjSgzdLtokWAky1zbztM3WRLCbZ70/3cTANmQmOdR7y2g+J0e2WXywy1yS468tY+IruqEww==} - brace-expansion@5.0.6: - resolution: {integrity: sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==} - engines: {node: 18 || 20 || >=22} + brace-expansion@5.0.9: + resolution: {integrity: sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==} + engines: {node: 20 || >=22} camelcase-keys@6.2.2: resolution: {integrity: sha512-YrwaA0vEKazPBkn0ipTiMpSajYDSe+KjQfrjhcBMxJt/znbvlHd8Pw/Vamaz5EB4Wfhs3SUR3Z9mwRu/P3s3Yg==} @@ -1220,8 +1223,8 @@ packages: js-tokens@4.0.0: resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} - js-yaml@4.2.0: - resolution: {integrity: sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==} + js-yaml@4.3.1: + resolution: {integrity: sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==} hasBin: true jsep@1.4.0: @@ -1374,8 +1377,8 @@ packages: ms@2.1.3: resolution: {integrity: sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==} - nanoid@3.3.12: - resolution: {integrity: sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==} + nanoid@3.3.18: + resolution: {integrity: sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==} engines: {node: ^10 || ^12 || ^13.7 || ^14 || >=15.0.1} hasBin: true @@ -1495,8 +1498,8 @@ packages: engines: {node: '>=18'} hasBin: true - postcss@8.5.15: - resolution: {integrity: sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==} + postcss@8.5.18: + resolution: {integrity: sha512-xdB1oSLHbz1vRWgCDalrCqEFTWzFlhqFC5tIHLMOSUIjhm3XXQ1qrFy8S/ESr1JYRRXqM3c1QFiMZUJdUTqyMQ==} engines: {node: ^10 || ^12 || >=14} quick-lru@4.0.1: @@ -1933,7 +1936,7 @@ snapshots: dependencies: '@octokit/rest': 20.1.2(@octokit/core@5.2.2) '@octokit/types': 13.10.0 - js-yaml: 4.2.0 + js-yaml: 4.3.1 minimatch: 10.2.5 '@iarna/toml@3.0.0': {} @@ -2174,7 +2177,7 @@ snapshots: boolbase@1.0.0: {} - brace-expansion@5.0.6: + brace-expansion@5.0.9: dependencies: balanced-match: 4.0.4 @@ -2468,7 +2471,7 @@ snapshots: js-tokens@4.0.0: {} - js-yaml@4.2.0: + js-yaml@4.3.1: dependencies: argparse: 2.0.1 @@ -2554,7 +2557,7 @@ snapshots: minimatch@10.2.5: dependencies: - brace-expansion: 5.0.6 + brace-expansion: 5.0.9 minimist-options@4.1.0: dependencies: @@ -2570,7 +2573,7 @@ snapshots: ms@2.1.3: {} - nanoid@3.3.12: {} + nanoid@3.3.18: {} neo-async@2.6.2: {} @@ -2695,9 +2698,9 @@ snapshots: optionalDependencies: fsevents: 2.3.2 - postcss@8.5.15: + postcss@8.5.18: dependencies: - nanoid: 3.3.12 + nanoid: 3.3.18 picocolors: 1.1.1 source-map-js: 1.2.1 @@ -2749,7 +2752,7 @@ snapshots: figures: 3.2.0 http-proxy-agent: 7.0.2 https-proxy-agent: 7.0.6 - js-yaml: 4.2.0 + js-yaml: 4.3.1 jsonpath-plus: 10.4.0(jsep@1.4.0) node-html-parser: 6.1.13 parse-github-repo-url: 1.4.1 @@ -2954,7 +2957,7 @@ snapshots: esbuild: 0.28.1 lightningcss: 1.32.0 picomatch: 4.0.4 - postcss: 8.5.15 + postcss: 8.5.18 rolldown: 1.0.3 tinyglobby: 0.2.17(picomatch@4.0.4) tsx: 4.21.0 diff --git a/package.json b/package.json index b802f0a..8841fcf 100644 --- a/package.json +++ b/package.json @@ -100,8 +100,11 @@ } }, "overrides": { - "brace-expansion": "5.0.6", + "brace-expansion": "5.0.9", "esbuild": "0.28.1", + "js-yaml": "4.3.1", + "nanoid": "3.3.18", + "postcss": "8.5.18", "vite": "8.0.16", "ws": "8.21.0" } From 739e3f8ce9cac9f6536f1a68a4af6dfca9eed964 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 10:32:01 +0000 Subject: [PATCH 02/21] feat: add record diff command for replayed screen diffs computeScreenHash can only say whether two screens differ. record diff replays two session event logs offline, extracts the canonical visible screens, and emits an LCS line diff (plus per-side screen hashes) as a JSON envelope and a unified-style human diff. --at-seq-a/--at-seq-b target intermediate Event Log sequences, so a session can be diffed against its own earlier state. --- docs/USAGE.md | 14 ++ src/cli/commands/record-diff.ts | 181 +++++++++++++++++++ src/cli/main.ts | 40 +++++ src/protocol/messages.ts | 12 ++ src/protocol/schemas.ts | 31 ++++ src/util/lineDiff.ts | 89 +++++++++ test/e2e/scrollback-demo.test.ts | 26 ++- test/integration/record-diff.test.ts | 213 ++++++++++++++++++++++ test/unit/commands/record-diff.test.ts | 240 +++++++++++++++++++++++++ test/unit/protocol/messages.test.ts | 64 +++++++ test/unit/util/lineDiff.test.ts | 74 ++++++++ 11 files changed, 978 insertions(+), 6 deletions(-) create mode 100644 src/cli/commands/record-diff.ts create mode 100644 src/util/lineDiff.ts create mode 100644 test/integration/record-diff.test.ts create mode 100644 test/unit/commands/record-diff.test.ts create mode 100644 test/unit/util/lineDiff.test.ts diff --git a/docs/USAGE.md b/docs/USAGE.md index 429fb14..242f7a9 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -206,6 +206,20 @@ Use `--renderer ghostty-web`, `AGENT_TTY_RENDERER=ghostty-web`, or Home `config. `ghostty-web` provides reference visual truth for reviewable artifacts; it does not promise exact pixel parity with native terminals. +## `record diff` + +`computeScreenHash` (see [Screen Hash](#screen-hash)) can only say _whether_ two screens differ. Use `record diff` to see _what_ changed: it replays two recorded sessions offline from their event logs and prints an LCS line diff of the canonical visible screens. + +```bash +agent-tty record diff --json +agent-tty record diff --at-seq-a 0 --json +``` + +- `--at-seq-a ` / `--at-seq-b `: replay each side up to an Event Log sequence (default: latest). Diffing a session against itself at an earlier sequence shows how its screen evolved. +- The JSON result carries `identical`, per-side `sessionId`/`capturedAtSeq`/`cols`/`rows`/`screenHash`, and a `diff` array of `{ op: equal | delete | add, text, aRow?, bRow? }` entries over the visible screen lines (0-based rows, no trimming or normalization — the same canonical lines that `screenHash` hashes). +- Human output is a unified-style diff with `---`/`+++` headers naming each side's session, sequence, and hash prefix. +- The command works entirely offline from `events.jsonl`; sessions may be running or exited. The exit code is `0` whether or not the screens differ — automation should read `identical` from the JSON result. + ## Isolation `--home ` stores manifests, sockets, event logs, and artifacts under an isolated agent-tty home. diff --git a/src/cli/commands/record-diff.ts b/src/cli/commands/record-diff.ts new file mode 100644 index 0000000..77a17c1 --- /dev/null +++ b/src/cli/commands/record-diff.ts @@ -0,0 +1,181 @@ +import type { CommandContext } from '../context.js'; +import type { + RecordDiffResult, + RecordDiffSide, +} from '../../protocol/messages.js'; + +import { emitSuccess } from '../output.js'; +import { CliError } from '../errors.js'; +import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { RecordDiffResultSchema } from '../../protocol/messages.js'; +import { + canonicalVisibleLines, + computeScreenHash, +} from '../../renderer/canonicalScreen.js'; +import { withOfflineReplayRenderer } from '../../replay/offlineReplay.js'; +import { readManifestIfExists } from '../../storage/manifests.js'; +import { manifestPath, sessionDir } from '../../storage/sessionPaths.js'; +import { diffLines } from '../../util/lineDiff.js'; +import { invariant } from '../../util/assert.js'; + +interface CommandOptions { + context: CommandContext; + json: boolean; + sessionIdA: string; + sessionIdB: string; + atSeqA: number | undefined; + atSeqB: number | undefined; +} + +interface ReplayedScreen { + readonly side: RecordDiffSide; + readonly visibleLines: readonly string[]; +} + +function assertValidAtSeq(value: number | undefined, flag: string): void { + if (value !== undefined && (!Number.isInteger(value) || value < 0)) { + throw makeCliError(ERROR_CODES.INVALID_INPUT, { + message: `${flag} must be a non-negative integer.`, + details: { [flag]: value }, + }); + } +} + +async function resolveSessionDirectory( + home: string, + sessionId: string, +): Promise { + let sessionDirectory: string; + try { + sessionDirectory = sessionDir(home, sessionId); + } catch (error) { + throw makeCliError(ERROR_CODES.INVALID_SESSION_ID, { + message: `Session ID "${sessionId}" is invalid.`, + details: { sessionId }, + cause: error, + }); + } + + const manifestFile = manifestPath(sessionDirectory); + const manifest = await readManifestIfExists(manifestFile); + if (manifest === null) { + throw makeCliError(ERROR_CODES.SESSION_NOT_FOUND, { + message: `Session "${sessionId}" was not found.`, + details: { sessionId, manifestPath: manifestFile }, + }); + } + + return sessionDirectory; +} + +async function replayScreen( + context: CommandContext, + sessionId: string, + targetSeq: number | undefined, +): Promise { + const sessionDirectory = await resolveSessionDirectory( + context.home, + sessionId, + ); + + try { + return await withOfflineReplayRenderer( + { + sessionDir: sessionDirectory, + rendererName: context.rendererDefault, + ...(targetSeq === undefined ? {} : { targetSeq }), + }, + async ({ backend }) => { + const snapshot = await backend.snapshot({ includeScrollback: false }); + return { + side: { + sessionId, + capturedAtSeq: snapshot.capturedAtSeq, + cols: snapshot.cols, + rows: snapshot.rows, + screenHash: computeScreenHash(snapshot), + }, + visibleLines: canonicalVisibleLines(snapshot), + }; + }, + ); + } catch (error) { + if (error instanceof CliError) { + throw error; + } + + throw makeCliError(ERROR_CODES.REPLAY_ERROR, { + message: `Failed to replay session "${sessionId}" for record diff.`, + details: { + sessionId, + ...(targetSeq === undefined ? {} : { targetSeq }), + }, + cause: error, + }); + } +} + +function buildResultLines(result: RecordDiffResult): string[] { + const header = [ + `--- ${result.a.sessionId} @seq ${String(result.a.capturedAtSeq)} (${result.a.screenHash.slice(0, 12)})`, + `+++ ${result.b.sessionId} @seq ${String(result.b.capturedAtSeq)} (${result.b.screenHash.slice(0, 12)})`, + ]; + + if (result.identical) { + return [...header, 'Screens are identical.']; + } + + const markers: Record<'equal' | 'delete' | 'add', string> = { + equal: ' ', + delete: '-', + add: '+', + }; + return [ + ...header, + ...result.diff.map((entry) => `${markers[entry.op]}${entry.text}`), + ]; +} + +export async function runRecordDiffCommand( + options: CommandOptions, +): Promise { + assertValidAtSeq(options.atSeqA, 'at-seq-a'); + assertValidAtSeq(options.atSeqB, 'at-seq-b'); + + const a = await replayScreen( + options.context, + options.sessionIdA, + options.atSeqA, + ); + const b = await replayScreen( + options.context, + options.sessionIdB, + options.atSeqB, + ); + + const identical = a.side.screenHash === b.side.screenHash; + const diff = identical ? [] : diffLines(a.visibleLines, b.visibleLines); + if (identical) { + invariant( + a.visibleLines.join('\n') === b.visibleLines.join('\n'), + 'equal screen hashes must imply equal canonical visible text', + ); + } + + const rawResult = { identical, a: a.side, b: b.side, diff }; + const parsedResult = RecordDiffResultSchema.safeParse(rawResult); + if (!parsedResult.success) { + throw makeCliError(ERROR_CODES.INTERNAL_ERROR, { + message: 'Generated record diff result did not match the schema.', + details: { issues: parsedResult.error.issues }, + cause: parsedResult.error, + }); + } + + emitSuccess({ + command: 'record diff', + json: options.json, + result: parsedResult.data, + lines: buildResultLines(parsedResult.data), + }); +} diff --git a/src/cli/main.ts b/src/cli/main.ts index 665dc82..58874a9 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -19,6 +19,7 @@ import { runListCommand } from './commands/list.js'; import { runMarkCommand } from './commands/mark.js'; import { runPasteCommand } from './commands/paste.js'; import { runRunCommand } from './commands/run.js'; +import { runRecordDiffCommand } from './commands/record-diff.js'; import { runRecordExportCommand } from './commands/record-export.js'; import { runResizeCommand } from './commands/resize.js'; import { runScreenshotCommand } from './commands/screenshot.js'; @@ -800,6 +801,45 @@ async function main(): Promise { .command('record') .description('Manage recorded session artifacts'); + recordCommand + .command('diff ') + .description('Diff the replayed visible screens of two recorded sessions') + .option( + '--at-seq-a ', + 'Replay session A up to this Event Log sequence (default: latest)', + parseIntegerOption, + ) + .option( + '--at-seq-b ', + 'Replay session B up to this Event Log sequence (default: latest)', + parseIntegerOption, + ) + .option('--json', 'Emit a JSON command envelope', false) + .action( + wrapAction( + 'record diff', + async ( + sessionIdA: string, + sessionIdB: string, + options: { + atSeqA?: number; + atSeqB?: number; + json: boolean; + }, + context: CommandContext, + ) => { + await runRecordDiffCommand({ + context, + json: options.json, + sessionIdA, + sessionIdB, + atSeqA: options.atSeqA, + atSeqB: options.atSeqB, + }); + }, + ), + ); + recordCommand .command('export ') .description('Export a recorded session artifact') diff --git a/src/protocol/messages.ts b/src/protocol/messages.ts index efad2a9..0434a15 100644 --- a/src/protocol/messages.ts +++ b/src/protocol/messages.ts @@ -1,6 +1,9 @@ import { z } from 'zod'; import type { + RecordDiffLine as RecordDiffLineType, + RecordDiffResult as RecordDiffResultType, + RecordDiffSide as RecordDiffSideType, RecordExportResult as RecordExportResultType, ReplayTimingMode as ReplayTimingModeType, RichSnapshotLine as RichSnapshotLineType, @@ -20,6 +23,9 @@ import { } from './schemas.js'; export { + RecordDiffLineSchema, + RecordDiffResultSchema, + RecordDiffSideSchema, RecordExportResultSchema, ReplayTimingModeSchema, RichSnapshotLineSchema, @@ -213,6 +219,12 @@ export type ScreenshotResult = z.infer; export type RecordExportResult = RecordExportResultType; +export type RecordDiffLine = RecordDiffLineType; + +export type RecordDiffSide = RecordDiffSideType; + +export type RecordDiffResult = RecordDiffResultType; + export const TypeParamsSchema = z .object({ text: z.string().min(1), diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 70dca88..ebfcd87 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -474,6 +474,37 @@ export const RecordExportResultSchema = z .strict(); export type RecordExportResult = z.infer; +export const RecordDiffLineSchema = z + .object({ + op: z.enum(['equal', 'delete', 'add']), + text: z.string(), + aRow: NonNegativeIntSchema.optional(), + bRow: NonNegativeIntSchema.optional(), + }) + .strict(); +export type RecordDiffLine = z.infer; + +export const RecordDiffSideSchema = z + .object({ + sessionId: NonEmptyStringSchema, + capturedAtSeq: NonNegativeIntSchema, + cols: PositiveIntSchema, + rows: PositiveIntSchema, + screenHash: Sha256HexSchema, + }) + .strict(); +export type RecordDiffSide = z.infer; + +export const RecordDiffResultSchema = z + .object({ + identical: z.boolean(), + a: RecordDiffSideSchema, + b: RecordDiffSideSchema, + diff: z.array(RecordDiffLineSchema), + }) + .strict(); +export type RecordDiffResult = z.infer; + export type WaitForRenderResult = z.infer; // --- Week 8: Capability and renderer-runtime schemas --- diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts new file mode 100644 index 0000000..060ca70 --- /dev/null +++ b/src/util/lineDiff.ts @@ -0,0 +1,89 @@ +import { invariant } from './assert.js'; + +/** + * One line of an LCS-based line diff between two screens. + * + * - `equal`: the line is present in both screens (`aRow` and `bRow` set). + * - `delete`: the line is only in screen A (`aRow` set). + * - `add`: the line is only in screen B (`bRow` set). + * + * Row indices are 0-based positions in the respective input arrays. + */ +export interface LineDiffEntry { + readonly op: 'equal' | 'delete' | 'add'; + readonly text: string; + readonly aRow?: number; + readonly bRow?: number; +} + +const MAX_DIFF_LINES = 10_000; + +/** + * LCS line diff of two ordered line arrays via dynamic programming. + * Deterministic: when a delete and an add are both possible, the delete is + * emitted first. Inputs are bounded to keep the O(a.length * b.length) table + * cheap; terminal screens are far below the limit. + */ +export function diffLines( + a: readonly string[], + b: readonly string[], +): LineDiffEntry[] { + invariant( + a.length <= MAX_DIFF_LINES && b.length <= MAX_DIFF_LINES, + `diffLines inputs must not exceed ${String(MAX_DIFF_LINES)} lines`, + ); + + // lcs[i][j] = LCS length of a[i..] and b[j..]. + const lcs: number[][] = Array.from({ length: a.length + 1 }, () => + new Array(b.length + 1).fill(0), + ); + for (let i = a.length - 1; i >= 0; i -= 1) { + const row = lcs[i]; + const nextRow = lcs[i + 1]; + invariant( + row !== undefined && nextRow !== undefined, + 'diffLines LCS rows must exist', + ); + for (let j = b.length - 1; j >= 0; j -= 1) { + row[j] = + a[i] === b[j] + ? (nextRow[j + 1] ?? 0) + 1 + : Math.max(nextRow[j] ?? 0, row[j + 1] ?? 0); + } + } + + const entries: LineDiffEntry[] = []; + let i = 0; + let j = 0; + while (i < a.length && j < b.length) { + const aText = a[i]; + const bText = b[j]; + invariant( + aText !== undefined && bText !== undefined, + 'diffLines lines must exist within bounds', + ); + if (aText === bText) { + entries.push({ op: 'equal', text: aText, aRow: i, bRow: j }); + i += 1; + j += 1; + } else if ((lcs[i + 1]?.[j] ?? 0) >= (lcs[i]?.[j + 1] ?? 0)) { + entries.push({ op: 'delete', text: aText, aRow: i }); + i += 1; + } else { + entries.push({ op: 'add', text: bText, bRow: j }); + j += 1; + } + } + for (; i < a.length; i += 1) { + const aText = a[i]; + invariant(aText !== undefined, 'diffLines trailing a line must exist'); + entries.push({ op: 'delete', text: aText, aRow: i }); + } + for (; j < b.length; j += 1) { + const bText = b[j]; + invariant(bText !== undefined, 'diffLines trailing b line must exist'); + entries.push({ op: 'add', text: bText, bRow: j }); + } + + return entries; +} diff --git a/test/e2e/scrollback-demo.test.ts b/test/e2e/scrollback-demo.test.ts index 95ae98b..195049c 100644 --- a/test/e2e/scrollback-demo.test.ts +++ b/test/e2e/scrollback-demo.test.ts @@ -3,6 +3,7 @@ import { readFile, stat } from 'node:fs/promises'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import type { + RecordDiffResult, ScreenshotResult, SnapshotResult, } from '../../src/protocol/messages.js'; @@ -128,12 +129,25 @@ describe('scrollback-demo e2e', { timeout: 60_000 }, () => { expect(scrollbackLines).toBeDefined(); expect(scrollbackLines?.length).toBeGreaterThan(0); - const visibleText = structuredSnapshotEnvelope.result.visibleLines - .map((line) => line.text) - .join('\n'); - - expect(visibleText).toContain('SCROLLBACK COMPLETE'); - expect(visibleText).not.toContain('LINE 001'); + // `record diff` against the session's own first event proves the viewport + // scrolled: the final screen (equal + add entries) gained the completion + // marker and no longer shows the first line. + const diffEnvelope = runCliJson>( + ['record', 'diff', sessionId, sessionId, '--at-seq-a', '0'], + env, + ); + expect(diffEnvelope.ok).toBe(true); + expect(diffEnvelope.command).toBe('record diff'); + expect(diffEnvelope.result.identical).toBe(false); + const finalScreenLines = diffEnvelope.result.diff + .filter((entry) => entry.op !== 'delete') + .map((entry) => entry.text); + expect( + finalScreenLines.some((line) => line.includes('SCROLLBACK COMPLETE')), + ).toBe(true); + expect(finalScreenLines.some((line) => line.includes('LINE 001'))).toBe( + false, + ); const screenshotEnvelope = runCliJson>( ['screenshot', sessionId], diff --git a/test/integration/record-diff.test.ts b/test/integration/record-diff.test.ts new file mode 100644 index 0000000..f026072 --- /dev/null +++ b/test/integration/record-diff.test.ts @@ -0,0 +1,213 @@ +import { mkdtemp, realpath } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import type { RecordDiffResult } from '../../src/protocol/messages.js'; + +import { + cleanupHome, + createSession, + runCli, + type SuccessEnvelope, + type WaitResult, +} from '../helpers.js'; + +interface ErrorEnvelope { + ok: false; + command: string; + error: { + code: string; + message: string; + retryable: boolean; + details?: Record; + }; +} + +function waitForExit(testHome: string, sessionId: string): void { + const result = runCli( + ['wait', sessionId, '--exit', '--timeout', '10000', '--json'], + { AGENT_TTY_HOME: testHome }, + 15_000, + ); + + expect(result.status).toBe(0); + const envelope = JSON.parse(result.stdout) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.result.timedOut).toBe(false); +} + +function runRecordDiff( + testHome: string, + args: string[], +): ReturnType { + return runCli(['record', 'diff', ...args, '--json'], { + AGENT_TTY_HOME: testHome, + }); +} + +describe('record diff integration', { timeout: 120_000 }, () => { + let testHome = ''; + + beforeEach(async () => { + // oxfmt-ignore + testHome = await realpath(await mkdtemp(join(tmpdir(), 'agent-tty-record-diff-'))); + }); + + afterEach(async () => { + await cleanupHome(testHome); + }); + + it('reports identical screens for sessions with identical output', () => { + const command = ['/bin/sh', '-c', "printf 'alpha\\nbeta\\n'"]; + const sessionA = createSession(testHome, command); + const sessionB = createSession(testHome, command); + waitForExit(testHome, sessionA); + waitForExit(testHome, sessionB); + + const result = runRecordDiff(testHome, [sessionA, sessionB]); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(''); + const envelope = JSON.parse( + result.stdout, + ) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.command).toBe('record diff'); + expect(envelope.result.identical).toBe(true); + expect(envelope.result.diff).toEqual([]); + expect(envelope.result.a.sessionId).toBe(sessionA); + expect(envelope.result.b.sessionId).toBe(sessionB); + expect(envelope.result.a.screenHash).toBe(envelope.result.b.screenHash); + expect(envelope.result.a.screenHash).toMatch(/^[0-9a-f]{64}$/); + }); + + it('reports an LCS line diff for sessions with differing output', () => { + const sessionA = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'shared\\nOLD LINE\\n'", + ]); + const sessionB = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'shared\\nNEW LINE\\n'", + ]); + waitForExit(testHome, sessionA); + waitForExit(testHome, sessionB); + + const result = runRecordDiff(testHome, [sessionA, sessionB]); + + expect(result.status).toBe(0); + const envelope = JSON.parse( + result.stdout, + ) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.result.identical).toBe(false); + expect(envelope.result.a.screenHash).not.toBe(envelope.result.b.screenHash); + + const deleted = envelope.result.diff.filter( + (entry) => entry.op === 'delete', + ); + const added = envelope.result.diff.filter((entry) => entry.op === 'add'); + expect(deleted.map((entry) => entry.text)).toContain('OLD LINE'); + expect(added.map((entry) => entry.text)).toContain('NEW LINE'); + expect( + envelope.result.diff.some( + (entry) => entry.op === 'equal' && entry.text === 'shared', + ), + ).toBe(true); + }); + + it('diffs the same session against an earlier --at-seq target', () => { + const sessionId = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'first\\n'; sleep 0.3; printf 'second\\n'", + ]); + waitForExit(testHome, sessionId); + + // Sequence 0 is the first event of the log: replaying to it yields the + // screen before 'second' was printed. + const result = runRecordDiff(testHome, [ + sessionId, + sessionId, + '--at-seq-a', + '0', + ]); + + expect(result.status).toBe(0); + const envelope = JSON.parse( + result.stdout, + ) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.result.identical).toBe(false); + expect(envelope.result.a.capturedAtSeq).toBe(0); + expect(envelope.result.b.capturedAtSeq).toBeGreaterThan(0); + expect( + envelope.result.diff + .filter((entry) => entry.op === 'add') + .map((entry) => entry.text), + ).toContain('second'); + }); + + it('fails with SESSION_NOT_FOUND for unknown sessions', () => { + const command = ['/bin/sh', '-c', "printf 'x\\n'"]; + const sessionA = createSession(testHome, command); + waitForExit(testHome, sessionA); + + const result = runRecordDiff(testHome, [ + sessionA, + 'session-does-not-exist', + ]); + + expect(result.status).not.toBe(0); + const envelope = JSON.parse(result.stdout) as ErrorEnvelope; + expect(envelope.ok).toBe(false); + expect(envelope.error.code).toBe('SESSION_NOT_FOUND'); + }); + + it('rejects negative --at-seq values', () => { + const command = ['/bin/sh', '-c', "printf 'x\\n'"]; + const sessionA = createSession(testHome, command); + waitForExit(testHome, sessionA); + + const result = runRecordDiff(testHome, [ + sessionA, + sessionA, + '--at-seq-a', + '-2', + ]); + + expect(result.status).not.toBe(0); + const envelope = JSON.parse(result.stdout) as ErrorEnvelope; + expect(envelope.ok).toBe(false); + expect(envelope.error.code).toBe('INVALID_INPUT'); + }); + + it('prints a unified-style diff in human mode', () => { + const sessionA = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'OLD\\n'", + ]); + const sessionB = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'NEW\\n'", + ]); + waitForExit(testHome, sessionA); + waitForExit(testHome, sessionB); + + const result = runCli(['record', 'diff', sessionA, sessionB], { + AGENT_TTY_HOME: testHome, + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain(`--- ${sessionA} @seq `); + expect(result.stdout).toContain(`+++ ${sessionB} @seq `); + expect(result.stdout).toContain('-OLD'); + expect(result.stdout).toContain('+NEW'); + }); +}); diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts new file mode 100644 index 0000000..1c7421b --- /dev/null +++ b/test/unit/commands/record-diff.test.ts @@ -0,0 +1,240 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ERROR_CODES } from '../../../src/protocol/errors.js'; + +const mocks = vi.hoisted(() => ({ + emitSuccess: vi.fn(), + readManifestIfExists: vi.fn(), + sessionDir: vi.fn(), + manifestPath: vi.fn(), + withOfflineReplayRenderer: vi.fn(), +})); + +vi.mock('../../../src/cli/output.js', () => ({ + emitSuccess: mocks.emitSuccess, +})); + +vi.mock('../../../src/replay/offlineReplay.js', () => ({ + withOfflineReplayRenderer: mocks.withOfflineReplayRenderer, +})); + +vi.mock('../../../src/storage/manifests.js', () => ({ + readManifestIfExists: mocks.readManifestIfExists, +})); + +vi.mock('../../../src/storage/sessionPaths.js', () => ({ + sessionDir: mocks.sessionDir, + manifestPath: mocks.manifestPath, +})); + +import { runRecordDiffCommand } from '../../../src/cli/commands/record-diff.js'; +import { computeScreenHash } from '../../../src/renderer/canonicalScreen.js'; +import { createLogger } from '../../../src/util/logger.js'; +import { + createTestSemanticSnapshot, + createTestSessionRecord, +} from '../../helpers.js'; + +const TEST_CONTEXT = { + home: '/tmp/agent-tty', + timeoutMs: undefined, + colorEnabled: true, + logLevel: 'info', + logger: createLogger('info', () => undefined), + profileDefault: undefined, + rendererDefault: 'ghostty-web', + rendererVisualDefault: 'ghostty-web', + explicitHome: false, + configFile: null, +} as const; + +interface SnapshotFixture { + sessionId: string; + visibleLines: { row: number; text: string }[]; + capturedAtSeq: number; +} + +function mockReplaySnapshots(fixtures: SnapshotFixture[]): void { + let call = 0; + mocks.withOfflineReplayRenderer.mockImplementation( + async ( + _options: unknown, + run: (context: { + backend: { snapshot: () => Promise }; + }) => Promise, + ) => { + const fixture = fixtures[call]; + call += 1; + if (fixture === undefined) { + throw new Error('unexpected extra replay call'); + } + return await run({ + backend: { + snapshot: () => Promise.resolve(createTestSemanticSnapshot(fixture)), + }, + }); + }, + ); +} + +function createOptions( + overrides: Partial[0]> = {}, +) { + return { + context: TEST_CONTEXT, + json: true, + sessionIdA: 'session-a', + sessionIdB: 'session-b', + atSeqA: undefined, + atSeqB: undefined, + ...overrides, + }; +} + +describe('runRecordDiffCommand', () => { + beforeEach(() => { + mocks.sessionDir.mockImplementation( + (home: string, sessionId: string) => `${home}/sessions/${sessionId}`, + ); + mocks.manifestPath.mockImplementation( + (sessionDirectory: string) => `${sessionDirectory}/session.json`, + ); + mocks.readManifestIfExists.mockResolvedValue(createTestSessionRecord()); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('reports identical screens with an empty diff', async () => { + const lines = [ + { row: 0, text: 'hello' }, + { row: 1, text: '' }, + ]; + mockReplaySnapshots([ + { sessionId: 'session-a', visibleLines: lines, capturedAtSeq: 4 }, + { sessionId: 'session-b', visibleLines: lines, capturedAtSeq: 9 }, + ]); + + await runRecordDiffCommand(createOptions()); + + const expectedHash = computeScreenHash({ visibleLines: lines }); + expect(mocks.emitSuccess).toHaveBeenCalledWith( + expect.objectContaining({ + command: 'record diff', + result: { + identical: true, + a: { + sessionId: 'session-a', + capturedAtSeq: 4, + cols: 80, + rows: 24, + screenHash: expectedHash, + }, + b: { + sessionId: 'session-b', + capturedAtSeq: 9, + cols: 80, + rows: 24, + screenHash: expectedHash, + }, + diff: [], + }, + }), + ); + }); + + it('reports differing screens with an LCS line diff', async () => { + mockReplaySnapshots([ + { + sessionId: 'session-a', + visibleLines: [ + { row: 0, text: 'shared' }, + { row: 1, text: 'old line' }, + ], + capturedAtSeq: 4, + }, + { + sessionId: 'session-b', + visibleLines: [ + { row: 0, text: 'shared' }, + { row: 1, text: 'new line' }, + ], + capturedAtSeq: 4, + }, + ]); + + await runRecordDiffCommand(createOptions()); + + expect(mocks.emitSuccess).toHaveBeenCalledWith( + expect.objectContaining({ + result: expect.objectContaining({ + identical: false, + diff: [ + { op: 'equal', text: 'shared', aRow: 0, bRow: 0 }, + { op: 'delete', text: 'old line', aRow: 1 }, + { op: 'add', text: 'new line', bRow: 1 }, + ], + }) as Record, + lines: [ + expect.stringMatching(/^--- session-a @seq 4 \([0-9a-f]{12}\)$/), + expect.stringMatching(/^\+\+\+ session-b @seq 4 \([0-9a-f]{12}\)$/), + ' shared', + '-old line', + '+new line', + ], + }), + ); + }); + + it('passes --at-seq targets through to offline replay', async () => { + const lines = [{ row: 0, text: 'x' }]; + mockReplaySnapshots([ + { sessionId: 'session-a', visibleLines: lines, capturedAtSeq: 2 }, + { sessionId: 'session-b', visibleLines: lines, capturedAtSeq: 7 }, + ]); + + await runRecordDiffCommand(createOptions({ atSeqA: 2, atSeqB: 7 })); + + expect(mocks.withOfflineReplayRenderer).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ targetSeq: 2 }), + expect.any(Function), + ); + expect(mocks.withOfflineReplayRenderer).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ targetSeq: 7 }), + expect.any(Function), + ); + }); + + it('rejects negative --at-seq values', async () => { + await expect( + runRecordDiffCommand(createOptions({ atSeqA: -1 })), + ).rejects.toMatchObject({ + code: ERROR_CODES.INVALID_INPUT, + }); + expect(mocks.withOfflineReplayRenderer).not.toHaveBeenCalled(); + }); + + it('fails with SESSION_NOT_FOUND when a session manifest is missing', async () => { + mocks.readManifestIfExists.mockResolvedValueOnce(null); + + await expect(runRecordDiffCommand(createOptions())).rejects.toMatchObject({ + code: ERROR_CODES.SESSION_NOT_FOUND, + details: { sessionId: 'session-a' }, + }); + expect(mocks.withOfflineReplayRenderer).not.toHaveBeenCalled(); + }); + + it('wraps replay failures in REPLAY_ERROR', async () => { + mocks.withOfflineReplayRenderer.mockRejectedValue( + new Error('backend boot failed'), + ); + + await expect(runRecordDiffCommand(createOptions())).rejects.toMatchObject({ + code: ERROR_CODES.REPLAY_ERROR, + details: { sessionId: 'session-a' }, + }); + }); +}); diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index f665eff..f246789 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -11,6 +11,7 @@ import { MarkResultSchema, PasteParamsSchema, SendKeysResultSchema, + RecordDiffResultSchema, RecordExportResultSchema, ReplayTimingModeSchema, ResizeResultSchema, @@ -678,6 +679,69 @@ describe('RPC message schemas', () => { ).toBe(true); }); + it('accepts valid record diff results', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 24, + screenHash: 'a'.repeat(64), + }; + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: side, + b: { ...side, sessionId: 'session-02' }, + diff: [], + }).success, + ).toBe(true); + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: { ...side, screenHash: 'b'.repeat(64) }, + diff: [ + { op: 'equal', text: 'shared', aRow: 0, bRow: 0 }, + { op: 'delete', text: 'old', aRow: 1 }, + { op: 'add', text: 'new', bRow: 1 }, + ], + }).success, + ).toBe(true); + }); + + it('rejects invalid record diff results', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 24, + screenHash: 'a'.repeat(64), + }; + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: { ...side, screenHash: 'not-a-hash' }, + b: side, + diff: [], + }).success, + ).toBe(false); + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: side, + diff: [{ op: 'replace', text: 'x' }], + }).success, + ).toBe(false); + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: side, + }).success, + ).toBe(false); + }); + it('accepts valid record export results', () => { expect( RecordExportResultSchema.safeParse({ diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts new file mode 100644 index 0000000..59c84fc --- /dev/null +++ b/test/unit/util/lineDiff.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest'; + +import { diffLines } from '../../../src/util/lineDiff.js'; + +describe('diffLines', () => { + it('returns all-equal entries for identical inputs', () => { + const lines = ['a', 'b', 'c']; + + expect(diffLines(lines, lines)).toEqual([ + { op: 'equal', text: 'a', aRow: 0, bRow: 0 }, + { op: 'equal', text: 'b', aRow: 1, bRow: 1 }, + { op: 'equal', text: 'c', aRow: 2, bRow: 2 }, + ]); + }); + + it('reports a replaced line as a delete followed by an add', () => { + expect(diffLines(['a', 'b', 'c'], ['a', 'x', 'c'])).toEqual([ + { op: 'equal', text: 'a', aRow: 0, bRow: 0 }, + { op: 'delete', text: 'b', aRow: 1 }, + { op: 'add', text: 'x', bRow: 1 }, + { op: 'equal', text: 'c', aRow: 2, bRow: 2 }, + ]); + }); + + it('reports pure insertions and deletions with row indices', () => { + expect(diffLines(['a', 'c'], ['a', 'b', 'c'])).toEqual([ + { op: 'equal', text: 'a', aRow: 0, bRow: 0 }, + { op: 'add', text: 'b', bRow: 1 }, + { op: 'equal', text: 'c', aRow: 1, bRow: 2 }, + ]); + expect(diffLines(['a', 'b', 'c'], ['a', 'c'])).toEqual([ + { op: 'equal', text: 'a', aRow: 0, bRow: 0 }, + { op: 'delete', text: 'b', aRow: 1 }, + { op: 'equal', text: 'c', aRow: 2, bRow: 1 }, + ]); + }); + + it('handles empty inputs', () => { + expect(diffLines([], [])).toEqual([]); + expect(diffLines([], ['a'])).toEqual([{ op: 'add', text: 'a', bRow: 0 }]); + expect(diffLines(['a'], [])).toEqual([ + { op: 'delete', text: 'a', aRow: 0 }, + ]); + }); + + it('preserves the longest common subsequence across scrolled screens', () => { + // Simulates a terminal scrolling by two lines. + const before = ['line 1', 'line 2', 'line 3', 'line 4']; + const after = ['line 3', 'line 4', 'line 5', 'line 6']; + + expect(diffLines(before, after)).toEqual([ + { op: 'delete', text: 'line 1', aRow: 0 }, + { op: 'delete', text: 'line 2', aRow: 1 }, + { op: 'equal', text: 'line 3', aRow: 2, bRow: 0 }, + { op: 'equal', text: 'line 4', aRow: 3, bRow: 1 }, + { op: 'add', text: 'line 5', bRow: 2 }, + { op: 'add', text: 'line 6', bRow: 3 }, + ]); + }); + + it('treats repeated identical lines positionally', () => { + expect(diffLines(['', '', 'x'], ['', 'x', ''])).toEqual([ + { op: 'equal', text: '', aRow: 0, bRow: 0 }, + { op: 'delete', text: '', aRow: 1 }, + { op: 'equal', text: 'x', aRow: 2, bRow: 1 }, + { op: 'add', text: '', bRow: 2 }, + ]); + }); + + it('rejects inputs beyond the line limit', () => { + const big = new Array(10_001).fill('x'); + expect(() => diffLines(big, [])).toThrow(/must not exceed 10000 lines/); + }); +}); From a2d05c7d89fc8ddd53d56807e4d0616b0a539fa3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 10:46:08 +0000 Subject: [PATCH 03/21] fix: address record diff review findings - Synthesize the valid initial blank screen for sessions whose event log is still empty (backends reject snapshot() before the first replayed event), so record diff works on freshly created sessions. - Bound the LCS DP table by total cell count (4M) in addition to the per-side line cap, keeping worst-case memory bounded. - Parse --at-seq-a/--at-seq-b with full-token number parsing so malformed values like '1.5' or '2junk' are rejected as INVALID_INPUT instead of being silently truncated. --- src/cli/commands/record-diff.ts | 24 +++++++- src/cli/main.ts | 4 +- src/util/lineDiff.ts | 12 +++- test/integration/record-diff.test.ts | 41 +++++++++++++ test/unit/commands/record-diff.test.ts | 82 +++++++++++++++++++++++++- test/unit/util/lineDiff.test.ts | 10 ++++ 6 files changed, 165 insertions(+), 8 deletions(-) diff --git a/src/cli/commands/record-diff.ts b/src/cli/commands/record-diff.ts index 77a17c1..1dbb052 100644 --- a/src/cli/commands/record-diff.ts +++ b/src/cli/commands/record-diff.ts @@ -85,7 +85,29 @@ async function replayScreen( rendererName: context.rendererDefault, ...(targetSeq === undefined ? {} : { targetSeq }), }, - async ({ backend }) => { + async ({ backend, replayInput }) => { + if (replayInput.targetSeq < 0) { + // The session has not emitted any events yet, so its screen is the + // valid initial blank grid. Backends reject snapshot() before the + // first replayed event, so synthesize the blank screen directly. + const blankLines = Array.from( + { length: replayInput.initialRows }, + () => '', + ); + return { + side: { + sessionId, + capturedAtSeq: 0, + cols: replayInput.initialCols, + rows: replayInput.initialRows, + screenHash: computeScreenHash({ + visibleLines: blankLines.map((text) => ({ text })), + }), + }, + visibleLines: blankLines, + }; + } + const snapshot = await backend.snapshot({ includeScrollback: false }); return { side: { diff --git a/src/cli/main.ts b/src/cli/main.ts index 58874a9..b63d7bc 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -807,12 +807,12 @@ async function main(): Promise { .option( '--at-seq-a ', 'Replay session A up to this Event Log sequence (default: latest)', - parseIntegerOption, + parseNumberOption, ) .option( '--at-seq-b ', 'Replay session B up to this Event Log sequence (default: latest)', - parseIntegerOption, + parseNumberOption, ) .option('--json', 'Emit a JSON command envelope', false) .action( diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts index 060ca70..c9e6918 100644 --- a/src/util/lineDiff.ts +++ b/src/util/lineDiff.ts @@ -17,12 +17,16 @@ export interface LineDiffEntry { } const MAX_DIFF_LINES = 10_000; +// Bounds the DP table's cell count (each cell is one JS number), so worst-case +// memory stays in the tens of megabytes instead of scaling to +// MAX_DIFF_LINES^2 cells. +const MAX_DIFF_CELLS = 4_000_000; /** * LCS line diff of two ordered line arrays via dynamic programming. * Deterministic: when a delete and an add are both possible, the delete is - * emitted first. Inputs are bounded to keep the O(a.length * b.length) table - * cheap; terminal screens are far below the limit. + * emitted first. Inputs are bounded per side and by the product of their + * lengths (the DP table size); terminal screens are far below both limits. */ export function diffLines( a: readonly string[], @@ -32,6 +36,10 @@ export function diffLines( a.length <= MAX_DIFF_LINES && b.length <= MAX_DIFF_LINES, `diffLines inputs must not exceed ${String(MAX_DIFF_LINES)} lines`, ); + invariant( + (a.length + 1) * (b.length + 1) <= MAX_DIFF_CELLS, + `diffLines input product must not exceed ${String(MAX_DIFF_CELLS)} table cells`, + ); // lcs[i][j] = LCS length of a[i..] and b[j..]. const lcs: number[][] = Array.from({ length: a.length + 1 }, () => diff --git a/test/integration/record-diff.test.ts b/test/integration/record-diff.test.ts index f026072..f76f9bf 100644 --- a/test/integration/record-diff.test.ts +++ b/test/integration/record-diff.test.ts @@ -152,6 +152,47 @@ describe('record diff integration', { timeout: 120_000 }, () => { ).toContain('second'); }); + it('reports identical blank screens for running sessions with no events yet', () => { + // A freshly created quiet process has an empty event log; record diff must + // synthesize the valid initial blank screen instead of failing replay. + const sessionId = createSession(testHome, ['/bin/sleep', '60']); + + const result = runRecordDiff(testHome, [sessionId, sessionId]); + + expect(result.status).toBe(0); + const envelope = JSON.parse( + result.stdout, + ) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.result.identical).toBe(true); + expect(envelope.result.diff).toEqual([]); + + const destroyResult = runCli(['destroy', sessionId, '--force', '--json'], { + AGENT_TTY_HOME: testHome, + }); + expect(destroyResult.status).toBe(0); + }); + + it('rejects malformed --at-seq tokens instead of truncating them', () => { + const command = ['/bin/sh', '-c', "printf 'x\\n'"]; + const sessionA = createSession(testHome, command); + waitForExit(testHome, sessionA); + + for (const token of ['1.5', '2junk']) { + const result = runRecordDiff(testHome, [ + sessionA, + sessionA, + '--at-seq-a', + token, + ]); + + expect(result.status).not.toBe(0); + const envelope = JSON.parse(result.stdout) as ErrorEnvelope; + expect(envelope.ok).toBe(false); + expect(envelope.error.code).toBe('INVALID_INPUT'); + } + }); + it('fails with SESSION_NOT_FOUND for unknown sessions', () => { const command = ['/bin/sh', '-c', "printf 'x\\n'"]; const sessionA = createSession(testHome, command); diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts index 1c7421b..06d3fd2 100644 --- a/test/unit/commands/record-diff.test.ts +++ b/test/unit/commands/record-diff.test.ts @@ -54,14 +54,17 @@ interface SnapshotFixture { capturedAtSeq: number; } +interface MockReplayContext { + backend: { snapshot: () => Promise }; + replayInput: { targetSeq: number; initialCols: number; initialRows: number }; +} + function mockReplaySnapshots(fixtures: SnapshotFixture[]): void { let call = 0; mocks.withOfflineReplayRenderer.mockImplementation( async ( _options: unknown, - run: (context: { - backend: { snapshot: () => Promise }; - }) => Promise, + run: (context: MockReplayContext) => Promise, ) => { const fixture = fixtures[call]; call += 1; @@ -72,6 +75,30 @@ function mockReplaySnapshots(fixtures: SnapshotFixture[]): void { backend: { snapshot: () => Promise.resolve(createTestSemanticSnapshot(fixture)), }, + replayInput: { + targetSeq: fixture.capturedAtSeq, + initialCols: 80, + initialRows: 24, + }, + }); + }, + ); +} + +function mockEmptyLogReplay(rows: number, cols: number): void { + mocks.withOfflineReplayRenderer.mockImplementation( + async ( + _options: unknown, + run: (context: MockReplayContext) => Promise, + ) => { + return await run({ + backend: { + snapshot: () => + Promise.reject( + new Error('snapshot() must not be called for an empty log'), + ), + }, + replayInput: { targetSeq: -1, initialCols: cols, initialRows: rows }, }); }, ); @@ -187,6 +214,55 @@ describe('runRecordDiffCommand', () => { ); }); + it('synthesizes blank screens for sessions with empty event logs', async () => { + // A running session that has not emitted its first event yet replays to + // targetSeq -1; the command must report the valid initial blank screen + // instead of calling snapshot() (which backends reject before replay). + mockEmptyLogReplay(24, 80); + + await runRecordDiffCommand(createOptions()); + + const blankHash = computeScreenHash({ + visibleLines: Array.from({ length: 24 }, () => ({ text: '' })), + }); + expect(mocks.emitSuccess).toHaveBeenCalledWith( + expect.objectContaining({ + result: { + identical: true, + a: { + sessionId: 'session-a', + capturedAtSeq: 0, + cols: 80, + rows: 24, + screenHash: blankHash, + }, + b: { + sessionId: 'session-b', + capturedAtSeq: 0, + cols: 80, + rows: 24, + screenHash: blankHash, + }, + diff: [], + }, + }), + ); + }); + + it('rejects non-integer --at-seq values including NaN', async () => { + await expect( + runRecordDiffCommand(createOptions({ atSeqA: 1.5 })), + ).rejects.toMatchObject({ + code: ERROR_CODES.INVALID_INPUT, + }); + await expect( + runRecordDiffCommand(createOptions({ atSeqB: Number.NaN })), + ).rejects.toMatchObject({ + code: ERROR_CODES.INVALID_INPUT, + }); + expect(mocks.withOfflineReplayRenderer).not.toHaveBeenCalled(); + }); + it('passes --at-seq targets through to offline replay', async () => { const lines = [{ row: 0, text: 'x' }]; mockReplaySnapshots([ diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts index 59c84fc..1038920 100644 --- a/test/unit/util/lineDiff.test.ts +++ b/test/unit/util/lineDiff.test.ts @@ -71,4 +71,14 @@ describe('diffLines', () => { const big = new Array(10_001).fill('x'); expect(() => diffLines(big, [])).toThrow(/must not exceed 10000 lines/); }); + + it('rejects input pairs whose DP table would exceed the cell limit', () => { + // Each side passes the per-side limit, but the product (2002^2 cells) + // exceeds the 4M-cell table bound. + const a = new Array(2_001).fill('a'); + const b = new Array(2_001).fill('b'); + expect(() => diffLines(a, b)).toThrow( + /product must not exceed 4000000 table cells/, + ); + }); }); From 8501e4d3c3bdba91e848c5a4da5e552030ce956c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 10:51:48 +0000 Subject: [PATCH 04/21] chore: relock aube after upstream repo transfer The aube repository moved from endevco/aube to jdx/aube. GitHub attestation lookups via the old path now find nothing, so mise 2026.8.x fails 'mise install --locked' with 'Lockfile requires github-attestations provenance ... but verification was not performed', breaking every CI job at setup. Regenerate the aube lockfile entries with current mise (mise lock aube): URLs now point at jdx/aube and the unverifiable provenance pins are dropped. All artifact sha256 checksums are unchanged, so the pinned binaries are identical. --- mise.lock | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/mise.lock b/mise.lock index 13dfd9d..2a74620 100644 --- a/mise.lock +++ b/mise.lock @@ -985,39 +985,33 @@ backend = "github:endevco/aube" [tools.aube."platforms.linux-arm64"] checksum = "sha256:45dc6d46e69f1da58360fa8ac353d83c2b84e1ca2e31edb9be912a7729c839c1" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-unknown-linux-gnu.tar.gz" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417134849" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-unknown-linux-gnu.tar.gz" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417134849" [tools.aube."platforms.linux-arm64-musl"] checksum = "sha256:8094e14b0906ecb2fcdb573700e7c9536abdeedc203a15051723b4fe6247237e" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-unknown-linux-musl.tar.gz" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417112128" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-unknown-linux-musl.tar.gz" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417112128" [tools.aube."platforms.linux-x64"] checksum = "sha256:234b5d01ab5818937740ebc773709595e18eb926f2c543273e36670c5c416851" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-unknown-linux-gnu.tar.gz" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417115596" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-unknown-linux-gnu.tar.gz" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417115596" [tools.aube."platforms.linux-x64-musl"] checksum = "sha256:c6bc24fa4a06f13cc8fa7fdb18bd15c55ec17b2f9ebce478c559b9664bc5abde" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-unknown-linux-musl.tar.gz" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417115736" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-unknown-linux-musl.tar.gz" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417115736" [tools.aube."platforms.macos-arm64"] checksum = "sha256:963d276cef7f039a5750bf18d30af7c2457f6d5804fa24bd784458e22a4d1a14" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-apple-darwin.tar.gz" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417151945" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-aarch64-apple-darwin.tar.gz" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417151945" [tools.aube."platforms.windows-x64"] checksum = "sha256:631566d6802cbae88c35a54d8746ac36f56d563a0a680b6c5d882db8b9d495d5" -url = "https://github.com/endevco/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-pc-windows-msvc.zip" -url_api = "https://api.github.com/repos/endevco/aube/releases/assets/417123778" -provenance = "github-attestations" +url = "https://github.com/jdx/aube/releases/download/v1.10.4/aube-v1.10.4-x86_64-pc-windows-msvc.zip" +url_api = "https://api.github.com/repos/jdx/aube/releases/assets/417123778" [[tools.communique]] version = "1.1.3" From 5e1e56d8c7a3dd1196c7929bae930ea52d3023cf Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:00:10 +0000 Subject: [PATCH 05/21] fix: address record diff round-2 review findings - Reuse the first replay when both diff selectors are identical, so 'record diff ' on an actively printing session cannot race between two event-log reads and report spurious differences. - Replace the hard DP-table cell bound with common prefix/suffix trimming plus a non-minimal delete-then-add fallback for oversized middles: every valid screen pair now diffs successfully instead of crashing with an uncaught AssertionError. --- src/cli/commands/record-diff.ts | 15 ++- src/util/lineDiff.ts | 121 ++++++++++++++++++++----- test/integration/record-diff.test.ts | 25 +++++ test/unit/commands/record-diff.test.ts | 24 +++++ test/unit/util/lineDiff.test.ts | 47 ++++++++-- 5 files changed, 193 insertions(+), 39 deletions(-) diff --git a/src/cli/commands/record-diff.ts b/src/cli/commands/record-diff.ts index 1dbb052..f5ab67d 100644 --- a/src/cli/commands/record-diff.ts +++ b/src/cli/commands/record-diff.ts @@ -169,11 +169,16 @@ export async function runRecordDiffCommand( options.sessionIdA, options.atSeqA, ); - const b = await replayScreen( - options.context, - options.sessionIdB, - options.atSeqB, - ); + // When both selectors are identical, reuse the first replay instead of + // re-reading the event log: a running session may append output between the + // two reads, which would make `record diff ` spuriously + // non-identical. + const sameSelector = + options.sessionIdA === options.sessionIdB && + options.atSeqA === options.atSeqB; + const b = sameSelector + ? a + : await replayScreen(options.context, options.sessionIdB, options.atSeqB); const identical = a.side.screenHash === b.side.screenHash; const diff = identical ? [] : diffLines(a.visibleLines, b.visibleLines); diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts index c9e6918..564b4e7 100644 --- a/src/util/lineDiff.ts +++ b/src/util/lineDiff.ts @@ -16,31 +16,20 @@ export interface LineDiffEntry { readonly bRow?: number; } -const MAX_DIFF_LINES = 10_000; -// Bounds the DP table's cell count (each cell is one JS number), so worst-case -// memory stays in the tens of megabytes instead of scaling to -// MAX_DIFF_LINES^2 cells. +const MAX_DIFF_LINES = 100_000; +// Bounds the DP table's cell count (each cell is one JS number), keeping +// worst-case memory in the tens of megabytes. When the middle section (after +// common prefix/suffix trimming) would exceed this, the diff degrades to a +// non-minimal delete-then-add block instead of failing: every valid screen +// pair diffs successfully. const MAX_DIFF_CELLS = 4_000_000; -/** - * LCS line diff of two ordered line arrays via dynamic programming. - * Deterministic: when a delete and an add are both possible, the delete is - * emitted first. Inputs are bounded per side and by the product of their - * lengths (the DP table size); terminal screens are far below both limits. - */ -export function diffLines( +function lcsEntries( a: readonly string[], b: readonly string[], + aOffset: number, + bOffset: number, ): LineDiffEntry[] { - invariant( - a.length <= MAX_DIFF_LINES && b.length <= MAX_DIFF_LINES, - `diffLines inputs must not exceed ${String(MAX_DIFF_LINES)} lines`, - ); - invariant( - (a.length + 1) * (b.length + 1) <= MAX_DIFF_CELLS, - `diffLines input product must not exceed ${String(MAX_DIFF_CELLS)} table cells`, - ); - // lcs[i][j] = LCS length of a[i..] and b[j..]. const lcs: number[][] = Array.from({ length: a.length + 1 }, () => new Array(b.length + 1).fill(0), @@ -71,26 +60,108 @@ export function diffLines( 'diffLines lines must exist within bounds', ); if (aText === bText) { - entries.push({ op: 'equal', text: aText, aRow: i, bRow: j }); + entries.push({ + op: 'equal', + text: aText, + aRow: aOffset + i, + bRow: bOffset + j, + }); i += 1; j += 1; } else if ((lcs[i + 1]?.[j] ?? 0) >= (lcs[i]?.[j + 1] ?? 0)) { - entries.push({ op: 'delete', text: aText, aRow: i }); + entries.push({ op: 'delete', text: aText, aRow: aOffset + i }); i += 1; } else { - entries.push({ op: 'add', text: bText, bRow: j }); + entries.push({ op: 'add', text: bText, bRow: bOffset + j }); j += 1; } } for (; i < a.length; i += 1) { const aText = a[i]; invariant(aText !== undefined, 'diffLines trailing a line must exist'); - entries.push({ op: 'delete', text: aText, aRow: i }); + entries.push({ op: 'delete', text: aText, aRow: aOffset + i }); } for (; j < b.length; j += 1) { const bText = b[j]; invariant(bText !== undefined, 'diffLines trailing b line must exist'); - entries.push({ op: 'add', text: bText, bRow: j }); + entries.push({ op: 'add', text: bText, bRow: bOffset + j }); + } + + return entries; +} + +// Non-minimal but always-valid fallback: delete every a line, add every b +// line. Used only when the trimmed middle would exceed MAX_DIFF_CELLS. +function fallbackEntries( + a: readonly string[], + b: readonly string[], + aOffset: number, + bOffset: number, +): LineDiffEntry[] { + const entries: LineDiffEntry[] = []; + for (const [i, text] of a.entries()) { + entries.push({ op: 'delete', text, aRow: aOffset + i }); + } + for (const [j, text] of b.entries()) { + entries.push({ op: 'add', text, bRow: bOffset + j }); + } + return entries; +} + +/** + * LCS line diff of two ordered line arrays via dynamic programming. + * Deterministic: when a delete and an add are both possible, the delete is + * emitted first. Common prefix and suffix lines are matched directly, so the + * quadratic DP table only covers the differing middle; if that middle is + * still larger than MAX_DIFF_CELLS the middle degrades to a non-minimal + * delete-then-add block rather than failing. Terminal screens are far below + * every limit in practice. + */ +export function diffLines( + a: readonly string[], + b: readonly string[], +): LineDiffEntry[] { + invariant( + a.length <= MAX_DIFF_LINES && b.length <= MAX_DIFF_LINES, + `diffLines inputs must not exceed ${String(MAX_DIFF_LINES)} lines`, + ); + + let prefix = 0; + while (prefix < a.length && prefix < b.length && a[prefix] === b[prefix]) { + prefix += 1; + } + let suffix = 0; + while ( + suffix < a.length - prefix && + suffix < b.length - prefix && + a[a.length - 1 - suffix] === b[b.length - 1 - suffix] + ) { + suffix += 1; + } + + const entries: LineDiffEntry[] = []; + for (let k = 0; k < prefix; k += 1) { + const text = a[k]; + invariant(text !== undefined, 'diffLines prefix line must exist'); + entries.push({ op: 'equal', text, aRow: k, bRow: k }); + } + + const aMiddle = a.slice(prefix, a.length - suffix); + const bMiddle = b.slice(prefix, b.length - suffix); + const withinBudget = + (aMiddle.length + 1) * (bMiddle.length + 1) <= MAX_DIFF_CELLS; + entries.push( + ...(withinBudget + ? lcsEntries(aMiddle, bMiddle, prefix, prefix) + : fallbackEntries(aMiddle, bMiddle, prefix, prefix)), + ); + + for (let k = 0; k < suffix; k += 1) { + const aRow = a.length - suffix + k; + const bRow = b.length - suffix + k; + const text = a[aRow]; + invariant(text !== undefined, 'diffLines suffix line must exist'); + entries.push({ op: 'equal', text, aRow, bRow }); } return entries; diff --git a/test/integration/record-diff.test.ts b/test/integration/record-diff.test.ts index f76f9bf..2a972ca 100644 --- a/test/integration/record-diff.test.ts +++ b/test/integration/record-diff.test.ts @@ -152,6 +152,31 @@ describe('record diff integration', { timeout: 120_000 }, () => { ).toContain('second'); }); + it('reports identical screens when diffing an actively printing session against itself', () => { + // The session appends output continuously; the identical-selector replay + // reuse must prevent a race between the two event-log reads. + const sessionId = createSession(testHome, [ + '/bin/sh', + '-c', + 'i=0; while [ $i -lt 200 ]; do echo "tick $i"; i=$((i+1)); sleep 0.05; done', + ]); + + const result = runRecordDiff(testHome, [sessionId, sessionId]); + + expect(result.status).toBe(0); + const envelope = JSON.parse( + result.stdout, + ) as SuccessEnvelope; + expect(envelope.ok).toBe(true); + expect(envelope.result.identical).toBe(true); + expect(envelope.result.diff).toEqual([]); + + const destroyResult = runCli(['destroy', sessionId, '--force', '--json'], { + AGENT_TTY_HOME: testHome, + }); + expect(destroyResult.status).toBe(0); + }); + it('reports identical blank screens for running sessions with no events yet', () => { // A freshly created quiet process has an empty event log; record diff must // synthesize the valid initial blank screen instead of failing replay. diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts index 06d3fd2..816b7d1 100644 --- a/test/unit/commands/record-diff.test.ts +++ b/test/unit/commands/record-diff.test.ts @@ -263,6 +263,30 @@ describe('runRecordDiffCommand', () => { expect(mocks.withOfflineReplayRenderer).not.toHaveBeenCalled(); }); + it('replays only once when both selectors are identical', async () => { + // Diffing a running session against itself must not read the event log + // twice: output appended between the reads would make the result + // spuriously non-identical. + const lines = [{ row: 0, text: 'busy output' }]; + mockReplaySnapshots([ + { sessionId: 'session-a', visibleLines: lines, capturedAtSeq: 3 }, + ]); + + await runRecordDiffCommand( + createOptions({ sessionIdA: 'session-a', sessionIdB: 'session-a' }), + ); + + expect(mocks.withOfflineReplayRenderer).toHaveBeenCalledTimes(1); + expect(mocks.emitSuccess).toHaveBeenCalledWith( + expect.objectContaining({ + result: expect.objectContaining({ + identical: true, + diff: [], + }) as Record, + }), + ); + }); + it('passes --at-seq targets through to offline replay', async () => { const lines = [{ row: 0, text: 'x' }]; mockReplaySnapshots([ diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts index 1038920..d66e5d9 100644 --- a/test/unit/util/lineDiff.test.ts +++ b/test/unit/util/lineDiff.test.ts @@ -68,17 +68,46 @@ describe('diffLines', () => { }); it('rejects inputs beyond the line limit', () => { - const big = new Array(10_001).fill('x'); - expect(() => diffLines(big, [])).toThrow(/must not exceed 10000 lines/); + const big = new Array(100_001).fill('x'); + expect(() => diffLines(big, [])).toThrow(/must not exceed 100000 lines/); }); - it('rejects input pairs whose DP table would exceed the cell limit', () => { - // Each side passes the per-side limit, but the product (2002^2 cells) - // exceeds the 4M-cell table bound. - const a = new Array(2_001).fill('a'); - const b = new Array(2_001).fill('b'); - expect(() => diffLines(a, b)).toThrow( - /product must not exceed 4000000 table cells/, + it('matches large common prefixes and suffixes without a quadratic table', () => { + // 40k shared lines on each side would need a ~1.6G-cell DP table; the + // prefix/suffix trim must reduce the middle to the single changed line. + const shared = Array.from( + { length: 40_000 }, + (_, i) => `line ${String(i)}`, ); + const a = [...shared, 'OLD', ...shared]; + const b = [...shared, 'NEW', ...shared]; + + const entries = diffLines(a, b); + + const changed = entries.filter((entry) => entry.op !== 'equal'); + expect(changed).toEqual([ + { op: 'delete', text: 'OLD', aRow: 40_000 }, + { op: 'add', text: 'NEW', bRow: 40_000 }, + ]); + expect(entries).toHaveLength(a.length + 1); + }); + + it('degrades to a delete-then-add block when the middle exceeds the cell budget', () => { + // Fully distinct 3000-line sides leave a middle whose DP table (~9M + // cells) exceeds the 4M budget; the diff must degrade, not fail. + const a = Array.from({ length: 3_000 }, (_, i) => `a ${String(i)}`); + const b = Array.from({ length: 3_000 }, (_, i) => `b ${String(i)}`); + + const entries = diffLines(a, b); + + expect(entries).toHaveLength(6_000); + expect( + entries.slice(0, 3_000).every((entry) => entry.op === 'delete'), + ).toBe(true); + expect(entries.slice(3_000).every((entry) => entry.op === 'add')).toBe( + true, + ); + expect(entries[0]).toEqual({ op: 'delete', text: 'a 0', aRow: 0 }); + expect(entries[5_999]).toEqual({ op: 'add', text: 'b 2999', bRow: 2_999 }); }); }); From a0acc036e2df7d86b4f1bfd2509f4acbcd9c8654 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:05:47 +0000 Subject: [PATCH 06/21] fix: append large diff middles iteratively Spreading the fallback middle into push() exceeds V8's function-argument limit for ~125k-entry middles, throwing RangeError instead of returning the JSON envelope. Append iteratively. --- src/util/lineDiff.ts | 13 ++++++++----- test/unit/util/lineDiff.test.ts | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts index 564b4e7..e2a5408 100644 --- a/src/util/lineDiff.ts +++ b/src/util/lineDiff.ts @@ -150,11 +150,14 @@ export function diffLines( const bMiddle = b.slice(prefix, b.length - suffix); const withinBudget = (aMiddle.length + 1) * (bMiddle.length + 1) <= MAX_DIFF_CELLS; - entries.push( - ...(withinBudget - ? lcsEntries(aMiddle, bMiddle, prefix, prefix) - : fallbackEntries(aMiddle, bMiddle, prefix, prefix)), - ); + const middleEntries = withinBudget + ? lcsEntries(aMiddle, bMiddle, prefix, prefix) + : fallbackEntries(aMiddle, bMiddle, prefix, prefix); + // Append iteratively: spreading into push() would exceed V8's + // function-argument limit for very large middles. + for (const entry of middleEntries) { + entries.push(entry); + } for (let k = 0; k < suffix; k += 1) { const aRow = a.length - suffix + k; diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts index d66e5d9..59a7627 100644 --- a/test/unit/util/lineDiff.test.ts +++ b/test/unit/util/lineDiff.test.ts @@ -92,6 +92,24 @@ describe('diffLines', () => { expect(entries).toHaveLength(a.length + 1); }); + it('handles very large distinct middles without exceeding argument limits', () => { + // Two fully distinct 63k-line screens produce a 126k-entry fallback + // middle; appending it must not use argument spreading, which would throw + // RangeError past V8's function-argument limit. + const a = Array.from({ length: 63_000 }, (_, i) => `a ${String(i)}`); + const b = Array.from({ length: 63_000 }, (_, i) => `b ${String(i)}`); + + const entries = diffLines(a, b); + + expect(entries).toHaveLength(126_000); + expect(entries[0]).toEqual({ op: 'delete', text: 'a 0', aRow: 0 }); + expect(entries[125_999]).toEqual({ + op: 'add', + text: 'b 62999', + bRow: 62_999, + }); + }); + it('degrades to a delete-then-add block when the middle exceeds the cell budget', () => { // Fully distinct 3000-line sides leave a middle whose DP table (~9M // cells) exceeds the 4M budget; the diff must degrade, not fail. From 3f6abaa1fdb110b79f1e57b653926805b02b4280 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:12:25 +0000 Subject: [PATCH 07/21] fix: tighten record diff schema contract - Model diff entries as a strict discriminated union keyed by op: equal requires both row indices, delete only aRow, add only bRow. - Refine the result schema so identical is true exactly when both screen hashes are equal, identical results carry an empty diff, and non-identical results carry at least one delete or add entry. --- src/protocol/schemas.ts | 62 +++++++++++++++++--- test/unit/protocol/messages.test.ts | 90 ++++++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 10 deletions(-) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index ebfcd87..4e12fd4 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -474,14 +474,30 @@ export const RecordExportResultSchema = z .strict(); export type RecordExportResult = z.infer; -export const RecordDiffLineSchema = z - .object({ - op: z.enum(['equal', 'delete', 'add']), - text: z.string(), - aRow: NonNegativeIntSchema.optional(), - bRow: NonNegativeIntSchema.optional(), - }) - .strict(); +export const RecordDiffLineSchema = z.discriminatedUnion('op', [ + z + .object({ + op: z.literal('equal'), + text: z.string(), + aRow: NonNegativeIntSchema, + bRow: NonNegativeIntSchema, + }) + .strict(), + z + .object({ + op: z.literal('delete'), + text: z.string(), + aRow: NonNegativeIntSchema, + }) + .strict(), + z + .object({ + op: z.literal('add'), + text: z.string(), + bRow: NonNegativeIntSchema, + }) + .strict(), +]); export type RecordDiffLine = z.infer; export const RecordDiffSideSchema = z @@ -502,7 +518,35 @@ export const RecordDiffResultSchema = z b: RecordDiffSideSchema, diff: z.array(RecordDiffLineSchema), }) - .strict(); + .strict() + .superRefine((value, ctx) => { + const hashesEqual = value.a.screenHash === value.b.screenHash; + if (value.identical !== hashesEqual) { + ctx.addIssue({ + code: 'custom', + message: + 'identical must be true exactly when both screen hashes are equal.', + path: ['identical'], + }); + } + + if (value.identical && value.diff.length > 0) { + ctx.addIssue({ + code: 'custom', + message: 'identical results must carry an empty diff.', + path: ['diff'], + }); + } + + if (!value.identical && !value.diff.some((entry) => entry.op !== 'equal')) { + ctx.addIssue({ + code: 'custom', + message: + 'non-identical results must carry at least one delete or add entry.', + path: ['diff'], + }); + } + }); export type RecordDiffResult = z.infer; export type WaitForRenderResult = z.infer; diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index f246789..75418fd 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -717,6 +717,7 @@ describe('RPC message schemas', () => { rows: 24, screenHash: 'a'.repeat(64), }; + const otherSide = { ...side, screenHash: 'b'.repeat(64) }; expect( RecordDiffResultSchema.safeParse({ identical: true, @@ -729,7 +730,7 @@ describe('RPC message schemas', () => { RecordDiffResultSchema.safeParse({ identical: false, a: side, - b: side, + b: otherSide, diff: [{ op: 'replace', text: 'x' }], }).success, ).toBe(false); @@ -737,7 +738,94 @@ describe('RPC message schemas', () => { RecordDiffResultSchema.safeParse({ identical: false, a: side, + b: otherSide, + }).success, + ).toBe(false); + }); + + it('requires per-op row fields on record diff entries', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 24, + screenHash: 'a'.repeat(64), + }; + const base = { + identical: false, + a: side, + b: { ...side, screenHash: 'b'.repeat(64) }, + }; + // equal entries require both rows; delete only aRow; add only bRow. + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [ + { op: 'equal', text: 'x' }, + { op: 'delete', text: 'old', aRow: 1 }, + ], + }).success, + ).toBe(false); + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [{ op: 'delete', text: 'old', aRow: 1, bRow: 0 }], + }).success, + ).toBe(false); + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [{ op: 'add', text: 'new', aRow: 0, bRow: 1 }], + }).success, + ).toBe(false); + }); + + it('rejects record diff results with contradictory identity invariants', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 24, + screenHash: 'a'.repeat(64), + }; + const otherSide = { ...side, screenHash: 'b'.repeat(64) }; + // identical: true with differing hashes. + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: side, + b: otherSide, + diff: [], + }).success, + ).toBe(false); + // identical: true with a non-empty diff. + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: side, b: side, + diff: [{ op: 'equal', text: 'x', aRow: 0, bRow: 0 }], + }).success, + ).toBe(false); + // identical: false with equal hashes. + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: side, + diff: [ + { op: 'delete', text: 'old', aRow: 0 }, + { op: 'add', text: 'new', bRow: 0 }, + ], + }).success, + ).toBe(false); + // identical: false without any delete/add entry. + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: otherSide, + diff: [{ op: 'equal', text: 'x', aRow: 0, bRow: 0 }], }).success, ).toBe(false); }); From 1146b266b200defab4e92018f0408fe4f795256d Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:24:18 +0000 Subject: [PATCH 08/21] fix: report capturedAtSeq -1 for empty-event-log diff sides The blank-screen fallback claimed capturedAtSeq 0, fabricating an event sequence that was never replayed (and that --at-seq 0 cannot target while the log is empty). Report -1, mirroring ReplayInput.targetSeq semantics for an empty log, and widen the side schema accordingly. --- docs/USAGE.md | 2 +- src/cli/commands/record-diff.ts | 3 ++- src/protocol/schemas.ts | 4 +++- test/unit/commands/record-diff.test.ts | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 242f7a9..8c78b06 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -216,7 +216,7 @@ agent-tty record diff --at-seq-a 0 --json ``` - `--at-seq-a ` / `--at-seq-b `: replay each side up to an Event Log sequence (default: latest). Diffing a session against itself at an earlier sequence shows how its screen evolved. -- The JSON result carries `identical`, per-side `sessionId`/`capturedAtSeq`/`cols`/`rows`/`screenHash`, and a `diff` array of `{ op: equal | delete | add, text, aRow?, bRow? }` entries over the visible screen lines (0-based rows, no trimming or normalization — the same canonical lines that `screenHash` hashes). +- The JSON result carries `identical`, per-side `sessionId`/`capturedAtSeq`/`cols`/`rows`/`screenHash`, and a `diff` array of `{ op: equal | delete | add, text, aRow?, bRow? }` entries over the visible screen lines (0-based rows, no trimming or normalization — the same canonical lines that `screenHash` hashes). A side with an empty event log reports the pre-event blank screen with `capturedAtSeq: -1` (no event was replayed). - Human output is a unified-style diff with `---`/`+++` headers naming each side's session, sequence, and hash prefix. - The command works entirely offline from `events.jsonl`; sessions may be running or exited. The exit code is `0` whether or not the screens differ — automation should read `identical` from the JSON result. diff --git a/src/cli/commands/record-diff.ts b/src/cli/commands/record-diff.ts index f5ab67d..77e159e 100644 --- a/src/cli/commands/record-diff.ts +++ b/src/cli/commands/record-diff.ts @@ -90,6 +90,7 @@ async function replayScreen( // The session has not emitted any events yet, so its screen is the // valid initial blank grid. Backends reject snapshot() before the // first replayed event, so synthesize the blank screen directly. + // capturedAtSeq -1 mirrors targetSeq: no event was replayed. const blankLines = Array.from( { length: replayInput.initialRows }, () => '', @@ -97,7 +98,7 @@ async function replayScreen( return { side: { sessionId, - capturedAtSeq: 0, + capturedAtSeq: -1, cols: replayInput.initialCols, rows: replayInput.initialRows, screenHash: computeScreenHash({ diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 4e12fd4..89c881d 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -503,7 +503,9 @@ export type RecordDiffLine = z.infer; export const RecordDiffSideSchema = z .object({ sessionId: NonEmptyStringSchema, - capturedAtSeq: NonNegativeIntSchema, + // -1 mirrors ReplayInput.targetSeq for an empty event log: the side is + // the pre-event blank screen and no event sequence was replayed. + capturedAtSeq: z.number().int().gte(-1), cols: PositiveIntSchema, rows: PositiveIntSchema, screenHash: Sha256HexSchema, diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts index 816b7d1..1e36a73 100644 --- a/test/unit/commands/record-diff.test.ts +++ b/test/unit/commands/record-diff.test.ts @@ -231,14 +231,14 @@ describe('runRecordDiffCommand', () => { identical: true, a: { sessionId: 'session-a', - capturedAtSeq: 0, + capturedAtSeq: -1, cols: 80, rows: 24, screenHash: blankHash, }, b: { sessionId: 'session-b', - capturedAtSeq: 0, + capturedAtSeq: -1, cols: 80, rows: 24, screenHash: blankHash, From 8d1cc3fa6b47fa3eb7511a09bfe09c78e629f62e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:38:21 +0000 Subject: [PATCH 09/21] fix: address record diff round-6 review findings - Parse --at-seq-a/--at-seq-b with a strict integer-token parser so empty or whitespace-only values (e.g. an unset shell variable) are rejected as INVALID_INPUT instead of resolving to sequence 0. - Add an explicit phase boundary to the scrollback-demo fixture so the completion marker lands in its own PTY chunk, keeping the record diff e2e assertion deterministic regardless of PTY chunk coalescing. --- src/cli/main.ts | 12 ++++++++++-- test/fixtures/apps/scrollback-demo/main.ts | 14 ++++++++++---- test/integration/record-diff.test.ts | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/cli/main.ts b/src/cli/main.ts index b63d7bc..98413b9 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -60,6 +60,14 @@ function parseNumberOption(value: string): number { return Number(value); } +// Strict integer-token parser: empty, whitespace-only, fractional, or +// partially numeric tokens (e.g. "", "1.5", "2junk") yield NaN so command +// validation rejects them instead of silently truncating. +function parseIntegerTokenOption(value: string): number { + const token = value.trim(); + return /^[+-]?\d+$/.test(token) ? Number.parseInt(token, 10) : Number.NaN; +} + function collectStringOption(value: string, previous: string[] = []): string[] { return [...previous, value]; } @@ -807,12 +815,12 @@ async function main(): Promise { .option( '--at-seq-a ', 'Replay session A up to this Event Log sequence (default: latest)', - parseNumberOption, + parseIntegerTokenOption, ) .option( '--at-seq-b ', 'Replay session B up to this Event Log sequence (default: latest)', - parseNumberOption, + parseIntegerTokenOption, ) .option('--json', 'Emit a JSON command envelope', false) .action( diff --git a/test/fixtures/apps/scrollback-demo/main.ts b/test/fixtures/apps/scrollback-demo/main.ts index 21d9811..8e461e8 100644 --- a/test/fixtures/apps/scrollback-demo/main.ts +++ b/test/fixtures/apps/scrollback-demo/main.ts @@ -2,6 +2,10 @@ import assert from 'node:assert/strict'; import process from 'node:process'; const HOLD_OPEN_MS = 1_200; +// Pause between the scroll phase and the completion marker so the marker +// lands in a separate PTY chunk (= separate Event Log output event). Tests +// that replay to an intermediate sequence rely on this phase boundary. +const PHASE_BOUNDARY_MS = 150; const LINE_SUFFIX = 'abcdefghijklmnopqrstuvwxyz'; const LINE_COUNT = 80; @@ -16,8 +20,10 @@ for (let i = 1; i <= LINE_COUNT; i += 1) { process.stdout.write(`LINE ${String(i).padStart(3, '0')} | ${LINE_SUFFIX}\n`); } -process.stdout.write('SCROLLBACK COMPLETE\n'); - setTimeout(() => { - process.exit(0); -}, HOLD_OPEN_MS); + process.stdout.write('SCROLLBACK COMPLETE\n'); + + setTimeout(() => { + process.exit(0); + }, HOLD_OPEN_MS); +}, PHASE_BOUNDARY_MS); diff --git a/test/integration/record-diff.test.ts b/test/integration/record-diff.test.ts index 2a972ca..7af9d78 100644 --- a/test/integration/record-diff.test.ts +++ b/test/integration/record-diff.test.ts @@ -203,7 +203,7 @@ describe('record diff integration', { timeout: 120_000 }, () => { const sessionA = createSession(testHome, command); waitForExit(testHome, sessionA); - for (const token of ['1.5', '2junk']) { + for (const token of ['1.5', '2junk', '', ' ']) { const result = runRecordDiff(testHome, [ sessionA, sessionA, From 9c3450a92bcb5e966bf2d28cbbf17a7c416ec1c0 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:48:07 +0000 Subject: [PATCH 10/21] fix: bound record diff row coordinates by side dimensions Diff entries index the padded visible screens, so aRow/bRow are always less than the corresponding side's rows; refine the schema to reject out-of-bounds coordinates. --- src/protocol/schemas.ts | 19 +++++++++++++ test/unit/protocol/messages.test.ts | 41 +++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 89c881d..4cfb5b5 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -548,6 +548,25 @@ export const RecordDiffResultSchema = z path: ['diff'], }); } + + // Diff rows index the padded visible screens, so they are bounded by + // each side's row count. + for (const [index, entry] of value.diff.entries()) { + if (entry.op !== 'add' && entry.aRow >= value.a.rows) { + ctx.addIssue({ + code: 'custom', + message: 'aRow must be less than side a rows.', + path: ['diff', index, 'aRow'], + }); + } + if (entry.op !== 'delete' && entry.bRow >= value.b.rows) { + ctx.addIssue({ + code: 'custom', + message: 'bRow must be less than side b rows.', + path: ['diff', index, 'bRow'], + }); + } + } }); export type RecordDiffResult = z.infer; diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 75418fd..08dae01 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -780,6 +780,47 @@ describe('RPC message schemas', () => { ).toBe(false); }); + it('rejects record diff entries with out-of-bounds row coordinates', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 24, + screenHash: 'a'.repeat(64), + }; + const base = { + identical: false, + a: side, + b: { ...side, screenHash: 'b'.repeat(64) }, + }; + // aRow == a.rows is out of bounds for the padded visible screen. + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [{ op: 'delete', text: 'old', aRow: 24 }], + }).success, + ).toBe(false); + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [ + { op: 'equal', text: 'x', aRow: 0, bRow: 24 }, + { op: 'add', text: 'new', bRow: 0 }, + ], + }).success, + ).toBe(false); + // In-bounds rows parse. + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [ + { op: 'delete', text: 'old', aRow: 23 }, + { op: 'add', text: 'new', bRow: 23 }, + ], + }).success, + ).toBe(true); + }); + it('rejects record diff results with contradictory identity invariants', () => { const side = { sessionId: 'session-01', From 4cf22c5955dfd6ef1455c9443e174fe965830789 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 11:56:14 +0000 Subject: [PATCH 11/21] fix: require record diff rows to enumerate both screens in order The diff is a complete traversal of both padded visible screens, so the schema now requires participating A and B coordinates to each enumerate 0..rows-1 exactly once in order, letting consumers reconstruct either side by filtering operations. --- src/protocol/schemas.ts | 46 +++++++++++++++++++------- test/unit/commands/record-diff.test.ts | 14 ++++++-- test/unit/protocol/messages.test.ts | 41 +++++++++++++++++------ 3 files changed, 75 insertions(+), 26 deletions(-) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 4cfb5b5..b2d86a2 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -549,21 +549,43 @@ export const RecordDiffResultSchema = z }); } - // Diff rows index the padded visible screens, so they are bounded by - // each side's row count. - for (const [index, entry] of value.diff.entries()) { - if (entry.op !== 'add' && entry.aRow >= value.a.rows) { - ctx.addIssue({ - code: 'custom', - message: 'aRow must be less than side a rows.', - path: ['diff', index, 'aRow'], - }); + // The diff is a complete traversal of both padded visible screens: + // participating A coordinates (equal/delete) and B coordinates + // (equal/add) must each enumerate 0..rows-1 exactly once, in order, so + // consumers can reconstruct either side by filtering operations. + if (!value.identical) { + let nextARow = 0; + let nextBRow = 0; + for (const [index, entry] of value.diff.entries()) { + if (entry.op !== 'add') { + if (entry.aRow !== nextARow) { + ctx.addIssue({ + code: 'custom', + message: `aRow must enumerate side a rows in order (expected ${String(nextARow)}).`, + path: ['diff', index, 'aRow'], + }); + return; + } + nextARow += 1; + } + if (entry.op !== 'delete') { + if (entry.bRow !== nextBRow) { + ctx.addIssue({ + code: 'custom', + message: `bRow must enumerate side b rows in order (expected ${String(nextBRow)}).`, + path: ['diff', index, 'bRow'], + }); + return; + } + nextBRow += 1; + } } - if (entry.op !== 'delete' && entry.bRow >= value.b.rows) { + if (nextARow !== value.a.rows || nextBRow !== value.b.rows) { ctx.addIssue({ code: 'custom', - message: 'bRow must be less than side b rows.', - path: ['diff', index, 'bRow'], + message: + 'diff must cover every row of both sides exactly once (0..rows-1).', + path: ['diff'], }); } } diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts index 1e36a73..6e80724 100644 --- a/test/unit/commands/record-diff.test.ts +++ b/test/unit/commands/record-diff.test.ts @@ -71,9 +71,17 @@ function mockReplaySnapshots(fixtures: SnapshotFixture[]): void { if (fixture === undefined) { throw new Error('unexpected extra replay call'); } + // Production snapshots pad visibleLines to exactly `rows`; mirror that + // so the emitted diff enumerates complete screens. return await run({ backend: { - snapshot: () => Promise.resolve(createTestSemanticSnapshot(fixture)), + snapshot: () => + Promise.resolve( + createTestSemanticSnapshot({ + ...fixture, + rows: fixture.visibleLines.length, + }), + ), }, replayInput: { targetSeq: fixture.capturedAtSeq, @@ -155,14 +163,14 @@ describe('runRecordDiffCommand', () => { sessionId: 'session-a', capturedAtSeq: 4, cols: 80, - rows: 24, + rows: 2, screenHash: expectedHash, }, b: { sessionId: 'session-b', capturedAtSeq: 9, cols: 80, - rows: 24, + rows: 2, screenHash: expectedHash, }, diff: [], diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 08dae01..31fc7a4 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -684,7 +684,7 @@ describe('RPC message schemas', () => { sessionId: 'session-01', capturedAtSeq: 7, cols: 80, - rows: 24, + rows: 2, screenHash: 'a'.repeat(64), }; expect( @@ -780,12 +780,12 @@ describe('RPC message schemas', () => { ).toBe(false); }); - it('rejects record diff entries with out-of-bounds row coordinates', () => { + it('requires record diff rows to enumerate both screens in order', () => { const side = { sessionId: 'session-01', capturedAtSeq: 7, cols: 80, - rows: 24, + rows: 2, screenHash: 'a'.repeat(64), }; const base = { @@ -793,32 +793,51 @@ describe('RPC message schemas', () => { a: side, b: { ...side, screenHash: 'b'.repeat(64) }, }; - // aRow == a.rows is out of bounds for the padded visible screen. + // Complete ordered enumeration of both 2-row screens parses. + expect( + RecordDiffResultSchema.safeParse({ + ...base, + diff: [ + { op: 'equal', text: 'shared', aRow: 0, bRow: 0 }, + { op: 'delete', text: 'old', aRow: 1 }, + { op: 'add', text: 'new', bRow: 1 }, + ], + }).success, + ).toBe(true); + // Missing rows (screens not fully covered) are rejected. expect( RecordDiffResultSchema.safeParse({ ...base, - diff: [{ op: 'delete', text: 'old', aRow: 24 }], + diff: [ + { op: 'delete', text: 'old', aRow: 0 }, + { op: 'add', text: 'new', bRow: 0 }, + ], }).success, ).toBe(false); + // Out-of-order coordinates are rejected. expect( RecordDiffResultSchema.safeParse({ ...base, diff: [ - { op: 'equal', text: 'x', aRow: 0, bRow: 24 }, + { op: 'delete', text: 'old', aRow: 1 }, + { op: 'delete', text: 'older', aRow: 0 }, { op: 'add', text: 'new', bRow: 0 }, + { op: 'add', text: 'newer', bRow: 1 }, ], }).success, ).toBe(false); - // In-bounds rows parse. + // Duplicate coordinates are rejected. expect( RecordDiffResultSchema.safeParse({ ...base, diff: [ - { op: 'delete', text: 'old', aRow: 23 }, - { op: 'add', text: 'new', bRow: 23 }, + { op: 'delete', text: 'old', aRow: 0 }, + { op: 'delete', text: 'older', aRow: 0 }, + { op: 'add', text: 'new', bRow: 0 }, + { op: 'add', text: 'newer', bRow: 1 }, ], }).success, - ).toBe(true); + ).toBe(false); }); it('rejects record diff results with contradictory identity invariants', () => { @@ -826,7 +845,7 @@ describe('RPC message schemas', () => { sessionId: 'session-01', capturedAtSeq: 7, cols: 80, - rows: 24, + rows: 1, screenHash: 'a'.repeat(64), }; const otherSide = { ...side, screenHash: 'b'.repeat(64) }; From 606810518119d05c296ccdc959cb6b1b70a6324e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:06:57 +0000 Subject: [PATCH 12/21] fix: accept every input size in diffLines Remove the per-side line cap: prefix/suffix trimming and the cell- budget fallback already bound the quadratic DP table, and all other work is linear, so no valid screen dimensions can crash record diff with an uncaught AssertionError. --- src/util/lineDiff.ts | 12 +++--------- test/unit/util/lineDiff.test.ts | 11 ++++++++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts index e2a5408..7928e69 100644 --- a/src/util/lineDiff.ts +++ b/src/util/lineDiff.ts @@ -16,12 +16,11 @@ export interface LineDiffEntry { readonly bRow?: number; } -const MAX_DIFF_LINES = 100_000; // Bounds the DP table's cell count (each cell is one JS number), keeping // worst-case memory in the tens of megabytes. When the middle section (after // common prefix/suffix trimming) would exceed this, the diff degrades to a // non-minimal delete-then-add block instead of failing: every valid screen -// pair diffs successfully. +// pair diffs successfully, so no input size is rejected. const MAX_DIFF_CELLS = 4_000_000; function lcsEntries( @@ -114,18 +113,13 @@ function fallbackEntries( * emitted first. Common prefix and suffix lines are matched directly, so the * quadratic DP table only covers the differing middle; if that middle is * still larger than MAX_DIFF_CELLS the middle degrades to a non-minimal - * delete-then-add block rather than failing. Terminal screens are far below - * every limit in practice. + * delete-then-add block rather than failing. Every input size is accepted: + * work and memory outside the capped DP table are linear in the inputs. */ export function diffLines( a: readonly string[], b: readonly string[], ): LineDiffEntry[] { - invariant( - a.length <= MAX_DIFF_LINES && b.length <= MAX_DIFF_LINES, - `diffLines inputs must not exceed ${String(MAX_DIFF_LINES)} lines`, - ); - let prefix = 0; while (prefix < a.length && prefix < b.length && a[prefix] === b[prefix]) { prefix += 1; diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts index 59a7627..2a236f7 100644 --- a/test/unit/util/lineDiff.test.ts +++ b/test/unit/util/lineDiff.test.ts @@ -67,9 +67,14 @@ describe('diffLines', () => { ]); }); - it('rejects inputs beyond the line limit', () => { - const big = new Array(100_001).fill('x'); - expect(() => diffLines(big, [])).toThrow(/must not exceed 100000 lines/); + it('accepts arbitrarily large single-sided inputs linearly', () => { + // No per-side cap: a one-sided diff needs no DP table and must not throw. + const big = new Array(150_000).fill('x'); + + const entries = diffLines(big, []); + + expect(entries).toHaveLength(150_000); + expect(entries.every((entry) => entry.op === 'delete')).toBe(true); }); it('matches large common prefixes and suffixes without a quadratic table', () => { From adc9c235e23a5d30bae6607d9332e11d1aec77cc Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:14:51 +0000 Subject: [PATCH 13/21] fix: verify record diff hashes against reconstructed screens - The result schema now reconstructs both sides from the diff traversal and requires each declared screenHash to match the reconstruction, so the diff, hashes, and identical flag can never contradict each other after successful validation. - Reject --at-seq tokens beyond Number.MAX_SAFE_INTEGER instead of silently rounding them to a different sequence. --- src/cli/main.ts | 12 ++++--- src/protocol/schemas.ts | 40 +++++++++++++++++------ test/unit/protocol/messages.test.ts | 49 ++++++++++++++++++++++++++--- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/cli/main.ts b/src/cli/main.ts index 98413b9..68b80c9 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -60,12 +60,16 @@ function parseNumberOption(value: string): number { return Number(value); } -// Strict integer-token parser: empty, whitespace-only, fractional, or -// partially numeric tokens (e.g. "", "1.5", "2junk") yield NaN so command -// validation rejects them instead of silently truncating. +// Strict integer-token parser: empty, whitespace-only, fractional, partially +// numeric (e.g. "", "1.5", "2junk"), or unsafe-magnitude tokens yield NaN so +// command validation rejects them instead of silently truncating or rounding. function parseIntegerTokenOption(value: string): number { const token = value.trim(); - return /^[+-]?\d+$/.test(token) ? Number.parseInt(token, 10) : Number.NaN; + if (!/^[+-]?\d+$/.test(token)) { + return Number.NaN; + } + const parsed = Number.parseInt(token, 10); + return Number.isSafeInteger(parsed) ? parsed : Number.NaN; } function collectStringOption(value: string, previous: string[] = []): string[] { diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index b2d86a2..94c3d41 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -5,6 +5,7 @@ import { MAX_WAIT_FOR_RENDER_TEXT_LENGTH, } from '../renderWait/limits.js'; import { RendererNameSchema } from '../renderer/names.js'; +import { sha256Hex } from '../util/hash.js'; const NonEmptyStringSchema = z.string().min(1); const TextMatchSchema = z.string().min(1).max(MAX_WAIT_FOR_RENDER_TEXT_LENGTH); @@ -554,39 +555,60 @@ export const RecordDiffResultSchema = z // (equal/add) must each enumerate 0..rows-1 exactly once, in order, so // consumers can reconstruct either side by filtering operations. if (!value.identical) { - let nextARow = 0; - let nextBRow = 0; + const aLines: string[] = []; + const bLines: string[] = []; for (const [index, entry] of value.diff.entries()) { if (entry.op !== 'add') { - if (entry.aRow !== nextARow) { + if (entry.aRow !== aLines.length) { ctx.addIssue({ code: 'custom', - message: `aRow must enumerate side a rows in order (expected ${String(nextARow)}).`, + message: `aRow must enumerate side a rows in order (expected ${String(aLines.length)}).`, path: ['diff', index, 'aRow'], }); return; } - nextARow += 1; + aLines.push(entry.text); } if (entry.op !== 'delete') { - if (entry.bRow !== nextBRow) { + if (entry.bRow !== bLines.length) { ctx.addIssue({ code: 'custom', - message: `bRow must enumerate side b rows in order (expected ${String(nextBRow)}).`, + message: `bRow must enumerate side b rows in order (expected ${String(bLines.length)}).`, path: ['diff', index, 'bRow'], }); return; } - nextBRow += 1; + bLines.push(entry.text); } } - if (nextARow !== value.a.rows || nextBRow !== value.b.rows) { + if (aLines.length !== value.a.rows || bLines.length !== value.b.rows) { ctx.addIssue({ code: 'custom', message: 'diff must cover every row of both sides exactly once (0..rows-1).', path: ['diff'], }); + return; + } + + // The reconstructed sides must hash to the declared screen hashes, so + // the diff, the hashes, and `identical` can never contradict each + // other after successful validation. + if (sha256Hex(aLines.join('\n')) !== value.a.screenHash) { + ctx.addIssue({ + code: 'custom', + message: + 'side a screenHash must match the screen reconstructed from the diff.', + path: ['a', 'screenHash'], + }); + } + if (sha256Hex(bLines.join('\n')) !== value.b.screenHash) { + ctx.addIssue({ + code: 'custom', + message: + 'side b screenHash must match the screen reconstructed from the diff.', + path: ['b', 'screenHash'], + }); } } }); diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 31fc7a4..9c285e0 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest'; +import { sha256Hex } from '../../../src/util/hash.js'; + import { CapabilityEntrySchema, DestroyParamsSchema, @@ -698,8 +700,12 @@ describe('RPC message schemas', () => { expect( RecordDiffResultSchema.safeParse({ identical: false, - a: side, - b: { ...side, screenHash: 'b'.repeat(64) }, + a: { ...side, screenHash: sha256Hex('shared\nold') }, + b: { + ...side, + sessionId: 'session-02', + screenHash: sha256Hex('shared\nnew'), + }, diff: [ { op: 'equal', text: 'shared', aRow: 0, bRow: 0 }, { op: 'delete', text: 'old', aRow: 1 }, @@ -790,8 +796,8 @@ describe('RPC message schemas', () => { }; const base = { identical: false, - a: side, - b: { ...side, screenHash: 'b'.repeat(64) }, + a: { ...side, screenHash: sha256Hex('shared\nold') }, + b: { ...side, screenHash: sha256Hex('shared\nnew') }, }; // Complete ordered enumeration of both 2-row screens parses. expect( @@ -840,6 +846,41 @@ describe('RPC message schemas', () => { ).toBe(false); }); + it('rejects record diff results whose hashes contradict the diff text', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: 7, + cols: 80, + rows: 1, + screenHash: sha256Hex('same'), + }; + // Reconstructing both sides yields 'same', so differing declared hashes + // (and identical: false) contradict the diff content. + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: side, + b: { ...side, screenHash: 'b'.repeat(64) }, + diff: [ + { op: 'delete', text: 'same', aRow: 0 }, + { op: 'add', text: 'same', bRow: 0 }, + ], + }).success, + ).toBe(false); + // Consistent hashes for genuinely different one-row screens parse. + expect( + RecordDiffResultSchema.safeParse({ + identical: false, + a: { ...side, screenHash: sha256Hex('old') }, + b: { ...side, screenHash: sha256Hex('new') }, + diff: [ + { op: 'delete', text: 'old', aRow: 0 }, + { op: 'add', text: 'new', bRow: 0 }, + ], + }).success, + ).toBe(true); + }); + it('rejects record diff results with contradictory identity invariants', () => { const side = { sessionId: 'session-01', From 71ad7a908032494fe4674541d8bb3b5b3a48958c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:20:54 +0000 Subject: [PATCH 14/21] fix: require equal row counts for identical record diff results Equal screen hashes imply equal canonical line sequences, which have one line per padded visible row, so identical results cannot report different side dimensions. --- src/protocol/schemas.ts | 10 ++++++++++ test/unit/protocol/messages.test.ts | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 94c3d41..2da7469 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -541,6 +541,16 @@ export const RecordDiffResultSchema = z }); } + // Equal screen hashes imply equal canonical line sequences, which have + // one line per padded visible row. + if (value.identical && value.a.rows !== value.b.rows) { + ctx.addIssue({ + code: 'custom', + message: 'identical results must have equal side row counts.', + path: ['b', 'rows'], + }); + } + if (!value.identical && !value.diff.some((entry) => entry.op !== 'equal')) { ctx.addIssue({ code: 'custom', diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 9c285e0..218af49 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -929,6 +929,16 @@ describe('RPC message schemas', () => { diff: [{ op: 'equal', text: 'x', aRow: 0, bRow: 0 }], }).success, ).toBe(false); + // identical: true with mismatched row counts (equal hashes imply equal + // canonical line counts). + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: side, + b: { ...side, rows: 2 }, + diff: [], + }).success, + ).toBe(false); }); it('accepts valid record export results', () => { From 31e0f5a6f945718529689cd69cc4cc4346e3cae3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:33:08 +0000 Subject: [PATCH 15/21] fix: address record diff round-12 review findings - Refine the schema so a pre-event side (capturedAtSeq -1) must hash to a blank screen of its declared row count. - Replace the scrollback-demo wall-clock phase pause with an opt-in stdin handshake (--wait-input-before-complete): the record diff e2e now waits for an observed pre-completion Event Log state, releases the fixture, and diffs against that sequence, eliminating the PTY chunk-coalescing race entirely. --- src/protocol/schemas.ts | 17 ++++ test/e2e/scrollback-demo.test.ts | 98 +++++++++++++++++----- test/fixtures/apps/scrollback-demo/main.ts | 20 +++-- test/unit/protocol/messages.test.ts | 28 +++++++ 4 files changed, 137 insertions(+), 26 deletions(-) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 2da7469..0b358c9 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -551,6 +551,23 @@ export const RecordDiffResultSchema = z }); } + // capturedAtSeq -1 marks the pre-event blank screen, so such a side must + // hash to `rows` empty canonical lines. + for (const sideKey of ['a', 'b'] as const) { + const side = value[sideKey]; + if ( + side.capturedAtSeq === -1 && + side.screenHash !== sha256Hex('\n'.repeat(side.rows - 1)) + ) { + ctx.addIssue({ + code: 'custom', + message: + 'a pre-event side (capturedAtSeq -1) must hash to a blank screen.', + path: [sideKey, 'screenHash'], + }); + } + } + if (!value.identical && !value.diff.some((entry) => entry.op !== 'equal')) { ctx.addIssue({ code: 'custom', diff --git a/test/e2e/scrollback-demo.test.ts b/test/e2e/scrollback-demo.test.ts index 195049c..bfbb018 100644 --- a/test/e2e/scrollback-demo.test.ts +++ b/test/e2e/scrollback-demo.test.ts @@ -6,6 +6,7 @@ import type { RecordDiffResult, ScreenshotResult, SnapshotResult, + WaitForRenderResult, } from '../../src/protocol/messages.js'; import { cleanupHome, @@ -129,26 +130,6 @@ describe('scrollback-demo e2e', { timeout: 60_000 }, () => { expect(scrollbackLines).toBeDefined(); expect(scrollbackLines?.length).toBeGreaterThan(0); - // `record diff` against the session's own first event proves the viewport - // scrolled: the final screen (equal + add entries) gained the completion - // marker and no longer shows the first line. - const diffEnvelope = runCliJson>( - ['record', 'diff', sessionId, sessionId, '--at-seq-a', '0'], - env, - ); - expect(diffEnvelope.ok).toBe(true); - expect(diffEnvelope.command).toBe('record diff'); - expect(diffEnvelope.result.identical).toBe(false); - const finalScreenLines = diffEnvelope.result.diff - .filter((entry) => entry.op !== 'delete') - .map((entry) => entry.text); - expect( - finalScreenLines.some((line) => line.includes('SCROLLBACK COMPLETE')), - ).toBe(true); - expect(finalScreenLines.some((line) => line.includes('LINE 001'))).toBe( - false, - ); - const screenshotEnvelope = runCliJson>( ['screenshot', sessionId], env, @@ -167,4 +148,81 @@ describe('scrollback-demo e2e', { timeout: 60_000 }, () => { ); expect(screenshotBytes.subarray(0, 8).toString('hex')).toBe(PNG_MAGIC_HEX); }); + + it('diffs the scrolled viewport against its pre-completion state', () => { + const env = testEnv(testHome); + // The stdin handshake keeps the fixture blocked before the completion + // marker, so the wait below observes an ingested Event Log state that is + // guaranteed to precede the marker — no PTY chunk-coalescing race. + const createEnvelope = runCliJson>( + [ + 'create', + '--rows', + '10', + '--cols', + '80', + '--', + ...fixtureCommand('scrollback-demo'), + '--wait-input-before-complete', + ], + env, + ); + expect(createEnvelope.ok).toBe(true); + const sessionId = createEnvelope.result.sessionId; + createdSessionIds.push(sessionId); + + const waitEnvelope = runCliJson>( + ['wait', sessionId, '--text', 'LINE 080', '--timeout', '15000'], + env, + ); + expect(waitEnvelope.ok).toBe(true); + expect(waitEnvelope.result.matched).toBe(true); + const preCompletionSeq = waitEnvelope.result.capturedAtSeq; + + const releaseEnvelope = runCliJson>( + ['send-keys', sessionId, 'Enter'], + env, + ); + expect(releaseEnvelope.ok).toBe(true); + + const exitEnvelope = runCliJson>( + ['wait', sessionId, '--exit', '--timeout', String(EXIT_WAIT_TIMEOUT_MS)], + env, + ); + expect(exitEnvelope.ok).toBe(true); + expect(exitEnvelope.result.exitCode).toBe(0); + + // `record diff` against the observed pre-completion sequence proves the + // viewport scrolled: the final screen (equal + add entries) gained the + // completion marker and no longer shows the first line. + const diffEnvelope = runCliJson>( + [ + 'record', + 'diff', + sessionId, + sessionId, + '--at-seq-a', + String(preCompletionSeq), + ], + env, + ); + expect(diffEnvelope.ok).toBe(true); + expect(diffEnvelope.command).toBe('record diff'); + expect(diffEnvelope.result.identical).toBe(false); + const preCompletionLines = diffEnvelope.result.diff + .filter((entry) => entry.op !== 'add') + .map((entry) => entry.text); + expect( + preCompletionLines.some((line) => line.includes('SCROLLBACK COMPLETE')), + ).toBe(false); + const finalScreenLines = diffEnvelope.result.diff + .filter((entry) => entry.op !== 'delete') + .map((entry) => entry.text); + expect( + finalScreenLines.some((line) => line.includes('SCROLLBACK COMPLETE')), + ).toBe(true); + expect(finalScreenLines.some((line) => line.includes('LINE 001'))).toBe( + false, + ); + }); }); diff --git a/test/fixtures/apps/scrollback-demo/main.ts b/test/fixtures/apps/scrollback-demo/main.ts index 8e461e8..66eff4d 100644 --- a/test/fixtures/apps/scrollback-demo/main.ts +++ b/test/fixtures/apps/scrollback-demo/main.ts @@ -2,12 +2,13 @@ import assert from 'node:assert/strict'; import process from 'node:process'; const HOLD_OPEN_MS = 1_200; -// Pause between the scroll phase and the completion marker so the marker -// lands in a separate PTY chunk (= separate Event Log output event). Tests -// that replay to an intermediate sequence rely on this phase boundary. -const PHASE_BOUNDARY_MS = 150; const LINE_SUFFIX = 'abcdefghijklmnopqrstuvwxyz'; const LINE_COUNT = 80; +// With --wait-input-before-complete, the fixture blocks until it receives any +// stdin byte before printing the completion marker. Tests that must observe +// an intermediate Event Log state first (e.g. record diff replays) use this +// handshake so the marker cannot coalesce into an earlier PTY chunk. +const waitForInput = process.argv.includes('--wait-input-before-complete'); assert( process.stdout.writable, @@ -20,10 +21,17 @@ for (let i = 1; i <= LINE_COUNT; i += 1) { process.stdout.write(`LINE ${String(i).padStart(3, '0')} | ${LINE_SUFFIX}\n`); } -setTimeout(() => { +function finish(): void { process.stdout.write('SCROLLBACK COMPLETE\n'); setTimeout(() => { process.exit(0); }, HOLD_OPEN_MS); -}, PHASE_BOUNDARY_MS); +} + +if (waitForInput) { + process.stdin.once('data', finish); + process.stdin.resume(); +} else { + finish(); +} diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 218af49..a5e4ee9 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -846,6 +846,34 @@ describe('RPC message schemas', () => { ).toBe(false); }); + it('requires pre-event record diff sides to hash to a blank screen', () => { + const blank = { + sessionId: 'session-01', + capturedAtSeq: -1, + cols: 80, + rows: 3, + screenHash: sha256Hex('\n\n'), + }; + // A pre-event side with the correct blank hash parses. + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: blank, + b: { ...blank, sessionId: 'session-02' }, + diff: [], + }).success, + ).toBe(true); + // A pre-event side whose hash is not the blank-screen hash is rejected. + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: { ...blank, screenHash: sha256Hex('not blank\n\n') }, + b: { ...blank, screenHash: sha256Hex('not blank\n\n') }, + diff: [], + }).success, + ).toBe(false); + }); + it('rejects record diff results whose hashes contradict the diff text', () => { const side = { sessionId: 'session-01', From 45dfeda8c00105a428999c9289d09bd8901021bc Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:41:09 +0000 Subject: [PATCH 16/21] fix: bound record diff side dimensions in the schema Cap cols/rows at a generous ceiling so validation-time work (such as hashing the blank pre-event screen) is proportional to a schema-checked limit instead of attacker-controlled input. --- src/protocol/schemas.ts | 10 ++++++++-- test/unit/protocol/messages.test.ts | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index 0b358c9..ee7e26f 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -501,14 +501,20 @@ export const RecordDiffLineSchema = z.discriminatedUnion('op', [ ]); export type RecordDiffLine = z.infer; +// Generous ceiling on diffable screen dimensions; real terminals are far +// below it. Bounding rows keeps validation-time work (e.g. hashing the +// blank pre-event screen) proportional to a schema-checked limit instead of +// attacker-controlled input. +const MAX_RECORD_DIFF_DIMENSION = 100_000; + export const RecordDiffSideSchema = z .object({ sessionId: NonEmptyStringSchema, // -1 mirrors ReplayInput.targetSeq for an empty event log: the side is // the pre-event blank screen and no event sequence was replayed. capturedAtSeq: z.number().int().gte(-1), - cols: PositiveIntSchema, - rows: PositiveIntSchema, + cols: PositiveIntSchema.lte(MAX_RECORD_DIFF_DIMENSION), + rows: PositiveIntSchema.lte(MAX_RECORD_DIFF_DIMENSION), screenHash: Sha256HexSchema, }) .strict(); diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index a5e4ee9..2d3940a 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -846,6 +846,25 @@ describe('RPC message schemas', () => { ).toBe(false); }); + it('bounds record diff side dimensions', () => { + const side = { + sessionId: 'session-01', + capturedAtSeq: -1, + cols: 80, + rows: 100_001, + screenHash: 'a'.repeat(64), + }; + // Oversized rows fail schema validation before any blank-screen hashing. + expect( + RecordDiffResultSchema.safeParse({ + identical: true, + a: side, + b: side, + diff: [], + }).success, + ).toBe(false); + }); + it('requires pre-event record diff sides to hash to a blank screen', () => { const blank = { sessionId: 'session-01', From ebe0ae9cdbf2758ccb50126f1ee84b9db601da82 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:47:55 +0000 Subject: [PATCH 17/21] fix: keep record diff dimensions unrestricted, bound only hash work Sessions may be created or resized to any positive dimensions, so the side schema must not narrow the public contract. Drop the cols/rows cap and instead skip the pre-event blank-hash equality check above a 100k-row work bound, keeping safeParse cost bounded without rejecting contract-valid results. --- src/protocol/schemas.ts | 19 +++++++++++-------- test/unit/protocol/messages.test.ts | 12 ++++++++---- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/protocol/schemas.ts b/src/protocol/schemas.ts index ee7e26f..784e2c9 100644 --- a/src/protocol/schemas.ts +++ b/src/protocol/schemas.ts @@ -501,11 +501,11 @@ export const RecordDiffLineSchema = z.discriminatedUnion('op', [ ]); export type RecordDiffLine = z.infer; -// Generous ceiling on diffable screen dimensions; real terminals are far -// below it. Bounding rows keeps validation-time work (e.g. hashing the -// blank pre-event screen) proportional to a schema-checked limit instead of -// attacker-controlled input. -const MAX_RECORD_DIFF_DIMENSION = 100_000; +// Work bound for validation-time blank-screen hashing only. Dimensions are +// deliberately NOT capped (the session contract accepts any positive size); +// above this row count the pre-event blank-hash equality check is skipped so +// safeParse never performs unbounded work on attacker-controlled input. +const MAX_BLANK_HASH_ROWS = 100_000; export const RecordDiffSideSchema = z .object({ @@ -513,8 +513,8 @@ export const RecordDiffSideSchema = z // -1 mirrors ReplayInput.targetSeq for an empty event log: the side is // the pre-event blank screen and no event sequence was replayed. capturedAtSeq: z.number().int().gte(-1), - cols: PositiveIntSchema.lte(MAX_RECORD_DIFF_DIMENSION), - rows: PositiveIntSchema.lte(MAX_RECORD_DIFF_DIMENSION), + cols: PositiveIntSchema, + rows: PositiveIntSchema, screenHash: Sha256HexSchema, }) .strict(); @@ -558,11 +558,14 @@ export const RecordDiffResultSchema = z } // capturedAtSeq -1 marks the pre-event blank screen, so such a side must - // hash to `rows` empty canonical lines. + // hash to `rows` empty canonical lines. Skipped above the work bound so + // validation cost stays bounded for arbitrarily large (but contract- + // valid) dimensions. for (const sideKey of ['a', 'b'] as const) { const side = value[sideKey]; if ( side.capturedAtSeq === -1 && + side.rows <= MAX_BLANK_HASH_ROWS && side.screenHash !== sha256Hex('\n'.repeat(side.rows - 1)) ) { ctx.addIssue({ diff --git a/test/unit/protocol/messages.test.ts b/test/unit/protocol/messages.test.ts index 2d3940a..808ecda 100644 --- a/test/unit/protocol/messages.test.ts +++ b/test/unit/protocol/messages.test.ts @@ -846,15 +846,18 @@ describe('RPC message schemas', () => { ).toBe(false); }); - it('bounds record diff side dimensions', () => { + it('accepts huge session dimensions without unbounded validation work', () => { + // Dimensions accepted by the session contract must validate here too; + // above the work bound the blank-hash equality check is skipped, so this + // parses quickly regardless of the declared hash. const side = { sessionId: 'session-01', capturedAtSeq: -1, cols: 80, - rows: 100_001, + rows: 1_000_000_000, screenHash: 'a'.repeat(64), }; - // Oversized rows fail schema validation before any blank-screen hashing. + const started = Date.now(); expect( RecordDiffResultSchema.safeParse({ identical: true, @@ -862,7 +865,8 @@ describe('RPC message schemas', () => { b: side, diff: [], }).success, - ).toBe(false); + ).toBe(true); + expect(Date.now() - started).toBeLessThan(1_000); }); it('requires pre-event record diff sides to hash to a blank screen', () => { From 17c86cd1362e9ccbdc83b480c2ef5f21f22372d2 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:53:24 +0000 Subject: [PATCH 18/21] docs: state the record diff minimality bound explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LCS is exact for any realistic screen; when the trimmed differing region exceeds the cell budget on both sides the diff degrades to a delete-then-add block. Document that public semantics boundary — identity, hashes, and reconstruction remain exact. --- docs/USAGE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/USAGE.md b/docs/USAGE.md index 8c78b06..21fa551 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -218,6 +218,7 @@ agent-tty record diff --at-seq-a 0 --json - `--at-seq-a ` / `--at-seq-b `: replay each side up to an Event Log sequence (default: latest). Diffing a session against itself at an earlier sequence shows how its screen evolved. - The JSON result carries `identical`, per-side `sessionId`/`capturedAtSeq`/`cols`/`rows`/`screenHash`, and a `diff` array of `{ op: equal | delete | add, text, aRow?, bRow? }` entries over the visible screen lines (0-based rows, no trimming or normalization — the same canonical lines that `screenHash` hashes). A side with an empty event log reports the pre-event blank screen with `capturedAtSeq: -1` (no event was replayed). - Human output is a unified-style diff with `---`/`+++` headers naming each side's session, sequence, and hash prefix. +- The line diff is LCS-minimal for any realistic screen. As a bounded-memory safeguard, when the differing region (after matching the common prefix and suffix) exceeds roughly 2,000 lines on **both** sides, that region degrades to a plain delete-then-add block instead of a minimal diff — `identical`, both `screenHash` values, and full-screen reconstruction remain exact; only diff minimality is reduced. - The command works entirely offline from `events.jsonl`; sessions may be running or exited. The exit code is `0` whether or not the screens differ — automation should read `identical` from the JSON result. ## Isolation From b12844def1ed52dd97755575e9badbb94cb59f23 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 12:59:34 +0000 Subject: [PATCH 19/21] docs: describe the diff minimality budget by its cell product --- docs/USAGE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 21fa551..a0c918e 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -218,7 +218,7 @@ agent-tty record diff --at-seq-a 0 --json - `--at-seq-a ` / `--at-seq-b `: replay each side up to an Event Log sequence (default: latest). Diffing a session against itself at an earlier sequence shows how its screen evolved. - The JSON result carries `identical`, per-side `sessionId`/`capturedAtSeq`/`cols`/`rows`/`screenHash`, and a `diff` array of `{ op: equal | delete | add, text, aRow?, bRow? }` entries over the visible screen lines (0-based rows, no trimming or normalization — the same canonical lines that `screenHash` hashes). A side with an empty event log reports the pre-event blank screen with `capturedAtSeq: -1` (no event was replayed). - Human output is a unified-style diff with `---`/`+++` headers naming each side's session, sequence, and hash prefix. -- The line diff is LCS-minimal for any realistic screen. As a bounded-memory safeguard, when the differing region (after matching the common prefix and suffix) exceeds roughly 2,000 lines on **both** sides, that region degrades to a plain delete-then-add block instead of a minimal diff — `identical`, both `screenHash` values, and full-screen reconstruction remain exact; only diff minimality is reduced. +- The line diff is LCS-minimal for any realistic screen. As a bounded-memory safeguard, when the differing region (after matching the common prefix and suffix) is so large that the product of its two side lengths exceeds a 4,000,000-cell budget (for example 1,000 × 4,001 lines), that region degrades to a plain delete-then-add block instead of a minimal diff — `identical`, both `screenHash` values, and full-screen reconstruction remain exact; only diff minimality is reduced. - The command works entirely offline from `events.jsonl`; sessions may be running or exited. The exit code is `0` whether or not the screens differ — automation should read `identical` from the JSON result. ## Isolation From f3cae9b6dfef99e62864ed40f630bbb0b97fc407 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 13:09:06 +0000 Subject: [PATCH 20/21] fix: reject missing event logs before synthesizing blank screens The host creates events.jsonl at startup, so an empty replay is only authoritative when the zero-length log exists. A deleted or never- written log now fails with REPLAY_ERROR instead of fabricating an identical blank-screen result for lost recordings. --- src/cli/commands/record-diff.ts | 30 +++++++++++++++++++++++++- test/integration/record-diff.test.ts | 22 ++++++++++++++++++- test/unit/commands/record-diff.test.ts | 26 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/record-diff.ts b/src/cli/commands/record-diff.ts index 77e159e..cfd1b94 100644 --- a/src/cli/commands/record-diff.ts +++ b/src/cli/commands/record-diff.ts @@ -1,3 +1,5 @@ +import { access } from 'node:fs/promises'; + import type { CommandContext } from '../context.js'; import type { RecordDiffResult, @@ -14,7 +16,11 @@ import { } from '../../renderer/canonicalScreen.js'; import { withOfflineReplayRenderer } from '../../replay/offlineReplay.js'; import { readManifestIfExists } from '../../storage/manifests.js'; -import { manifestPath, sessionDir } from '../../storage/sessionPaths.js'; +import { + eventLogPath, + manifestPath, + sessionDir, +} from '../../storage/sessionPaths.js'; import { diffLines } from '../../util/lineDiff.js'; import { invariant } from '../../util/assert.js'; @@ -68,6 +74,22 @@ async function resolveSessionDirectory( return sessionDirectory; } +async function assertEventLogExists( + sessionDirectory: string, + sessionId: string, +): Promise { + const eventsFile = eventLogPath(sessionDirectory); + try { + await access(eventsFile); + } catch (error) { + throw makeCliError(ERROR_CODES.REPLAY_ERROR, { + message: `Session "${sessionId}" has no event log; the canonical log was deleted or never written.`, + details: { sessionId, eventLogPath: eventsFile }, + cause: error, + }); + } +} + async function replayScreen( context: CommandContext, sessionId: string, @@ -91,6 +113,12 @@ async function replayScreen( // valid initial blank grid. Backends reject snapshot() before the // first replayed event, so synthesize the blank screen directly. // capturedAtSeq -1 mirrors targetSeq: no event was replayed. + // + // The host creates events.jsonl at startup, so an empty replay is + // only authoritative when the (zero-length) log file exists; a + // missing file means the canonical log was deleted or never + // written and must not be reported as a blank screen. + await assertEventLogExists(sessionDirectory, sessionId); const blankLines = Array.from( { length: replayInput.initialRows }, () => '', diff --git a/test/integration/record-diff.test.ts b/test/integration/record-diff.test.ts index 7af9d78..a29250f 100644 --- a/test/integration/record-diff.test.ts +++ b/test/integration/record-diff.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, realpath } from 'node:fs/promises'; +import { mkdtemp, realpath, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -218,6 +218,26 @@ describe('record diff integration', { timeout: 120_000 }, () => { } }); + it('fails with REPLAY_ERROR when the canonical event log was deleted', async () => { + const sessionId = createSession(testHome, [ + '/bin/sh', + '-c', + "printf 'x\\n'", + ]); + waitForExit(testHome, sessionId); + + // Simulate a lost recording: the manifest survives but events.jsonl is + // gone. record diff must not fabricate a blank-screen result. + await rm(join(testHome, 'sessions', sessionId, 'events.jsonl')); + + const result = runRecordDiff(testHome, [sessionId, sessionId]); + + expect(result.status).not.toBe(0); + const envelope = JSON.parse(result.stdout) as ErrorEnvelope; + expect(envelope.ok).toBe(false); + expect(envelope.error.code).toBe('REPLAY_ERROR'); + }); + it('fails with SESSION_NOT_FOUND for unknown sessions', () => { const command = ['/bin/sh', '-c', "printf 'x\\n'"]; const sessionA = createSession(testHome, command); diff --git a/test/unit/commands/record-diff.test.ts b/test/unit/commands/record-diff.test.ts index 6e80724..a3cd907 100644 --- a/test/unit/commands/record-diff.test.ts +++ b/test/unit/commands/record-diff.test.ts @@ -7,9 +7,15 @@ const mocks = vi.hoisted(() => ({ readManifestIfExists: vi.fn(), sessionDir: vi.fn(), manifestPath: vi.fn(), + eventLogPath: vi.fn(), + access: vi.fn(), withOfflineReplayRenderer: vi.fn(), })); +vi.mock('node:fs/promises', () => ({ + access: mocks.access, +})); + vi.mock('../../../src/cli/output.js', () => ({ emitSuccess: mocks.emitSuccess, })); @@ -25,6 +31,7 @@ vi.mock('../../../src/storage/manifests.js', () => ({ vi.mock('../../../src/storage/sessionPaths.js', () => ({ sessionDir: mocks.sessionDir, manifestPath: mocks.manifestPath, + eventLogPath: mocks.eventLogPath, })); import { runRecordDiffCommand } from '../../../src/cli/commands/record-diff.js'; @@ -135,6 +142,10 @@ describe('runRecordDiffCommand', () => { (sessionDirectory: string) => `${sessionDirectory}/session.json`, ); mocks.readManifestIfExists.mockResolvedValue(createTestSessionRecord()); + mocks.eventLogPath.mockImplementation( + (sessionDirectory: string) => `${sessionDirectory}/events.jsonl`, + ); + mocks.access.mockResolvedValue(undefined); }); afterEach(() => { @@ -257,6 +268,21 @@ describe('runRecordDiffCommand', () => { ); }); + it('fails with REPLAY_ERROR when the event log file is missing', async () => { + // An empty replay is only authoritative when the zero-length log exists; + // a deleted or never-written log must not synthesize a blank screen. + mockEmptyLogReplay(24, 80); + mocks.access.mockRejectedValue( + Object.assign(new Error('ENOENT'), { code: 'ENOENT' }), + ); + + await expect(runRecordDiffCommand(createOptions())).rejects.toMatchObject({ + code: ERROR_CODES.REPLAY_ERROR, + message: expect.stringContaining('has no event log') as string, + }); + expect(mocks.emitSuccess).not.toHaveBeenCalled(); + }); + it('rejects non-integer --at-seq values including NaN', async () => { await expect( runRecordDiffCommand(createOptions({ atSeqA: 1.5 })), From e33e20359b63b2ecc6be4dd041b920ff551af34e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 21 Aug 2026 13:15:01 +0000 Subject: [PATCH 21/21] fix: skip the LCS table for one-sided diff middles An empty middle side needs no comparisons; construct the delete/add entries directly so huge one-sided middles stay linear in memory instead of allocating millions of one-element table rows. --- src/util/lineDiff.ts | 6 ++++++ test/unit/util/lineDiff.test.ts | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/util/lineDiff.ts b/src/util/lineDiff.ts index 7928e69..5dc7006 100644 --- a/src/util/lineDiff.ts +++ b/src/util/lineDiff.ts @@ -29,6 +29,12 @@ function lcsEntries( aOffset: number, bOffset: number, ): LineDiffEntry[] { + // An empty side needs no LCS comparisons; skip the table so one-sided + // middles stay linear in memory as well as time. + if (a.length === 0 || b.length === 0) { + return fallbackEntries(a, b, aOffset, bOffset); + } + // lcs[i][j] = LCS length of a[i..] and b[j..]. const lcs: number[][] = Array.from({ length: a.length + 1 }, () => new Array(b.length + 1).fill(0), diff --git a/test/unit/util/lineDiff.test.ts b/test/unit/util/lineDiff.test.ts index 2a236f7..9cfcc4a 100644 --- a/test/unit/util/lineDiff.test.ts +++ b/test/unit/util/lineDiff.test.ts @@ -77,6 +77,27 @@ describe('diffLines', () => { expect(entries.every((entry) => entry.op === 'delete')).toBe(true); }); + it('handles a huge one-sided middle without allocating the DP table', () => { + // 3M distinct rows vs one shared row: the trimmed middle is one-sided, + // which must bypass the table (3M one-element rows would be ~hundreds of + // MiB) and complete quickly. + const a = Array.from({ length: 3_000_000 }, (_, i) => `row ${String(i)}`); + const b = ['row 0']; + + const started = Date.now(); + const entries = diffLines(a, b); + + expect(Date.now() - started).toBeLessThan(10_000); + expect(entries).toHaveLength(3_000_000); + expect(entries[0]).toEqual({ + op: 'equal', + text: 'row 0', + aRow: 0, + bRow: 0, + }); + expect(entries.slice(1).every((entry) => entry.op === 'delete')).toBe(true); + }); + it('matches large common prefixes and suffixes without a quadratic table', () => { // 40k shared lines on each side would need a ~1.6G-cell DP table; the // prefix/suffix trim must reduce the middle to the single changed line.