From ef812236b01c18b93bcfbb629403e52b38867e41 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 16 Aug 2026 20:43:43 +0200 Subject: [PATCH] fix: read a row-addressed repaint as lines in the preview Measured against a live Claude pane: an Ink TUI paints by ROW and emits almost no newlines, so dropping cursor-position sequences collapsed a whole screen into one unreadable line, and a tail cut mid-sequence printed the remains of it (";1H") as text. Now a jump to column 1 starts a display line, a jump inside a row moves the write position (capped, since a stream may address a column no terminal has), and a severed CSI head is dropped before parsing. The preview is readable against a real session as a result: tool calls, the working line and the composer all land where they belong. Also drop the repeated session name from a search row, whose snippet opens with the name the row already shows in its first column. Co-Authored-By: Claude Fable 5 --- src/tui/tui-ansi.ts | 60 ++++++++++++++++++++++++++++++++++++++ src/tui/tui-app.ts | 5 ++-- src/tui/tui-model.ts | 15 ++++++++-- test/tui/tui-ansi.test.ts | 36 +++++++++++++++++++++-- test/tui/tui-model.test.ts | 4 ++- 5 files changed, 113 insertions(+), 7 deletions(-) diff --git a/src/tui/tui-ansi.ts b/src/tui/tui-ansi.ts index 80118c36..76e0c73f 100644 --- a/src/tui/tui-ansi.ts +++ b/src/tui/tui-ansi.ts @@ -8,10 +8,20 @@ * cursor is dropped, and a `\r` is honored as "back to column 0" so a spinner * that repaints its line 200 times contributes one line instead of 200. * + * CURSOR ADDRESSING (`ESC [ r ; c H`) is honored too, and it has to be: an Ink + * TUI like Claude Code repaints by ROW and emits almost no newlines, so + * dropping those sequences collapses a whole screen into one unreadable line + * (measured against a live pane, 2026-08-16). A jump to column 1 starts a new + * display line, a jump within a row moves the write position, which is the same + * reading `normalizeCapturedFrame` in `web/approval-inbox.ts` takes of the same + * kind of frame. + * * Two approximations are deliberate, because the alternative is an emulator: * a carriage-return overwrite counts CODE POINTS, not display columns (so a * repaint over CJK text can land one cell off), and tab stops are counted the * same way. Neither can corrupt output, they only shift a repaint's alignment. + * Absolute ROW numbers are ignored as well: rows arrive in the order they are + * painted, which for a tail is the order worth reading. * * @module tui/tui-ansi */ @@ -27,6 +37,8 @@ export const SGR_RESET = '\x1b[0m'; const TAB_WIDTH = 8; /** Cap on remembered SGR sequences per cell, so a pathological stream cannot grow one unboundedly. */ const MAX_ACTIVE_SGR = 32; +/** Ceiling on a display line's cells: a stream may address column 99999, a terminal has none. */ +const MAX_LINE_CELLS = 1000; // ───────────────────────────────────────────────────────────────────────────── // Escape-sequence scanning @@ -37,6 +49,15 @@ interface EscapeScan { next: number; /** The sequence itself, only when it is SGR (`CSI ... m`) and therefore kept. */ sgr?: string; + /** 1-based column of a cursor-position sequence (`CSI r ; c H` or `f`). */ + column?: number; +} + +/** The column a `CSI r ; c H` addresses. Both parameters default to 1. */ +function cursorColumn(params: string): number { + const parts = params.split(';'); + const column = Number.parseInt(parts[1] ?? '', 10); + return Number.isSafeInteger(column) && column > 0 ? column : 1; } /** Scan a CSI body starting at `from` (params, then intermediates, then a final byte). */ @@ -47,6 +68,9 @@ function readCsi(text: string, start: number, from: number, keepSgr: boolean): E if (j >= text.length) return { next: text.length }; const next = j + 1; if (keepSgr && text[j] === 'm') return { next, sgr: text.slice(start, next) }; + if (keepSgr && (text[j] === 'H' || text[j] === 'f')) { + return { next, column: cursorColumn(text.slice(from, j)) }; + } return { next }; } @@ -320,6 +344,17 @@ export function toDisplayLines(raw: string): string[] { col = 0; }; + /** + * Park the write position at a column, padding the gap so the cell array + * never grows a hole (a hole would crash the replay, and a stream can address + * any column it likes). + */ + const moveTo = (column: number): void => { + const target = Math.min(column, MAX_LINE_CELLS); + while (cells.length < target) cells.push({ text: ' ', sgr: '' }); + col = target; + }; + const write = (text: string, width: number): void => { if (width === 0) { // A combining mark belongs to the character it follows, never to a cell @@ -340,6 +375,11 @@ export function toDisplayLines(raw: string): string[] { if (scan.sgr !== undefined) { active = applySgr(active, scan.sgr); sgr = active.join(''); + } else if (scan.column !== undefined) { + // Column 1 is a fresh row, which is the only thing a repainting TUI + // gives us to split lines on. + if (scan.column <= 1) endLine(); + else moveTo(scan.column - 1); } i = scan.next; continue; @@ -377,6 +417,26 @@ export function toDisplayLines(raw: string): string[] { return lines; } +/** + * The parameter bytes plus final byte of a CSI sequence whose `ESC [` was cut + * off. Requires at least one parameter byte, so ordinary text starting with a + * letter is never mistaken for one. + */ +const SEVERED_CSI = /^[0-9;?:<>=]+[A-Za-z]/; + +/** + * Drop the remains of an escape sequence a byte-sliced tail begins in the + * middle of. + * + * `GET /api/sessions/:id/terminal?tail=N` cuts the buffer at a byte offset, so + * a tail can start inside `ESC [ 12 ; 1 H` and hand the parser `;1H` as text, + * which is exactly what it then prints (observed against a live Claude pane). + * Only the severed head is dropped, never a whole line. + */ +export function dropSeveredEscape(raw: string): string { + return raw.replace(SEVERED_CSI, ''); +} + /** * Drop every escape sequence, keeping the visible text. Needed because the * preview carries the session's OWN colors: under NO_COLOR the frame must not diff --git a/src/tui/tui-app.ts b/src/tui/tui-app.ts index 760f65f6..7e485e05 100644 --- a/src/tui/tui-app.ts +++ b/src/tui/tui-app.ts @@ -50,7 +50,7 @@ import chalk from 'chalk'; import { palette, table, tint, type Tone } from '../cli-style.js'; import { CODEMAN_INSTANCE, resolveTmuxSocketName } from '../config/instance.js'; import { getErrorMessage } from '../types/api.js'; -import { toDisplayLines } from './tui-ansi.js'; +import { dropSeveredEscape, toDisplayLines } from './tui-ansi.js'; import { approvalAnswerForKey, newApprovalIds } from './tui-approvals.js'; import { composerScroll, composerStep, composerText, createComposer, type TuiComposerState } from './tui-composer.js'; import { formatAwayDigest } from './tui-digest.js'; @@ -895,7 +895,8 @@ class TuiApp { try { const raw = await this.client.fetchTerminalTail(sessionId, PREVIEW_TAIL_BYTES); if (this.previewSessionId !== sessionId) return; - this.applyPreview({ sessionId, lines: toDisplayLines(raw).slice(-PREVIEW_MAX_LINES) }); + const lines = toDisplayLines(dropSeveredEscape(raw)).slice(-PREVIEW_MAX_LINES); + this.applyPreview({ sessionId, lines }); } catch { // A tail that cannot be read is a pane-level fact, not a connection one: // the list stays exactly as it is and only this pane says so. diff --git a/src/tui/tui-model.ts b/src/tui/tui-model.ts index aebaa15d..59c81c18 100644 --- a/src/tui/tui-model.ts +++ b/src/tui/tui-model.ts @@ -217,6 +217,16 @@ const SEARCH_GROUP_LABELS: Record = { * has a session id too, but selecting it would move the cursor to a row that is * not on the list. */ +/** + * A session snippet opens with the session's own name (`w1-alpha — /tmp/alpha`), + * which the row already shows in its first column. Dropping the repeat is what + * keeps a result row from reading as a stutter. + */ +function withoutLabelPrefix(snippet: string, label: string): string { + const rest = snippet.startsWith(label) ? snippet.slice(label.length) : snippet; + return rest === snippet ? snippet : rest.replace(/^\s*(?:[—:-]\s*)?/, ''); +} + export function buildSearchEntries( groups: readonly SearchResultGroup[], isLive: (sessionId: string) => boolean @@ -227,10 +237,11 @@ export function buildSearchEntries( entries.push({ kind: 'header', text: SEARCH_GROUP_LABELS[group.type] ?? group.type.toUpperCase() }); for (const result of group.results) { const live = result.jumpTo.kind === 'session' && isLive(result.sessionId); + const label = result.jumpTo.relativePath ?? result.sessionName ?? result.sessionId.slice(0, 8); entries.push({ kind: 'result', - text: result.jumpTo.relativePath ?? result.sessionName ?? result.sessionId.slice(0, 8), - detail: result.snippet, + text: label, + detail: withoutLabelPrefix(result.snippet, label), sessionId: result.sessionId, live, }); diff --git a/test/tui/tui-ansi.test.ts b/test/tui/tui-ansi.test.ts index 0dc3b83c..1a0687df 100644 --- a/test/tui/tui-ansi.test.ts +++ b/test/tui/tui-ansi.test.ts @@ -10,6 +10,7 @@ import { describe, it, expect } from 'vitest'; import { clipStyledLine, + dropSeveredEscape, padDisplay, stripStyles, toDisplayLines, @@ -42,14 +43,31 @@ describe('toDisplayLines', () => { expect(toDisplayLines('\x1b]0;window title\x1b\\text')).toEqual(['text']); }); - it('strips DECSET/DECRST, cursor movement and charset selection', () => { + it('strips DECSET/DECRST, relative cursor movement and charset selection', () => { expect(toDisplayLines('\x1b[?25lvisible\x1b[?25h')).toEqual(['visible']); expect(toDisplayLines('a\x1b[5Cb')).toEqual(['ab']); - expect(toDisplayLines('\x1b[2J\x1b[H\x1b[1;1Hhome')).toEqual(['home']); expect(toDisplayLines('\x1b(0lqk\x1b(B')).toEqual(['lqk']); expect(toDisplayLines('\x1b=app\x1b>')).toEqual(['app']); }); + it('splits a row-addressed repaint into lines, which is how an Ink TUI paints', () => { + // Claude Code emits almost no newlines: without this the whole screen is + // one line and nothing in the preview is readable. + expect(toDisplayLines('\x1b[1;1Hfirst\x1b[2;1Hsecond\x1b[3;1Hthird')).toEqual(['', 'first', 'second', 'third']); + // A jump inside a row is a write position, not a new line. + expect(toDisplayLines('\x1b[1;1Hab\x1b[1;5Hcd')).toEqual(['', 'ab cd']); + expect(toDisplayLines('\x1b[1;1Habcdef\x1b[1;2HXY')).toEqual(['', 'aXYdef']); + // Both parameters default to 1, so a bare CUP is a fresh row. + expect(toDisplayLines('a\x1b[Hb')).toEqual(['a', 'b']); + expect(toDisplayLines('a\x1b[3;1fb')).toEqual(['a', 'b']); + }); + + it('refuses to allocate a line for a column no terminal has', () => { + const lines = toDisplayLines('\x1b[1;99999Hx'); + expect(lines).toHaveLength(1); + expect(visibleWidth(lines[0])).toBeLessThanOrEqual(1001); + }); + it('strips C1 controls and their sequences', () => { expect(toDisplayLines('a\x9b31mb')).toEqual(['ab']); expect(toDisplayLines('a\x9d0;title\x9cb')).toEqual(['ab']); @@ -89,6 +107,20 @@ describe('toDisplayLines', () => { }); }); +describe('dropSeveredEscape', () => { + it('drops the remains of a sequence a byte-sliced tail starts inside', () => { + expect(dropSeveredEscape(';1Hstill here')).toBe('still here'); + expect(dropSeveredEscape('12;3Htext')).toBe('text'); + expect(dropSeveredEscape('31mred')).toBe('red'); + }); + + it('leaves ordinary text alone', () => { + expect(dropSeveredEscape('hello world')).toBe('hello world'); + expect(dropSeveredEscape('\x1b[31mred')).toBe('\x1b[31mred'); + expect(dropSeveredEscape('')).toBe(''); + }); +}); + describe('visibleWidth', () => { it('ignores escape sequences', () => { expect(visibleWidth(`${RED}abc${RESET}`)).toBe(3); diff --git a/test/tui/tui-model.test.ts b/test/tui/tui-model.test.ts index 1a9c1924..4076d5b4 100644 --- a/test/tui/tui-model.test.ts +++ b/test/tui/tui-model.test.ts @@ -332,7 +332,7 @@ describe('search results', () => { sessionId: 'live-1', sessionName: 'w1-alpha', timestamp: NOW, - snippet: '/tmp/alpha', + snippet: 'w1-alpha — /tmp/alpha', exactMatch: true, jumpTo: { kind: 'session', sessionId: 'live-1' }, }, @@ -368,6 +368,8 @@ describe('search results', () => { expect(entries.map((entry) => entry.kind)).toEqual(['header', 'result', 'result', 'header', 'result']); expect(entries[0].text).toBe('SESSIONS'); expect(entries[1]).toMatchObject({ text: 'w1-alpha', sessionId: 'live-1', live: true }); + // The snippet opens with the session name, which the row already shows. + expect(entries[1].detail).toBe('/tmp/alpha'); // A session that is not on the list cannot be selected into. expect(entries[2]).toMatchObject({ text: 'w9-old', live: false }); expect(entries[4]).toMatchObject({ text: 'docs/notes.md', live: false });