fix(terminal): gate the row-preserving skips on a capture, not the query flag

Review of the previous commit found the guard inverted: the three skips keyed
on `?full=1`, which is only what the client asked for. When the capture comes
back null — ENOBUFS, a timeout, a vanished pane, or a session with no mux at
all — the reply falls back to the byte history, which IS a stream of
successive frames and still needs stripping. Gating on the request returned it
whole: measured at 82KB against 4KB for the same buffer without `full=1`. A
direct-PTY session takes that path on every first selection, not only during
an outage. The skips now key on `isFullCapture`, meaning a capture arrived.

Three further defects the same review surfaced, all on this path:

Keeping the trailing rows is only sound when a cursor move follows to count
back up from them. On the two branches where the cursor query fails there is
no move, so the caret was left at the bottom of the pane — worse than before.
The cursor is now read first and settles both decisions together.

The move is relative rather than absolute. `CUP` numbers rows from the top of
the browser's screen, so it is only right while the browser's row count equals
the pane's, and `resizeWindow` does not wait for tmux, so a capture can be
taken before a requested resize applies. Measured against real tmux with a
browser four rows shorter than the pane: the absolute move lands on a blank
row, the relative one lands on the caret's row.

An all-blank pane no longer reads as content. Retaining trailing rows and
appending a move made it non-empty, and the caller treats non-empty as "replay
this", so a blank screen would have replaced real history — the downgrade
`_replayWouldShrinkBuffer` refuses, arriving from the server side where that
guard cannot see it.

The documentation claimed one line per screen row. `-J` joins a hard-wrapped
row into its logical line, so that is false whenever any row wrapped: measured
at 10 lines for a 12-row pane. Both entries now say what actually holds, and
the stale "NOT repositioned" contract in the mux interface is updated too.

Tests: the byte-history fallback is stripped, an empty capture leaves history
intact, and the extracted helpers are unit-tested directly rather than through
source-text matching. The slice window in the capture test is bounded at the
next method, having overrun into its neighbours.
This commit is contained in:
Michael Grundberg
2026-09-09 15:35:02 +02:00
parent 323730a29d
commit 2b57c595df
7 changed files with 233 additions and 74 deletions
+52
View File
@@ -878,6 +878,58 @@ describe('session-routes', () => {
expect(body.data.terminalBuffer.split('\r\n')).toHaveLength(rendered.split('\r\n').length);
});
it('full reload (?full=1) still strips the byte history when no capture came back', async () => {
// The row-preserving skips exist for a rendered pane. When the capture is
// unavailable the reply IS the byte stream — successive Ink frames, no row
// alignment to protect — so keying the skips on the query parameter rather
// than on the capture returned it unstripped, which is the whole reason
// stripInkRedrawBloat exists. A session with no mux takes this path on
// every first selection, not just during an outage.
harness.ctx._session.mode = 'claude';
// A VPA cluster the stripper will collapse: >= 10 sequences, spanning the
// 32KB minimum, with real content after it.
const frame = '\x1b[12d' + 'spinner frame '.repeat(240);
harness.ctx._session.terminalBuffer = frame.repeat(20) + 'REAL CONTENT AFTER THE BLOAT';
(harness.ctx.mux as { captureActivePaneBuffer?: unknown }).captureActivePaneBuffer = vi.fn(() => null);
const res = await harness.app.inject({
method: 'GET',
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
});
expect(res.statusCode).toBe(200);
const body = JSON.parse(res.body);
expect(body.data.source).toBe('history');
expect(body.data.terminalBuffer).toContain('REAL CONTENT AFTER THE BLOAT');
// Stripped, not passed through whole.
expect(body.data.terminalBuffer.length).toBeLessThan(harness.ctx._session.terminalBuffer.length);
});
it('full reload (?full=1) keeps the byte history when the capture is empty', async () => {
// Pins the contract the capture side relies on: an empty capture means
// "nothing to replay" and the byte history survives. capturePaneBuffer
// returns '' for an all-blank pane precisely to reach this branch, since
// retaining trailing blank rows and appending a cursor move would
// otherwise make a blank screen non-empty and replace the history with it.
// (The blank-pane decision itself is unit-tested on hasVisibleContent —
// capturePaneBuffer short-circuits under IS_TEST_MODE and cannot run here.)
harness.ctx._session.mode = 'claude';
harness.ctx._session.terminalBuffer = 'a real conversation worth keeping';
(harness.ctx.mux as { captureActivePaneBuffer?: unknown }).captureActivePaneBuffer = vi.fn(
(_name: string, opts?: { fullHistory?: boolean }) => (opts?.fullHistory ? '' : 'visible frame')
);
const res = await harness.app.inject({
method: 'GET',
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
});
expect(res.statusCode).toBe(200);
const body = JSON.parse(res.body);
expect(body.data.source).toBe('history');
expect(body.data.terminalBuffer).toContain('a real conversation worth keeping');
});
it('full reload (?full=1) falls back to the byte history when the capture is unavailable', async () => {
harness.ctx._session.mode = 'claude';
harness.ctx._session.terminalBuffer = 'byte history survives';
+57 -14
View File
@@ -11,11 +11,15 @@
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
import { describe, expect, it } from 'vitest';
import { formatCursorRestore, hasVisibleContent } from '../src/tmux-manager.js';
describe('tmux full-history pane capture (COD-47)', () => {
const source = readFileSync(resolve(import.meta.dirname, '../src/tmux-manager.ts'), 'utf8');
const methodStart = source.indexOf('capturePaneBuffer(muxName: string');
const methodBody = source.slice(methodStart, methodStart + 6500);
// Bounded at the next method so `methodBody` really is one method: the
// ordering assertions below would otherwise be satisfiable by a neighbour.
const methodEnd = source.indexOf('captureActivePaneBuffer(muxName: string', methodStart);
const methodBody = source.slice(methodStart, methodEnd);
it('capturePaneBuffer accepts pane-capture options with a fullHistory flag', () => {
expect(methodStart).toBeGreaterThan(-1);
@@ -42,32 +46,37 @@ describe('tmux full-history pane capture (COD-47)', () => {
});
it('returns full-history capture as raw scrollback (skips the single-screen repaint)', () => {
// When fullHistory, return the normalized buffer BEFORE the
// formatPaneSnapshot repaint (which is single-screen and would clip a
// multi-screen history).
const normalize = methodBody.indexOf('normalizeScrollbackEol(buffer)');
// The fullHistory branch returns before the formatPaneSnapshot repaint,
// which is single-screen and would clip a multi-screen history.
const branch = methodBody.indexOf('if (fullHistory) {\n // Without geometry');
const snapshot = methodBody.indexOf('formatPaneSnapshot(');
expect(normalize).toBeGreaterThan(-1);
expect(branch).toBeGreaterThan(-1);
expect(snapshot).toBeGreaterThan(-1);
expect(normalize).toBeLessThan(snapshot);
expect(branch).toBeLessThan(snapshot);
// …and what it returns is normalized linear scrollback, not a repaint.
expect(methodBody.slice(branch, snapshot)).toContain('normalizeScrollbackEol(');
});
it('appends the pane cursor to the full-history capture', () => {
// A linear replay leaves the caret wherever the last character landed — the
// status line, for an agent CLI — and every cursor-relative update the CLI
// sends afterwards is then measured from the wrong row.
const restore = methodBody.indexOf('return `${normalized}');
const restore = methodBody.indexOf('formatCursorRestore(geometry)');
const snapshot = methodBody.indexOf('formatPaneSnapshot(');
expect(restore).toBeGreaterThan(-1);
expect(restore).toBeLessThan(snapshot);
expect(methodBody).toContain('cursorY + 1};${cursorX + 1}H');
});
it('keeps the trailing rows of a full-history capture', () => {
// The visible path drops trailing blank rows because it repaints each row
// absolutely afterwards. Dropping them on the linear path would move the
// frame up and leave the restored cursor pointing at the wrong line.
expect(methodBody).toContain("fullHistory ? rawCapture.replace(/\\n$/, '')");
it('keeps the trailing rows only when a cursor move will follow', () => {
// Trailing blank rows are the bottom of the screen and the cursor move counts
// up from them, so the two decisions travel together: no geometry, no move,
// and the old trim applies instead.
expect(methodBody).toContain("rawCapture.replace(/\\n$/, '')");
expect(methodBody).toContain("if (!geometry) return normalizeScrollbackEol(rawCapture.replace(/\\n+$/g, ''))");
});
it('defers to the byte history when the pane holds nothing visible', () => {
expect(methodBody).toContain("if (!hasVisibleContent(trimmed)) return ''");
});
it('captureActivePaneBuffer forwards the capture options', () => {
@@ -78,3 +87,37 @@ describe('tmux full-history pane capture (COD-47)', () => {
expect(body).toContain('this.capturePaneBuffer(muxName, target, opts)');
});
});
describe('full-history cursor restore', () => {
it('counts up from the last replayed row rather than down from the top', () => {
// Relative, not `CUP`: absolute row addressing is only correct while the
// browser's row count equals the pane's, and resizeWindow does not wait for
// tmux, so a capture can be taken before a requested resize has applied.
expect(formatCursorRestore({ cols: 80, rows: 24, cursorX: 2, cursorY: 20 })).toBe('\x1b[3A\r\x1b[2C');
});
it('emits no row move when the caret is already on the last row', () => {
expect(formatCursorRestore({ cols: 80, rows: 24, cursorX: 5, cursorY: 23 })).toBe('\r\x1b[5C');
});
it('emits no column move for column zero', () => {
expect(formatCursorRestore({ cols: 80, rows: 10, cursorX: 0, cursorY: 0 })).toBe('\x1b[9A\r');
});
});
describe('hasVisibleContent', () => {
it('is false for a pane of blank rows', () => {
expect(hasVisibleContent('\n'.repeat(23))).toBe(false);
});
it('is false for blank rows carrying only SGR attributes', () => {
// `capture-pane -e` styles every row, so an all-blank pane is not an empty
// string. Treating it as content would replace the byte history with a
// blank screen.
expect(hasVisibleContent('\x1b[m \x1b[0m\n\x1b[m \x1b[0m')).toBe(false);
});
it('is true as soon as one row carries a character', () => {
expect(hasVisibleContent('\x1b[m \x1b[0m\n\x1b[m x \x1b[0m')).toBe(true);
});
});