From bfa6d52d6699f9fee5aaccc7fa2b52a94a7afdfe Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 23 Aug 2026 21:58:13 -0700 Subject: [PATCH 1/2] Shrink the approval overlay's context text to keep choices and the prompt box visible on short terminals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decision overlay's context body used a fixed 8-line budget regardless of terminal height. On a short terminal the resulting chrome could exceed the overlay's own render minimum (border + title + header + at least one choice), so the geometry resolver's "nothing left to collapse" fallback accepted an overlay smaller than that minimum — the box rendered past its assigned rows or lost every choice, while the prompt box stayed on screen but unanswerable. The context budget is now computed from the terminal height, the prompt floor, and the overlay's fraction cap, shrinking (down to 0 lines) so the header and at least one choice row always fit. Recomputed on resize too. --- CHANGELOG.md | 12 ++ src/tui/approval-prompt-visibility.test.ts | 165 +++++++++++++++++++++ src/tui/overlay-body.ts | 8 +- src/tui/overlay-overflow.test.ts | 5 +- src/tui/shell.ts | 84 ++++++++++- 5 files changed, 266 insertions(+), 8 deletions(-) create mode 100644 src/tui/approval-prompt-visibility.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 44f306fa5..6d4314291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,18 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename as an unhandled rejection. A failed close now short-circuits the rebuild with a clear, catchable error instead of retrying a doomed second acquisition. +### TUI + +- **The approval overlay no longer pushes the prompt box off screen or clips + itself at the bottom.** On a short terminal the fixed context budget behind + a permission/operator approval could ask for more rows than its frame had, + so the resolver fell back to sizing it below its own render minimum — the + overlay's border and choices painted past the frame, or vanished entirely, + while the prompt box was still on screen but the thing the operator needed + to answer was not. The overlay's context text now shrinks (down to dropping + it entirely on the shortest terminals) so the header, every choice's row + budget, and the prompt box at its floor always fit together; choices win the + row budget over context detail when a terminal is too short for both. ## [0.2.107] - 2026-08-24 diff --git a/src/tui/approval-prompt-visibility.test.ts b/src/tui/approval-prompt-visibility.test.ts new file mode 100644 index 000000000..8e7d3e754 --- /dev/null +++ b/src/tui/approval-prompt-visibility.test.ts @@ -0,0 +1,165 @@ +/** + * CL-5750: the approval surface must never push the prompt box off screen, + * and its choices must always be visible/reachable — an unanswerable + * approval deadlocks the session, so the choices win the row budget over + * the prompt box's growth and over the overlay's own context text. + */ +import { describe, expect, test } from "bun:test"; +import { withTestRenderer } from "./harness.js"; +import { createAppShell, appendStreamRow, type AppShell } from "./shell.js"; +import { openPermissionsOverlay, makePermissionItems } from "./overlays.js"; + +const WIDTH = 80; +// Deliberately spans from far below the documented 24-row baseline down to +// the shortest terminals the shell claims to support, plus a comfortable one. +const HEIGHTS = [10, 12, 15, 24, 40] as const; + +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 { + // In-flow layout: the overlay host competes with the prompt for rows the + // same way a live approval does mid-session (not the landing screen). + appendStreamRow(shell, { role: "assistant", text: "session underway" }); +} + +interface ApprovalSnapshot { + readonly frame: string; + readonly promptHeight: number; + readonly promptVisible: boolean; + readonly listHeight: number | null; + readonly hasOverlayList: boolean; +} + +async function paintApproval(height: number, itemCount = 6): Promise { + return withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: WIDTH, rows: height }, + run: "idle", + }); + try { + primeSession(shell); + openPermissionsOverlay(shell, { + items: makePermissionItems(itemCount), + body: APPROVAL_BODY, + }); + await h.renderOnce(); + await h.renderOnce(); + const frame = h.captureCharFrame().replace(/\n$/, ""); + return { + frame, + promptHeight: shell.layout.heights.prompt, + promptVisible: shell.promptBox.visible, + listHeight: shell.overlayList?.height ?? null, + hasOverlayList: shell.overlayList !== null, + }; + } finally { + shell.dispose(); + } + }, + { width: WIDTH, height }, + ); +} + +describe("approval overlay keeps the prompt box on screen (CL-5750)", () => { + for (const height of HEIGHTS) { + test(`prompt box stays visible and unclipped at ${height} rows`, async () => { + const snap = await paintApproval(height); + + // The prompt box is always assigned rows, never displaced entirely. + expect(snap.promptHeight).toBeGreaterThan(0); + expect(snap.promptVisible).toBe(true); + + // The painted frame actually shows the prompt box's border, not just + // internal state — the bug was a visual displacement, not a state one. + const lines = snap.frame.split("\n"); + expect(lines.length).toBeLessThanOrEqual(height); + const promptTopBorder = lines.findIndex((l) => l.includes("╭")); + const promptBottomBorder = lines.findIndex((l) => l.includes("╰")); + expect(promptTopBorder).toBeGreaterThanOrEqual(0); + expect(promptBottomBorder).toBeGreaterThan(promptTopBorder); + // The prompt box's own bottom rule (with the cwd/branding) must be + // fully painted within the frame, not cut off past the last row. + expect(promptBottomBorder).toBeLessThan(lines.length); + }); + + test(`at least one approval choice is painted and reachable at ${height} rows`, async () => { + const snap = await paintApproval(height); + + expect(snap.hasOverlayList).toBe(true); + // The active choice is always inside the viewport window state... + expect(snap.listHeight).toBeGreaterThanOrEqual(1); + + // ...and it is actually painted on screen, not just tracked in state: + // the marked active choice's label must appear in the frame. + expect(snap.frame).toContain("Allow once"); + }); + } + + test("overlay host never displaces the prompt box out of the frame even with a tall body", async () => { + const tallBody = Array.from({ length: 20 }, (_, i) => `context line ${i}`).join("\n"); + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: WIDTH, rows: 12 }, + run: "idle", + }); + try { + primeSession(shell); + openPermissionsOverlay(shell, { items: makePermissionItems(10), body: tallBody }); + await h.renderOnce(); + await h.renderOnce(); + const frame = h.captureCharFrame().replace(/\n$/, ""); + const lines = frame.split("\n"); + expect(lines.length).toBeLessThanOrEqual(12); + expect(shell.layout.heights.prompt).toBeGreaterThan(0); + expect(lines.some((l) => l.includes("╭"))).toBe(true); + expect(lines.some((l) => l.includes("╰"))).toBe(true); + expect(shell.overlayList).not.toBeNull(); + expect(shell.overlayList!.height).toBeGreaterThanOrEqual(1); + } finally { + shell.dispose(); + } + }, + { width: WIDTH, height: 12 }, + ); + }); + + test("no dead space between the transcript and the overlay: overlay sits directly above the prompt", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: WIDTH, rows: 24 }, + run: "idle", + }); + try { + primeSession(shell); + openPermissionsOverlay(shell, { items: makePermissionItems(6), body: APPROVAL_BODY }); + await h.renderOnce(); + await h.renderOnce(); + const heights = shell.layout.heights; + // Paint order stacks transcript, then overlay_host, then the other + // zones, then prompt — with nothing charged a height between the + // overlay and the prompt box, the overlay's bottom edge abuts the + // zones immediately above the prompt rather than leaving a gap. + // These zones are all off in this idle, no-task/no-agents scenario, + // so nothing should separate the overlay from the prompt. + const between = + heights.agents + + heights.task + + heights.plugin_banner + + heights.command_banner + + heights.settings_notice; + expect(between).toBe(0); + } finally { + shell.dispose(); + } + }, + { width: WIDTH, height: 24 }, + ); + }); +}); diff --git a/src/tui/overlay-body.ts b/src/tui/overlay-body.ts index eacfa62f3..c01bad2c8 100644 --- a/src/tui/overlay-body.ts +++ b/src/tui/overlay-body.ts @@ -152,7 +152,11 @@ export function composeDecisionBody( width: number, contextLines: number, ): OverlayBodyRow[] { - const budget = Math.max(1, Math.floor(contextLines)); + // Zero is a valid budget: on a terminal too short to spare a row of air + // plus a line of context on top of the header, the context section is + // dropped entirely rather than forced to cost at least one row it cannot + // afford — the choices below it must win that row instead. + const budget = Math.max(0, Math.floor(contextLines)); const lines = text.split("\n"); const headIndex = lines.findIndex((l) => l.trim().length > 0); if (headIndex < 0) return []; @@ -168,7 +172,7 @@ export function composeDecisionBody( }); }); - const rest = lines.slice(headIndex + 1).filter((l) => l.trim().length > 0); + const rest = budget > 0 ? lines.slice(headIndex + 1).filter((l) => l.trim().length > 0) : []; if (rest.length > 0) { rows.push({ text: "", fg: UI.textDim }); // Continuation rows are indented so a wrapped chain segment can never be diff --git a/src/tui/overlay-overflow.test.ts b/src/tui/overlay-overflow.test.ts index 73bef2c68..0e3f7be34 100644 --- a/src/tui/overlay-overflow.test.ts +++ b/src/tui/overlay-overflow.test.ts @@ -351,7 +351,10 @@ describe("gate-wire approval overflow on short terminal", () => { emitter.emit("permission.gate", { request, resolve: () => {} }); const body = permissionBodyFromRequest(request, { hint: true }); - expect(shell.overlayBodyLines.join("\n")).toContain(" r.text); shell.overlayBodyFgs = rows.map((r) => r.fg); return; @@ -5896,6 +5962,13 @@ export function createAppShell(renderer: ShellRenderer, options?: AppShellOption const onResize = (width: number, height: number): void => { if (disposed) return; const bag = internals.get(shell); + // A decision overlay's body was shaped against the old height's context + // budget; a shorter terminal can no longer afford as much of it without + // crowding out the choices, so it is re-shaped before asking for rows. + if (shell.overlayList && isDecisionOverlay(shell.overlayKind) && bag) { + applyOverlayBodyText(shell, bag.overlayRawBodyText, 0, height); + relayoutOverlayHost(shell, shell.overlayItems.length); + } relayout(shell, { columns: width, rows: height, @@ -6015,6 +6088,7 @@ export function createAppShell(renderer: ShellRenderer, options?: AppShellOption overlayMode: "closed", overlayBodyRows: undefined, overlayMinBodyRows: undefined, + overlayRawBodyText: "", priorOverlay: null, overlayItemIds: [], overlayItemValues: [], From d7839db5439a932467b8a72c6baab7d99b2bdf82 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 23 Aug 2026 22:20:32 -0700 Subject: [PATCH 2/2] Stop a stacked palette from clobbering the approval overlay's cached body text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit applyOverlayBodyText cached every opened overlay's raw body text for a resize to re-shape against, but a palette stacked over an open permission or operator overlay called it too, with its own (empty) body — overwriting the approval's cache. Popping the palette restores overlayBodyLines from the snapshot but not this cache, so a resize right after re-shaped the approval from the palette's stale empty string, blanking its body entirely (header included). The cache write is now scoped to decision overlays, since a palette never reads it back. Also tightens decisionContextBudget's docblock and a test comment that overclaimed the guarantee held below 10 rows; the resolver's own collapse fallback (unrelated to this budget) still has a gap below that floor. --- src/tui/approval-prompt-visibility.test.ts | 2 +- src/tui/overlay-body-cache-staleness.test.ts | 68 ++++++++++++++++++++ src/tui/shell.ts | 20 +++++- 3 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 src/tui/overlay-body-cache-staleness.test.ts diff --git a/src/tui/approval-prompt-visibility.test.ts b/src/tui/approval-prompt-visibility.test.ts index 8e7d3e754..9c25c652e 100644 --- a/src/tui/approval-prompt-visibility.test.ts +++ b/src/tui/approval-prompt-visibility.test.ts @@ -11,7 +11,7 @@ import { openPermissionsOverlay, makePermissionItems } from "./overlays.js"; const WIDTH = 80; // Deliberately spans from far below the documented 24-row baseline down to -// the shortest terminals the shell claims to support, plus a comfortable one. +// 10 rows, the shortest terminal this fix guarantees, plus a comfortable one. const HEIGHTS = [10, 12, 15, 24, 40] as const; const APPROVAL_BODY = [ diff --git a/src/tui/overlay-body-cache-staleness.test.ts b/src/tui/overlay-body-cache-staleness.test.ts new file mode 100644 index 000000000..5a595368f --- /dev/null +++ b/src/tui/overlay-body-cache-staleness.test.ts @@ -0,0 +1,68 @@ +/** + * CL-5750 follow-up: a palette stacked over an open approval must not + * clobber the approval's cached raw body text, which a resize re-shapes + * against the new height's context budget (see `decisionContextBudget` / + * `applyOverlayBodyText` in shell.ts). + */ +import { describe, expect, test } from "bun:test"; +import { withTestRenderer } from "./harness.js"; +import { + createAppShell, + appendStreamRow, + closeInsetOverlay, + openPalette, + type AppShell, +} from "./shell.js"; +import { openPermissionsOverlay, makePermissionItems } from "./overlays.js"; + +function primeSession(shell: AppShell): void { + appendStreamRow(shell, { role: "assistant", text: "session underway" }); +} + +describe("decision overlay body cache survives a stacked palette", () => { + test("resize after popping a stacked palette re-shapes the approval's own body, not a blanked one", async () => { + await withTestRenderer( + async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }); + try { + primeSession(shell); + openPermissionsOverlay(shell, { + items: makePermissionItems(3), + body: "run_shell\nRun shell command\nSome context about the risky command.", + }); + await h.renderOnce(); + expect(shell.overlayBodyLines.length).toBeGreaterThan(0); + + // Stack a palette over the open permissions overlay — its own + // (empty) body must not overwrite the approval's cached raw text. + openPalette(shell, { + catalog: [{ id: "foo", label: "foo" }], + title: "commands", + }); + await h.renderOnce(); + expect(shell.overlayKind).toBe("palette"); + + // Pop the palette back to the permissions overlay underneath. + closeInsetOverlay(shell); + await h.renderOnce(); + expect(shell.overlayKind).toBe("permissions"); + expect(shell.overlayBodyLines.length).toBeGreaterThan(0); + + // Resize: the body must still show the permission context, not be + // blanked by re-shaping from the palette's stale empty cache. + h.resize(80, 20); + await h.renderOnce(); + await h.renderOnce(); + expect(shell.overlayBodyLines.length).toBeGreaterThan(0); + expect(shell.overlayBodyLines.join("\n")).toContain("run_shell"); + } finally { + shell.dispose(); + } + }, + { width: 80, height: 24 }, + ); + }); +}); diff --git a/src/tui/shell.ts b/src/tui/shell.ts index 4b42cbb33..30b87f97a 100644 --- a/src/tui/shell.ts +++ b/src/tui/shell.ts @@ -3372,8 +3372,8 @@ const DECISION_CONTEXT_BLANK_ROWS = 1; /** * Shrink the decision body's context budget so its own chrome never crowds - * out the one thing that must survive any terminal height: at least one - * choice row, with the prompt box still seated at its floor below it. A + * out the one thing this fix guarantees down to a 10-row terminal: at least + * one choice row, with the prompt box still seated at its floor below it. A * generous, fixed context budget reads fine on a tall terminal, but on a * short one it can consume the entire overlay host, leaving no room to paint * a single option — the operator is then asked to decide between choices @@ -3381,6 +3381,12 @@ const DECISION_CONTEXT_BLANK_ROWS = 1; * on the shortest terminals, is the deliberate trade: the header (which tool, * 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. */ function decisionContextBudget( shell: AppShell, @@ -3409,7 +3415,15 @@ function applyOverlayBodyText( ): void { const width = overlayRowWidth(shell); const bag = internals.get(shell); - if (bag) bag.overlayRawBodyText = text; + // Scoped to decision overlays: a palette stacked over an open approval + // calls this too, with its own (usually empty) body text. Caching that + // would overwrite the approval's cached raw text with the palette's, and + // popping the palette restores the approval's `overlayBodyLines` but not + // this cache (`PriorOverlaySnapshot` never carried it) — so a resize right + // after would re-shape the approval's body from the palette's stale empty + // string instead of its own, blanking it. The palette itself never reads + // this cache (not a decision overlay), so it never needs to be cached. + if (bag && isDecisionOverlay(shell.overlayKind)) bag.overlayRawBodyText = text; if (text.length === 0) { shell.overlayBodyLines = []; shell.overlayBodyFgs = [];