From 7d40b7083490d14e6ece2d4732fce6980ab9c72b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 12:27:30 -0700 Subject: [PATCH] Arm queued gates' display-dependent timers only once shown The permission gate's goal-mode timeout and the operator gate's own new timeout/abort safety net both used to start (or, for the operator gate, not exist at all) the moment a request was raised, even while it sat behind another overlay on the shared host. A request queued behind others could burn its entire timeout, or the run could abort, before the operator ever saw it, presenting as a hung session with commands firing but no visible response. The timeout now arms inside each gate's own open() callback, which only runs once the shared overlay host actually displays it; the abort-signal listener stays registered immediately, since a tool having already finished is true whether or not its gate is on screen. The operator gate gains the same abort/timeout/auto-cancel treatment the permission gate already had, including the goal-mode timeout and the ALS tool-budget signal, via a shared attachApprovalBudget helper used by both the permission gate and every operator-gate emission site (ask_operator, MCP TOFU). Unlike the permission gate, operator gates had no queue module to register with, so a gate still queued behind another overlay at session teardown was dropped without ever settling its awaited promise. wireGates now tracks each outstanding operator gate's teardown callback and cancels every one of them on dispose, the same guarantee permissionQueue.drain() already gives permission requests. --- src/tui-opentui/gate-wire.test.ts | 275 +++++++++++++++++++++++++++++- src/tui-opentui/gate-wire.ts | 171 ++++++++++++++----- src/tui/gate-events.ts | 13 ++ src/tui/request-approval.test.ts | 47 ++++- src/tui/request-approval.ts | 70 +++++--- src/tui/runner.ts | 47 +++-- 6 files changed, 537 insertions(+), 86 deletions(-) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 575c20264..266e1e623 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -640,7 +640,7 @@ describe("each gate decision appends exactly one transcript row", () => { }) }) - test("a queued gate's timeout settles once and records once without ever opening", async () => { + test("a queued gate's timeout settles once and records once, only after it is displayed", async () => { await withTestRenderer(async (h) => { const shell = createAppShell(h.renderer, { terminal: { columns: 80, rows: 24 }, @@ -663,10 +663,17 @@ describe("each gate decision appends exactly one transcript row", () => { timeoutMs: 5, }) + // Still behind the first gate — the timeout must not be ticking yet. + await new Promise((r) => setTimeout(r, 20)) + expect(resolveCount).toBe(0) + expect(shell.streamLog.length).toBe(before) + + // Closing the first gate displays the queued one, arming its timer. + acceptOverlaySelection(shell) await new Promise((r) => setTimeout(r, 20)) expect(resolveCount).toBe(1) - expect(shell.streamLog.length - before).toBe(1) + expect(shell.streamLog.length - before).toBe(2) // first gate's row + the queued gate's timeout row } finally { shell.dispose() } @@ -906,7 +913,7 @@ describe("permission.gate auto-deny", () => { }) }) - test("a queued gate's timeout fires while it waits, without disturbing the open one", async () => { + test("a queued gate's timeout does not start until it is displayed", async () => { await withTestRenderer(async (h) => { const shell = createAppShell(h.renderer, { terminal: { columns: 80, rows: 24 }, @@ -934,26 +941,280 @@ describe("permission.gate auto-deny", () => { // Second gate has not opened yet — it is waiting behind the first. expect(shell.overlayKind).toBe("permissions") + // Well past the nominal 5ms timeout — the queued gate must survive + // this because it has never been shown to the operator. await new Promise((r) => setTimeout(r, 20)) + expect(secondResolved).toBeUndefined() + expect(firstResolved).toBeUndefined() + expect(shell.overlayKind).toBe("permissions") - // The queued gate resolved on its own without ever opening... + // Closing the first gate displays the second, which arms its timer + // only now — this is when the queued gate's clock should start. + acceptOverlaySelection(shell) + expect(firstResolved).toEqual({ allow: false }) + expect(secondResolved).toBeUndefined() + + await new Promise((r) => setTimeout(r, 20)) expect(secondResolved).toEqual({ allow: false, message: "queued gate timed out", }) - // ...and the open overlay (the first gate) is undisturbed. - expect(firstResolved).toBeUndefined() + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) +}) + +describe("operator.gate auto-cancel", () => { + test("timeoutMs elapsing auto-cancels with the timeout label 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("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: (result: unknown) => { + resolved = result + }, + timeoutMs: 5, + timeoutMessage: "goal mode: no answer in time", + }) + expect(shell.overlayKind).toBe("operator") + + await new Promise((r) => setTimeout(r, 20)) + + expect(resolved).toEqual(operatorCancelResult()) + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) + + test("aborting the signal while the overlay is open auto-cancels 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("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: (result: unknown) => { + resolved = result + }, + signal: controller.signal, + }) + expect(shell.overlayKind).toBe("operator") + + controller.abort() + + expect(resolved).toEqual(operatorCancelResult()) + expect(shell.overlayList).toBeNull() + } finally { + shell.dispose() + } + }) + }) + + // The queue-behind hazard from CL-5664: a stuck overlay in front of an + // ask_operator question must not hang the run forever with nothing on + // screen to answer. The abort listener is not display-dependent, so it + // must settle the queued gate even though it never opened. + test("aborting the run while the operator gate is still queued settles it without ever opening", 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: () => {}, + }) + emitter.emit("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: (result: unknown) => { + resolved = result + }, + signal: controller.signal, + }) + // Still queued behind the permission overlay. + expect(shell.overlayKind).toBe("permissions") + + controller.abort() + + expect(resolved).toEqual(operatorCancelResult()) + // The permission overlay in front is undisturbed. + expect(shell.overlayKind).toBe("permissions") + } finally { + shell.dispose() + } + }) + }) + + test("a queued operator gate's timeout does not start until it is displayed", 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("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: (result: unknown) => { + secondResolved = result + }, + timeoutMs: 5, + timeoutMessage: "queued operator gate timed out", + }) expect(shell.overlayKind).toBe("permissions") + // Well past the nominal 5ms timeout — must survive because it has + // never been shown to the operator. + await new Promise((r) => setTimeout(r, 20)) + expect(secondResolved).toBeUndefined() + acceptOverlaySelection(shell) expect(firstResolved).toEqual({ allow: false }) - // No queued gate left to open once the first closes. + expect(secondResolved).toBeUndefined() + expect(shell.overlayKind).toBe("operator") + + await new Promise((r) => setTimeout(r, 20)) + expect(secondResolved).toEqual(operatorCancelResult()) 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 + try { + wireGates(emitter, shell) + emitter.emit("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: () => { + resolveCount += 1 + }, + timeoutMs: 10, + }) + + acceptOverlaySelection(shell) + expect(resolveCount).toBe(1) + + await new Promise((r) => setTimeout(r, 25)) + expect(resolveCount).toBe(1) + } finally { + shell.dispose() + } + }) + }) + + test("each terminal path writes exactly one transcript row", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + try { + wireGates(emitter, shell) + const before = shell.streamLog.length + emitter.emit("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: () => {}, + timeoutMs: 5, + }) + await new Promise((r) => setTimeout(r, 20)) + expect(shell.streamLog.length - before).toBe(1) + } finally { + shell.dispose() + } + }) + }) + + // Mirrors the permission queue's "disposing with a request still queued" + // coverage: unlike permissionQueue, the operator gate has no queue module + // of its own, so wireGates must track outstanding operator gates itself to + // settle them on teardown. + test("disposing with a gate still queued settles it instead of hanging", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + let openResolved: unknown + let queuedResolved: unknown + const dispose = wireGates(emitter, shell) + emitter.emit("operator.gate", { + question: "Proceed?", + options: ["Yes", "No"], + resolve: (r: unknown) => { + openResolved = r + }, + }) + // Occupies the overlay host so this second gate queues instead of + // opening — dispose must cancel it without ever displaying it. + const before = shell.streamLog.length + emitter.emit("operator.gate", { + question: "Also proceed?", + options: ["Yes", "No"], + resolve: (r: unknown) => { + queuedResolved = r + }, + }) + + dispose() + + expect(openResolved).toEqual(operatorCancelResult()) + expect(queuedResolved).toEqual(operatorCancelResult()) + expect(shell.streamLog.length - before).toBe(2) + for (const row of shell.streamLog.slice(-2)) { + expect(row.text).toContain("Cancelled (session ended)") + } + shell.dispose() + }) + }) }) describe("Esc on a gate overlay settles the awaited promise", () => { diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index 253a30669..07ebc2386 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -332,6 +332,17 @@ export function wireGates( emitter, permissionQueue, ) + // Operator gates have no queue module of their own (unlike permission + // requests, which register with permissionQueue so dispose can drain + // them) — each one registers its own teardown callback here for the + // lifetime it is outstanding, so a gate still queued behind another + // overlay at session teardown still settles instead of hanging its + // awaited promise forever. Every settle path (onAccept, onTextAnswer, + // onCancel, settleOnce) is responsible for deregistering its own entry + // before resolving — a future settle path that forgets this leaks its + // gate into dispose's teardown sweep after it has already resolved + // (harmless, since `settled` guards the double-resolve, but wasted work). + const operatorTeardowns = new Set<() => void>() // Bumped every time any gate (permission or operator) opens on the shared // host. A settle path that only knows "my overlay was opened" cannot tell // whether the host has since moved on to a newer one — the shell closes an @@ -439,6 +450,11 @@ export function wireGates( const open = (): void => { openedGeneration = overlayGeneration + if (ev.timeoutMs !== undefined) { + timer = setTimeout(() => { + autoDeny(ev.timeoutMessage ?? "approval timed out; request denied") + }, ev.timeoutMs) + } openPermissionsOverlay(shell, { items: choices.items, itemIds: choices.itemIds, @@ -476,6 +492,13 @@ export function wireGates( // queued open) parked forever. Whichever fires first settles the queue // entry, which is itself the single-resolve guard, so the other side is // simply a no-op once it runs. + // + // The goal-mode timeout is display-dependent and arms inside `open` + // (below), not here: a request sitting behind others in `pending` must + // not burn its timeout while the operator has never seen it. Abort is not + // display-dependent — it reflects the tool having already finished or + // been cancelled, which is true whether or not this gate is on screen — + // so its listener is registered immediately. let timer: ReturnType | undefined const clearTimers = (): void => { if (timer !== undefined) clearTimeout(timer) @@ -493,11 +516,6 @@ export function wireGates( 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 @@ -516,42 +534,109 @@ export function wireGates( // 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, - // recordOperatorDecision below is the authoritative transcript row for - // every terminal path — the overlay's own accept/answer echo would - // duplicate it. - echoChoice: false, - onAccept: (sel: OverlaySelection) => { - if (settled) return - settled = true - recordOperatorDecision(shell, ev.question, sel.label) - resolve( - operatorResultFromSelection(ev.options, { - index: sel.index, - ...(sel.id !== undefined ? { id: sel.id } : {}), - }), - ) - }, - // 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) => { - if (settled) return - settled = true - recordOperatorDecision(shell, ev.question, text) - 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 - recordOperatorDecision(shell, ev.question, "Cancelled") - resolve(operatorCancelResult()) - }, - })) + // Set only while this gate's own overlay is the one on screen — mirrors + // openedGeneration on the permission path (see its comment above): a + // settle path that only knows "my overlay was opened" cannot tell + // whether the host has since moved on to a newer one. + let openedGeneration: number | undefined + + // Mirrors the permission gate: watchdog abort and the goal-mode timeout + // both race an operator who may never answer, and unlike the permission + // path this gate previously had no safety net at all — a queued question + // behind a stuck overlay hung the run forever. The timeout is + // display-dependent and arms inside `open` (below); abort is not, so its + // listener is registered immediately. + let timer: ReturnType | undefined + const clearTimers = (): void => { + if (timer !== undefined) clearTimeout(timer) + ev.signal?.removeEventListener("abort", onAbort) + } + + const open = (): void => { + openedGeneration = overlayGeneration + if (ev.timeoutMs !== undefined) { + timer = setTimeout(() => { + autoCancel(ev.timeoutMessage ?? "Cancelled (timed out)") + }, ev.timeoutMs) + } + openOperatorOverlay(shell, { + body: ev.question, + choices: choices.items, + itemIds: choices.itemIds, + // recordOperatorDecision below is the authoritative transcript row + // for every terminal path — the overlay's own accept/answer echo + // would duplicate it. + echoChoice: false, + onAccept: (sel: OverlaySelection) => { + if (settled) return + settled = true + clearTimers() + operatorTeardowns.delete(teardown) + recordOperatorDecision(shell, ev.question, sel.label) + resolve( + operatorResultFromSelection(ev.options, { + index: sel.index, + ...(sel.id !== undefined ? { id: sel.id } : {}), + }), + ) + }, + // 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) => { + if (settled) return + settled = true + clearTimers() + operatorTeardowns.delete(teardown) + recordOperatorDecision(shell, ev.question, text) + 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. + // Esc already closes this overlay through the shell's own key + // handling, so — unlike autoCancel below — this does not re-invoke + // closeInsetOverlay itself; doing so would reenter this same + // onCancel (see the permission gate's identical note on `settle`). + onCancel: () => { + if (settled) return + settled = true + clearTimers() + operatorTeardowns.delete(teardown) + recordOperatorDecision(shell, ev.question, "Cancelled") + resolve(operatorCancelResult()) + }, + }) + } + + const settleOnce = (label: string, result: OperatorResult): void => { + if (settled) return + settled = true + clearTimers() + operatorTeardowns.delete(teardown) + if (openedGeneration === undefined) { + unqueue(open) + } else if (openedGeneration === overlayGeneration) { + closeInsetOverlay(shell) + } + recordOperatorDecision(shell, ev.question, label) + resolve(result) + } + const autoCancel = (label: string): void => { + settleOnce(label, operatorCancelResult()) + } + const teardown = (): void => { + autoCancel("Cancelled (session ended)") + } + function onAbort(): void { + autoCancel("Cancelled (tool no longer running)") + } + if (ev.signal?.aborted === true) { + autoCancel("Cancelled (tool no longer running)") + return + } + ev.signal?.addEventListener("abort", onAbort, { once: true }) + operatorTeardowns.add(teardown) + + openOrQueue(open) } emitter.on("permission.gate", onPermission) @@ -566,5 +651,9 @@ export function wireGates( // Deny anything still queued so its awaited evaluate() call never hangs // past session teardown. permissionQueue.drain() + // Cancel every outstanding operator gate (queued or displayed) so its + // awaited resolve() never hangs past session teardown either — the + // permission-side equivalent of the drain() call above. + for (const teardown of [...operatorTeardowns]) teardown() } } diff --git a/src/tui/gate-events.ts b/src/tui/gate-events.ts index 80ad92b68..7e68ef108 100644 --- a/src/tui/gate-events.ts +++ b/src/tui/gate-events.ts @@ -5,6 +5,19 @@ export type OperatorGateEvent = { question: string; options: string[]; resolve: (result: OperatorResult) => void; + /** + * When set (goal mode active), auto-cancel if the operator has not answered + * within this many ms so an unattended goal cannot park on the modal forever. + */ + timeoutMs?: number; + /** Override the agent-facing cancel message on timeout. */ + timeoutMessage?: string; + /** + * Tool-execution budget signal. When aborted (watchdog timeout or parent + * cancel), this operator entry is auto-cancelled even if it is not the head + * of the queue — so the modal cannot outlive a tool that already finished. + */ + signal?: AbortSignal; }; export type PermissionGateEvent = { diff --git a/src/tui/request-approval.test.ts b/src/tui/request-approval.test.ts index 89bbcdf24..755af762a 100644 --- a/src/tui/request-approval.test.ts +++ b/src/tui/request-approval.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { createGateRequestApproval } from "./request-approval.js"; +import { attachApprovalBudget, createGateRequestApproval } from "./request-approval.js"; import { getToolApprovalBudget, runWithToolExecutionWatchdog, @@ -103,3 +103,48 @@ describe("createGateRequestApproval", () => { expect((await pending).allow).toBe(true); }); }); + +// attachApprovalBudget is the mechanism createGateRequestApproval builds on +// (see above) and is reused directly by the operator-gate emission sites in +// runner.ts (ask_operator, MCP TOFU) — it must generalize past +// ApprovalOutcome and enforce single-resolution on its own. +describe("attachApprovalBudget", () => { + test("pauses the budget at call time and resumes exactly once no matter how many times finish is called", async () => { + let resolveCount = 0; + let lastValue: string | undefined; + await runWithToolExecutionWatchdog( + { id: "3", name: "ask_operator", arguments: {} }, + new AbortController().signal, + 60, + async () => { + const { finish } = attachApprovalBudget( + (value) => { + resolveCount += 1; + lastValue = value; + }, + { tool: "ask_operator", kind: "operator" }, + ); + // Longer than the budget — frozen while the question is pending, + // exactly like the permission gate's budget pause. + await new Promise((r) => setTimeout(r, 120)); + expect(getToolApprovalBudget()?.signal.aborted).toBe(false); + finish("answered"); + finish("answered again"); + return { callId: "3", content: "done" }; + }, + { salvageGraceMs: 80, waitForApproval: true }, + ); + expect(resolveCount).toBe(1); + expect(lastValue).toBe("answered"); + }); + + test("has no signal when called outside a tool run", () => { + let resolved: unknown; + const { finish, signal } = attachApprovalBudget((value) => { + resolved = value; + }, { tool: "ask_operator", kind: "operator" }); + expect(signal).toBeUndefined(); + finish("cancel"); + expect(resolved).toBe("cancel"); + }); +}); diff --git a/src/tui/request-approval.ts b/src/tui/request-approval.ts index 0e3263bdb..255804c09 100644 --- a/src/tui/request-approval.ts +++ b/src/tui/request-approval.ts @@ -1,7 +1,6 @@ import { getLogger } from "@intx/log"; import { LOG_NAMESPACE_ROOT } from "../branding.js"; import type { ApprovalOutcome, PermissionRequest, RequestApproval } from "../permission/types.js"; -import type { ChainedPauseToken } from "./tool-execution-watchdog.js"; import { getToolApprovalBudget } from "./tool-execution-watchdog.js"; import type { PermissionGateEvent } from "./gate-events.js"; @@ -14,42 +13,63 @@ export type CreateGateRequestApprovalArgs = { const logger = getLogger([LOG_NAMESPACE_ROOT, "tui", "permission"]); +/** + * Wires a gate's settle callback to the ALS tool-approval budget, so every + * gate (permission, ask_operator, MCP TOFU) freezes the tool's wall-clock + * budget the same way while a human decides. The budget is paused the moment + * the gate is raised, not deferred until its overlay is actually shown: it + * guards the tool's own execution timeout, which keeps running for any tool + * call regardless of whether an approval overlay is on screen, so deferring + * the pause would let a queued-and-invisible request burn its tool timeout + * with no protection at all — a worse failure than the one being fixed. + * `resolve` is called at most once no matter how many times the returned + * `finish` is invoked. The budget handle is captured at gate time: `finish` + * may run on the UI thread outside the tool ALS, so an ALS re-lookup there + * would no-op on resume. + */ +export function attachApprovalBudget( + resolve: (value: T) => void, + logContext: { tool: string; kind: string }, +): { finish: (value: T) => void; signal?: AbortSignal } { + const budget = getToolApprovalBudget(); + if (budget === undefined) { + // Every TUI tool call runs under the watchdog ALS; an absent store means + // the gate fired outside a tool run or the ALS context was lost. + logger.warn("{kind} gate reached with no tool budget in ALS for {tool}", logContext); + } + const pauseToken = budget?.waitForApproval ? budget.pause() : undefined; + let settled = false; + const finish = (value: T): void => { + if (settled) return; + settled = true; + if (pauseToken !== undefined) budget?.resume(pauseToken); + resolve(value); + }; + return { + finish, + ...(budget !== undefined ? { signal: budget.signal } : {}), + }; +} + /** * Builds the permission gate's requestApproval callback for the TUI. * - * Freezes the tool wall-clock budget while the operator decides (when - * waitForApproval is on). Always attaches the budget signal so a timeout with - * waitForApproval off dismisses the modal instead of leaving a ghost. The - * budget handle is captured at gate time: finish() runs on the UI thread - * outside the tool ALS, so an ALS re-lookup there would no-op on resume. + * Always attaches the budget signal so a timeout with waitForApproval off + * dismisses the modal instead of leaving a ghost. */ export function createGateRequestApproval(args: CreateGateRequestApprovalArgs): RequestApproval { return (request: PermissionRequest) => new Promise((resolve) => { - const budget = getToolApprovalBudget(); - if (budget === undefined) { - // Every TUI tool call runs under the watchdog ALS; an absent store - // means the gate fired outside a tool run or the ALS context was lost. - logger.warn("permission gate reached with no tool budget in ALS for {tool}", { - tool: request.tool, - }); - } - const pauseToken: ChainedPauseToken | undefined = budget?.waitForApproval - ? budget.pause() - : undefined; - let settled = false; - const finish = (outcome: ApprovalOutcome): void => { - if (settled) return; - settled = true; - if (pauseToken !== undefined) budget?.resume(pauseToken); - resolve(outcome); - }; + const { finish, signal } = attachApprovalBudget(resolve, { + tool: request.tool, + kind: "permission", + }); const goal = args.goalTimeout(); const event: PermissionGateEvent = { request, resolve: finish, ...(goal !== undefined ? goal : {}), - ...(budget !== undefined ? { signal: budget.signal } : {}), + ...(signal !== undefined ? { signal } : {}), }; if (!args.emitGate(event)) { // Pre-mount or post-unmount: no gate queue exists, so the prompt would diff --git a/src/tui/runner.ts b/src/tui/runner.ts index a7e70be1a..80f109244 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -46,7 +46,7 @@ import { unconnectedProviderChoices } from "../tui-opentui/provider-setup.js"; import { connectProviderInline } from "../tui-opentui/provider-connect.js"; import type { SessionModeScope } from "../tui-opentui/command-surfaces.js"; import { resolveWaitForApproval, type ToolWatchdogConfig } from "./tool-execution-watchdog.js"; -import { createGateRequestApproval } from "./request-approval.js"; +import { attachApprovalBudget, createGateRequestApproval } from "./request-approval.js"; import { codexProfileFromProviderName } from "../config/codex-providers.js"; import { xaiProfileFromProviderName } from "../config/xai-providers.js"; import type { PluginsAdmin, PluginDescriptor } from "../plugins/admin.js"; @@ -642,6 +642,19 @@ export async function runTUI(initialConfig: Config): Promise { current: null, }; + // Shared by the permission gate and every operator-gate emission site: an + // unattended goal-mode run must not park on any gate forever, whichever + // kind it is. + const goalTimeout = (): { timeoutMs: number; timeoutMessage: string } | undefined => { + const snap = goalGovernorRef.current?.get() ?? null; + return isGoalApprovalTimeoutActive(snap?.status) + ? { + timeoutMs: DEFAULT_GOAL_APPROVAL_TIMEOUT_MS, + timeoutMessage: goalApprovalTimeoutMessage(DEFAULT_GOAL_APPROVAL_TIMEOUT_MS), + } + : undefined; + }; + const seededApprovals = await loadSeededApprovals(config.cwd, sessionId); const permissionGate = createPermissionGate({ approvals: seededApprovals, @@ -651,15 +664,7 @@ export async function runTUI(initialConfig: Config): Promise { model: config.model, requestApproval: createGateRequestApproval({ emitGate: (event) => emitter.emit("permission.gate", event), - goalTimeout: () => { - const snap = goalGovernorRef.current?.get() ?? null; - return isGoalApprovalTimeoutActive(snap?.status) - ? { - timeoutMs: DEFAULT_GOAL_APPROVAL_TIMEOUT_MS, - timeoutMessage: goalApprovalTimeoutMessage(DEFAULT_GOAL_APPROVAL_TIMEOUT_MS), - } - : undefined; - }, + goalTimeout, }), persist: createApprovalPersist(config.cwd, activeProviderModel), interactive: true, @@ -1067,7 +1072,18 @@ export async function runTUI(initialConfig: Config): Promise { ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), onOperatorGate: (question, options) => new Promise((resolve) => { - const event: OperatorGateEvent = { question, options, resolve }; + const { finish, signal } = attachApprovalBudget(resolve, { + tool: "ask_operator", + kind: "operator", + }); + const goal = goalTimeout(); + const event: OperatorGateEvent = { + question, + options, + resolve: finish, + ...(goal !== undefined ? goal : {}), + ...(signal !== undefined ? { signal } : {}), + }; emitter.emit("operator.gate", event); }), sessionMode: liveSessionMode, @@ -1078,6 +1094,11 @@ export async function runTUI(initialConfig: Config): Promise { requestMcpTrust: async (server) => { // TOFU via operator gate: Trust this local MCP server? const result = await new Promise((resolve) => { + const { finish, signal } = attachApprovalBudget(resolve, { + tool: `mcp:${server.name}`, + kind: "operator", + }); + const goal = goalTimeout(); const event: OperatorGateEvent = { question: `Trust local MCP server "${server.name}" for this project?` @@ -1087,7 +1108,9 @@ export async function runTUI(initialConfig: Config): Promise { ? `\nURL: ${server.url}` : ""), options: ["Trust and connect", "Deny"], - resolve, + resolve: finish, + ...(goal !== undefined ? goal : {}), + ...(signal !== undefined ? { signal } : {}), }; emitter.emit("operator.gate", event); });