Skip to content

Commit 5313822

Browse files
Make ask_operator use a full-shell overlay (#686)
* Make ask_operator use a full-shell overlay Inset mode clipped long questions and option lists. Operator asks now open full_shell; permission gates stay inset. Agents must put long rationale in chat first, then ask with short labels. * Retarget landing inset overlay test to permissions Operator asks now open full_shell (CL-7067), which hides the landing. The slide-only-as-needed coverage belongs on the inset permission path. * Harden landing overlay tests for inset pressure and full_shell budget A three-item empty permissions open left message delta 0, so the slide test would pass without the float moving. The many-choices landing case still opened operator full_shell, which hides landing. Retarget both to heavy inset permissions, and pin that full_shell keeps more decision context rows than inset for the same long body. * Format overlays.test.ts choices array for Prettier
1 parent c4ffba8 commit 5313822

8 files changed

Lines changed: 149 additions & 18 deletions

File tree

src/agent/director.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ const PathArgSchema = type({ path: "string" });
114114
export const askOperatorDefinition: ToolDefinition = {
115115
name: "ask_operator",
116116
description:
117-
"Pause execution and ask the operator a clarifying question. Execution resumes when the operator selects an option. " +
117+
"Pause execution and ask the operator a short clarifying question with short option labels. " +
118+
"Put any long rationale, trade-offs, or context in a normal transcript reply first, then call this " +
119+
"with only a brief question and brief option labels — the overlay is not a place for essays. " +
120+
"Execution resumes when the operator selects an option. " +
118121
"If the question is really asking permission to run one specific shell command, pass that exact command as `command` " +
119122
"instead of just describing it in the option text — approval here then covers the matching run_shell call too, so the " +
120123
"operator is not asked to approve the same action twice.",
@@ -123,11 +126,12 @@ export const askOperatorDefinition: ToolDefinition = {
123126
properties: {
124127
question: {
125128
type: "string",
126-
description: "The question to ask the operator",
129+
description:
130+
"Short question shown in the overlay (one or two lines). Put long rationale in a transcript reply first.",
127131
},
128132
options: {
129133
type: "array",
130-
description: "List of options the operator can choose from",
134+
description: "Short option labels the operator can choose from (keep each label brief)",
131135
items: { type: "string" },
132136
minItems: 1,
133137
},

src/agent/directors/skywalker/package.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Quick routing:
5454
- After multi-file builder landings → default a critic (or greybeard when architecture is in play) on the diff/criteria in a fresh context
5555
5656
Prefer typed spawn: intent, success_criteria, do_not, report_focus, agent when specialist.
57-
Parallelize independent lanes with spawn_agent, then wait_agents. manage_tasks for your checklist. ask_operator when blocked or ambiguous.
57+
Parallelize independent lanes with spawn_agent, then wait_agents. manage_tasks for your checklist. ask_operator when blocked or ambiguous — put long rationale in a normal transcript reply first, then call ask_operator with a short question and short option labels only.
5858
5959
# Fetch URLs (primary-mounted)
6060
@@ -117,7 +117,7 @@ Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shak
117117
118118
## If ORCHESTRATION → coordinate
119119
120-
Track with manage_tasks. Parallelize independent lanes via spawn_agent + wait_agents. After each spawn wave, update the operator before blocking. Escalate blockers with ask_operator. This is your core role.
120+
Track with manage_tasks. Parallelize independent lanes via spawn_agent + wait_agents. After each spawn wave, update the operator before blocking. Escalate blockers with ask_operator (chat rationale first, then short ask_operator). This is your core role.
121121
122122
## If COMMUNICATION → answer directly
123123

src/agent/prompt-contract.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export const CHAT_PROMPT_QUALITY_MARKERS = [
1111
"Scope and conventions:",
1212
"DIY tiny/single-file/one-route",
1313
"ask_operator only when permission blocks you",
14+
"short question and short option labels only",
1415
"Touch only code required for the task",
1516
"load the style and philosophy skills",
1617
"grep or search_files",

src/agent/prompts.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ export function buildGuidelines(
134134
]
135135
: [
136136
"- Clear, bounded coding requests: proceed autonomously; use ask_operator only when permission blocks you or the request is genuinely ambiguous (missing repro, conflicting instructions, destructive choice).",
137+
"- Before ask_operator: put long rationale in a normal transcript reply first, then call ask_operator with a short question and short option labels only.",
137138
"- Questions, reviews, and product/visual feedback: answer or diagnose first; do not edit until the user wants a change.",
138139
"- Preserve unrelated user edits; never revert changes you did not make unless asked.",
139140
"- Unexpected changes in files you did not touch: stop and ask_operator.",
@@ -219,7 +220,8 @@ const TOOL_SUMMARIES: Record<string, string> = {
219220
"find agent profiles by role or team before spawning with task(agent=...); results include full system prompt / body so you need not read_file plugin roots outside the workspace",
220221
manage_tasks: "maintain your work checklist — create/replace, update status, append, cancel",
221222
submit_output: "signal the task is complete — the only way to finish",
222-
ask_operator: "pause and ask the user when blocked or genuinely ambiguous",
223+
ask_operator:
224+
"pause and ask the user when blocked or genuinely ambiguous; put long rationale in a transcript reply first, then call with a short question and short option labels only",
223225
present:
224226
"dynamically render aligned/structured output using the layout primitives (stack/row/grid/text etc)",
225227
tool_search: "load more tools by capability when you need them",

src/tui/landing.test.ts

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
surfaceSystemNotice,
2424
toggleTasksPanel,
2525
} from "./shell";
26-
import { makeOperatorQuestion, openOperatorOverlay } from "./overlays";
26+
import { makePermissionItems, openPermissionsOverlay } from "./overlays";
2727
import {
2828
LANDING_HINTS,
2929
LANDING_SUGGESTIONS,
@@ -429,7 +429,26 @@ describe("landing screen", () => {
429429
// together before the overlay opens.
430430
expect(was).toEqual([...was].sort((a, b) => a - b));
431431

432-
openOperatorOverlay(shell);
432+
// Heavy inset permission overlay: many choices plus a multi-line body
433+
// so the float must take real headroom from the landing split. A
434+
// three-item empty body leaves message delta 0 and would pass even if
435+
// the split never slid. Operator asks use full_shell (CL-7067) and
436+
// hide the landing instead — that path is covered in overlays.test.ts.
437+
const heavyBody = [
438+
"run_shell",
439+
"Run shell command",
440+
"Proposed: git reset --hard origin/main && rm -rf node_modules",
441+
"Files at risk: 128 modified, 12 untracked.",
442+
"Continue only if you accept discarding local work.",
443+
"Also note: this path was requested by the explore agent.",
444+
"Scopes include session, project, and once-only grants.",
445+
"Review carefully before approving this request.",
446+
].join("\n");
447+
openPermissionsOverlay(shell, {
448+
items: makePermissionItems(16),
449+
body: heavyBody,
450+
});
451+
expect(shell.layout.overlayMode).toBe("inset");
433452
await settle(h);
434453
const after = rows(h);
435454
// Every landing anchor is still on screen and in the same relative
@@ -442,6 +461,9 @@ describe("landing screen", () => {
442461
expect(nowAt.every((index) => index > 0)).toBe(true);
443462
expect(nowAt).toEqual([...nowAt].sort((a, b) => a - b));
444463
expect(new Set(nowAt).size).toBe(nowAt.length);
464+
// Real geometry pressure: the prompt field moves so the inset can
465+
// claim rows the even split would not have given it.
466+
expect(nowAt[0]).not.toBe(was[0]);
445467
expect(h.captureCharFrame()).toContain("Esc cancel");
446468
} finally {
447469
shell.dispose();
@@ -451,12 +473,14 @@ describe("landing screen", () => {
451473
);
452474
});
453475

454-
// A question with more choices than the even top/bottom split would leave
455-
// room for used to get its list starved down to whatever that split
456-
// happened to allow — as little as one or two choices — because the float
457-
// only asked the split for one choice row of headroom. It now asks for the
458-
// overlay's real, already fraction-capped content height, so a terminal
476+
// An inset permission list with more choices than the even top/bottom split
477+
// would leave room for used to get its list starved down to whatever that
478+
// split happened to allow — as little as one or two choices — because the
479+
// float only asked the split for one choice row of headroom. It now asks for
480+
// the overlay's real, already fraction-capped content height, so a terminal
459481
// tall enough for that content shows every choice without scrolling.
482+
// Operator full_shell hides the landing (CL-7067); this coverage stays on
483+
// the inset permission path that still floats over landing.
460484
test("a landing overlay with many choices shows them all when there is room", async () => {
461485
await withTestRenderer(
462486
async (h) => {
@@ -466,10 +490,15 @@ describe("landing screen", () => {
466490
run: "idle",
467491
});
468492
try {
469-
openOperatorOverlay(shell);
493+
const items = makePermissionItems(8);
494+
openPermissionsOverlay(shell, {
495+
items,
496+
body: "run_shell\nRun shell command\nbun test src/tui",
497+
});
498+
expect(shell.layout.overlayMode).toBe("inset");
470499
await settle(h);
471500
const frame = h.captureCharFrame();
472-
for (const choice of makeOperatorQuestion().choices) {
501+
for (const choice of items) {
473502
expect(frame).toContain(choice);
474503
}
475504
} finally {

src/tui/overlays.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ describe("operator question overlay", () => {
192192
try {
193193
openOperatorOverlay(shell);
194194
expect(shell.overlayKind).toBe("operator");
195+
expect(shell.layout.overlayMode).toBe("full_shell");
195196
expect(shell.overlayBodyLines.length).toBeGreaterThan(0);
196197
expect(shell.overlayItems.length).toBeGreaterThan(3);
197198
expect(focusOwner(shell.focus)).toBe("overlay");
@@ -222,6 +223,68 @@ describe("operator question overlay", () => {
222223
{ width: 80, height: 24 },
223224
);
224225
});
226+
227+
test("full_shell decisionContext budget shows more body rows than inset", async () => {
228+
// Same long body under both modes: inset stays capped at DECISION_CONTEXT_ROWS
229+
// (8), while full_shell raises the cap with terminal height so a long ask
230+
// stays readable (CL-7067). Mode-only asserts are not enough — pin the
231+
// budget branch that actually shapes overlayBodyLines.
232+
const longBody = [
233+
"Should we proceed with the destructive reset of the working tree?",
234+
...Array.from({ length: 24 }, (_, i) => `Context line ${i + 1}.`),
235+
].join("\n");
236+
const choices = [
237+
"Cancel — keep working tree",
238+
"Allow this once",
239+
"Allow for this session",
240+
] as const;
241+
const size = { width: 80, height: 40 } as const;
242+
243+
async function bodyLineCount(mode: "inset" | "full_shell"): Promise<{
244+
readonly lines: number;
245+
readonly body: readonly string[];
246+
readonly frame: string;
247+
}> {
248+
return withTestRenderer(async (h) => {
249+
const shell = createAppShell(h.renderer, {
250+
terminal: { columns: size.width, rows: size.height },
251+
wireKeys: false,
252+
run: "idle",
253+
});
254+
try {
255+
openListOverlay(shell, {
256+
kind: "operator",
257+
title: "",
258+
body: longBody,
259+
items: [...choices],
260+
overlayMode: mode,
261+
frameId: `overlay-operator-${mode}`,
262+
});
263+
expect(shell.layout.overlayMode).toBe(mode);
264+
await h.renderOnce();
265+
await h.renderOnce();
266+
return {
267+
lines: shell.overlayBodyLines.length,
268+
body: shell.overlayBodyLines,
269+
frame: h.captureCharFrame(),
270+
};
271+
} finally {
272+
shell.dispose();
273+
}
274+
}, size);
275+
}
276+
277+
const inset = await bodyLineCount("inset");
278+
const full = await bodyLineCount("full_shell");
279+
expect(full.lines).toBeGreaterThan(inset.lines);
280+
// Short, non-wrapping context lines map 1:1 to the budget. Inset caps at
281+
// eight context rows (then dither + tail); full_shell's raised cap keeps a
282+
// mid-body line the inset path drops.
283+
expect(full.body.some((line) => line.includes("Context line 12."))).toBe(true);
284+
expect(inset.body.some((line) => line.includes("Context line 12."))).toBe(false);
285+
expect(full.frame).toContain("Context line 12.");
286+
expect(inset.frame).not.toContain("Context line 12.");
287+
});
225288
});
226289

227290
describe("model / provider picker", () => {

src/tui/overlays.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ export function openOperatorOverlay(shell: AppShell, opts?: OpenOperatorOpts): v
148148
items: choices,
149149
activeIndex: opts?.activeIndex ?? 0,
150150
frameId: "overlay-operator",
151+
// Full shell so long questions and option lists stay readable (CL-7067).
152+
// Permission gates keep the default inset path.
153+
overlayMode: "full_shell",
151154
...(opts?.itemIds !== undefined ? { itemIds: opts.itemIds } : {}),
152155
...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}),
153156
...(opts?.onTextAnswer !== undefined ? { onTextAnswer: opts.onTextAnswer } : {}),

src/tui/shell.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,8 +1282,12 @@ function relayoutOverlayHost(shell: AppShell, itemCount: number): void {
12821282
const perItem = overlayRowsPerItem(shell.overlayKind);
12831283
const hostRows = overlayHostRows(shell, shell.overlayBodyLines.length, itemCount * perItem);
12841284
const minHostRows = overlayMinHostRows(shell, shell.overlayBodyLines.length, itemCount > 0);
1285+
// Preserve the mode the open chose (operator uses full_shell; others inset).
1286+
// Hardcoding inset here would collapse a full_shell ask on every list refresh.
1287+
const bag = internals.get(shell);
1288+
const mode: OverlayMode = bag?.overlayMode === "full_shell" ? "full_shell" : "inset";
12851289
relayout(shell, {
1286-
overlayMode: "inset",
1290+
overlayMode: mode,
12871291
overlayBodyRows: hostRows,
12881292
overlayMinBodyRows: minHostRows,
12891293
});
@@ -1937,6 +1941,8 @@ interface PriorOverlaySnapshot {
19371941
readonly onCancel: (() => void) | null;
19381942
readonly addProviderHint: boolean;
19391943
readonly setDefaultHint: boolean;
1944+
/** Geometry mode the primary used before the palette stacked over it. */
1945+
readonly overlayMode: OverlayMode;
19401946
}
19411947

19421948
interface ShellInternals {
@@ -3429,6 +3435,16 @@ function decisionContextBudget(
34293435
): number {
34303436
const fixedChrome =
34313437
OVERLAY_HOST_BORDER_ROWS + overlayTitleRows(kind) + DECISION_HEADER_AND_TRAILER_ROWS;
3438+
// full_shell owns residual after chrome — drop the inset fraction cap so a
3439+
// longer question can use the extra rows instead of staying clipped at eight.
3440+
const bag = internals.get(shell);
3441+
if (bag?.overlayMode === "full_shell") {
3442+
const maxOverlayRows = Math.max(0, terminalHeight - PROMPT_BASE_ROWS);
3443+
const baseline =
3444+
maxOverlayRows - DECISION_CHOICE_ROWS - fixedChrome - DECISION_CONTEXT_BLANK_ROWS;
3445+
const fullCap = Math.max(DECISION_CONTEXT_ROWS, Math.floor(terminalHeight / 2));
3446+
return Math.max(0, Math.min(fullCap, baseline));
3447+
}
34323448
// The resolver never lets the overlay host past the fraction cap even when
34333449
// the transcript floor and every other zone have already given up their
34343450
// rows, so that cap — not just the prompt floor — bounds how much context
@@ -3554,6 +3570,12 @@ export interface OpenListOverlayOpts {
35543570
* server needs authorization.
35553571
*/
35563572
readonly echoChoice?: boolean;
3573+
/**
3574+
* Geometry mode for this open. Defaults to inset. Operator asks use
3575+
* full_shell so long questions and option lists stay readable; permission
3576+
* gates stay inset unless a caller opts in.
3577+
*/
3578+
readonly overlayMode?: "inset" | "full_shell";
35573579
/**
35583580
* Claim printable keys for a `>` filter row so the list narrows as you type.
35593581
* Opt-in per open (model picker, palette). Overlays without it keep j/k
@@ -3608,6 +3630,7 @@ export function openListOverlay(shell: AppShell, opts?: OpenListOverlayOpts): vo
36083630
onCancel: bag.overlayOnCancel,
36093631
addProviderHint: bag.overlayAddProviderHint,
36103632
setDefaultHint: bag.overlaySetDefaultHint,
3633+
overlayMode: bag.overlayMode === "full_shell" ? "full_shell" : "inset",
36113634
};
36123635
}
36133636
// Leave prior overlay focus frame; palette will stack above it.
@@ -3627,6 +3650,9 @@ export function openListOverlay(shell: AppShell, opts?: OpenListOverlayOpts): vo
36273650

36283651
const bag = internals.get(shell);
36293652
if (bag) {
3653+
// Mode must land before applyOverlayBodyText so decisionContextBudget can
3654+
// size the body against full_shell vs inset. Palette is always inset.
3655+
bag.overlayMode = !isPalette && opts?.overlayMode === "full_shell" ? "full_shell" : "inset";
36303656
// Palette open does not own primary accept; leave prior snapshot's callback.
36313657
if (!isPalette) {
36323658
bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [];
@@ -4105,6 +4131,7 @@ export function closeInsetOverlay(shell: AppShell): void {
41054131
bag.overlayOnCancel = prior.onCancel;
41064132
bag.overlayAddProviderHint = prior.addProviderHint;
41074133
bag.overlaySetDefaultHint = prior.setDefaultHint;
4134+
bag.overlayMode = prior.overlayMode === "full_shell" ? "full_shell" : "inset";
41084135
// If focus was not stacked (edge case), re-open overlay frame.
41094136
if (focusOwner(shell.focus) !== "overlay") {
41104137
shell.focus = openOverlay(shell.focus, OVERLAY_FRAME_ID, {
@@ -4116,7 +4143,7 @@ export function closeInsetOverlay(shell: AppShell): void {
41164143
const hostRows = overlayHostRows(shell, prior.bodyLines.length, listH);
41174144
const minHostRows = overlayMinHostRows(shell, prior.bodyLines.length, prior.list.count > 0);
41184145
relayout(shell, {
4119-
overlayMode: "inset",
4146+
overlayMode: bag.overlayMode,
41204147
overlayBodyRows: hostRows,
41214148
overlayMinBodyRows: minHostRows,
41224149
});
@@ -4225,8 +4252,10 @@ export function setOverlayBody(shell: AppShell, text: string, maxLines = 8): voi
42254252
shell.overlayBodyLines.length,
42264253
shell.overlayItems.length > 0,
42274254
);
4255+
const bag = internals.get(shell);
4256+
const mode: OverlayMode = bag?.overlayMode === "full_shell" ? "full_shell" : "inset";
42284257
relayout(shell, {
4229-
overlayMode: "inset",
4258+
overlayMode: mode,
42304259
overlayBodyRows: hostRows,
42314260
overlayMinBodyRows: minHostRows,
42324261
});

0 commit comments

Comments
 (0)