mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
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.
This commit is contained in:
+33
-11
@@ -430,6 +430,26 @@ function truncatePaneLineByVisibleColumns(line: string, maxColumns: number): str
|
|||||||
return result;
|
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(
|
export function formatPaneSnapshot(
|
||||||
lines: string[],
|
lines: string[],
|
||||||
geometry: { cols: number; rows: number; cursorX: number; cursorY: number }
|
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 {
|
capturePaneBuffer(muxName: string, paneTarget?: string, opts?: { fullHistory?: boolean }): string | null {
|
||||||
if (IS_TEST_MODE) return '';
|
if (IS_TEST_MODE) return '';
|
||||||
if (!isValidMuxName(muxName)) {
|
const target = resolveTmuxPaneTarget(muxName, paneTarget);
|
||||||
console.error('[TmuxManager] Invalid session name in capturePaneBuffer:', muxName);
|
if (!target) {
|
||||||
|
console.error('[TmuxManager] Invalid pane target in capturePaneBuffer:', { muxName, paneTarget });
|
||||||
return null;
|
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;
|
const fullHistory = opts?.fullHistory === true;
|
||||||
|
|
||||||
@@ -2227,9 +2242,12 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
|||||||
timeout: EXEC_TIMEOUT_MS,
|
timeout: EXEC_TIMEOUT_MS,
|
||||||
}).replace(/\n+$/g, '');
|
}).replace(/\n+$/g, '');
|
||||||
// Full-history spans many screens — return it as raw linear scrollback
|
// 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) {
|
if (fullHistory) {
|
||||||
return buffer;
|
return normalizeScrollbackEol(buffer);
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
const cursor = execSync(
|
const cursor = execSync(
|
||||||
@@ -2255,7 +2273,11 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
|||||||
} catch (cursorErr) {
|
} catch (cursorErr) {
|
||||||
console.error('[TmuxManager] Failed to query pane cursor after capture:', 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) {
|
} catch (err) {
|
||||||
console.error('[TmuxManager] Failed to capture pane buffer:', err);
|
console.error('[TmuxManager] Failed to capture pane buffer:', err);
|
||||||
return null;
|
return null;
|
||||||
|
|||||||
@@ -1026,7 +1026,7 @@ export function registerSessionRoutes(
|
|||||||
// During long thinking phases, Ink rewrites the same rows thousands of times
|
// During long thinking phases, Ink rewrites the same rows thousands of times
|
||||||
// (500KB+). Without stripping, tail mode returns only spinner frames and
|
// (500KB+). Without stripping, tail mode returns only spinner frames and
|
||||||
// the terminal appears empty when switching tabs.
|
// 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
|
// Strip alt-screen toggles and scrollback-erase from Codex/Claude byte
|
||||||
// streams. xterm.js obeys them by switching to its scrollback-less alt
|
// streams. xterm.js obeys them by switching to its scrollback-less alt
|
||||||
|
|||||||
@@ -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(/(?<!\r)\n/.test(out)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not double up carriage returns on already-CRLF input', () => {
|
||||||
|
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('');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user