Skip to content

Commit d928db5

Browse files
committed
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 1a063d4 commit d928db5

3 files changed

Lines changed: 86 additions & 4 deletions

File tree

src/tui/approval-prompt-visibility.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { openPermissionsOverlay, makePermissionItems } from "./overlays.js";
1111

1212
const WIDTH = 80;
1313
// Deliberately spans from far below the documented 24-row baseline down to
14-
// the shortest terminals the shell claims to support, plus a comfortable one.
14+
// 10 rows, the shortest terminal this fix guarantees, plus a comfortable one.
1515
const HEIGHTS = [10, 12, 15, 24, 40] as const;
1616

1717
const APPROVAL_BODY = [
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/shell.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3372,15 +3372,21 @@ const DECISION_CONTEXT_BLANK_ROWS = 1;
33723372

33733373
/**
33743374
* Shrink the decision body's context budget so its own chrome never crowds
3375-
* out the one thing that must survive any terminal height: at least one
3376-
* choice row, with the prompt box still seated at its floor below it. A
3375+
* out the one thing this fix guarantees down to a 10-row terminal: at least
3376+
* one choice row, with the prompt box still seated at its floor below it. A
33773377
* generous, fixed context budget reads fine on a tall terminal, but on a
33783378
* short one it can consume the entire overlay host, leaving no room to paint
33793379
* a single option — the operator is then asked to decide between choices
33803380
* they cannot see. Shrinking the context first, down to dropping it entirely
33813381
* on the shortest terminals, is the deliberate trade: the header (which tool,
33823382
* which question) and the choices are the two things an approval cannot
33833383
* render without; the surrounding detail can give way first.
3384+
*
3385+
* Below 10 rows this budget alone cannot save the frame: the resolver's own
3386+
* collapse fallback (`resolveGeometry` in geometry/resolve.ts) can still hand
3387+
* the overlay host fewer rows than its render minimum once every other zone
3388+
* is already at floor, which is a pre-existing gap in the resolver, not
3389+
* something this budget controls.
33843390
*/
33853391
function decisionContextBudget(
33863392
shell: AppShell,
@@ -3409,7 +3415,15 @@ function applyOverlayBodyText(
34093415
): void {
34103416
const width = overlayRowWidth(shell);
34113417
const bag = internals.get(shell);
3412-
if (bag) bag.overlayRawBodyText = text;
3418+
// Scoped to decision overlays: a palette stacked over an open approval
3419+
// calls this too, with its own (usually empty) body text. Caching that
3420+
// would overwrite the approval's cached raw text with the palette's, and
3421+
// popping the palette restores the approval's `overlayBodyLines` but not
3422+
// this cache (`PriorOverlaySnapshot` never carried it) — so a resize right
3423+
// after would re-shape the approval's body from the palette's stale empty
3424+
// string instead of its own, blanking it. The palette itself never reads
3425+
// this cache (not a decision overlay), so it never needs to be cached.
3426+
if (bag && isDecisionOverlay(shell.overlayKind)) bag.overlayRawBodyText = text;
34133427
if (text.length === 0) {
34143428
shell.overlayBodyLines = [];
34153429
shell.overlayBodyFgs = [];

0 commit comments

Comments
 (0)