Skip to content

Commit b6d9ccc

Browse files
committed
Grant an open overlay its minimum even below the prompt floor
An unanswerable approval deadlocks the session; a cramped prompt does not. Best-effort geometry re-checks the overlay minimum and may take rows from below the prompt floor so the border still closes and at least one choice remains painted.
1 parent 55d3d3e commit b6d9ccc

11 files changed

Lines changed: 243 additions & 37 deletions

docs/TUI.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,11 @@ The geometry resolver iteratively collapses optional chrome to make room for
364364
both the transcript floor and this overlay minimum before it ever accepts a
365365
transcript-below-floor outcome; only when nothing is left to collapse does it
366366
fall back to best effort (`resolveGeometry`'s collapse loop in
367-
`geometry/resolve.ts`). An overlay must never paint past the box it was
368-
actually assigned.
367+
`geometry/resolve.ts`). Best effort re-checks the assigned overlay height
368+
against that minimum: if the overlay is still short, it may take rows from
369+
below the prompt floor (`PROMPT_BASE_ROWS`). An unanswerable approval
370+
deadlocks the session; a cramped prompt does not. An overlay must never paint
371+
past the box it was actually assigned, and its border must always close.
369372

370373
Escape dismisses the open overlay and, for a permission or operator prompt,
371374
that dismissal **denies** the request rather than leaving it unresolved

src/tui/decision-truncation.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ describe("decision choice rendering", () => {
5454
await withTestRenderer(async (h) => {
5555
const contentWidth = 60;
5656
const list = createOverlayList(h.renderer, { count: 1, items: 4 });
57+
list.setHeight(list.height, DECISION_CHOICE_ROWS);
5758
const view = createOverlayView(h.renderer);
5859
h.renderer.root.add(view.host);
5960
view.host.visible = true;

src/tui/geometry/resolve.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,17 +389,33 @@ export function resolveGeometry(input: GeometryInput): GeometryLayout {
389389
// Need more space: collapse one zone, then retry.
390390
const cut = collapseOnce(heights, collapsed);
391391
if (cut === null) {
392-
// Nothing left — relax the transcript floor rather than leave the
393-
// overlay under its own render minimum; accept best effort past that.
394-
heights.overlay_host = desiredOverlayHeight(
392+
// Nothing left to collapse. Relax the transcript floor, then re-check
393+
// against the overlay's own render minimum. An unanswerable approval
394+
// deadlocks the session; a cramped prompt does not — so the overlay
395+
// may take rows from below PROMPT_BASE_ROWS when even that still
396+
// cannot seat minOverlay.
397+
let overlay = desiredOverlayHeight(
395398
{ ...input, terminal },
396399
mode,
397400
chrome,
398401
0,
399402
);
403+
if (overlay < minOverlay) {
404+
const grant = Math.min(minOverlay, terminal.rows);
405+
const available = Math.max(0, terminal.rows - chrome);
406+
const deficit = grant - Math.min(overlay, available);
407+
if (deficit > 0 && heights.prompt > 0) {
408+
heights.prompt -= Math.min(deficit, heights.prompt);
409+
}
410+
overlay = Math.min(
411+
grant,
412+
Math.max(0, terminal.rows - sumChrome(heights)),
413+
);
414+
}
415+
heights.overlay_host = overlay;
400416
heights.transcript = Math.max(
401417
0,
402-
terminal.rows - sumChrome(heights) - heights.overlay_host,
418+
terminal.rows - sumChrome(heights) - overlay,
403419
);
404420
break;
405421
}

src/tui/geometry/zones.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ export const OVERLAY_MAX_FRACTION = 0.7;
169169
* one content row. The transcript floor exists to keep conversation visible,
170170
* but it must not starve an overlay the operator just opened below the rows
171171
* its own border costs — that renders past its box instead of shrinking.
172+
* When even this minimum cannot be granted beside the prompt floor, the
173+
* overlay may take rows from below PROMPT_BASE_ROWS.
172174
*/
173175
export const OVERLAY_MIN_ROWS = 3;
174176

src/tui/overlay-body.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,10 @@ const DECISION_CONTEXT_BLANK_ROWS = 1;
307307
* which question) and the choices are the two things an approval cannot
308308
* render without; the surrounding detail can give way first.
309309
*
310-
* Below 10 rows this budget alone cannot save the frame: the resolver's own
311-
* collapse fallback (`resolveGeometry` in geometry/resolve.ts) can still hand
312-
* the overlay host fewer rows than its render minimum once every other zone
313-
* is already at floor, which is a pre-existing gap in the resolver, not
314-
* something this budget controls.
310+
* Below 10 rows this budget alone cannot save the frame: the resolver then
311+
* falls back to best effort (`resolveGeometry` in geometry/resolve.ts) and
312+
* may take rows from below the prompt floor so the overlay still meets its
313+
* render minimum. This budget does not control that fallback.
315314
*/
316315
export function decisionContextBudget(input: {
317316
readonly terminalHeight: number;
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* CL-6986: an approval on a terminal shorter than the 10-row guarantee must
3+
* still be answerable. Geometry may steal from the prompt floor; the painted
4+
* overlay border must close; at least one choice row must appear in the frame.
5+
*/
6+
7+
import { describe, expect, test } from "bun:test";
8+
import { makePermissionItems, withTestRenderer } from "./harness.js";
9+
import {
10+
OVERLAY_MIN_ROWS,
11+
PROMPT_BASE_ROWS,
12+
resolveGeometry,
13+
} from "./geometry/index.js";
14+
import { appendStreamRow } from "./shell/chrome.js";
15+
import { createAppShell } from "./shell/index.js";
16+
import type { AppShell } from "./shell/internals.js";
17+
import { openPermissionsOverlay } from "./overlays.js";
18+
19+
const WIDTH = 80;
20+
const HEIGHTS = [6, 7, 9] as const;
21+
const MIN_APPROVAL_ROWS = 7;
22+
23+
const APPROVAL_BODY = [
24+
"run_shell",
25+
"Run shell command",
26+
"This is context describing what the tool is about to do to the workspace.",
27+
].join("\n");
28+
29+
function primeSession(shell: AppShell): void {
30+
appendStreamRow(shell, { role: "assistant", text: "session underway" });
31+
}
32+
33+
function overlayBorderLines(frame: string): {
34+
readonly top: number;
35+
readonly bottom: number;
36+
readonly lines: readonly string[];
37+
} {
38+
const lines = frame.replace(/\n$/, "").split("\n");
39+
const top = lines.findIndex((l) => l.trimStart().startsWith("┌"));
40+
const bottom = lines.findIndex(
41+
(l, i) => i > top && l.trimStart().startsWith("└"),
42+
);
43+
return { top, bottom, lines };
44+
}
45+
46+
describe("resolveGeometry — best-effort overlay minimum", () => {
47+
for (const rows of HEIGHTS) {
48+
test(`grants minOverlay on a ${rows}-row terminal, stealing from the prompt if needed`, () => {
49+
const layout = resolveGeometry({
50+
terminal: { columns: WIDTH, rows },
51+
overlay: {
52+
mode: "inset",
53+
bodyRows: 48,
54+
minBodyRows: MIN_APPROVAL_ROWS,
55+
},
56+
});
57+
const granted = Math.min(MIN_APPROVAL_ROWS, rows);
58+
expect(layout.overlayHeight).toBeGreaterThanOrEqual(granted);
59+
expect(
60+
layout.chromeHeight + layout.overlayHeight + layout.transcriptHeight,
61+
).toBe(rows);
62+
if (rows < MIN_APPROVAL_ROWS + PROMPT_BASE_ROWS) {
63+
expect(layout.heights.prompt).toBeLessThan(PROMPT_BASE_ROWS);
64+
}
65+
});
66+
67+
test(`never sizes an open overlay below OVERLAY_MIN_ROWS when a ${rows}-row terminal can seat it`, () => {
68+
const layout = resolveGeometry({
69+
terminal: { columns: WIDTH, rows },
70+
overlay: { mode: "inset", bodyRows: 48 },
71+
});
72+
expect(layout.overlayHeight).toBeGreaterThanOrEqual(
73+
Math.min(OVERLAY_MIN_ROWS, rows),
74+
);
75+
});
76+
}
77+
});
78+
79+
describe("approval overlay remains answerable below 10 rows", () => {
80+
for (const height of HEIGHTS) {
81+
test(`closed overlay border and a painted choice at ${height} rows`, async () => {
82+
await withTestRenderer(
83+
async (h) => {
84+
const shell = createAppShell(h.renderer, {
85+
terminal: { columns: WIDTH, rows: height },
86+
run: "idle",
87+
});
88+
try {
89+
primeSession(shell);
90+
openPermissionsOverlay(shell, {
91+
items: makePermissionItems(6),
92+
body: APPROVAL_BODY,
93+
});
94+
await h.renderOnce();
95+
await h.renderOnce();
96+
const frame = h.captureCharFrame();
97+
const { top, bottom, lines } = overlayBorderLines(frame);
98+
99+
expect(lines.length).toBeLessThanOrEqual(height);
100+
expect(top).toBeGreaterThanOrEqual(0);
101+
expect(bottom).toBeGreaterThan(top);
102+
expect(bottom).toBeLessThan(lines.length);
103+
104+
const borderOnly = /^[]+$/;
105+
for (const idx of [top, bottom]) {
106+
const trimmed = lines[idx]?.trim() ?? "";
107+
expect(borderOnly.test(trimmed)).toBe(true);
108+
}
109+
110+
expect(frame).toContain("Allow once");
111+
} finally {
112+
shell.dispose();
113+
}
114+
},
115+
{ width: WIDTH, height },
116+
);
117+
});
118+
}
119+
});

src/tui/overlay-view.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ export function overlayChromeRows(
108108
* Smallest host rows the open overlay can render into without spilling past
109109
* its own box: fixed chrome (border, title, body lines) plus one row of the
110110
* list when it has anything to show. Below this the resolver must give ground
111-
* elsewhere (transcript floor) rather than starve the overlay itself.
111+
* elsewhere (transcript floor, then the prompt floor) rather than starve the
112+
* overlay itself.
112113
*/
113114
export function overlayMinHostRows(
114115
chromeRows: number,
@@ -383,11 +384,11 @@ export function createOverlayView(ctx: RenderContext) {
383384
paintDescriptionZone(presentation.describe, contentWidth);
384385
return;
385386
}
386-
const decision = isDecisionOverlay(presentation.kind);
387387
// Choice labels are bare action names (scope hints paint in the body
388388
// above), so each one paints SelectRenderable's name row plus its reserved
389-
// second row of air — nothing wraps, nothing clips.
390-
list.setHeight(list.height, decision ? DECISION_CHOICE_ROWS : 1);
389+
// second row of air — nothing wraps, nothing clips. A cramped host may
390+
// have already dropped that air to keep one choice inside the box.
391+
list.setHeight(list.height, list.rowsPerItem);
391392
list.select.showSelectionIndicator = true;
392393
list.select.options = presentation.items.map((label, i) => {
393394
const id = presentation.itemIds?.[i];

src/tui/palette-paint.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,12 @@ describe("command list height cap", () => {
424424
);
425425

426426
// Every plugin-inflated catalog and every terminal size gets a bounded
427-
// frame: the border-to-border row count above the prompt box never grows
428-
// past the terminal, and the box below stays intact and readable.
427+
// frame: the border-to-border row count never grows past the terminal.
428+
// Below the 10-row comfort line the overlay may take rows from the prompt
429+
// floor so the list stays painted; a cramped prompt is preferred to an
430+
// overflowed host.
429431
for (const height of [24, 16, 12, 8, 6]) {
430-
test(`stays within a ${height}-row terminal and keeps the prompt box intact`, async () => {
432+
test(`stays within a ${height}-row terminal`, async () => {
431433
await withTestRenderer(
432434
async (h) => {
433435
const shell = createAppShell(h.renderer, {
@@ -441,7 +443,10 @@ describe("command list height cap", () => {
441443
// captureCharFrame's trailing newline yields one extra split
442444
// element — the frame itself must not exceed the terminal rows.
443445
expect(lines.length).toBeLessThanOrEqual(height + 1);
444-
expect(lines.some((l) => l.includes("message…"))).toBe(true);
446+
expect(lines.some((l) => l.includes("Fake command"))).toBe(true);
447+
if (height >= 12) {
448+
expect(lines.some((l) => l.includes("message…"))).toBe(true);
449+
}
445450
},
446451
{ width: 80, height },
447452
);

src/tui/shell/chrome.ts

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,20 @@ export function activeOverlayItemId(
470470
}
471471

472472
export function paintOverlayList(shell: AppShell): void {
473+
const list = shell.overlayList;
474+
if (!list) return;
475+
// Geometry's assigned host rows, not overlayHost.height: OpenTUI still
476+
// reports the dummy height 1 until the next layout pass, and fitting to
477+
// that dummy drops the decision header on a host that is actually tall.
478+
const hostH = Math.max(0, shell.layout.overlayHeight);
479+
if (hostH > 0) {
480+
fitOverlayListToHost(shell, hostH);
481+
return;
482+
}
483+
paintOverlayListContents(shell);
484+
}
485+
486+
function paintOverlayListContents(shell: AppShell): void {
473487
const list = shell.overlayList;
474488
if (!list) return;
475489
const bag = shellInternals(shell);
@@ -624,6 +638,57 @@ export function paintPromptBorder(shell: AppShell): void {
624638
shell.promptBottomRule.content = new StyledText(ruleChunks(shell, bottom));
625639
}
626640

641+
/**
642+
* Shrink overlay body and list so they fit the assigned host. A short
643+
* terminal can leave fewer rows than chrome plus a full-height choice;
644+
* dropping context first keeps one choice row inside the box so the
645+
* operator can still answer.
646+
*/
647+
function fitOverlayListToHost(shell: AppShell, hostH: number): void {
648+
const list = shell.overlayList;
649+
if (!list || hostH <= 0) return;
650+
const bag = shellInternals(shell);
651+
const hasDesc = !!bag?.primaryBindings.describe;
652+
const hasAnswer = overlayAnswerState(shell) !== null;
653+
const perItem = overlayRowsPerItem(shell.overlayKind);
654+
const hasItems = shell.overlayItems.length > 0;
655+
const choiceWant = hasItems ? perItem : 0;
656+
const choiceMin = hasItems ? 1 : 0;
657+
let bodyCount = shell.overlayBodyLines.length;
658+
const chromeOf = (n: number): number =>
659+
overlayChromeRows(shell.overlayKind, n, hasDesc, hasAnswer);
660+
let chrome = chromeOf(bodyCount);
661+
while (bodyCount > 0 && hostH - chrome < choiceMin) {
662+
bodyCount -= 1;
663+
chrome = chromeOf(bodyCount);
664+
}
665+
while (bodyCount > 0 && hostH - chrome < choiceWant) {
666+
bodyCount -= 1;
667+
chrome = chromeOf(bodyCount);
668+
}
669+
const bodyH = Math.max(0, hostH - chrome);
670+
if (bodyH >= perItem) {
671+
list.setHeight(
672+
Math.max(1, Math.floor(bodyH / perItem)),
673+
isDecisionOverlay(shell.overlayKind) ? DECISION_CHOICE_ROWS : 1,
674+
);
675+
} else if (bodyH >= 1 && hasItems) {
676+
list.setHeight(1, 1);
677+
}
678+
const savedLines = shell.overlayBodyLines;
679+
const savedFgs = shell.overlayBodyFgs;
680+
if (bodyCount < savedLines.length) {
681+
shell.overlayBodyLines = savedLines.slice(0, bodyCount);
682+
shell.overlayBodyFgs = savedFgs.slice(0, bodyCount);
683+
}
684+
try {
685+
paintOverlayListContents(shell);
686+
} finally {
687+
shell.overlayBodyLines = savedLines;
688+
shell.overlayBodyFgs = savedFgs;
689+
}
690+
}
691+
627692
export function applyLayout(shell: AppShell, layout: GeometryLayout): void {
628693
// Rows lay themselves out against the column budget (right-aligned bubbles,
629694
// pre-wrapped reasoning blocks), so a width change invalidates every painted
@@ -718,11 +783,16 @@ export function applyLayout(shell: AppShell, layout: GeometryLayout): void {
718783
shell.notice.height = noticeH > 0 ? noticeH : 1;
719784
shell.notice.visible = noticeH > 0;
720785

721-
const promptH = Math.max(1, h.prompt);
722-
shell.promptBox.height = promptH;
786+
const promptH = Math.max(0, h.prompt);
787+
shell.promptBox.height = promptH > 0 ? promptH : 1;
723788
shell.promptBox.visible = promptH > 0;
789+
const showPromptRules = promptH >= 2;
790+
const showPromptField = promptH >= 3;
791+
shell.promptTopRule.visible = showPromptRules || promptH === 1;
792+
shell.promptBottomRule.visible = showPromptRules;
793+
shell.promptField.visible = showPromptField;
724794
// The field takes whatever the box has left once both labelled rules are paid.
725-
const promptInnerH = Math.max(1, promptH - 2);
795+
const promptInnerH = showPromptField ? Math.max(1, promptH - 2) : 1;
726796
shell.promptField.height = promptInnerH;
727797
// Sized explicitly rather than left to grow with its content: past the cap the
728798
// 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 {
745815
shell.overlayHost.height = hostH > 0 ? hostH : 1;
746816
shell.overlayHost.visible = hostH > 0;
747817
if (hostH > 0 && shell.overlayList) {
748-
const chrome = overlayChromeRows(
749-
shell.overlayKind,
750-
shell.overlayBodyLines.length,
751-
!!bag?.primaryBindings.describe,
752-
overlayAnswerState(shell) !== null,
753-
);
754-
const bodyH = Math.max(1, hostH - chrome);
755-
// The viewport counts items, not rows; a decision overlay spends several
756-
// rows per item, so the row budget has to be divided back down.
757-
const perItem = overlayRowsPerItem(shell.overlayKind);
758-
shell.overlayList.setHeight(
759-
Math.max(1, Math.floor(bodyH / perItem)),
760-
isDecisionOverlay(shell.overlayKind) ? DECISION_CHOICE_ROWS : 1,
761-
);
762-
paintOverlayList(shell);
818+
fitOverlayListToHost(shell, hostH);
763819
}
764820

765821
paintPromptBorder(shell);

src/tui/shell/internals.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ export interface OverlayList {
602602
readonly activeIndex: number;
603603
/** Item-row capacity reserved by layout (not the renderable's row height). */
604604
readonly height: number;
605+
readonly rowsPerItem: number;
605606
readonly offset: number;
606607
readonly count: number;
607608
move(delta: number): void;

0 commit comments

Comments
 (0)