Skip to content

Commit 34beb9e

Browse files
committed
Answer operator questions, and cap search output on hosts without ripgrep
A gate arriving while another overlay was open was dropped: the host refuses a second non-palette open, so the handler returned without resolving and the run blocked forever with nothing on screen. Gates now queue and take the host when it frees. An approval and a question in the same turn is enough to hit it. The overlay had no way to type an answer, though the tool has always promised one and OperatorResult already carried it. There is now an answer field, and a question with no options opens straight into it rather than offering a chooser with nothing to choose. The GitHub runner has no ripgrep, so CI exercised the fallback walker for every search test — and the fallback never capped its output. An unbounded grep could reach the model on any host without rg. The cap now belongs to the plugin, and CI installs ripgrep so the real path is covered.
1 parent 14126c5 commit 34beb9e

9 files changed

Lines changed: 538 additions & 21 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ jobs:
2323
with:
2424
bun-version: "1.3.14"
2525

26+
# The runner image has no ripgrep, so the grep plugin silently exercised
27+
# its fallback walker and left the ripgrep path untested.
28+
- name: Install ripgrep
29+
run: sudo apt-get install -y ripgrep
30+
2631
- name: Install dependencies
2732
run: bun install --frozen-lockfile
2833

src/plugins/rg-output.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export function createRgCollector(maxOutputBytes: number): RgCollector {
4242
return settle({
4343
kind: "partial",
4444
stdout: truncateToWholeLines(stdout, maxOutputBytes),
45-
notice: `ripgrep output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`,
45+
notice: `search output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`,
4646
});
4747
};
4848

src/plugins/rg-run.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { createRgCollector, type RgOutcome } from "./rg-output.js";
55
const RG_TIMEOUT_MS = 10_000;
66
// Cap collected stdout so a runaway pattern cannot OOM the process before the
77
// line-cap post-processing runs.
8-
const MAX_OUTPUT_BYTES = 512_000;
8+
export const MAX_OUTPUT_BYTES = 512_000;
99

1010
export type RgResult = RgOutcome | { kind: "unavailable" };
1111

src/plugins/ripgrep-plugin.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import {
77
runBoundedSearchFiles,
88
type BoundedGrepArgs,
99
} from "./bounded-grep-fallback.js";
10-
import { runRg, type RgLimits } from "./rg-run.js";
10+
import { createRgCollector } from "./rg-output.js";
11+
import { MAX_OUTPUT_BYTES, runRg, type RgLimits } from "./rg-run.js";
1112

1213
// A grep over a large tree with the pure-TypeScript walker enumerates the whole
1314
// directory (node_modules, build output, the lot) before searching, which stalls
@@ -35,6 +36,15 @@ function partialContent(stdout: string, maxResults: number, notice: string): str
3536
return `${capped}\n... ${notice}`;
3637
}
3738

39+
// The fallback walker collects its whole result in memory before returning, so
40+
// the byte cap has to be applied here. Without this the cap simply does not
41+
// exist on a host without ripgrep, and an unbounded grep reaches the model.
42+
function boundedContent(content: string, maxResults: number, maxOutputBytes: number): string {
43+
const breach = createRgCollector(maxOutputBytes).push(content);
44+
if (breach?.kind !== "partial") return capLines(content, maxResults);
45+
return partialContent(breach.stdout, maxResults, breach.notice);
46+
}
47+
3848
function str(value: unknown): string | undefined {
3949
return typeof value === "string" && value.length > 0 ? value : undefined;
4050
}
@@ -57,6 +67,7 @@ function searchLocation(path: string, fallbackCwd: string): { cwd: string; targe
5767
}
5868

5969
export function ripgrepPlugin(cwd: string, limits: RgLimits = {}): ToolPlugin {
70+
const maxBytes = limits.maxOutputBytes ?? MAX_OUTPUT_BYTES;
6071
return {
6172
middleware: (next) => async (call, signal) => {
6273
if (call.name === "grep") {
@@ -86,7 +97,7 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}): ToolPlugin {
8697
};
8798
if (glob !== undefined) boundedArgs.glob = glob;
8899
const content = await runBoundedGrep(boundedArgs, signal, rgCwd);
89-
return { callId: call.id, content: capLines(content, maxResults) };
100+
return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) };
90101
} catch (err) {
91102
return {
92103
callId: call.id,
@@ -122,7 +133,7 @@ export function ripgrepPlugin(cwd: string, limits: RgLimits = {}): ToolPlugin {
122133
signal,
123134
rgCwd,
124135
);
125-
return { callId: call.id, content: capLines(content, maxResults) };
136+
return { callId: call.id, content: boundedContent(content, maxResults, maxBytes) };
126137
} catch (err) {
127138
return {
128139
callId: call.id,

src/tui-opentui/gate-wire.test.ts

Lines changed: 179 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,16 @@
44
import { EventEmitter } from "node:events"
55
import { describe, expect, test } from "bun:test"
66
import type { PermissionRequest } from "../permission/types.js"
7-
import { withTestRenderer } from "./harness.js"
7+
import type { KeyEvent } from "@opentui/core"
8+
import { withTestRenderer, type Harness } from "./harness.js"
89
import { OVERLAY_MAX_FRACTION } from "./geometry/index.js"
910
import {
1011
acceptOverlaySelection,
1112
createAppShell,
13+
exitOverlayAnswerMode,
14+
handleOverlayAnswerKey,
15+
moveOverlaySelection,
16+
setOverlayAnswerActive,
1217
toggleOverlayExpand,
1318
type AppShell,
1419
} from "./shell.js"
@@ -479,3 +484,176 @@ describe("permission overlay height", () => {
479484
expect(capped).toBeLessThan(await hostRowsFor(rows, 1) + 40)
480485
})
481486
})
487+
488+
describe("operator question overlay", () => {
489+
const emitOperator = (
490+
shell: AppShell,
491+
options: readonly string[],
492+
onResolve: (result: unknown) => void,
493+
): void => {
494+
const emitter = new EventEmitter()
495+
wireGates(emitter, shell)
496+
emitter.emit("operator.gate", {
497+
question: "Scope for this run is still <SCOPE>. What should it be?",
498+
options: [...options],
499+
resolve: onResolve,
500+
})
501+
}
502+
503+
const keyOf = (seq: string, name?: string): KeyEvent =>
504+
({
505+
name: name ?? seq,
506+
sequence: seq,
507+
ctrl: false,
508+
meta: false,
509+
option: false,
510+
}) as unknown as KeyEvent
511+
512+
const withOperator = async (
513+
rows: number,
514+
options: readonly string[],
515+
body: (
516+
h: Harness,
517+
shell: AppShell,
518+
resolved: () => unknown,
519+
) => void | Promise<void>,
520+
): Promise<void> => {
521+
await withTestRenderer(
522+
async (h) => {
523+
const shell = createAppShell(h.renderer, {
524+
terminal: { columns: 96, rows },
525+
run: "idle",
526+
})
527+
let resolved: unknown = undefined
528+
try {
529+
emitOperator(shell, options, (r) => {
530+
resolved = r
531+
})
532+
await body(h, shell, () => resolved)
533+
} finally {
534+
shell.dispose()
535+
}
536+
},
537+
{ width: 96, height: rows },
538+
)
539+
}
540+
541+
const frameOf = async (
542+
h: Harness,
543+
): Promise<string> => {
544+
await h.renderOnce()
545+
await h.renderOnce()
546+
return h.captureCharFrame()
547+
}
548+
549+
for (const rows of [24, 60]) {
550+
test(`several options render and resolve by index at ${rows} rows`, async () => {
551+
await withOperator(rows, ["repo only", "docs too", "everything"], (h, shell, resolved) => {
552+
expect(shell.overlayKind).toBe("operator")
553+
expect(shell.overlayItems).toEqual(["repo only", "docs too", "everything"])
554+
moveOverlaySelection(shell, 1)
555+
acceptOverlaySelection(shell)
556+
expect(resolved()).toEqual({ kind: "option", index: 1 })
557+
void h
558+
})
559+
})
560+
561+
test(`a single option still renders a choosable row at ${rows} rows`, async () => {
562+
await withOperator(rows, ["only this"], (h, shell, resolved) => {
563+
expect(shell.overlayItems).toEqual(["only this"])
564+
acceptOverlaySelection(shell)
565+
expect(resolved()).toEqual({ kind: "option", index: 0 })
566+
void h
567+
})
568+
})
569+
570+
test(`no options opens straight into the answer field at ${rows} rows`, async () => {
571+
await withOperator(rows, [], async (h, shell, resolved) => {
572+
expect(shell.overlayKind).toBe("operator")
573+
expect(shell.overlayItems).toEqual([])
574+
const frame = await frameOf(h)
575+
// Never offer a chooser with nothing to choose.
576+
expect(frame).not.toContain("Enter choose")
577+
expect(frame).toContain("Enter send")
578+
expect(frame).toContain("answer>")
579+
// Enter with nothing typed must not resolve the gate at all.
580+
acceptOverlaySelection(shell)
581+
expect(resolved()).toBeUndefined()
582+
})
583+
})
584+
}
585+
586+
test("the answer field is advertised on screen next to the choices", async () => {
587+
await withOperator(40, ["repo only", "everything"], async (h) => {
588+
const frame = await frameOf(h)
589+
expect(frame).toContain("Tab type an answer")
590+
expect(frame).toContain("type your own answer")
591+
})
592+
})
593+
594+
test("a typed answer round-trips as a custom OperatorResult", async () => {
595+
await withOperator(40, ["repo only", "everything"], (h, shell, resolved) => {
596+
expect(setOverlayAnswerActive(shell, true)).toBe(true)
597+
for (const ch of "src and docs") {
598+
expect(handleOverlayAnswerKey(shell, keyOf(ch))).toBe(true)
599+
}
600+
expect(handleOverlayAnswerKey(shell, keyOf("x", "backspace"))).toBe(true)
601+
expect(handleOverlayAnswerKey(shell, keyOf("", "return"))).toBe(true)
602+
expect(resolved()).toEqual({ kind: "custom", text: "src and doc" })
603+
// Submitting closes the overlay, so the host is free for the next gate.
604+
expect(shell.overlayList).toBeNull()
605+
void h
606+
})
607+
})
608+
609+
test("Esc in the answer field returns to the choices instead of cancelling", async () => {
610+
await withOperator(40, ["repo only"], (h, shell, resolved) => {
611+
setOverlayAnswerActive(shell, true)
612+
expect(exitOverlayAnswerMode(shell)).toBe(true)
613+
expect(shell.overlayList).not.toBeNull()
614+
expect(resolved()).toBeUndefined()
615+
void h
616+
})
617+
})
618+
619+
test("a gate arriving while another overlay is open opens once that one closes", async () => {
620+
await withTestRenderer(
621+
async (h) => {
622+
const shell = createAppShell(h.renderer, {
623+
terminal: { columns: 96, rows: 40 },
624+
run: "idle",
625+
})
626+
const emitter = new EventEmitter()
627+
let approved: unknown = undefined
628+
let answered: unknown = undefined
629+
try {
630+
wireGates(emitter, shell)
631+
emitter.emit("permission.gate", {
632+
request: baseRequest(),
633+
resolve: (o: unknown) => {
634+
approved = o
635+
},
636+
})
637+
emitter.emit("operator.gate", {
638+
question: "Scope for this run?",
639+
options: ["repo only"],
640+
resolve: (r: unknown) => {
641+
answered = r
642+
},
643+
})
644+
expect(shell.overlayKind).toBe("permissions")
645+
646+
acceptOverlaySelection(shell)
647+
expect(approved).toEqual({ allow: false })
648+
// The queued question is not lost: it takes the host as it frees up.
649+
expect(shell.overlayKind).toBe("operator")
650+
acceptOverlaySelection(shell)
651+
expect(answered).toEqual({ kind: "option", index: 0 })
652+
} finally {
653+
shell.dispose()
654+
}
655+
},
656+
{ width: 96, height: 40 },
657+
)
658+
})
659+
})

src/tui-opentui/gate-wire.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type {
1414
PermissionRequest,
1515
} from "../permission/types.js"
1616
import type { AppShell, OverlaySelection } from "./shell.js"
17-
import { appendStreamRow, setOverlayBody } from "./shell.js"
17+
import { appendStreamRow, onOverlayClosed, setOverlayBody } from "./shell.js"
1818
import { EXPAND_KEY } from "./stream.js"
1919

2020
/** Stable sentinel ids for the always-present deny / once rows. */
@@ -237,6 +237,25 @@ export function wireGates(
237237
emitter: EventEmitter,
238238
shell: AppShell,
239239
): () => void {
240+
// The shell has one overlay host, and opening onto a busy one is a no-op.
241+
// Gates cannot be dropped that way — a lost ask_operator blocks the run with
242+
// nothing on screen to answer — so a gate that arrives while another overlay
243+
// is up waits here and opens as soon as the host frees up.
244+
const pending: Array<() => void> = []
245+
246+
function openOrQueue(open: () => void): void {
247+
if (shell.overlayList !== null) {
248+
pending.push(open)
249+
return
250+
}
251+
open()
252+
}
253+
254+
const disposeClosed = onOverlayClosed(shell, () => {
255+
const next = pending.shift()
256+
if (next) next()
257+
})
258+
240259
function onPermission(ev: PermissionGateEvent): void {
241260
const choices = permissionChoicesFromRequest(ev.request)
242261
const collapsedBody = permissionBodyFromRequest(ev.request, { hint: true })
@@ -264,7 +283,7 @@ export function wireGates(
264283
})
265284
}
266285

267-
openPermissionsOverlay(shell, {
286+
openOrQueue(() => openPermissionsOverlay(shell, {
268287
items: choices.items,
269288
itemIds: choices.itemIds,
270289
body: collapsedBody,
@@ -277,12 +296,12 @@ export function wireGates(
277296
recordDecision(shell, ev.request, choices, gateSelection)
278297
ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
279298
},
280-
})
299+
}))
281300
}
282301

283302
function onOperator(ev: OperatorGateEvent): void {
284303
const choices = operatorChoicesFromOptions(ev.options)
285-
openOperatorOverlay(shell, {
304+
openOrQueue(() => openOperatorOverlay(shell, {
286305
body: ev.question,
287306
choices: choices.items,
288307
itemIds: choices.itemIds,
@@ -294,7 +313,10 @@ export function wireGates(
294313
}),
295314
)
296315
},
297-
})
316+
// The ask_operator contract offers a free-form answer, so the overlay
317+
// must be able to send one back rather than only an option index.
318+
onTextAnswer: (text: string) => ev.resolve(operatorCustomResult(text)),
319+
}))
298320
}
299321

300322
emitter.on("permission.gate", onPermission)
@@ -303,5 +325,7 @@ export function wireGates(
303325
return () => {
304326
emitter.off("permission.gate", onPermission)
305327
emitter.off("operator.gate", onOperator)
328+
disposeClosed()
329+
pending.length = 0
306330
}
307331
}

src/tui-opentui/overlays.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,22 +118,38 @@ export type OpenOperatorOpts = {
118118
readonly activeIndex?: number
119119
/** Per-open accept; host binds OperatorResult mapping. */
120120
readonly onAccept?: (selection: OverlaySelection) => void
121+
/** Per-open free-text answer; host binds the custom OperatorResult. */
122+
readonly onTextAnswer?: (text: string) => void
121123
}
122124

125+
/**
126+
* Line appended to the question when the operator can neither pick nor type.
127+
* The overlay must always say what its one available action is rather than
128+
* offering "Enter choose" against an empty list.
129+
*/
130+
const NO_WAY_TO_ANSWER =
131+
"No options were offered and this question takes no typed answer. Press Esc to dismiss it."
132+
123133
export function openOperatorOverlay(
124134
shell: AppShell,
125135
opts?: OpenOperatorOpts,
126136
): void {
127137
const fixture = makeOperatorQuestion()
138+
const choices = opts?.choices ?? fixture.choices
139+
const body = opts?.body ?? fixture.body
140+
const stranded = choices.length === 0 && opts?.onTextAnswer === undefined
128141
openListOverlay(shell, {
129142
kind: "operator",
130143
title: "operator question",
131-
body: opts?.body ?? fixture.body,
132-
items: opts?.choices ?? fixture.choices,
144+
body: stranded ? `${body}\n\n${NO_WAY_TO_ANSWER}` : body,
145+
items: choices,
133146
activeIndex: opts?.activeIndex ?? 0,
134147
frameId: "overlay-operator",
135148
...(opts?.itemIds !== undefined ? { itemIds: opts.itemIds } : {}),
136149
...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}),
150+
...(opts?.onTextAnswer !== undefined
151+
? { onTextAnswer: opts.onTextAnswer }
152+
: {}),
137153
})
138154
}
139155

0 commit comments

Comments
 (0)