diff --git a/docs/TUI.md b/docs/TUI.md index 39e1596a4..ef65e7077 100644 --- a/docs/TUI.md +++ b/docs/TUI.md @@ -364,8 +364,11 @@ The geometry resolver iteratively collapses optional chrome to make room for both the transcript floor and this overlay minimum before it ever accepts a transcript-below-floor outcome; only when nothing is left to collapse does it fall back to best effort (`resolveGeometry`'s collapse loop in -`geometry/resolve.ts`). An overlay must never paint past the box it was -actually assigned. +`geometry/resolve.ts`). Best effort re-checks the assigned overlay height +against that minimum: if the overlay is still short, it may take rows from +below the prompt floor (`PROMPT_BASE_ROWS`). An unanswerable approval +deadlocks the session; a cramped prompt does not. An overlay must never paint +past the box it was actually assigned, and its border must always close. Escape dismisses the open overlay and, for a permission or operator prompt, that dismissal **denies** the request rather than leaving it unresolved diff --git a/src/tui/decision-truncation.test.ts b/src/tui/decision-truncation.test.ts index 606a244c2..d0b5e95a6 100644 --- a/src/tui/decision-truncation.test.ts +++ b/src/tui/decision-truncation.test.ts @@ -54,6 +54,7 @@ describe("decision choice rendering", () => { await withTestRenderer(async (h) => { const contentWidth = 60; const list = createOverlayList(h.renderer, { count: 1, items: 4 }); + list.setHeight(list.height, DECISION_CHOICE_ROWS); const view = createOverlayView(h.renderer); h.renderer.root.add(view.host); view.host.visible = true; diff --git a/src/tui/geometry/resolve.ts b/src/tui/geometry/resolve.ts index 3415e9527..b6733c49f 100644 --- a/src/tui/geometry/resolve.ts +++ b/src/tui/geometry/resolve.ts @@ -389,17 +389,33 @@ export function resolveGeometry(input: GeometryInput): GeometryLayout { // Need more space: collapse one zone, then retry. const cut = collapseOnce(heights, collapsed); if (cut === null) { - // Nothing left — relax the transcript floor rather than leave the - // overlay under its own render minimum; accept best effort past that. - heights.overlay_host = desiredOverlayHeight( + // Nothing left to collapse. Relax the transcript floor, then re-check + // against the overlay's own render minimum. An unanswerable approval + // deadlocks the session; a cramped prompt does not — so the overlay + // may take rows from below PROMPT_BASE_ROWS when even that still + // cannot seat minOverlay. + let overlay = desiredOverlayHeight( { ...input, terminal }, mode, chrome, 0, ); + if (overlay < minOverlay) { + const grant = Math.min(minOverlay, terminal.rows); + const available = Math.max(0, terminal.rows - chrome); + const deficit = grant - Math.min(overlay, available); + if (deficit > 0 && heights.prompt > 0) { + heights.prompt -= Math.min(deficit, heights.prompt); + } + overlay = Math.min( + grant, + Math.max(0, terminal.rows - sumChrome(heights)), + ); + } + heights.overlay_host = overlay; heights.transcript = Math.max( 0, - terminal.rows - sumChrome(heights) - heights.overlay_host, + terminal.rows - sumChrome(heights) - overlay, ); break; } diff --git a/src/tui/geometry/zones.ts b/src/tui/geometry/zones.ts index 04a692db7..de70250bc 100644 --- a/src/tui/geometry/zones.ts +++ b/src/tui/geometry/zones.ts @@ -169,6 +169,8 @@ export const OVERLAY_MAX_FRACTION = 0.7; * one content row. The transcript floor exists to keep conversation visible, * but it must not starve an overlay the operator just opened below the rows * its own border costs — that renders past its box instead of shrinking. + * When even this minimum cannot be granted beside the prompt floor, the + * overlay may take rows from below PROMPT_BASE_ROWS. */ export const OVERLAY_MIN_ROWS = 3; diff --git a/src/tui/overlay-body.ts b/src/tui/overlay-body.ts index 75a1b2d41..87c550b56 100644 --- a/src/tui/overlay-body.ts +++ b/src/tui/overlay-body.ts @@ -307,11 +307,10 @@ const DECISION_CONTEXT_BLANK_ROWS = 1; * which question) and the choices are the two things an approval cannot * render without; the surrounding detail can give way first. * - * Below 10 rows this budget alone cannot save the frame: the resolver's own - * collapse fallback (`resolveGeometry` in geometry/resolve.ts) can still hand - * the overlay host fewer rows than its render minimum once every other zone - * is already at floor, which is a pre-existing gap in the resolver, not - * something this budget controls. + * Below 10 rows this budget alone cannot save the frame: the resolver then + * falls back to best effort (`resolveGeometry` in geometry/resolve.ts) and + * may take rows from below the prompt floor so the overlay still meets its + * render minimum. This budget does not control that fallback. */ export function decisionContextBudget(input: { readonly terminalHeight: number; diff --git a/src/tui/overlay-min-geometry.test.ts b/src/tui/overlay-min-geometry.test.ts new file mode 100644 index 000000000..d4e7ef5b6 --- /dev/null +++ b/src/tui/overlay-min-geometry.test.ts @@ -0,0 +1,119 @@ +/** + * CL-6986: an approval on a terminal shorter than the 10-row guarantee must + * still be answerable. Geometry may steal from the prompt floor; the painted + * overlay border must close; at least one choice row must appear in the frame. + */ + +import { describe, expect, test } from "bun:test"; +import { makePermissionItems, withTestRenderer } from "./harness.js"; +import { + OVERLAY_MIN_ROWS, + PROMPT_BASE_ROWS, + resolveGeometry, +} from "./geometry/index.js"; +import { appendStreamRow } from "./shell/chrome.js"; +import { createAppShell } from "./shell/index.js"; +import type { AppShell } from "./shell/internals.js"; +import { openPermissionsOverlay } from "./overlays.js"; + +const WIDTH = 80; +const HEIGHTS = [6, 7, 9] as const; +const MIN_APPROVAL_ROWS = 7; + +const APPROVAL_BODY = [ + "run_shell", + "Run shell command", + "This is context describing what the tool is about to do to the workspace.", +].join("\n"); + +function primeSession(shell: AppShell): void { + appendStreamRow(shell, { role: "assistant", text: "session underway" }); +} + +function overlayBorderLines(frame: string): { + readonly top: number; + readonly bottom: number; + readonly lines: readonly string[]; +} { + const lines = frame.replace(/\n$/, "").split("\n"); + const top = lines.findIndex((l) => l.trimStart().startsWith("┌")); + const bottom = lines.findIndex( + (l, i) => i > top && l.trimStart().startsWith("└"), + ); + return { top, bottom, lines }; +} + +describe("resolveGeometry — best-effort overlay minimum", () => { + for (const rows of HEIGHTS) { + test(`grants minOverlay on a ${rows}-row terminal, stealing from the prompt if needed`, () => { + const layout = resolveGeometry({ + terminal: { columns: WIDTH, rows }, + overlay: { + mode: "inset", + bodyRows: 48, + minBodyRows: MIN_APPROVAL_ROWS, + }, + }); + const granted = Math.min(MIN_APPROVAL_ROWS, rows); + expect(layout.overlayHeight).toBeGreaterThanOrEqual(granted); + expect( + layout.chromeHeight + layout.overlayHeight + layout.transcriptHeight, + ).toBe(rows); + if (rows < MIN_APPROVAL_ROWS + PROMPT_BASE_ROWS) { + expect(layout.heights.prompt).toBeLessThan(PROMPT_BASE_ROWS); + } + }); + + test(`never sizes an open overlay below OVERLAY_MIN_ROWS when a ${rows}-row terminal can seat it`, () => { + const layout = resolveGeometry({ + terminal: { columns: WIDTH, rows }, + overlay: { mode: "inset", bodyRows: 48 }, + }); + expect(layout.overlayHeight).toBeGreaterThanOrEqual( + Math.min(OVERLAY_MIN_ROWS, rows), + ); + }); + } +}); + +describe("approval overlay remains answerable below 10 rows", () => { + for (const height of HEIGHTS) { + test(`closed overlay border and a painted choice at ${height} rows`, async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: WIDTH, rows: height }, + run: "idle", + }); + try { + primeSession(shell); + openPermissionsOverlay(shell, { + items: makePermissionItems(6), + body: APPROVAL_BODY, + }); + await h.renderOnce(); + await h.renderOnce(); + const frame = h.captureCharFrame(); + const { top, bottom, lines } = overlayBorderLines(frame); + + expect(lines.length).toBeLessThanOrEqual(height); + expect(top).toBeGreaterThanOrEqual(0); + expect(bottom).toBeGreaterThan(top); + expect(bottom).toBeLessThan(lines.length); + + const borderOnly = /^[┌└├┬┐┘─┤┴]+$/; + for (const idx of [top, bottom]) { + const trimmed = lines[idx]?.trim() ?? ""; + expect(borderOnly.test(trimmed)).toBe(true); + } + + expect(frame).toContain("Allow once"); + } finally { + shell.dispose(); + } + }, + { width: WIDTH, height }, + ); + }); + } +}); diff --git a/src/tui/overlay-view.ts b/src/tui/overlay-view.ts index 6afa138b6..3f0fb8bbe 100644 --- a/src/tui/overlay-view.ts +++ b/src/tui/overlay-view.ts @@ -108,7 +108,8 @@ export function overlayChromeRows( * Smallest host rows the open overlay can render into without spilling past * its own box: fixed chrome (border, title, body lines) plus one row of the * list when it has anything to show. Below this the resolver must give ground - * elsewhere (transcript floor) rather than starve the overlay itself. + * elsewhere (transcript floor, then the prompt floor) rather than starve the + * overlay itself. */ export function overlayMinHostRows( chromeRows: number, @@ -383,11 +384,11 @@ export function createOverlayView(ctx: RenderContext) { paintDescriptionZone(presentation.describe, contentWidth); return; } - const decision = isDecisionOverlay(presentation.kind); // Choice labels are bare action names (scope hints paint in the body // above), so each one paints SelectRenderable's name row plus its reserved - // second row of air — nothing wraps, nothing clips. - list.setHeight(list.height, decision ? DECISION_CHOICE_ROWS : 1); + // second row of air — nothing wraps, nothing clips. A cramped host may + // have already dropped that air to keep one choice inside the box. + list.setHeight(list.height, list.rowsPerItem); list.select.showSelectionIndicator = true; list.select.options = presentation.items.map((label, i) => { const id = presentation.itemIds?.[i]; diff --git a/src/tui/palette-paint.test.ts b/src/tui/palette-paint.test.ts index a6e3ff685..16aa46101 100644 --- a/src/tui/palette-paint.test.ts +++ b/src/tui/palette-paint.test.ts @@ -424,10 +424,12 @@ describe("command list height cap", () => { ); // 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. + // frame: the border-to-border row count never grows past the terminal. + // Below the 10-row comfort line the overlay may take rows from the prompt + // floor so the list stays painted; a cramped prompt is preferred to an + // overflowed host. for (const height of [24, 16, 12, 8, 6]) { - test(`stays within a ${height}-row terminal and keeps the prompt box intact`, async () => { + test(`stays within a ${height}-row terminal`, async () => { await withTestRenderer( async (h) => { const shell = createAppShell(h.renderer, { @@ -441,7 +443,10 @@ describe("command list height cap", () => { // 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); + expect(lines.some((l) => l.includes("Fake command"))).toBe(true); + if (height >= 12) { + expect(lines.some((l) => l.includes("message…"))).toBe(true); + } }, { width: 80, height }, ); diff --git a/src/tui/shell/chrome.ts b/src/tui/shell/chrome.ts index 00a2b9bf3..3409c9b17 100644 --- a/src/tui/shell/chrome.ts +++ b/src/tui/shell/chrome.ts @@ -470,6 +470,20 @@ export function activeOverlayItemId( } export function paintOverlayList(shell: AppShell): void { + const list = shell.overlayList; + if (!list) return; + // Geometry's assigned host rows, not overlayHost.height: OpenTUI still + // reports the dummy height 1 until the next layout pass, and fitting to + // that dummy drops the decision header on a host that is actually tall. + const hostH = Math.max(0, shell.layout.overlayHeight); + if (hostH > 0) { + fitOverlayListToHost(shell, hostH); + return; + } + paintOverlayListContents(shell); +} + +function paintOverlayListContents(shell: AppShell): void { const list = shell.overlayList; if (!list) return; const bag = shellInternals(shell); @@ -624,6 +638,57 @@ export function paintPromptBorder(shell: AppShell): void { shell.promptBottomRule.content = new StyledText(ruleChunks(shell, bottom)); } +/** + * Shrink overlay body and list so they fit the assigned host. A short + * terminal can leave fewer rows than chrome plus a full-height choice; + * dropping context first keeps one choice row inside the box so the + * operator can still answer. + */ +function fitOverlayListToHost(shell: AppShell, hostH: number): void { + const list = shell.overlayList; + if (!list || hostH <= 0) return; + const bag = shellInternals(shell); + const hasDesc = !!bag?.primaryBindings.describe; + const hasAnswer = overlayAnswerState(shell) !== null; + const perItem = overlayRowsPerItem(shell.overlayKind); + const hasItems = shell.overlayItems.length > 0; + const choiceWant = hasItems ? perItem : 0; + const choiceMin = hasItems ? 1 : 0; + let bodyCount = shell.overlayBodyLines.length; + const chromeOf = (n: number): number => + overlayChromeRows(shell.overlayKind, n, hasDesc, hasAnswer); + let chrome = chromeOf(bodyCount); + while (bodyCount > 0 && hostH - chrome < choiceMin) { + bodyCount -= 1; + chrome = chromeOf(bodyCount); + } + while (bodyCount > 0 && hostH - chrome < choiceWant) { + bodyCount -= 1; + chrome = chromeOf(bodyCount); + } + const bodyH = Math.max(0, hostH - chrome); + if (bodyH >= perItem) { + list.setHeight( + Math.max(1, Math.floor(bodyH / perItem)), + isDecisionOverlay(shell.overlayKind) ? DECISION_CHOICE_ROWS : 1, + ); + } else if (bodyH >= 1 && hasItems) { + list.setHeight(1, 1); + } + const savedLines = shell.overlayBodyLines; + const savedFgs = shell.overlayBodyFgs; + if (bodyCount < savedLines.length) { + shell.overlayBodyLines = savedLines.slice(0, bodyCount); + shell.overlayBodyFgs = savedFgs.slice(0, bodyCount); + } + try { + paintOverlayListContents(shell); + } finally { + shell.overlayBodyLines = savedLines; + shell.overlayBodyFgs = savedFgs; + } +} + export function applyLayout(shell: AppShell, layout: GeometryLayout): void { // Rows lay themselves out against the column budget (right-aligned bubbles, // pre-wrapped reasoning blocks), so a width change invalidates every painted @@ -718,11 +783,16 @@ export function applyLayout(shell: AppShell, layout: GeometryLayout): void { shell.notice.height = noticeH > 0 ? noticeH : 1; shell.notice.visible = noticeH > 0; - const promptH = Math.max(1, h.prompt); - shell.promptBox.height = promptH; + const promptH = Math.max(0, h.prompt); + shell.promptBox.height = promptH > 0 ? promptH : 1; shell.promptBox.visible = promptH > 0; + const showPromptRules = promptH >= 2; + const showPromptField = promptH >= 3; + shell.promptTopRule.visible = showPromptRules || promptH === 1; + shell.promptBottomRule.visible = showPromptRules; + shell.promptField.visible = showPromptField; // The field takes whatever the box has left once both labelled rules are paid. - const promptInnerH = Math.max(1, promptH - 2); + const promptInnerH = showPromptField ? Math.max(1, promptH - 2) : 1; shell.promptField.height = promptInnerH; // Sized explicitly rather than left to grow with its content: past the cap the // input has to scroll inside a fixed window instead of pushing the frame open. @@ -745,21 +815,7 @@ export function applyLayout(shell: AppShell, layout: GeometryLayout): void { shell.overlayHost.height = hostH > 0 ? hostH : 1; shell.overlayHost.visible = hostH > 0; if (hostH > 0 && shell.overlayList) { - const chrome = overlayChromeRows( - shell.overlayKind, - shell.overlayBodyLines.length, - !!bag?.primaryBindings.describe, - overlayAnswerState(shell) !== null, - ); - const bodyH = Math.max(1, hostH - chrome); - // The viewport counts items, not rows; a decision overlay spends several - // rows per item, so the row budget has to be divided back down. - const perItem = overlayRowsPerItem(shell.overlayKind); - shell.overlayList.setHeight( - Math.max(1, Math.floor(bodyH / perItem)), - isDecisionOverlay(shell.overlayKind) ? DECISION_CHOICE_ROWS : 1, - ); - paintOverlayList(shell); + fitOverlayListToHost(shell, hostH); } paintPromptBorder(shell); diff --git a/src/tui/shell/internals.ts b/src/tui/shell/internals.ts index f11397755..775948fff 100644 --- a/src/tui/shell/internals.ts +++ b/src/tui/shell/internals.ts @@ -602,6 +602,7 @@ export interface OverlayList { readonly activeIndex: number; /** Item-row capacity reserved by layout (not the renderable's row height). */ readonly height: number; + readonly rowsPerItem: number; readonly offset: number; readonly count: number; move(delta: number): void; diff --git a/src/tui/shell/overlay-list.ts b/src/tui/shell/overlay-list.ts index 748265766..ef2a28c7d 100644 --- a/src/tui/shell/overlay-list.ts +++ b/src/tui/shell/overlay-list.ts @@ -215,6 +215,9 @@ export function createOverlayList( get height() { return shape.items; }, + get rowsPerItem() { + return shape.rowsPerItem; + }, get offset() { return selectScrollState(select).offset; },