From c7967d4b55edab724dc74643c6d13ac03d4de9ac Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 20 Jun 2026 09:43:05 -0400 Subject: [PATCH] COD-138 normalize shell scrollback to CRLF so replay doesn't staircase A shell terminal could render output diagonally (each line shifted one column right) after a full page reload or a cursor-query-failure replay. Root cause: capturePaneBuffer's full-history path (capture-pane -p -e -S -) and its cursor-query-failure fallback returned raw scrollback, which tmux joins with a BARE \n. The browser xterm uses convertEol:false (correct for the live PTY stream, which carries real \r\n), so each bare \n dropped a row without returning the cursor to column 0 -> staircase. The visible / tab-switch path (formatPaneSnapshot) was immune because it repaints each row with an absolute cursor CSI. Fix: new pure helper normalizeScrollbackEol() (\r?\n -> \r\n, idempotent on CRLF, leaves lone \r overwrites untouched, adds/removes no rows) applied at both raw-return seams. The absolute-positioned snapshot path is unchanged. Tests: test/tmux-scrollback-eol.test.ts pins the invariant (no LF without a preceding CR) + CRLF idempotency + lone-CR preservation. 136/136 across tmux-scrollback-eol + tmux-capture-full-history + tmux-manager + routes/session-routes; build, tsc, prettier, frontend-syntax clean. --- src/tmux-manager.ts | 44 ++++++++++++++++++------ src/web/routes/session-routes.ts | 2 +- test/tmux-scrollback-eol.test.ts | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 test/tmux-scrollback-eol.test.ts diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 42106e54..055964ca 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -430,6 +430,26 @@ function truncatePaneLineByVisibleColumns(line: string, maxColumns: number): str return result; } +/** + * Normalize scrollback line endings to `\r\n` so a fresh xterm replays each line + * at column 0 (COD-138). + * + * `capture-pane -p -e -S -` (full-history capture) joins scrollback rows with a + * BARE `\n`. The browser xterm is created with the default `convertEol: false` + * (correct for the live PTY stream, which already carries real `\r\n`), so a bare + * `\n` drops a row without returning the cursor to column 0. Replaying that raw + * buffer on a full page reload makes every line start one column further right — + * the diagonal "staircase". The visible/tab-switch path avoids this by repainting + * each row with an absolute cursor CSI (`formatPaneSnapshot`); the full-history + * path returns raw scrollback, so it must be CRLF-normalized here. + * + * `\r?\n → \r\n` is idempotent on already-CRLF input and leaves a lone `\r` (an + * intentional in-line column reset / overwrite) untouched. + */ +export function normalizeScrollbackEol(buffer: string): string { + return buffer.replace(/\r?\n/g, '\r\n'); +} + export function formatPaneSnapshot( lines: string[], geometry: { cols: number; rows: number; cursorX: number; cursorY: number } @@ -2206,16 +2226,11 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { */ capturePaneBuffer(muxName: string, paneTarget?: string, opts?: { fullHistory?: boolean }): string | null { if (IS_TEST_MODE) return ''; - if (!isValidMuxName(muxName)) { - console.error('[TmuxManager] Invalid session name in capturePaneBuffer:', muxName); + const target = resolveTmuxPaneTarget(muxName, paneTarget); + if (!target) { + console.error('[TmuxManager] Invalid pane target in capturePaneBuffer:', { muxName, paneTarget }); return null; } - if (!SAFE_PANE_TARGET_PATTERN.test(paneTarget)) { - console.error('[TmuxManager] Invalid pane target:', paneTarget); - return null; - } - - const target = paneTarget.startsWith('%') ? `${muxName}.${paneTarget}` : `${muxName}.%${paneTarget}`; const fullHistory = opts?.fullHistory === true; @@ -2227,9 +2242,12 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { timeout: EXEC_TIMEOUT_MS, }).replace(/\n+$/g, ''); // Full-history spans many screens — return it as raw linear scrollback - // rather than repainting rows at single-screen absolute positions. + // rather than repainting rows at single-screen absolute positions. tmux + // joins scrollback rows with a bare `\n`; normalize to `\r\n` so a fresh + // xterm (convertEol:false) starts each replayed line at column 0 instead + // of staircasing diagonally (COD-138). if (fullHistory) { - return buffer; + return normalizeScrollbackEol(buffer); } try { const cursor = execSync( @@ -2255,7 +2273,11 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { } catch (cursorErr) { console.error('[TmuxManager] Failed to query pane cursor after capture:', cursorErr); } - return buffer; + // Cursor query failed or geometry was invalid, so we skip the absolute- + // positioned snapshot repaint and fall back to the raw capture. Normalize + // its bare `\n` line endings to `\r\n` so the replay doesn't staircase + // diagonally in a fresh xterm (COD-138, same reason as the fullHistory path). + return normalizeScrollbackEol(buffer); } catch (err) { console.error('[TmuxManager] Failed to capture pane buffer:', err); return null; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index bc823b43..70858bff 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1026,7 +1026,7 @@ export function registerSessionRoutes( // During long thinking phases, Ink rewrites the same rows thousands of times // (500KB+). Without stripping, tail mode returns only spinner frames and // the terminal appears empty when switching tabs. - let strippedBuffer = stripInkRedrawBloat(rawBuffer); + let strippedBuffer = session.mode === 'shell' ? rawBuffer : stripInkRedrawBloat(rawBuffer); // Strip alt-screen toggles and scrollback-erase from Codex/Claude byte // streams. xterm.js obeys them by switching to its scrollback-less alt diff --git a/test/tmux-scrollback-eol.test.ts b/test/tmux-scrollback-eol.test.ts new file mode 100644 index 00000000..8d9c1bdd --- /dev/null +++ b/test/tmux-scrollback-eol.test.ts @@ -0,0 +1,59 @@ +/** + * COD-138: shell terminal staircase / diagonal replay after reload. + * + * Root cause: the full-history tmux capture (`capture-pane -p -e -S -`) returns + * scrollback lines joined by a BARE `\n` (no `\r`). The visible/tab-switch path + * repaints each row with an absolute cursor CSI via `formatPaneSnapshot`, so it + * never staircases — but the full-history path returns the raw buffer. The + * browser xterm is created with the default `convertEol: false` (correct for the + * live PTY stream, which carries real `\r\n`), so on a full page reload each + * bare `\n` drops a row WITHOUT returning the cursor to column 0. Every replayed + * line then starts one column further right → the diagonal staircase. + * + * Fix: normalize the full-history scrollback to `\r\n` line endings before it is + * shipped to the browser, so a fresh xterm starts every replayed line at col 0. + * + * This exercises the pure transform (`normalizeScrollbackEol`). Under VITEST, + * TmuxManager no-ops execSync (IS_TEST_MODE), so the real capture path can't be + * driven end-to-end here; the transform is the load-bearing seam. + */ +import { describe, expect, it } from 'vitest'; +import { normalizeScrollbackEol } from '../src/tmux-manager.js'; + +describe('normalizeScrollbackEol (COD-138 staircase fix)', () => { + it('adds carriage returns so bare-LF scrollback lines start at column 0', () => { + // tmux capture-pane joins rows with bare \n. Without a preceding \r, xterm + // (convertEol:false) keeps the column → staircase. + const raw = 'line one\nline two\nline three'; + expect(normalizeScrollbackEol(raw)).toBe('line one\r\nline two\r\nline three'); + }); + + it('every newline in the result is preceded by a carriage return', () => { + const raw = 'a\nb\nc\nd'; + const out = normalizeScrollbackEol(raw); + // The staircase invariant: no LF may appear without a CR immediately before it. + expect(/(? { + const raw = 'line one\r\nline two\r\nline three'; + expect(normalizeScrollbackEol(raw)).toBe('line one\r\nline two\r\nline three'); + }); + + it('normalizes a mix of CRLF and bare LF to uniform CRLF', () => { + const raw = 'crlf\r\nbare\ncrlf2\r\nbare2'; + expect(normalizeScrollbackEol(raw)).toBe('crlf\r\nbare\r\ncrlf2\r\nbare2'); + }); + + it('preserves a lone trailing carriage return (in-line overwrite, not an EOL)', () => { + // A bare \r not followed by \n is a column-0 reset the TUI emitted on purpose; + // it must survive untouched so we do not corrupt an overwrite. + const raw = 'progress\rdone'; + expect(normalizeScrollbackEol(raw)).toBe('progress\rdone'); + }); + + it('is a no-op on content without newlines', () => { + expect(normalizeScrollbackEol('single frame')).toBe('single frame'); + expect(normalizeScrollbackEol('')).toBe(''); + }); +});