test(terminal): pin checkpoint-only cold restore of a large checkpoint (#10500)

* test(terminal): pin checkpoint-only cold restore of a large checkpoint Triaging a report that checkpoint-only cold restore renders blank at every checkpoint size found no v1.4.156 regression: an A/B of v1.4.155 against origin/main returned byte-identical ColdRestoreInfo at 1.01, 5.56, 11.30 and 20.61 MiB, all 300 marker lines intact on both refs. Blankness tracks the meta.endedAt eligibility gate, not size — a cleanly ended session refuses to cold-restore at any size, which is by design and unchanged between the refs. What the triage did surface is a coverage gap. #10179's 16MiB checkpoint read cap sat under what the unbounded writer emits, so a large checkpoint threw, was swallowed to checkpoint=null, and the pane reopened empty; #10479 raised the cap but nothing pinned the round trip it had broken. history-reader-memory covers the bounded reader at its limit, not writer→reader recovery. Adds that round trip through the real HistoryManager writer and HistoryReader over a header-only log, and pins the endedAt gate that has now been mistaken for a size regression twice. Fails at the pre-#10479 cap with the exact production symptom (detectColdRestore returns null). * test(terminal): fail loudly when writeSync is unavailable writeLargeScrollback ignored writeSync's boolean, which is false when xterm's private _core.writeSync goes away. The size assertions catch that in the large-checkpoint test (416 bytes vs 16MiB), but the endedAt gate is size-independent, so that test silently passed on an empty snapshot — pinning the gate over no scrollback at all. * test(terminal): cut large-checkpoint fixture peak RSS from 1.2GiB to 800MiB The filler colored per line, not per cell as its comment claimed, so the serialized seed only tracked plain-text size and needed 26k buffer rows to clear 16MiB. The xterm buffer costs rows x cols, which made this single file raise the whole src/main/daemon/ suite's peak RSS 5.2x (241MiB -> 1244MiB) — a real OOM risk on CI right after #10299 bounded readers for that reason. Carrying the bytes in SGR runs instead of rows reaches a larger seed from 5.5k rows: suite peak RSS 1244MiB -> 793MiB, and the margins improve too (seed 25.16MiB = 1.57x the threshold vs 1.19x before, checkpoint.json 46.1MiB = 2.88x vs 1.20x). Also makes the fixture match its own comment and be more representative of real colored agent output. Re-verified all three mutations still fail: cap at 16MiB -> "expected null not to be null"; endedAt gate deleted -> test 2 fails; writeSync unavailable -> both fail. * test(terminal): tighten timeout, reuse prod dir-name helper, dispose first Review follow-ups, all test-only: - Import getHistorySessionDirName instead of hand-rolling encodeURIComponent. Equivalent today, but that helper exists to absorb encoding changes, so the hand-rolled copy would silently diverge from the writer it is checking. - Index FILLER_ROWS by its own length so growing the array cannot leave rows unused. - 300_000ms -> 60_000ms. The file runs in ~2.8s and vitest's own default is 30s; a 5-minute ceiling turns a hung regression into a stalled job rather than a failure. 60s keeps Windows headroom. - Take the snapshot, dispose, then checkpoint, so a throwing checkpoint() cannot leak the buffer. Note this is memory-neutral, not a saving: peak RSS is set at getSnapshot(), where the buffer and the serialized strings coexist, and measured ~800MiB either way. Mutations re-verified: cap at 16MiB -> "expected null not to be null"; endedAt gate deleted -> test 2 fails; writeSync unavailable -> both fail. (cherry picked from commit a0971e0f9b)
2026-10-20 00:03:45 +00:00 · 2026-07-25 00:05:00 -07:00
parent 869eb41948
commit cfb4151524
1 changed files with 120 additions and 0 deletions
@@ -0,0 +1,120 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { mkdtempSync, rmSync, statSync } from 'node:fs'
+import { HistoryManager } from './history-manager'
+import { HistoryReader } from './history-reader'
+import { HeadlessEmulator } from './headless-emulator'
+import { LOG_HEADER_BYTES } from './terminal-history-log'
+import { getHistorySessionDirName } from './history-paths'
+
+// Guards checkpoint-only cold restore: the route taken when the daemon dies right after a
+// checkpoint, so output.log is header-only and checkpoint.json is the sole recovery source.
+// #10179 shipped a read cap (16MiB) under what the unbounded checkpoint writer can emit; past it
+// the read threw, the catch swallowed it to checkpoint=null, and the pane reopened empty (#10479
+// raised the cap; nothing pinned the round trip that the cap silently broke). The
+// reader-level bound is covered in history-reader-memory.test.ts — this asserts the round trip
+// through the real writer, which is what actually decides whether a user sees their scrollback.
+
+const MARKERS = 300
+const SESSION_ID = 'repo-1::/Users/dev/large-scrollback'
+const COLS = 800
+const ROWS = 40
+
+// Why per-cell color: the restore seed must clear 16MiB to cover the pre-#10479 cap, and
+// SGR-per-cell is how real agent/build output inflates a snapshot well past its plain-text size.
+// It is also what keeps the fixture affordable — the xterm buffer costs rows x cols, so carrying
+// the bytes in SGR runs instead of rows reaches 25MiB of seed from 5.5k rows rather than 26k,
+// cutting this file's peak RSS from ~1.2GiB to ~800MiB. Only 8 distinct rows exist under
+// (row + col) % 8, so build them once rather than per line.
+const FILLER_ROWS = Array.from(
+  { length: 8 },
+  (_unused, row) =>
+    `${Array.from({ length: COLS - 1 }, (_cell, col) => `\x1b[3${(row + col) % 8}mx`).join('')}\x1b[0m\r\n`
+)
+
+function writeLargeScrollback(emulator: HeadlessEmulator, fillerLines: number): void {
+  let written = true
+  for (let index = 0; index < fillerLines; index += 1) {
+    written = emulator.writeSync(FILLER_ROWS[index % FILLER_ROWS.length]) && written
+  }
+  for (let index = 0; index < MARKERS; index += 1) {
+    written = emulator.writeSync(`MARKER-${index}\r\n`) && written
+  }
+  // Why assert: writeSync returns false if xterm's private _core.writeSync ever goes away, which
+  // would leave an empty snapshot. The size assertions below catch that, but the endedAt gate is
+  // size-independent and would still pass — pinning the gate over no scrollback at all.
+  expect(written).toBe(true)
+}
+
+describe('checkpoint-only cold restore of a large checkpoint', () => {
+  let dir: string
+
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'large-checkpoint-restore-'))
+  })
+
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true })
+  })
+
+  it('recovers the full scrollback from a checkpoint larger than the pre-fix 16MiB read cap', async () => {
+    const manager = new HistoryManager(dir)
+    const reader = new HistoryReader(dir)
+    const emulator = new HeadlessEmulator({ cols: COLS, rows: ROWS, scrollback: 100_000 })
+    writeLargeScrollback(emulator, 5_500)
+
+    await manager.openSession(SESSION_ID, {
+      cwd: '/Users/dev/large-scrollback',
+      cols: COLS,
+      rows: ROWS
+    })
+    // Why dispose first: a throwing checkpoint() must not leak the buffer for the worker's life.
+    const snapshot = emulator.getSnapshot()
+    emulator.dispose()
+    await manager.checkpoint(SESSION_ID, snapshot)
+
+    const sessionDir = join(dir, getHistorySessionDirName(SESSION_ID))
+    // checkpoint() resets the log to its header, so this is genuinely checkpoint-only: any
+    // recovered content had to come through the checkpoint read, not incremental log replay.
+    expect(statSync(join(sessionDir, 'output.log')).size).toBe(LOG_HEADER_BYTES)
+    expect(statSync(join(sessionDir, 'checkpoint.json')).size).toBeGreaterThan(16 * 1024 * 1024)
+
+    const info = await reader.detectColdRestore(SESSION_ID)
+    expect(info).not.toBeNull()
+    // The adapter seeds a non-alt-screen restore with rehydrateSequences + snapshotAnsi.
+    const seed = info!.rehydrateSequences + info!.snapshotAnsi
+    expect(info!.modes.alternateScreen).toBe(false)
+    for (const index of [0, MARKERS - 1]) {
+      expect(seed).toContain(`MARKER-${index}`)
+    }
+    expect(seed.length).toBeGreaterThan(16 * 1024 * 1024)
+  }, 60_000)
+
+  // Why pinned: this gate, not any size cap, is what makes a checkpoint-only restore come back
+  // blank. Two separate investigations mistook a cleanly-ended session for a size regression.
+  it('restores after an unclean exit but not after a clean one, at the same checkpoint size', async () => {
+    const restoreAfter = async (endCleanly: boolean): Promise<boolean> => {
+      const manager = new HistoryManager(dir)
+      const emulator = new HeadlessEmulator({ cols: COLS, rows: ROWS, scrollback: 100_000 })
+      writeLargeScrollback(emulator, 50)
+      await manager.openSession(SESSION_ID, {
+        cwd: '/Users/dev/large-scrollback',
+        cols: COLS,
+        rows: ROWS
+      })
+      const snapshot = emulator.getSnapshot()
+      emulator.dispose()
+      await manager.checkpoint(SESSION_ID, snapshot)
+      if (endCleanly) {
+        await manager.closeSession(SESSION_ID, 0)
+      }
+      const info = await new HistoryReader(dir).detectColdRestore(SESSION_ID)
+      rmSync(join(dir, getHistorySessionDirName(SESSION_ID)), { recursive: true, force: true })
+      return info !== null
+    }
+
+    expect(await restoreAfter(false)).toBe(true)
+    expect(await restoreAfter(true)).toBe(false)
+  }, 60_000)
+})