fix(terminal): stop cold restore dropping all scrollback on large checkpoints (#10479)

* fix(terminal): stop cold restore dropping all scrollback on large checkpoints

The checkpoint read cap shipped without its write bound. history-reader.ts
reads checkpoint.json through a 16MiB cap that throws past the limit, and the
catch swallows it to checkpoint=null; history-manager.ts still writes the
checkpoint with an unbounded JSON.stringify. Every fallback then collapses
(stale-generation log, unlinked legacy scrollback.bin), so the terminal
reopens empty with nothing surfaced.

Raise the read cap to cover the largest checkpoint the writer can legitimately
emit, derived from the scrollback policy's 50k-row max preset so it cannot
drift back under the writer. A bound is kept so a corrupt file still cannot
OOM the main process.

Bounding the writer instead would not recover the scrollback: the stringify
throw lands in handleWriteError, which adds the session to disabledSessions
and permanently stops history recording for it.

* fix(terminal): anchor checkpoint read cap to its own reasoning

The cap was derived as 2 * LEGACY_TERMINAL_SCROLLBACK_BYTES_100_MB, but that
constant is a legacy byte-preset setting value with no other consumer, and the
50k-row bucket it was attributed to has no upper byte bound. Same value, stated
without the false policy linkage.

* fix(terminal): assert the checkpoint byte cap, correct its rationale

The retained oversized-checkpoint test passed with the byte guard removed
entirely — 200MB of NUL fails JSON.parse, so detectColdRestore returned null
either way. Assert the bounded reader directly, as the amplification test does.

Also: a 50k-row max preset of ordinary text measures ~14MB serialized, not
'far below' by an unbounded margin — per-cell-colored output can still exceed
the cap, which is what a writer-side snapshot trim has to fix.

(cherry picked from commit 6c03ecf8e1)
This commit is contained in:
Brennan Benson
2026-07-24 23:53:06 -07:00
parent ab128cb13b
commit e4057f26f9
2 changed files with 29 additions and 7 deletions
+24 -6
View File
@@ -22,6 +22,8 @@ import {
TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS
} from './terminal-history-file-reader'
const RETIRED_CHECKPOINT_READ_CAP_BYTES = 16 * 1024 * 1024
const directories: string[] = []
function createSession(sessionId: string): { basePath: string; sessionPath: string } {
@@ -49,7 +51,7 @@ function createSparseFile(path: string, bytes: number): void {
closeSync(descriptor)
}
function checkpoint(): string {
function checkpoint(overrides?: Record<string, unknown>): string {
return JSON.stringify({
snapshotAnsi: 'safe checkpoint',
scrollbackAnsi: '',
@@ -62,7 +64,8 @@ function checkpoint(): string {
mouseTracking: false,
applicationCursor: false,
alternateScreen: false
}
},
...overrides
})
}
@@ -82,12 +85,27 @@ describe('terminal history restore memory limits', () => {
expect(restore?.snapshotAnsi).toBe('safe checkpoint')
})
// Why: the read cap once sat at 16MiB while the writer stayed unbounded, so a big-scrollback
// session cold-restored to an empty terminal — checkpoint nulled, and every fallback collapsed.
it('restores a checkpoint larger than the retired 16MiB read cap', async () => {
const { basePath, sessionPath } = createSession('large-checkpoint')
const scrollbackAnsi = 'x'.repeat(RETIRED_CHECKPOINT_READ_CAP_BYTES + 1)
writeFileSync(join(sessionPath, 'checkpoint.json'), checkpoint({ scrollbackAnsi }))
const restore = await new HistoryReader(basePath).detectColdRestore('large-checkpoint')
expect(restore?.scrollbackAnsi).toBe(scrollbackAnsi)
})
// Why the cap survives at all: it bounds a corrupt/runaway file, well above legitimate output.
it('ignores oversized checkpoint and metadata files before parsing', async () => {
const checkpointSession = createSession('oversized-checkpoint')
createSparseFile(
join(checkpointSession.sessionPath, 'checkpoint.json'),
TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES + 1
)
const oversizedCheckpoint = join(checkpointSession.sessionPath, 'checkpoint.json')
createSparseFile(oversizedCheckpoint, TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES + 1)
// Why assert the reader directly too: detectColdRestore returns null for any unparseable
// checkpoint, so it would stay green with the byte cap removed entirely.
expect(() =>
readTerminalHistoryJson(oversizedCheckpoint, TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES)
).toThrow(/File too large/)
expect(
await new HistoryReader(checkpointSession.basePath).detectColdRestore('oversized-checkpoint')
).toBeNull()
@@ -1,4 +1,8 @@
export const TERMINAL_HISTORY_META_MAX_BYTES = 64 * 1024
export const TERMINAL_HISTORY_LOG_MAX_BYTES = 5 * 1024 * 1024
export const TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES = 16 * 1024 * 1024
// Why deliberately generous: the checkpoint writer is unbounded, and a read cap under what it
// can emit silently drops ALL scrollback on cold restore. This guards a corrupt/runaway file,
// not retention — a 50k-row max preset of ordinary text serializes to ~14MB, ~15x under. Output
// colored per cell can still exceed this; trimming the snapshot writer-side is the real fix.
export const TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES = 200_000_000
export const TERMINAL_HISTORY_LEGACY_SCROLLBACK_MAX_BYTES = 16 * 1024 * 1024