From 5b53a00a5fc7cc345c562d85b32be20caf174b98 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 29 Jul 2026 08:16:32 -0700 Subject: [PATCH] fix(tui): restore the header box's right border MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header box rendered with a left edge and a top rule that ran off the screen, no right border at all. It was exactly two columns too wide. `appLayout` sized the intro banner and session panel with `composer.cols - 2`. That is the *composer's* reserve; `composer.cols` is the terminal width and the composer separately subtracts its prompt prefix. The transcript's reserve is different and larger: a scrollbar gutter beside the ScrollBox (`marginLeft={1}` plus the scrollbar's own `width={1}`) and `paddingX={1}` on the inner column -- four columns, not two. The panel drew itself two columns wider than its container, and ScrollBox clipped the overflow, taking the right border with it. Two changes, either of which fixes the symptom; both are worth having: - Add `transcriptPanelWidth()` next to `stableComposerColumns()`, naming the transcript's reserve so the two budgets can't be confused again, and use it for both `Banner` and `SessionPanel`. - Drop the explicit `width={cols}` from the panel's outer Box. That prop made the border a *prediction* of the container width; without it the box simply stretches to whatever the container is, so a caller that over-reports can no longer push the border off screen. The feed column gains `flexGrow` to absorb the real remainder, and its footer rule becomes a top-border-only Box rather than `'─'.repeat(w)`, since a repeat count can't track a column that flexes. `width={cols}` was tuned against a harness whose container was the whole terminal, which is also why the render test missed this. The test now renders inside a stand-in for appLayout's real wrapper and asserts rows match the container width rather than the terminal width. A plain `` wrapper is not sufficient: it has flexShrink, so an over-wide child is quietly shrunk and the bug vanishes. ScrollBox constrains its children instead, so the child is clipped -- the on-screen behavior. Verified by mutation: restoring `width={cols}` fails 4 tests under the ScrollBox wrapper and 0 under a Box one. Adds a test that the panel ignores an over-reported `maxWidth`, plus unit coverage for `transcriptPanelWidth`. Suite: 8 failed / 1576 passed -- the same 8 pre-existing failures (createGatewayEventHandler, cursorDriftRegression, statusRule, useConfigSync, virtualHeights) as baseline. Co-Authored-By: Claude Opus 5 --- .../src/__tests__/brandingRenderWidth.test.ts | 104 +++++++++++++++--- .../__tests__/termuxComposerLayout.test.ts | 27 ++++- ui-tui/src/components/appLayout.tsx | 7 +- ui-tui/src/components/branding.tsx | 32 ++++-- ui-tui/src/lib/inputMetrics.ts | 18 +++ 5 files changed, 160 insertions(+), 28 deletions(-) diff --git a/ui-tui/src/__tests__/brandingRenderWidth.test.ts b/ui-tui/src/__tests__/brandingRenderWidth.test.ts index 5960d391f..8166c0bf7 100644 --- a/ui-tui/src/__tests__/brandingRenderWidth.test.ts +++ b/ui-tui/src/__tests__/brandingRenderWidth.test.ts @@ -1,10 +1,15 @@ import { PassThrough } from 'stream' -import { renderSync, stringWidth } from '@clawcodex/ink' +import { Box, renderSync, ScrollBox, stringWidth } from '@clawcodex/ink' import React from 'react' import { afterEach, describe, expect, it } from 'vitest' import { SessionPanel } from '../components/branding.js' +import { + TRANSCRIPT_PADDING_X, + TRANSCRIPT_SCROLLBAR_GUTTER, + transcriptPanelWidth +} from '../lib/inputMetrics.js' import { stripAnsi } from '../lib/text.js' import { DEFAULT_THEME } from '../theme.js' import type { SessionInfo, Theme } from '../types.js' @@ -58,7 +63,18 @@ const info = (over: Partial = {}): SessionInfo => ({ ...over }) -async function boxRows(cols: number, sessionInfo: SessionInfo, theme: Theme = DEFAULT_THEME): Promise { +const boxRows = (cols: number, sessionInfo: SessionInfo, theme: Theme = DEFAULT_THEME) => + render(cols, transcriptPanelWidth(cols), sessionInfo, theme) + +/** Same wrapper, but the panel is told a width its container does not have. */ +const boxRowsWithMaxWidth = (cols: number, maxWidth: number) => render(cols, maxWidth, info(), DEFAULT_THEME) + +async function render( + cols: number, + maxWidth: number, + sessionInfo: SessionInfo, + theme: Theme = DEFAULT_THEME +): Promise { Object.defineProperty(process.stdout, 'columns', { configurable: true, value: cols, writable: true }) const stdout = new PassThrough() @@ -74,12 +90,41 @@ async function boxRows(cols: number, sessionInfo: SessionInfo, theme: Theme = DE captured += chunk.toString() }) - const instance = renderSync(React.createElement(SessionPanel, { info: sessionInfo, sid: 'a1b2c3d4', t: theme }), { - patchConsole: false, - stderr: stderr as NodeJS.WriteStream, - stdin: stdin as NodeJS.ReadStream, - stdout: stdout as NodeJS.WriteStream - }) + // Mirrors appLayout's actual wrapper, NOT the root and NOT a plain Box. + // + // Both details are load-bearing. Rendering at the root is what let a + // two-column overflow pass this file originally. And a plain stand-in + // is not equivalent either: it has flexShrink, so an over-wide child is + // quietly shrunk to fit and the bug disappears. ScrollBox constrains its + // children instead of being expanded by them, so an over-wide child is + // CLIPPED — which is why the right border vanished on screen but not in a + // Box-wrapped test. + const instance = renderSync( + React.createElement( + Box, + { flexDirection: 'row', height: 40, width: cols }, + React.createElement( + ScrollBox, + { flexDirection: 'column', flexGrow: 1, flexShrink: 1 }, + React.createElement( + Box, + { flexDirection: 'column', paddingX: TRANSCRIPT_PADDING_X / 2 }, + React.createElement(SessionPanel, { info: sessionInfo, maxWidth, sid: 'a1b2c3d4', t: theme }) + ) + ), + React.createElement(Box, { + flexShrink: 0, + marginLeft: TRANSCRIPT_SCROLLBAR_GUTTER - 1, + width: TRANSCRIPT_SCROLLBAR_GUTTER - 1 + }) + ), + { + patchConsole: false, + stderr: stderr as NodeJS.WriteStream, + stdin: stdin as NodeJS.ReadStream, + stdout: stdout as NodeJS.WriteStream + } + ) try { await delay(30) @@ -90,8 +135,13 @@ async function boxRows(cols: number, sessionInfo: SessionInfo, theme: Theme = DE // pass against `undefined`. const plain = stripAnsi(captured) - // Only the box rows; the panel also emits a trailing margin line. - return plain.split('\n').filter(line => /^[╭│╰]/.test(line)) + // Only the box rows; the panel also emits a trailing margin line. The + // wrapper's left padding means rows no longer start at column 0. + return plain + .split('\n') + .map(line => line.trimEnd()) + .filter(line => /[╭│╰]/.test(line)) + .map(line => line.slice(line.search(/[╭│╰]/))) } finally { instance.unmount() instance.cleanup() @@ -101,13 +151,14 @@ async function boxRows(cols: number, sessionInfo: SessionInfo, theme: Theme = DE const WIDTHS = [34, 40, 60, 72, 88, 96, 100, 140, 200] describe('header box render width', () => { - it.each(WIDTHS)('emits rows exactly %i columns wide', async cols => { + it.each(WIDTHS)('fills its container exactly at %i terminal columns', async cols => { const rows = await boxRows(cols, info()) expect(rows.length).toBeGreaterThan(0) + // The container, not the terminal: overflowing it clips the right border. for (const row of rows) { - expect(stringWidth(row)).toBe(cols) + expect(stringWidth(row)).toBe(transcriptPanelWidth(cols)) } }) @@ -130,6 +181,27 @@ describe('header box render width', () => { } }) + // The shipped regression: appLayout passed `composer.cols - 2` (the COMPOSER's + // reserve) instead of the transcript's, so the panel believed it had two more + // columns than its container had. Combined with an explicit `width={cols}` on + // the box, the right border was pushed off screen — the box rendered with a + // left edge, a top rule running to the terminal edge, and no `╮`/`│`/`╯`. + // + // The panel must now survive a caller that over-reports: with no hard width it + // stretches to its real container regardless of what it was told. + it.each([34, 60, 100, 140])('ignores an over-reported maxWidth at %i columns', async cols => { + const rows = await boxRowsWithMaxWidth(cols, cols - 2) + const top = rows.find(r => r.startsWith('╭')) + + expect(rows.length).toBeGreaterThan(0) + expect(top).toBeDefined() + expect(top?.endsWith('╮')).toBe(true) + + for (const row of rows) { + expect(stringWidth(row)).toBe(transcriptPanelWidth(cols)) + } + }) + // A branded install pushes the title past `cols - 2` at ordinary widths. it('degrades a long brand title instead of destroying the top border', async () => { const branded: Theme = { ...DEFAULT_THEME, brand: { ...DEFAULT_THEME.brand, name: 'Acme Engineering Assistant' } } @@ -139,7 +211,7 @@ describe('header box render width', () => { const top = rows.find(r => r.startsWith('╭')) expect(top).toBeDefined() - expect(stringWidth(top ?? '')).toBe(cols) + expect(stringWidth(top ?? '')).toBe(transcriptPanelWidth(cols)) expect(top?.endsWith('╮')).toBe(true) } }) @@ -154,7 +226,7 @@ describe('header box render width', () => { expect(rows.length).toBeGreaterThan(0) for (const row of rows) { - expect(stringWidth(row)).toBe(100) + expect(stringWidth(row)).toBe(transcriptPanelWidth(100)) } }) @@ -164,7 +236,7 @@ describe('header box render width', () => { expect(rows.length).toBeGreaterThan(0) for (const row of rows) { - expect(stringWidth(row)).toBe(100) + expect(stringWidth(row)).toBe(transcriptPanelWidth(100)) } }) @@ -178,7 +250,7 @@ describe('header box render width', () => { expect(rows.length).toBeGreaterThan(0) for (const row of rows) { - expect(stringWidth(row)).toBe(100) + expect(stringWidth(row)).toBe(transcriptPanelWidth(100)) } } }) diff --git a/ui-tui/src/__tests__/termuxComposerLayout.test.ts b/ui-tui/src/__tests__/termuxComposerLayout.test.ts index 68e26cb4c..e31dfbaa6 100644 --- a/ui-tui/src/__tests__/termuxComposerLayout.test.ts +++ b/ui-tui/src/__tests__/termuxComposerLayout.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { stableComposerColumns, transcriptBodyWidth } from '../lib/inputMetrics.js' +import { stableComposerColumns, transcriptBodyWidth, transcriptPanelWidth } from '../lib/inputMetrics.js' import { composerPromptText } from '../lib/prompt.js' describe('Termux composer prompt + width guards', () => { @@ -31,3 +31,28 @@ describe('Termux composer prompt + width guards', () => { expect(transcriptBodyWidth(24, 'user', 'upstr >')).toBe(20) }) }) + +describe('transcriptPanelWidth', () => { + // The transcript's reserve is NOT the composer's. appLayout used to hand the + // intro banner and session panel `composer.cols - 2` — the composer's outer + // padding — while the transcript actually gives up 4 columns: a scrollbar + // gutter (marginLeft 1 + width 1) plus paddingX on both sides. The boxed + // panel was therefore two columns too wide and its right border was clipped + // off the screen entirely. + it('reserves the scrollbar gutter and the transcript padding', () => { + expect(transcriptPanelWidth(100)).toBe(96) + expect(transcriptPanelWidth(80)).toBe(76) + }) + + it('is strictly narrower than the composer reserve it replaced', () => { + for (const cols of [40, 60, 100, 200]) { + expect(transcriptPanelWidth(cols)).toBeLessThan(cols - 2) + } + }) + + it('never goes below one column', () => { + for (const cols of [0, 1, 2, 3, 4, 5]) { + expect(transcriptPanelWidth(cols)).toBeGreaterThanOrEqual(1) + } + }) +}) diff --git a/ui-tui/src/components/appLayout.tsx b/ui-tui/src/components/appLayout.tsx index 26eb82190..c99aeab66 100644 --- a/ui-tui/src/components/appLayout.tsx +++ b/ui-tui/src/components/appLayout.tsx @@ -14,7 +14,8 @@ import { COMPOSER_PROMPT_GAP_WIDTH, composerPromptWidth, inputVisualHeight, - stableComposerColumns + stableComposerColumns, + transcriptPanelWidth } from '../lib/inputMetrics.js' import { PerfPane } from '../lib/perfPane.js' import { composerPromptText } from '../lib/prompt.js' @@ -124,13 +125,13 @@ const TranscriptPane = memo(function TranscriptPane({ {row.msg.kind === 'intro' ? ( - + {row.msg.info && ( diff --git a/ui-tui/src/components/branding.tsx b/ui-tui/src/components/branding.tsx index 498d73feb..542963119 100644 --- a/ui-tui/src/components/branding.tsx +++ b/ui-tui/src/components/branding.tsx @@ -387,6 +387,7 @@ export function SessionPanel({ info, logoPalette, maxWidth, sid, t }: SessionPan [welcome, `${MODEL_LABEL}${modelLine}`, `${PATH_LABEL}${cwdLine}`, permsLine], heroW ) + const wide = fitsHorizontal(cols, leftW) // Stacked: the column IS the interior (border + paddingX = BORDER_PADDING). // Flooring this at MIN_RIGHT_WIDTH would overflow a narrow box — that floor @@ -675,10 +676,13 @@ export function SessionPanel({ info, logoPalette, maxWidth, sid, t }: SessionPan marginBottom={1} paddingX={1} paddingY={1} - // Without this the border stretches to the full terminal while the column - // math above is budgeted against `cols` (which honors maxWidth), so the - // right column stops short of its own border. - width={cols} + // Deliberately NO explicit width: the box stretches to its container, so + // the right border is wherever the container actually ends. Pinning it to + // `cols` instead makes the border a *prediction* of the container width, + // and when that prediction was two columns high (the transcript reserves a + // scrollbar gutter the composer budget doesn't) the right border was + // pushed off screen entirely. `cols` still drives the column split below, + // where being slightly off only shifts a truncation point. > {wide && ( @@ -700,7 +704,10 @@ export function SessionPanel({ info, logoPalette, maxWidth, sid, t }: SessionPan /> )} - + {/* flexGrow so the feed column absorbs the container's *actual* remainder + rather than the `w` we predicted; `w` stays the flex basis and the + truncation budget. */} + {/* Narrow layout drops the column split; keep the identity block above the feed so nothing is lost. */} {!wide && ( @@ -722,9 +729,18 @@ export function SessionPanel({ info, logoPalette, maxWidth, sid, t }: SessionPan ))} - - {'─'.repeat(Math.max(1, w))} - + {/* A top-border-only box, not `'─'.repeat(w)`: the column can flex away + from `w`, and a repeat-count rule would then under- or overshoot its + own column. A border stretches to whatever width the column got. */} + {toolsTotal} tools{' · '} diff --git a/ui-tui/src/lib/inputMetrics.ts b/ui-tui/src/lib/inputMetrics.ts index 6622a55bd..ae3395405 100644 --- a/ui-tui/src/lib/inputMetrics.ts +++ b/ui-tui/src/lib/inputMetrics.ts @@ -191,6 +191,24 @@ export function transcriptBodyWidth(totalCols: number, role: Role, userPrompt: s return Math.max(20, available) } +/** Scrollbar gutter beside the transcript: `marginLeft={1}` + `width={1}` on + * the `TranscriptScrollbar` wrapper in appLayout. Always reserved — the + * scrollbar renders a `width={1}` spacer even when it has nothing to show. */ +export const TRANSCRIPT_SCROLLBAR_GUTTER = 2 +/** `paddingX={1}` on the transcript's inner column, both sides. */ +export const TRANSCRIPT_PADDING_X = 2 + +/** Width actually available to a full-bleed transcript child (the intro banner + * and session panel), i.e. the ScrollBox interior. + * + * Not the same as `stableComposerColumns`, which budgets the *composer's* text + * area and subtracts the prompt prefix. Passing `totalCols - 2` here — the + * composer's reserve rather than the transcript's — makes a boxed child two + * columns too wide, which silently clips its right border off the screen. */ +export function transcriptPanelWidth(totalCols: number) { + return Math.max(1, totalCols - TRANSCRIPT_SCROLLBAR_GUTTER - TRANSCRIPT_PADDING_X) +} + export function stableComposerColumns(totalCols: number, promptWidth: number, termuxMode = false) { // Physical render/wrap width. Always reserve outer composer padding and // prompt prefix. Only reserve the transcript scrollbar gutter when the