Skip to content

Commit c3a8cb6

Browse files
Merge pull request #1566 from corbitsdev/cl-11362-long-approval-prompts-hide-accept-once-leaving-only-reject
fix(tui): keep every approval choice on screen under a tall body
2 parents bd2512e + 285642e commit c3a8cb6

2 files changed

Lines changed: 120 additions & 3 deletions

File tree

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
* box's growth and over the overlay's own context text.
66
*/
77
import { describe, expect, test } from "bun:test";
8+
import { EventEmitter } from "node:events";
89
import { defined } from "../../testkit/defined.js";
10+
import { PRIMARY_ASK_OPERATOR_SOURCE } from "./gate-events.js";
11+
import { wireGates } from "./gate-wire.js";
912
import { makePermissionItems, withTestRenderer } from "./harness.js";
1013
import { appendStreamRow } from "./shell/chrome.js";
1114
import { createAppShell } from "./shell/index.js";
@@ -178,3 +181,110 @@ describe("approval overlay keeps the prompt box on screen (CL-5750)", () => {
178181
);
179182
});
180183
});
184+
185+
// A long multi-line command makes the permission body taller than the host.
186+
// Only the first choice was guaranteed rows, so the prompt showed Reject alone
187+
// and Accept once sat scrolled out of view (CL-11362).
188+
const LONG_COMMAND = [
189+
"set +e",
190+
"TARGET=/Users/operator/code/worktrees/corbits-code/stack-1559-1561-1562-1563-cl-10768",
191+
'FIXTURE="$(mktemp -d /tmp/corbits-linear-release.XXXXXX)"',
192+
...Array.from(
193+
{ length: 28 },
194+
(_, i) =>
195+
`echo "probe step ${i}: checking the release issue list" >> "$FIXTURE/log"`,
196+
),
197+
].join("\n");
198+
199+
const FIXED_CHOICES = ["Reject", "Reject and stop", "Accept once"] as const;
200+
201+
const TALL_QUESTION = [
202+
"Which release branch should the probe target?",
203+
...Array.from(
204+
{ length: 30 },
205+
(_, i) => `context line ${i}: the probe found a candidate branch`,
206+
),
207+
].join("\n");
208+
209+
const OPERATOR_CHOICES = [
210+
"Use the current branch",
211+
"Use the release branch",
212+
"Skip the probe",
213+
"Stop and ask later",
214+
] as const;
215+
216+
describe("a tall approval body never hides a choice (CL-11362)", () => {
217+
for (const [columns, rows] of [
218+
[80, 24],
219+
[100, 30],
220+
] as const) {
221+
test(`every choice is painted for a long command at ${columns}x${rows}`, async () => {
222+
await withTestRenderer(
223+
async (h) => {
224+
const shell = createAppShell(h.renderer, {
225+
terminal: { columns, rows },
226+
run: "idle",
227+
});
228+
const emitter = new EventEmitter();
229+
const dispose = wireGates(emitter, shell);
230+
try {
231+
primeSession(shell);
232+
emitter.emit("permission.gate", {
233+
id: "gate-1",
234+
request: {
235+
tool: "run_shell",
236+
action: "Run shell command",
237+
subject: LONG_COMMAND,
238+
scopes: [],
239+
},
240+
resolve: () => undefined,
241+
});
242+
await h.renderOnce();
243+
await h.renderOnce();
244+
const frame = h.captureCharFrame();
245+
for (const choice of FIXED_CHOICES) expect(frame).toContain(choice);
246+
} finally {
247+
dispose();
248+
shell.dispose();
249+
}
250+
},
251+
{ width: columns, height: rows },
252+
);
253+
});
254+
255+
// The operator ask is the other decision overlay: its choices are
256+
// caller-supplied, so a tall question must not push any of them out.
257+
test(`every operator choice is painted for a tall question at ${columns}x${rows}`, async () => {
258+
await withTestRenderer(
259+
async (h) => {
260+
const shell = createAppShell(h.renderer, {
261+
terminal: { columns, rows },
262+
run: "idle",
263+
});
264+
const emitter = new EventEmitter();
265+
const dispose = wireGates(emitter, shell);
266+
try {
267+
primeSession(shell);
268+
emitter.emit("operator.gate", {
269+
id: "ask-1",
270+
question: TALL_QUESTION,
271+
options: [...OPERATOR_CHOICES],
272+
resolve: () => undefined,
273+
source: PRIMARY_ASK_OPERATOR_SOURCE,
274+
});
275+
await h.renderOnce();
276+
await h.renderOnce();
277+
const frame = h.captureCharFrame();
278+
for (const choice of OPERATOR_CHOICES) {
279+
expect(frame).toContain(choice);
280+
}
281+
} finally {
282+
dispose();
283+
shell.dispose();
284+
}
285+
},
286+
{ width: columns, height: rows },
287+
);
288+
});
289+
}
290+
});

‎src/tui/shell/chrome.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -830,8 +830,9 @@ export function paintPromptBorder(shell: AppShell): void {
830830
shell.promptBottomRule.content = new StyledText(ruleChunks(shell, bottom));
831831
}
832832

833-
/** Shrink overlay body and list to fit the host; on a short terminal,
834-
* drop context first so one choice stays answerable. */
833+
/** Shrink overlay body and list to fit the host, dropping context before
834+
* choices: a decision overlay keeps every choice on screen, any other overlay
835+
* keeps one, and a host too short for every choice still keeps one. */
835836
function fitOverlayListToHost(shell: AppShell, hostH: number): void {
836837
const list = shell.overlayList;
837838
if (!list || hostH <= 0) return;
@@ -840,7 +841,13 @@ function fitOverlayListToHost(shell: AppShell, hostH: number): void {
840841
const hasAnswer = overlayAnswerState(shell) !== null;
841842
const perItem = overlayRowsPerItem(shell.overlayKind);
842843
const hasItems = shell.overlayItems.length > 0;
843-
const choiceWant = hasItems ? perItem : 0;
844+
// A decision's first choice is Reject, so reserving one choice can leave
845+
// the only approving choice scrolled out of view under a tall body.
846+
const choiceWant = !hasItems
847+
? 0
848+
: isDecisionOverlay(shell.overlayKind)
849+
? shell.overlayItems.length * perItem
850+
: perItem;
844851
const choiceMin = hasItems ? 1 : 0;
845852
let bodyCount = shell.overlayBodyLines.length;
846853
const chromeOf = (n: number): number =>

0 commit comments

Comments
 (0)