Skip to content

Commit 8af97ac

Browse files
Fix approval overlay pushing the prompt box off screen on short terminals (#597)
* Shrink the approval overlay's context text to keep choices and the prompt box visible on short terminals 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. * Stop a stacked palette from clobbering the approval overlay's cached body text 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.
1 parent e52cac0 commit 8af97ac

6 files changed

Lines changed: 348 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
4747
as an unhandled rejection. A failed close now short-circuits the rebuild
4848
with a clear, catchable error instead of retrying a doomed second
4949
acquisition.
50+
### TUI
51+
52+
- **The approval overlay no longer pushes the prompt box off screen or clips
53+
itself at the bottom.** On a short terminal the fixed context budget behind
54+
a permission/operator approval could ask for more rows than its frame had,
55+
so the resolver fell back to sizing it below its own render minimum — the
56+
overlay's border and choices painted past the frame, or vanished entirely,
57+
while the prompt box was still on screen but the thing the operator needed
58+
to answer was not. The overlay's context text now shrinks (down to dropping
59+
it entirely on the shortest terminals) so the header, every choice's row
60+
budget, and the prompt box at its floor always fit together; choices win the
61+
row budget over context detail when a terminal is too short for both.
5062

5163
## [0.2.107] - 2026-08-24
5264

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* CL-5750: the approval surface must never push the prompt box off screen,
3+
* and its choices must always be visible/reachable — an unanswerable
4+
* approval deadlocks the session, so the choices win the row budget over
5+
* the prompt box's growth and over the overlay's own context text.
6+
*/
7+
import { describe, expect, test } from "bun:test";
8+
import { withTestRenderer } from "./harness.js";
9+
import { createAppShell, appendStreamRow, type AppShell } from "./shell.js";
10+
import { openPermissionsOverlay, makePermissionItems } from "./overlays.js";
11+
12+
const WIDTH = 80;
13+
// Deliberately spans from far below the documented 24-row baseline down to
14+
// 10 rows, the shortest terminal this fix guarantees, plus a comfortable one.
15+
const HEIGHTS = [10, 12, 15, 24, 40] as const;
16+
17+
const APPROVAL_BODY = [
18+
"run_shell",
19+
"Run shell command",
20+
"This is context describing what the tool is about to do to the workspace.",
21+
].join("\n");
22+
23+
function primeSession(shell: AppShell): void {
24+
// In-flow layout: the overlay host competes with the prompt for rows the
25+
// same way a live approval does mid-session (not the landing screen).
26+
appendStreamRow(shell, { role: "assistant", text: "session underway" });
27+
}
28+
29+
interface ApprovalSnapshot {
30+
readonly frame: string;
31+
readonly promptHeight: number;
32+
readonly promptVisible: boolean;
33+
readonly listHeight: number | null;
34+
readonly hasOverlayList: boolean;
35+
}
36+
37+
async function paintApproval(height: number, itemCount = 6): Promise<ApprovalSnapshot> {
38+
return withTestRenderer(
39+
async (h) => {
40+
const shell = createAppShell(h.renderer, {
41+
terminal: { columns: WIDTH, rows: height },
42+
run: "idle",
43+
});
44+
try {
45+
primeSession(shell);
46+
openPermissionsOverlay(shell, {
47+
items: makePermissionItems(itemCount),
48+
body: APPROVAL_BODY,
49+
});
50+
await h.renderOnce();
51+
await h.renderOnce();
52+
const frame = h.captureCharFrame().replace(/\n$/, "");
53+
return {
54+
frame,
55+
promptHeight: shell.layout.heights.prompt,
56+
promptVisible: shell.promptBox.visible,
57+
listHeight: shell.overlayList?.height ?? null,
58+
hasOverlayList: shell.overlayList !== null,
59+
};
60+
} finally {
61+
shell.dispose();
62+
}
63+
},
64+
{ width: WIDTH, height },
65+
);
66+
}
67+
68+
describe("approval overlay keeps the prompt box on screen (CL-5750)", () => {
69+
for (const height of HEIGHTS) {
70+
test(`prompt box stays visible and unclipped at ${height} rows`, async () => {
71+
const snap = await paintApproval(height);
72+
73+
// The prompt box is always assigned rows, never displaced entirely.
74+
expect(snap.promptHeight).toBeGreaterThan(0);
75+
expect(snap.promptVisible).toBe(true);
76+
77+
// The painted frame actually shows the prompt box's border, not just
78+
// internal state — the bug was a visual displacement, not a state one.
79+
const lines = snap.frame.split("\n");
80+
expect(lines.length).toBeLessThanOrEqual(height);
81+
const promptTopBorder = lines.findIndex((l) => l.includes("╭"));
82+
const promptBottomBorder = lines.findIndex((l) => l.includes("╰"));
83+
expect(promptTopBorder).toBeGreaterThanOrEqual(0);
84+
expect(promptBottomBorder).toBeGreaterThan(promptTopBorder);
85+
// The prompt box's own bottom rule (with the cwd/branding) must be
86+
// fully painted within the frame, not cut off past the last row.
87+
expect(promptBottomBorder).toBeLessThan(lines.length);
88+
});
89+
90+
test(`at least one approval choice is painted and reachable at ${height} rows`, async () => {
91+
const snap = await paintApproval(height);
92+
93+
expect(snap.hasOverlayList).toBe(true);
94+
// The active choice is always inside the viewport window state...
95+
expect(snap.listHeight).toBeGreaterThanOrEqual(1);
96+
97+
// ...and it is actually painted on screen, not just tracked in state:
98+
// the marked active choice's label must appear in the frame.
99+
expect(snap.frame).toContain("Allow once");
100+
});
101+
}
102+
103+
test("overlay host never displaces the prompt box out of the frame even with a tall body", async () => {
104+
const tallBody = Array.from({ length: 20 }, (_, i) => `context line ${i}`).join("\n");
105+
await withTestRenderer(
106+
async (h) => {
107+
const shell = createAppShell(h.renderer, {
108+
terminal: { columns: WIDTH, rows: 12 },
109+
run: "idle",
110+
});
111+
try {
112+
primeSession(shell);
113+
openPermissionsOverlay(shell, { items: makePermissionItems(10), body: tallBody });
114+
await h.renderOnce();
115+
await h.renderOnce();
116+
const frame = h.captureCharFrame().replace(/\n$/, "");
117+
const lines = frame.split("\n");
118+
expect(lines.length).toBeLessThanOrEqual(12);
119+
expect(shell.layout.heights.prompt).toBeGreaterThan(0);
120+
expect(lines.some((l) => l.includes("╭"))).toBe(true);
121+
expect(lines.some((l) => l.includes("╰"))).toBe(true);
122+
expect(shell.overlayList).not.toBeNull();
123+
expect(shell.overlayList!.height).toBeGreaterThanOrEqual(1);
124+
} finally {
125+
shell.dispose();
126+
}
127+
},
128+
{ width: WIDTH, height: 12 },
129+
);
130+
});
131+
132+
test("no dead space between the transcript and the overlay: overlay sits directly above the prompt", async () => {
133+
await withTestRenderer(
134+
async (h) => {
135+
const shell = createAppShell(h.renderer, {
136+
terminal: { columns: WIDTH, rows: 24 },
137+
run: "idle",
138+
});
139+
try {
140+
primeSession(shell);
141+
openPermissionsOverlay(shell, { items: makePermissionItems(6), body: APPROVAL_BODY });
142+
await h.renderOnce();
143+
await h.renderOnce();
144+
const heights = shell.layout.heights;
145+
// Paint order stacks transcript, then overlay_host, then the other
146+
// zones, then prompt — with nothing charged a height between the
147+
// overlay and the prompt box, the overlay's bottom edge abuts the
148+
// zones immediately above the prompt rather than leaving a gap.
149+
// These zones are all off in this idle, no-task/no-agents scenario,
150+
// so nothing should separate the overlay from the prompt.
151+
const between =
152+
heights.agents +
153+
heights.task +
154+
heights.plugin_banner +
155+
heights.command_banner +
156+
heights.settings_notice;
157+
expect(between).toBe(0);
158+
} finally {
159+
shell.dispose();
160+
}
161+
},
162+
{ width: WIDTH, height: 24 },
163+
);
164+
});
165+
});
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/**
2+
* CL-5750 follow-up: a palette stacked over an open approval must not
3+
* clobber the approval's cached raw body text, which a resize re-shapes
4+
* against the new height's context budget (see `decisionContextBudget` /
5+
* `applyOverlayBodyText` in shell.ts).
6+
*/
7+
import { describe, expect, test } from "bun:test";
8+
import { withTestRenderer } from "./harness.js";
9+
import {
10+
createAppShell,
11+
appendStreamRow,
12+
closeInsetOverlay,
13+
openPalette,
14+
type AppShell,
15+
} from "./shell.js";
16+
import { openPermissionsOverlay, makePermissionItems } from "./overlays.js";
17+
18+
function primeSession(shell: AppShell): void {
19+
appendStreamRow(shell, { role: "assistant", text: "session underway" });
20+
}
21+
22+
describe("decision overlay body cache survives a stacked palette", () => {
23+
test("resize after popping a stacked palette re-shapes the approval's own body, not a blanked one", async () => {
24+
await withTestRenderer(
25+
async (h) => {
26+
const shell = createAppShell(h.renderer, {
27+
terminal: { columns: 80, rows: 24 },
28+
run: "idle",
29+
});
30+
try {
31+
primeSession(shell);
32+
openPermissionsOverlay(shell, {
33+
items: makePermissionItems(3),
34+
body: "run_shell\nRun shell command\nSome context about the risky command.",
35+
});
36+
await h.renderOnce();
37+
expect(shell.overlayBodyLines.length).toBeGreaterThan(0);
38+
39+
// Stack a palette over the open permissions overlay — its own
40+
// (empty) body must not overwrite the approval's cached raw text.
41+
openPalette(shell, {
42+
catalog: [{ id: "foo", label: "foo" }],
43+
title: "commands",
44+
});
45+
await h.renderOnce();
46+
expect(shell.overlayKind).toBe("palette");
47+
48+
// Pop the palette back to the permissions overlay underneath.
49+
closeInsetOverlay(shell);
50+
await h.renderOnce();
51+
expect(shell.overlayKind).toBe("permissions");
52+
expect(shell.overlayBodyLines.length).toBeGreaterThan(0);
53+
54+
// Resize: the body must still show the permission context, not be
55+
// blanked by re-shaping from the palette's stale empty cache.
56+
h.resize(80, 20);
57+
await h.renderOnce();
58+
await h.renderOnce();
59+
expect(shell.overlayBodyLines.length).toBeGreaterThan(0);
60+
expect(shell.overlayBodyLines.join("\n")).toContain("run_shell");
61+
} finally {
62+
shell.dispose();
63+
}
64+
},
65+
{ width: 80, height: 24 },
66+
);
67+
});
68+
});

src/tui/overlay-body.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,11 @@ export function composeDecisionBody(
152152
width: number,
153153
contextLines: number,
154154
): OverlayBodyRow[] {
155-
const budget = Math.max(1, Math.floor(contextLines));
155+
// Zero is a valid budget: on a terminal too short to spare a row of air
156+
// plus a line of context on top of the header, the context section is
157+
// dropped entirely rather than forced to cost at least one row it cannot
158+
// afford — the choices below it must win that row instead.
159+
const budget = Math.max(0, Math.floor(contextLines));
156160
const lines = text.split("\n");
157161
const headIndex = lines.findIndex((l) => l.trim().length > 0);
158162
if (headIndex < 0) return [];
@@ -168,7 +172,7 @@ export function composeDecisionBody(
168172
});
169173
});
170174

171-
const rest = lines.slice(headIndex + 1).filter((l) => l.trim().length > 0);
175+
const rest = budget > 0 ? lines.slice(headIndex + 1).filter((l) => l.trim().length > 0) : [];
172176
if (rest.length > 0) {
173177
rows.push({ text: "", fg: UI.textDim });
174178
// Continuation rows are indented so a wrapped chain segment can never be

src/tui/overlay-overflow.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,10 @@ describe("gate-wire approval overflow on short terminal", () => {
351351
emitter.emit("permission.gate", { request, resolve: () => {} });
352352

353353
const body = permissionBodyFromRequest(request, { hint: true });
354-
expect(shell.overlayBodyLines.join("\n")).toContain("<message,");
354+
// The raw body still carries the collapsed-command hint — only what
355+
// gets painted is squeezed. On this short a terminal (CL-5750) the
356+
// choices win the row budget over the hint text, so the rendered
357+
// lines are not required to contain it.
355358
expect(body).toContain("e expand");
356359
expect(shell.layout.heights.overlay_host).toBeLessThanOrEqual(
357360
Math.floor(SHORT.height * OVERLAY_MAX_FRACTION),

0 commit comments

Comments
 (0)