From 533f6da77afe949d406f5241044e2b7ad17f3c70 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 22:09:04 -0700 Subject: [PATCH 1/2] Restore permission-gate auto-deny for timeouts and aborts A local redeclaration of the gate event type omitted timeoutMs, timeoutMessage, and signal, so goal-mode timeouts and watchdog aborts were silently dropped. An unattended run could park on the permission modal forever with nobody able to answer it. Import the shared type instead, so the compiler catches a missing field, and wire a timer plus an abort listener into the permission handler that auto-deny and close the overlay. All three exit paths (accept, timeout, abort) guard on a settled flag so none can double-resolve, and a queued gate resolves on its own timeout without disturbing whichever gate is currently on screen. --- src/tui-opentui/gate-wire.test.ts | 149 ++++++++++++++++++++++++++++++ src/tui-opentui/gate-wire.ts | 97 +++++++++++++------ 2 files changed, 220 insertions(+), 26 deletions(-) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 19852553a..222bc6d73 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -423,6 +423,155 @@ describe("wireGates", () => { }) }) +describe("permission.gate auto-deny", () => { + test("timeoutMs elapsing auto-denies with the timeout message and closes the overlay", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let resolved: unknown + try { + wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: (outcome: unknown) => { + resolved = outcome + }, + timeoutMs: 5, + timeoutMessage: "goal mode: no answer in time", + }) + expect(shell.overlayKind).toBe("permissions") + + await new Promise((r) => setTimeout(r, 20)) + + expect(resolved).toEqual({ + allow: false, + message: "goal mode: no answer in time", + }) + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) + + test("aborting the signal while the overlay is open auto-denies and closes it", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + const controller = new AbortController() + let resolved: unknown + try { + wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: (outcome: unknown) => { + resolved = outcome + }, + signal: controller.signal, + }) + expect(shell.overlayKind).toBe("permissions") + + controller.abort() + + expect(resolved).toEqual({ + allow: false, + message: "tool no longer running; permission request denied", + }) + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) + + test("resolving normally clears the timer instead of firing it later", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let resolveCount = 0 + let lastOutcome: unknown + try { + wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: (outcome: unknown) => { + resolveCount += 1 + lastOutcome = outcome + }, + timeoutMs: 10, + }) + + acceptOverlaySelection(shell) + expect(resolveCount).toBe(1) + expect(lastOutcome).toEqual({ allow: false }) + + await new Promise((r) => setTimeout(r, 25)) + expect(resolveCount).toBe(1) + } finally { + shell.dispose() + } + }) + }) + + test("a queued gate's timeout fires while it waits, without disturbing the open one", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let firstResolved: unknown + let secondResolved: unknown + try { + wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: (outcome: unknown) => { + firstResolved = outcome + }, + }) + emitter.emit("permission.gate", { + request: baseRequest({ tool: "queued_tool" }), + resolve: (outcome: unknown) => { + secondResolved = outcome + }, + timeoutMs: 5, + timeoutMessage: "queued gate timed out", + }) + // Second gate has not opened yet — it is waiting behind the first. + expect(shell.overlayKind).toBe("permissions") + + await new Promise((r) => setTimeout(r, 20)) + + // The queued gate resolved on its own without ever opening... + expect(secondResolved).toEqual({ + allow: false, + message: "queued gate timed out", + }) + // ...and the open overlay (the first gate) is undisturbed. + expect(firstResolved).toBeUndefined() + expect(shell.overlayKind).toBe("permissions") + + acceptOverlaySelection(shell) + expect(firstResolved).toEqual({ allow: false }) + // No queued gate left to open once the first closes. + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) +}) + describe("permission overlay height", () => { const openGate = (shell: AppShell, scopeCount: number): void => { const emitter = new EventEmitter() diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index 30b2e5486..66d36e66f 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -14,8 +14,17 @@ import type { PermissionRequest, } from "../permission/types.js" import type { AppShell, OverlaySelection } from "./shell.js" -import { appendStreamRow, onOverlayClosed, setOverlayBody } from "./shell.js" +import { + appendStreamRow, + closeInsetOverlay, + onOverlayClosed, + setOverlayBody, +} from "./shell.js" import { EXPAND_KEY } from "./stream.js" +import type { + OperatorGateEvent, + PermissionGateEvent, +} from "../tui/gate-events.js" /** Stable sentinel ids for the always-present deny / once rows. */ export const PERMISSION_DENY_ID = "__deny__" as const @@ -218,17 +227,6 @@ function recordDecision( appendStreamRow(shell, { role: "system", text, meta: "permission" }) } -type PermissionGateEvent = { - request: PermissionRequest - resolve: (outcome: ApprovalOutcome) => void -} - -type OperatorGateEvent = { - question: string - options: string[] - resolve: (result: OperatorResult) => void -} - /** * Subscribe the permission/operator gate events to the shell's overlays. * Returns a dispose function that removes exactly the listeners this call added. @@ -264,6 +262,8 @@ export function wireGates( const collapsedAnything = formatCommandForApproval(ev.request.subject).payloadCount > 0 let expanded = false + let settled = false + let isOpen = false const onToggleExpand = (): void => { expanded = !expanded @@ -283,20 +283,65 @@ export function wireGates( }) } - openOrQueue(() => openPermissionsOverlay(shell, { - items: choices.items, - itemIds: choices.itemIds, - body: collapsedBody, - ...(collapsedAnything ? { onToggleExpand } : {}), - onAccept: (sel: OverlaySelection) => { - const gateSelection = { - index: sel.index, - ...(sel.id !== undefined ? { id: sel.id } : {}), - } - recordDecision(shell, ev.request, choices, gateSelection) - ev.resolve(approvalOutcomeFromSelection(choices, gateSelection)) - }, - })) + const open = (): void => { + isOpen = true + openPermissionsOverlay(shell, { + items: choices.items, + itemIds: choices.itemIds, + body: collapsedBody, + ...(collapsedAnything ? { onToggleExpand } : {}), + onAccept: (sel: OverlaySelection) => { + if (settled) return + settled = true + clearTimers() + const gateSelection = { + index: sel.index, + ...(sel.id !== undefined ? { id: sel.id } : {}), + } + recordDecision(shell, ev.request, choices, gateSelection) + ev.resolve(approvalOutcomeFromSelection(choices, gateSelection)) + }, + }) + } + + // Watchdog abort (tool budget expired / parent run cancelled) and the + // goal-mode timeout both race an operator who may never answer — each + // must resolve the gate itself rather than leave the overlay (or the + // queued open) parked forever. autoDeny wins the race exactly once: + // whichever fires first tears down the other and, if the overlay is + // already on screen for this gate, closes it so nothing stale lingers. + let timer: ReturnType | undefined + const clearTimers = (): void => { + if (timer !== undefined) clearTimeout(timer) + ev.signal?.removeEventListener("abort", onAbort) + } + const autoDeny = (message: string): void => { + if (settled) return + settled = true + clearTimers() + if (isOpen) { + closeInsetOverlay(shell) + } else { + const idx = pending.indexOf(open) + if (idx >= 0) pending.splice(idx, 1) + } + ev.resolve({ allow: false, message }) + } + function onAbort(): void { + autoDeny("tool no longer running; permission request denied") + } + if (ev.timeoutMs !== undefined) { + timer = setTimeout(() => { + autoDeny(ev.timeoutMessage ?? "approval timed out; request denied") + }, ev.timeoutMs) + } + if (ev.signal?.aborted === true) { + autoDeny("tool no longer running; permission request denied") + return + } + ev.signal?.addEventListener("abort", onAbort, { once: true }) + + openOrQueue(open) } function onOperator(ev: OperatorGateEvent): void { From dbf21e4b3a5688ca9aa800905deda74de1d064ee Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 6 Aug 2026 22:09:49 -0700 Subject: [PATCH 2/2] Resolve gate overlays on Esc instead of hanging Esc on a permission or operator overlay called closeInsetOverlay, which dropped the accept callback without ever invoking the caller's awaited resolve. The run deadlocked until the process was killed. Add an onCancel hook to the overlay opts and thread it through the open/close paths for the promise-backed overlay kinds only, so the gate wiring can resolve deny/cancel from it. Deliberate accept and answer-submit paths null the hook first so closing there does not also fire it. The operator gate also gains its own settled flag mirroring the permission gate, so double-resolve is prevented by construction rather than by callers remembering to null the hook. Update the operator "no way to answer" copy now that Esc actually settles the question. --- src/tui-opentui/gate-wire.test.ts | 66 +++++++++++++++++++++++++++++++ src/tui-opentui/gate-wire.ts | 33 +++++++++++++++- src/tui-opentui/overlays.ts | 8 +++- src/tui-opentui/shell.ts | 37 ++++++++++++++++- 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 222bc6d73..dd588556d 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -9,6 +9,7 @@ import { withTestRenderer, type Harness } from "./harness.js" import { OVERLAY_MAX_FRACTION } from "./geometry/index.js" import { acceptOverlaySelection, + closeInsetOverlay, createAppShell, exitOverlayAnswerMode, handleOverlayAnswerKey, @@ -572,6 +573,71 @@ describe("permission.gate auto-deny", () => { }) }) +describe("Esc on a gate overlay settles the awaited promise", () => { + test("permission.gate: Esc denies instead of abandoning the promise", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let resolved: unknown + let resolveCount = 0 + try { + wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: (outcome: unknown) => { + resolveCount += 1 + resolved = outcome + }, + }) + expect(shell.overlayKind).toBe("permissions") + + closeInsetOverlay(shell) + + expect(shell.overlayList).toBeNull() + expect(resolveCount).toBe(1) + expect(resolved).toEqual({ allow: false }) + } finally { + shell.dispose() + } + }) + }) + + test("operator.gate: Esc cancels instead of abandoning the promise", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let resolved: unknown + let resolveCount = 0 + try { + wireGates(emitter, shell) + emitter.emit("operator.gate", { + question: "Proceed?", + options: ["Cancel", "Continue"], + resolve: (result: unknown) => { + resolveCount += 1 + resolved = result + }, + }) + expect(shell.overlayKind).toBe("operator") + + closeInsetOverlay(shell) + + expect(shell.overlayList).toBeNull() + expect(resolveCount).toBe(1) + expect(resolved).toEqual({ kind: "cancel" }) + } finally { + shell.dispose() + } + }) + }) +}) + describe("permission overlay height", () => { const openGate = (shell: AppShell, scopeCount: number): void => { const emitter = new EventEmitter() diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index 66d36e66f..c19053d79 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -301,6 +301,19 @@ export function wireGates( recordDecision(shell, ev.request, choices, gateSelection) ev.resolve(approvalOutcomeFromSelection(choices, gateSelection)) }, + // Esc must settle the awaited promise (as a deny), not abandon it — + // an unresolved gate hangs the run until the process is killed. + onCancel: () => { + if (settled) return + settled = true + clearTimers() + ev.resolve( + approvalOutcomeFromSelection(choices, { + index: 0, + id: PERMISSION_DENY_ID, + }), + ) + }, }) } @@ -346,11 +359,18 @@ export function wireGates( function onOperator(ev: OperatorGateEvent): void { const choices = operatorChoicesFromOptions(ev.options) + // Guarded the same way as the permission gate: correctness must not rest + // on callers of closeInsetOverlay remembering to null the cancel hook + // before dispatching accept — a future accept-via-close path that forgets + // would otherwise double-resolve this promise. + let settled = false openOrQueue(() => openOperatorOverlay(shell, { body: ev.question, choices: choices.items, itemIds: choices.itemIds, onAccept: (sel: OverlaySelection) => { + if (settled) return + settled = true ev.resolve( operatorResultFromSelection(ev.options, { index: sel.index, @@ -360,7 +380,18 @@ export function wireGates( }, // The ask_operator contract offers a free-form answer, so the overlay // must be able to send one back rather than only an option index. - onTextAnswer: (text: string) => ev.resolve(operatorCustomResult(text)), + onTextAnswer: (text: string) => { + if (settled) return + settled = true + ev.resolve(operatorCustomResult(text)) + }, + // Esc must settle the awaited promise (as a cancel), not abandon it — + // an unresolved gate hangs the run until the process is killed. + onCancel: () => { + if (settled) return + settled = true + ev.resolve(operatorCancelResult()) + }, })) } diff --git a/src/tui-opentui/overlays.ts b/src/tui-opentui/overlays.ts index f495549fb..91ef61f39 100644 --- a/src/tui-opentui/overlays.ts +++ b/src/tui-opentui/overlays.ts @@ -89,6 +89,8 @@ export type OpenPermissionsOpts = { readonly onAccept?: (selection: OverlaySelection) => void /** Per-open expand/collapse of collapsed command payloads. */ readonly onToggleExpand?: () => void + /** Per-open Esc/dismiss; host binds resolve(ApprovalOutcome) so Esc denies instead of hanging. */ + readonly onCancel?: () => void } export function openPermissionsOverlay( @@ -108,6 +110,7 @@ export function openPermissionsOverlay( ? { onToggleExpand: opts.onToggleExpand } : {}), ...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}), + ...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}), }) } @@ -120,6 +123,8 @@ export type OpenOperatorOpts = { readonly onAccept?: (selection: OverlaySelection) => void /** Per-open free-text answer; host binds the custom OperatorResult. */ readonly onTextAnswer?: (text: string) => void + /** Per-open Esc/dismiss; host binds resolve(cancel) so Esc cancels instead of hanging. */ + readonly onCancel?: () => void } /** @@ -128,7 +133,7 @@ export type OpenOperatorOpts = { * offering "Enter choose" against an empty list. */ const NO_WAY_TO_ANSWER = - "No options were offered and this question takes no typed answer. Press Esc to dismiss it." + "No options were offered and this question takes no typed answer. Press Esc to cancel it." export function openOperatorOverlay( shell: AppShell, @@ -150,6 +155,7 @@ export function openOperatorOverlay( ...(opts?.onTextAnswer !== undefined ? { onTextAnswer: opts.onTextAnswer } : {}), + ...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}), }) } diff --git a/src/tui-opentui/shell.ts b/src/tui-opentui/shell.ts index 432d9f872..c2879dc0c 100644 --- a/src/tui-opentui/shell.ts +++ b/src/tui-opentui/shell.ts @@ -1685,6 +1685,7 @@ type PriorOverlaySnapshot = { readonly onAction: ((itemId: string, key: KeyEvent) => boolean) | null readonly answer: OverlayAnswerState | null readonly titleText: string + readonly onCancel: (() => void) | null } type ShellInternals = { @@ -1712,6 +1713,12 @@ type ShellInternals = { overlayAnswer: OverlayAnswerState | null /** Bare title of the open overlay, so its key hints can be re-composed. */ overlayTitleText: string + /** + * Per-open dismiss hook for promise-backed overlays (permissions, operator). + * Esc/closeInsetOverlay invokes this instead of silently dropping the + * pending promise the way palette/mentions/copy overlays correctly do. + */ + overlayOnCancel: (() => void) | null /** Fired once the shell has no overlay open, so queued gates can re-open. */ overlayClosedListeners: Set<() => void> /** @@ -2715,6 +2722,12 @@ export type OpenListOverlayOpts = { * Scoped to this open only, the way `onToggleExpand` and `typeToFilter` are. */ readonly onCycle?: (itemId: string, direction: -1 | 1) => void + /** + * Per-open Esc/dismiss hook for promise-backed overlays (permissions, + * operator). Invoked by closeInsetOverlay before the accept path is + * cleared, so the caller's awaited promise resolves instead of hanging. + */ + readonly onCancel?: () => void /** * Description-zone source. Called with the focused item's id on every move * (falling back to its label when no `itemIds` were supplied). Returning @@ -2788,6 +2801,7 @@ export function openListOverlay( onAction: bag.overlayOnAction, answer: bag.overlayAnswer, titleText: bag.overlayTitleText, + onCancel: bag.overlayOnCancel, } } // Leave prior overlay focus frame; palette will stack above it. @@ -2816,6 +2830,7 @@ export function openListOverlay( bag.overlayOnCycle = opts?.onCycle ?? null bag.overlayDescribe = opts?.describe ?? null bag.overlayOnAction = opts?.onAction ?? null + bag.overlayOnCancel = opts?.onCancel ?? null } else if (!bag.priorOverlay) { // Bare palette (no primary under it): no accept payload. bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : [] @@ -2825,6 +2840,7 @@ export function openListOverlay( bag.overlayOnCycle = opts?.onCycle ?? null bag.overlayDescribe = opts?.describe ?? null bag.overlayOnAction = opts?.onAction ?? null + bag.overlayOnCancel = opts?.onCancel ?? null } if (!isPalette) { bag.overlayAnswer = @@ -3072,6 +3088,10 @@ export function handleOverlayAnswerKey( text: `answered: ${text}`, meta: "overlay", }) + // Deliberate submit, not a dismiss — closeInsetOverlay must not also fire + // the Esc/cancel path. + const bag = internals.get(shell) + if (bag) bag.overlayOnCancel = null closeInsetOverlay(shell) submit(text) return true @@ -3157,6 +3177,13 @@ export function closeInsetOverlay(shell: AppShell): void { } const bag = internals.get(shell) const prior = wasPalette ? bag?.priorOverlay ?? null : null + // Permissions/operator overlays back a caller awaiting ev.resolve — Esc must + // still settle that promise (as a deny/cancel) or the caller hangs forever. + // Palette/mentions/copy have no such awaited caller, so they drop silently. + const cancelable = + !prior && + (shell.overlayKind === "permissions" || shell.overlayKind === "operator") + const onCancel = cancelable ? bag?.overlayOnCancel ?? null : null shell.overlayList = null shell.overlayKind = null @@ -3165,7 +3192,8 @@ export function closeInsetOverlay(shell: AppShell): void { shell.paletteCommands = [] shell.copyTargets = null clearOverlayBody(shell) - // Esc / dismiss: drop accept path without invoking callbacks. + // Esc / dismiss: drop accept path without invoking it (onCancel above is + // captured before this clears, and is invoked separately once state settles). if (bag && !prior) { bag.overlayItemIds = [] bag.overlayOnAccept = null @@ -3174,6 +3202,7 @@ export function closeInsetOverlay(shell: AppShell): void { bag.overlayDescribe = null bag.overlayOnAction = null bag.overlayAnswer = null + bag.overlayOnCancel = null } // Pop exactly one frame (palette or overlay). @@ -3202,6 +3231,7 @@ export function closeInsetOverlay(shell: AppShell): void { bag.overlayOnAction = prior.onAction bag.overlayAnswer = prior.answer bag.overlayTitleText = prior.titleText + bag.overlayOnCancel = prior.onCancel // If focus was not stacked (edge case), re-open overlay frame. if (focusOwner(shell.focus) !== "overlay") { shell.focus = openOverlay(shell.focus, OVERLAY_FRAME_ID, { @@ -3230,6 +3260,7 @@ export function closeInsetOverlay(shell: AppShell): void { relayout(shell, { overlayMode: "closed" }) applyFocus(shell) notifyOverlayClosed(shell) + onCancel?.() } /** @@ -3436,6 +3467,9 @@ export function acceptOverlaySelection(shell: AppShell): void { } // Capture before close clears per-open state. const perOpen = bag?.overlayOnAccept ?? null + // This is a deliberate accept, not a dismiss — closeInsetOverlay must not + // also fire the Esc/cancel path below. + if (bag) bag.overlayOnCancel = null if (bag?.overlayEchoChoice !== false) { appendStreamRow(shell, { @@ -5023,6 +5057,7 @@ export function createAppShell( overlayOnAction: null, overlayAnswer: null, overlayTitleText: "", + overlayOnCancel: null, overlayClosedListeners: new Set(), paletteCatalog: paletteCatalogOpt, paletteFilter: null,