Merge provider-setup short-terminal garbling fix

Author: TheGreatAxios

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 },
+    )
+  })
+})
+

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")
+  })
+})

src/tui-opentui/provider-setup.ts

@@ -645,6 +645,15 @@ export async function runProviderSetup(
   })
 
   function listHeight(): number {
+    // 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 - 14))
   }
@@ -669,25 +678,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 +796,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 +837,7 @@ export async function runProviderSetup(
     id: "provider-setup-footer",
     content: "",
     fg: UI.textFaint,
+    flexShrink: 0,
   })
 
   root.add(header)