diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 19852553a..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, @@ -423,6 +424,220 @@ 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("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 30b2e5486..c19053d79 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,29 +283,94 @@ 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)) + }, + // 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, + }), + ) + }, + }) + } + + // 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 { 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, @@ -315,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,