From 5e1d9d18a80dfc30f3969335ba123f6dc1856e01 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 07:27:51 -0700 Subject: [PATCH 1/4] Stop the provider setup screen from garbling text on short terminals Every direct child of the setup screen's root column needs flexShrink: 0. header, intro, step, instruction, statusLine, guidance, and footer were all missing it while every sibling box already had it, so a terminal too short for the full column let the flex algorithm compress these unprotected single-line rows onto each other instead of clipping from the bottom. Reproduces on the provider pick-list and independently on the failed-connection-test screen, where statusLine and guidance are populated together. The list height budget also reserved more chrome rows than the picker actually uses, so the footer could go missing even on terminals that had room for it. --- src/tui-opentui/provider-setup.test.ts | 83 ++++++++++++++++++++++++++ src/tui-opentui/provider-setup.ts | 22 ++++++- 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/src/tui-opentui/provider-setup.test.ts b/src/tui-opentui/provider-setup.test.ts index d8dad1ee9..ff0437569 100644 --- a/src/tui-opentui/provider-setup.test.ts +++ b/src/tui-opentui/provider-setup.test.ts @@ -693,3 +693,86 @@ describe("runProviderSetup paste", () => { expect(values?.apiKey).toBe(key) }) }) + +describe("runProviderSetup pick-list height cap", () => { + // Every terminal size gets a bounded frame — no chrome row overlaps + // another (the header/intro/step/instruction rows used to compress into + // each other when the flex column ran out of room), and the picker never + // paints past the terminal's own row count. + for (const height of [24, 16, 12, 8, 6]) { + test(`stays within a ${height}-row terminal with no overlapping chrome`, async () => { + const harness = await createHarness({ width: 80, height }) + runProviderSetup({ + onSubmit: async () => {}, + showTelemetryNotice: false, + createRenderer: async () => harness.renderer, + }) + await harness.renderOnce() + await harness.renderOnce() + const lines = harness.captureCharFrame().split("\n") + expect(lines.length).toBeLessThanOrEqual(height + 1) + // The garbled-overlap bug glued the step line and the intro line + // together on one row; each survives as its own line, or is clipped + // entirely, but never merges into the other. + const stepLine = lines.find((l) => l.includes("step 1 of 3")) + if (stepLine !== undefined) { + expect(stepLine).not.toContain("connect an inference provider") + } + }) + } + + test("keyboard navigation scrolls a long provider list and keeps the active row visible", async () => { + const harness = await createHarness({ width: 80, height: 16 }) + runProviderSetup({ + onSubmit: async () => {}, + showTelemetryNotice: false, + createRenderer: async () => harness.renderer, + }) + await harness.renderOnce() + await harness.renderOnce() + const ids = providerChoiceRows(providerChoices()).map((r) => r.id) + for (let i = 0; i < ids.length - 1; i++) harness.pressKey("ARROW_DOWN") + await harness.renderOnce() + const frame = harness.captureCharFrame() + const last = providerChoiceRows(providerChoices()).at(-1) + expect(last).toBeDefined() + expect(frame).toContain(last!.label.slice(0, 20)) + }) + + // statusLine and guidance are both blank on the first screen these tests + // exercised — the garbling only showed up once a failed connection test + // populates both of them at once, so walk the flow there instead of + // stopping at the provider pick-list. + test("a failed connection test at a short terminal shows status and guidance on their own lines", async () => { + const harness = await createHarness({ width: 80, height: 16 }) + runProviderSetup({ + onSubmit: async (_values, _setPhase, opts) => { + if (!opts.skipValidation) throw new Error("connection refused") + }, + showTelemetryNotice: false, + createRenderer: async () => harness.renderer, + }) + await harness.renderOnce() + await harness.renderOnce() + await pickRow(harness, PROVIDER_IDS, "openai") + type(harness, "sk-key") + harness.pressKey("Enter") + await harness.renderOnce() + harness.pressKey("Enter") + await harness.renderOnce() + await new Promise((r) => setTimeout(r, 0)) + await harness.renderOnce() + + const lines = harness.captureCharFrame().split("\n") + expect(lines.length).toBeLessThanOrEqual(17) + const statusRow = lines.find((l) => l.includes("connection refused")) + const guidanceRow = lines.find((l) => l.includes("esc to re-enter")) + expect(statusRow).toBeDefined() + expect(guidanceRow).toBeDefined() + // The garbling bug glued these two rows together; each must survive as + // its own line, never merged into the other. + expect(statusRow).not.toBe(guidanceRow) + expect(statusRow).not.toContain("esc to re-enter") + expect(guidanceRow).not.toContain("connection refused") + }) +}) diff --git a/src/tui-opentui/provider-setup.ts b/src/tui-opentui/provider-setup.ts index e82cc3081..117379e04 100644 --- a/src/tui-opentui/provider-setup.ts +++ b/src/tui-opentui/provider-setup.ts @@ -645,8 +645,15 @@ export async function runProviderSetup( }) function listHeight(): number { + // Fixed chrome above the list: root padding, header, intro, step, + // instruction, one populated summary row, and the list box's own + // padding — plus the footer below it, and slack for a long label + // wrapping onto a second terminal row. A tighter budget than the old + // flat -14 so the list actually uses the room a short terminal leaves it + // instead of reserving rows nothing else needs and sitting short. + const chromeRows = 12 const rows = renderer.height || 24 - return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - 14)) + return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - chromeRows)) } const steps = (): readonly SetupStep[] => stepsFor(choice) @@ -669,25 +676,35 @@ export async function runProviderSetup( paddingRight: margin, }) + // Every direct child of `root` needs flexShrink: 0, full stop — a plain + // TextRenderable defaults to shrinkable, and a short terminal makes the + // flex algorithm compress unprotected single-line rows into each other + // (garbled overlapping text) instead of clipping the column from the + // bottom. header/intro/step/instruction here, and statusLine/guidance/ + // footer further down, all needed this; it is not specific to one step. const header = new TextRenderable(renderer, { id: "provider-setup-header", content: `${PRODUCT_NAME.toLowerCase()} · setup`, fg: UI.inFlightBright, + flexShrink: 0, }) const intro = new TextRenderable(renderer, { id: "provider-setup-welcome", content: "connect an inference provider — switch later with /model", fg: UI.textDim, + flexShrink: 0, }) const step = new TextRenderable(renderer, { id: "provider-setup-step", content: "", fg: UI.action, + flexShrink: 0, }) const instruction = new TextRenderable(renderer, { id: "provider-setup-instruction", content: "", fg: UI.text, + flexShrink: 0, }) const summary = new BoxRenderable(renderer, { @@ -777,11 +794,13 @@ export async function runProviderSetup( id: "provider-setup-status", content: "", fg: UI.textDim, + flexShrink: 0, }) const guidance = new TextRenderable(renderer, { id: "provider-setup-guidance", content: "", fg: UI.textDim, + flexShrink: 0, }) const telemetry = new BoxRenderable(renderer, { id: "provider-setup-telemetry", @@ -816,6 +835,7 @@ export async function runProviderSetup( id: "provider-setup-footer", content: "", fg: UI.textFaint, + flexShrink: 0, }) root.add(header) From dd66dadd02ce0e02f463511b3291f7c38a154c2c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 07:27:57 -0700 Subject: [PATCH 2/4] Cover the slash-command picker's height bounding with tests Investigation found the picker already routes through the shared list-viewport/resolveGeometry machinery: a 50-entry catalog stays bounded at 24, 16, 12, 8, and 6 terminal rows, the prompt box below it stays intact, and moving the selection scrolls the window so the active row is always visible. No sizing logic was missing, so this adds the regression coverage without a second windowing path. --- src/tui-opentui/palette-paint.test.ts | 62 +++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/tui-opentui/palette-paint.test.ts b/src/tui-opentui/palette-paint.test.ts index 689224421..03ba39c86 100644 --- a/src/tui-opentui/palette-paint.test.ts +++ b/src/tui-opentui/palette-paint.test.ts @@ -8,9 +8,11 @@ import type { KeyEvent } from "@opentui/core" import { withTestRenderer } from "./harness" import { openModelPickerOverlay } from "./overlays" +import type { PaletteCommand } from "./palette" import { createAppShell, handlePaletteFilterKey, + moveOverlaySelection, openPalette, type AppShell, } from "./shell" @@ -256,3 +258,63 @@ describe("command palette selection colour", () => { }) }) +describe("command palette height cap", () => { + const BIG_CATALOG: readonly PaletteCommand[] = Array.from( + { length: 50 }, + (_, i) => ({ + id: `cmd_${String(i)}`, + label: `Fake command number ${String(i)} with a longish label`, + dispatch: "command" as const, + }), + ) + + // Every plugin-inflated catalog and every terminal size gets a bounded + // frame: the border-to-border row count above the prompt box never grows + // past the terminal, and the box below stays intact and readable. + for (const height of [24, 16, 12, 8, 6]) { + test(`stays within a ${height}-row terminal and keeps the prompt box intact`, async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: height }, + wireKeys: false, + run: "idle", + }) + openPalette(shell, { catalog: BIG_CATALOG, title: "commands · /" }) + await h.renderOnce() + const lines = h.captureCharFrame().split("\n") + // captureCharFrame's trailing newline yields one extra split + // element — the frame itself must not exceed the terminal rows. + expect(lines.length).toBeLessThanOrEqual(height + 1) + expect(lines.some((l) => l.includes("message…"))).toBe(true) + }, + { width: 80, height }, + ) + }) + } + + test("scrolling the selection keeps the active row inside the window", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 12 }, + wireKeys: false, + run: "idle", + }) + openPalette(shell, { catalog: BIG_CATALOG, title: "commands · /" }) + await h.renderOnce() + for (let i = 0; i < 20; i++) moveOverlaySelection(shell, 1) + await h.renderOnce() + expect(shell.overlayList?.activeIndex).toBe(20) + const offset = shell.overlayList?.offset ?? 0 + const height = shell.overlayList?.height ?? 0 + expect(offset).toBeLessThanOrEqual(20) + expect(offset + height).toBeGreaterThan(20) + const frame = h.captureCharFrame() + expect(frame).toContain(`Fake command number 20`) + }, + { width: 80, height: 12 }, + ) + }) +}) + From 9e27ac38cc3a96e9390f42c91c0a5fc28e27b6bd Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 07:39:16 -0700 Subject: [PATCH 3/4] Derive the list-view chrome budget instead of guessing it listHeight() used a flat constant for the rows above and below the pick-list, re-guessed by hand whenever a row was added or removed. Compute it from the same building blocks the tree is made of instead: the seven always-visible single-line rows, root and listBox padding, the summary box's per-step rows, and the telemetry notice when shown. --- src/tui-opentui/provider-setup.ts | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/tui-opentui/provider-setup.ts b/src/tui-opentui/provider-setup.ts index 117379e04..58236f8d5 100644 --- a/src/tui-opentui/provider-setup.ts +++ b/src/tui-opentui/provider-setup.ts @@ -645,13 +645,20 @@ export async function runProviderSetup( }) function listHeight(): number { - // Fixed chrome above the list: root padding, header, intro, step, - // instruction, one populated summary row, and the list box's own - // padding — plus the footer below it, and slack for a long label - // wrapping onto a second terminal row. A tighter budget than the old - // flat -14 so the list actually uses the room a short terminal leaves it - // instead of reserving rows nothing else needs and sitting short. - const chromeRows = 12 + // Every row root carries besides listBox itself, during the steps where + // the list is shown (provider pick, or model pick for preset/oauth): + // loginBox and inputFrame are hidden then, so they cost nothing. + const singleLineRows = 7 // header, intro, step, instruction, statusLine, guidance, footer + const rootPadding = 1 + const listBoxPadding = 1 + // summary always renders one row per step in the active flow (values + // filled in as "done", the rest as "—"); PRESET_STEPS and OAUTH_STEPS + // are the only flows whose model/provider steps show the list, and both + // are the same length today, so take the max in case that changes. + const summaryRows = 1 + Math.max(PRESET_STEPS.length, OAUTH_STEPS.length) + const telemetryRows = config.showTelemetryNotice ? 1 + TELEMETRY_ROWS : 0 + const chromeRows = + rootPadding + singleLineRows + summaryRows + listBoxPadding + telemetryRows const rows = renderer.height || 24 return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - chromeRows)) } From 0d021d4b99db9e11643460aef5d94998c70ae0d6 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 08:38:32 -0700 Subject: [PATCH 4/4] Go back to a flat guess for the list-view chrome budget The derived chromeRows computation (singleLineRows, rootPadding, listBoxPadding, summaryRows) reads as principled but is still a hand-maintained count of the layout, now spread across four constants instead of one. Nothing catches it going stale when a row is added, since the tests cover height bounding, not the count, and the Math.max hedge covers a PRESET/OAUTH steps divergence that does not exist. Restored rows - 14 with a comment stating plainly that it is a guess: listHeight() runs before root is constructed, so there is no OpenTUI layout pass yet to measure a real chrome height from. Renderable.height and scrollHeight only reflect the last completed layout, populated post-mount, so they cannot help here. A shared, derived chrome budget for this and shell.ts's picker is tracked separately. --- src/tui-opentui/provider-setup.ts | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/tui-opentui/provider-setup.ts b/src/tui-opentui/provider-setup.ts index 58236f8d5..483d969e0 100644 --- a/src/tui-opentui/provider-setup.ts +++ b/src/tui-opentui/provider-setup.ts @@ -645,22 +645,17 @@ export async function runProviderSetup( }) function listHeight(): number { - // Every row root carries besides listBox itself, during the steps where - // the list is shown (provider pick, or model pick for preset/oauth): - // loginBox and inputFrame are hidden then, so they cost nothing. - const singleLineRows = 7 // header, intro, step, instruction, statusLine, guidance, footer - const rootPadding = 1 - const listBoxPadding = 1 - // summary always renders one row per step in the active flow (values - // filled in as "done", the rest as "—"); PRESET_STEPS and OAUTH_STEPS - // are the only flows whose model/provider steps show the list, and both - // are the same length today, so take the max in case that changes. - const summaryRows = 1 + Math.max(PRESET_STEPS.length, OAUTH_STEPS.length) - const telemetryRows = config.showTelemetryNotice ? 1 + TELEMETRY_ROWS : 0 - const chromeRows = - rootPadding + singleLineRows + summaryRows + listBoxPadding + telemetryRows + // This budget is a guess, not a derivation: it runs before `root` is + // even constructed below, so there has been no layout pass yet and + // nothing in OpenTUI to measure — Renderable.height and scrollHeight + // only reflect the last completed layout, populated post-mount. -14 + // is a hand count of the chrome rows above and below the list (header, + // intro, step, instruction, summary, statusLine, guidance, footer, and + // padding) with slack for a wrapped label; it goes stale if that chrome + // changes and nothing here will catch it. A shared, derived chrome + // budget for this and shell.ts's picker is tracked separately. const rows = renderer.height || 24 - return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - chromeRows)) + return Math.max(LIST_ROWS_MIN, Math.min(LIST_ROWS_MAX, rows - 14)) } const steps = (): readonly SetupStep[] => stepsFor(choice)