Skip to content

Commit 47f94b8

Browse files
committed
Restore permission-gate auto-deny and fix Esc leaving gate promises unresolved
gate-wire.ts redeclared a local PermissionGateEvent missing timeoutMs, timeoutMessage and signal, so goal-mode timeouts and watchdog aborts were silently dropped and an unattended run could hang on the modal forever. Import the shared type instead, and wire a timer/abort listener into onPermission that auto-denies and tears itself down cleanly. Esc on a permission or operator overlay called closeInsetOverlay without ever invoking the awaited resolve, deadlocking the session until the process was killed. Add an onCancel hook to the overlay opts, thread it through openListOverlay/closeInsetOverlay for the promise-backed overlay kinds only, and have gate-wire resolve deny/cancel from it.
1 parent cf3bb84 commit 47f94b8

4 files changed

Lines changed: 295 additions & 28 deletions

File tree

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

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { withTestRenderer, type Harness } from "./harness.js"
99
import { OVERLAY_MAX_FRACTION } from "./geometry/index.js"
1010
import {
1111
acceptOverlaySelection,
12+
closeInsetOverlay,
1213
createAppShell,
1314
exitOverlayAnswerMode,
1415
handleOverlayAnswerKey,
@@ -423,6 +424,171 @@ describe("wireGates", () => {
423424
})
424425
})
425426

427+
describe("permission.gate auto-deny", () => {
428+
test("timeoutMs elapsing auto-denies with the timeout message and closes the overlay", async () => {
429+
await withTestRenderer(async (h) => {
430+
const shell = createAppShell(h.renderer, {
431+
terminal: { columns: 80, rows: 24 },
432+
run: "idle",
433+
})
434+
const emitter = new EventEmitter()
435+
let resolved: unknown
436+
try {
437+
wireGates(emitter, shell)
438+
emitter.emit("permission.gate", {
439+
request: baseRequest(),
440+
resolve: (outcome: unknown) => {
441+
resolved = outcome
442+
},
443+
timeoutMs: 5,
444+
timeoutMessage: "goal mode: no answer in time",
445+
})
446+
expect(shell.overlayKind).toBe("permissions")
447+
448+
await new Promise((r) => setTimeout(r, 20))
449+
450+
expect(resolved).toEqual({
451+
allow: false,
452+
message: "goal mode: no answer in time",
453+
})
454+
expect(shell.overlayList).toBeNull()
455+
} finally {
456+
shell.dispose()
457+
}
458+
})
459+
})
460+
461+
test("aborting the signal while the overlay is open auto-denies and closes it", async () => {
462+
await withTestRenderer(async (h) => {
463+
const shell = createAppShell(h.renderer, {
464+
terminal: { columns: 80, rows: 24 },
465+
run: "idle",
466+
})
467+
const emitter = new EventEmitter()
468+
const controller = new AbortController()
469+
let resolved: unknown
470+
try {
471+
wireGates(emitter, shell)
472+
emitter.emit("permission.gate", {
473+
request: baseRequest(),
474+
resolve: (outcome: unknown) => {
475+
resolved = outcome
476+
},
477+
signal: controller.signal,
478+
})
479+
expect(shell.overlayKind).toBe("permissions")
480+
481+
controller.abort()
482+
483+
expect(resolved).toEqual({
484+
allow: false,
485+
message: "tool no longer running; permission request denied",
486+
})
487+
expect(shell.overlayList).toBeNull()
488+
} finally {
489+
shell.dispose()
490+
}
491+
})
492+
})
493+
494+
test("resolving normally clears the timer instead of firing it later", async () => {
495+
await withTestRenderer(async (h) => {
496+
const shell = createAppShell(h.renderer, {
497+
terminal: { columns: 80, rows: 24 },
498+
run: "idle",
499+
})
500+
const emitter = new EventEmitter()
501+
let resolveCount = 0
502+
let lastOutcome: unknown
503+
try {
504+
wireGates(emitter, shell)
505+
emitter.emit("permission.gate", {
506+
request: baseRequest(),
507+
resolve: (outcome: unknown) => {
508+
resolveCount += 1
509+
lastOutcome = outcome
510+
},
511+
timeoutMs: 10,
512+
})
513+
514+
acceptOverlaySelection(shell)
515+
expect(resolveCount).toBe(1)
516+
expect(lastOutcome).toEqual({ allow: false })
517+
518+
await new Promise((r) => setTimeout(r, 25))
519+
expect(resolveCount).toBe(1)
520+
} finally {
521+
shell.dispose()
522+
}
523+
})
524+
})
525+
})
526+
527+
describe("Esc on a gate overlay settles the awaited promise", () => {
528+
test("permission.gate: Esc denies instead of abandoning the promise", async () => {
529+
await withTestRenderer(async (h) => {
530+
const shell = createAppShell(h.renderer, {
531+
terminal: { columns: 80, rows: 24 },
532+
run: "idle",
533+
})
534+
const emitter = new EventEmitter()
535+
let resolved: unknown
536+
let resolveCount = 0
537+
try {
538+
wireGates(emitter, shell)
539+
emitter.emit("permission.gate", {
540+
request: baseRequest(),
541+
resolve: (outcome: unknown) => {
542+
resolveCount += 1
543+
resolved = outcome
544+
},
545+
})
546+
expect(shell.overlayKind).toBe("permissions")
547+
548+
closeInsetOverlay(shell)
549+
550+
expect(shell.overlayList).toBeNull()
551+
expect(resolveCount).toBe(1)
552+
expect(resolved).toEqual({ allow: false })
553+
} finally {
554+
shell.dispose()
555+
}
556+
})
557+
})
558+
559+
test("operator.gate: Esc cancels instead of abandoning the promise", async () => {
560+
await withTestRenderer(async (h) => {
561+
const shell = createAppShell(h.renderer, {
562+
terminal: { columns: 80, rows: 24 },
563+
run: "idle",
564+
})
565+
const emitter = new EventEmitter()
566+
let resolved: unknown
567+
let resolveCount = 0
568+
try {
569+
wireGates(emitter, shell)
570+
emitter.emit("operator.gate", {
571+
question: "Proceed?",
572+
options: ["Cancel", "Continue"],
573+
resolve: (result: unknown) => {
574+
resolveCount += 1
575+
resolved = result
576+
},
577+
})
578+
expect(shell.overlayKind).toBe("operator")
579+
580+
closeInsetOverlay(shell)
581+
582+
expect(shell.overlayList).toBeNull()
583+
expect(resolveCount).toBe(1)
584+
expect(resolved).toEqual({ kind: "cancel" })
585+
} finally {
586+
shell.dispose()
587+
}
588+
})
589+
})
590+
})
591+
426592
describe("permission overlay height", () => {
427593
const openGate = (shell: AppShell, scopeCount: number): void => {
428594
const emitter = new EventEmitter()

src/tui-opentui/gate-wire.ts

Lines changed: 86 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,17 @@ import type {
1414
PermissionRequest,
1515
} from "../permission/types.js"
1616
import type { AppShell, OverlaySelection } from "./shell.js"
17-
import { appendStreamRow, onOverlayClosed, setOverlayBody } from "./shell.js"
17+
import {
18+
appendStreamRow,
19+
closeInsetOverlay,
20+
onOverlayClosed,
21+
setOverlayBody,
22+
} from "./shell.js"
1823
import { EXPAND_KEY } from "./stream.js"
24+
import type {
25+
OperatorGateEvent,
26+
PermissionGateEvent,
27+
} from "../tui/gate-events.js"
1928

2029
/** Stable sentinel ids for the always-present deny / once rows. */
2130
export const PERMISSION_DENY_ID = "__deny__" as const
@@ -218,17 +227,6 @@ function recordDecision(
218227
appendStreamRow(shell, { role: "system", text, meta: "permission" })
219228
}
220229

221-
type PermissionGateEvent = {
222-
request: PermissionRequest
223-
resolve: (outcome: ApprovalOutcome) => void
224-
}
225-
226-
type OperatorGateEvent = {
227-
question: string
228-
options: string[]
229-
resolve: (result: OperatorResult) => void
230-
}
231-
232230
/**
233231
* Subscribe the permission/operator gate events to the shell's overlays.
234232
* Returns a dispose function that removes exactly the listeners this call added.
@@ -264,6 +262,8 @@ export function wireGates(
264262
const collapsedAnything =
265263
formatCommandForApproval(ev.request.subject).payloadCount > 0
266264
let expanded = false
265+
let settled = false
266+
let isOpen = false
267267

268268
const onToggleExpand = (): void => {
269269
expanded = !expanded
@@ -283,20 +283,77 @@ export function wireGates(
283283
})
284284
}
285285

286-
openOrQueue(() => openPermissionsOverlay(shell, {
287-
items: choices.items,
288-
itemIds: choices.itemIds,
289-
body: collapsedBody,
290-
...(collapsedAnything ? { onToggleExpand } : {}),
291-
onAccept: (sel: OverlaySelection) => {
292-
const gateSelection = {
293-
index: sel.index,
294-
...(sel.id !== undefined ? { id: sel.id } : {}),
295-
}
296-
recordDecision(shell, ev.request, choices, gateSelection)
297-
ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
298-
},
299-
}))
286+
const open = (): void => {
287+
isOpen = true
288+
openPermissionsOverlay(shell, {
289+
items: choices.items,
290+
itemIds: choices.itemIds,
291+
body: collapsedBody,
292+
...(collapsedAnything ? { onToggleExpand } : {}),
293+
onAccept: (sel: OverlaySelection) => {
294+
settled = true
295+
clearTimers()
296+
const gateSelection = {
297+
index: sel.index,
298+
...(sel.id !== undefined ? { id: sel.id } : {}),
299+
}
300+
recordDecision(shell, ev.request, choices, gateSelection)
301+
ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
302+
},
303+
// Esc must settle the awaited promise (as a deny), not abandon it —
304+
// an unresolved gate hangs the run until the process is killed.
305+
onCancel: () => {
306+
if (settled) return
307+
settled = true
308+
clearTimers()
309+
ev.resolve(
310+
approvalOutcomeFromSelection(choices, {
311+
index: 0,
312+
id: PERMISSION_DENY_ID,
313+
}),
314+
)
315+
},
316+
})
317+
}
318+
319+
// Watchdog abort (tool budget expired / parent run cancelled) and the
320+
// goal-mode timeout both race an operator who may never answer — each
321+
// must resolve the gate itself rather than leave the overlay (or the
322+
// queued open) parked forever. autoDeny wins the race exactly once:
323+
// whichever fires first tears down the other and, if the overlay is
324+
// already on screen for this gate, closes it so nothing stale lingers.
325+
let timer: ReturnType<typeof setTimeout> | undefined
326+
const clearTimers = (): void => {
327+
if (timer !== undefined) clearTimeout(timer)
328+
ev.signal?.removeEventListener("abort", onAbort)
329+
}
330+
const autoDeny = (message: string): void => {
331+
if (settled) return
332+
settled = true
333+
clearTimers()
334+
if (isOpen) {
335+
closeInsetOverlay(shell)
336+
} else {
337+
const idx = pending.indexOf(open)
338+
if (idx >= 0) pending.splice(idx, 1)
339+
}
340+
ev.resolve({ allow: false, message })
341+
}
342+
function onAbort(): void {
343+
autoDeny("tool no longer running; permission request denied")
344+
}
345+
if (ev.timeoutMs !== undefined) {
346+
timer = setTimeout(() => {
347+
autoDeny(ev.timeoutMessage ?? "approval timed out; request denied")
348+
}, ev.timeoutMs)
349+
}
350+
if (ev.signal?.aborted === true) {
351+
autoDeny("tool no longer running; permission request denied")
352+
return
353+
}
354+
ev.signal?.addEventListener("abort", onAbort, { once: true })
355+
356+
openOrQueue(open)
300357
}
301358

302359
function onOperator(ev: OperatorGateEvent): void {
@@ -316,6 +373,9 @@ export function wireGates(
316373
// The ask_operator contract offers a free-form answer, so the overlay
317374
// must be able to send one back rather than only an option index.
318375
onTextAnswer: (text: string) => ev.resolve(operatorCustomResult(text)),
376+
// Esc must settle the awaited promise (as a cancel), not abandon it —
377+
// an unresolved gate hangs the run until the process is killed.
378+
onCancel: () => ev.resolve(operatorCancelResult()),
319379
}))
320380
}
321381

src/tui-opentui/overlays.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ export type OpenPermissionsOpts = {
8989
readonly onAccept?: (selection: OverlaySelection) => void
9090
/** Per-open expand/collapse of collapsed command payloads. */
9191
readonly onToggleExpand?: () => void
92+
/** Per-open Esc/dismiss; host binds resolve(ApprovalOutcome) so Esc denies instead of hanging. */
93+
readonly onCancel?: () => void
9294
}
9395

9496
export function openPermissionsOverlay(
@@ -108,6 +110,7 @@ export function openPermissionsOverlay(
108110
? { onToggleExpand: opts.onToggleExpand }
109111
: {}),
110112
...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}),
113+
...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}),
111114
})
112115
}
113116

@@ -120,6 +123,8 @@ export type OpenOperatorOpts = {
120123
readonly onAccept?: (selection: OverlaySelection) => void
121124
/** Per-open free-text answer; host binds the custom OperatorResult. */
122125
readonly onTextAnswer?: (text: string) => void
126+
/** Per-open Esc/dismiss; host binds resolve(cancel) so Esc cancels instead of hanging. */
127+
readonly onCancel?: () => void
123128
}
124129

125130
/**
@@ -128,7 +133,7 @@ export type OpenOperatorOpts = {
128133
* offering "Enter choose" against an empty list.
129134
*/
130135
const NO_WAY_TO_ANSWER =
131-
"No options were offered and this question takes no typed answer. Press Esc to dismiss it."
136+
"No options were offered and this question takes no typed answer. Press Esc to cancel it."
132137

133138
export function openOperatorOverlay(
134139
shell: AppShell,
@@ -150,6 +155,7 @@ export function openOperatorOverlay(
150155
...(opts?.onTextAnswer !== undefined
151156
? { onTextAnswer: opts.onTextAnswer }
152157
: {}),
158+
...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}),
153159
})
154160
}
155161

0 commit comments

Comments
 (0)