From 547b6d608784ab06b4c39dbfcc1e869492e209cc Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 08:07:28 -0700 Subject: [PATCH 1/6] Add a headless queued-approval reconciliation queue The permission layer already judges whether one grant covers an already-queued request (isRequestCoveredByGrant); nothing owned the walk that acts on it. This queue enqueues live requests, settles them by id exactly once, drains coverage without a prompt when a grant widens, and denies whatever is left on session teardown so a pending entry can never hang an awaited resolve. Any approval surface can wire it to permission.grant with wirePermissionGrantReconciliation instead of reimplementing the walk. --- src/permission/queue.test.ts | 164 +++++++++++++++++++++++++++++++++++ src/permission/queue.ts | 112 ++++++++++++++++++++++++ 2 files changed, 276 insertions(+) create mode 100644 src/permission/queue.test.ts create mode 100644 src/permission/queue.ts diff --git a/src/permission/queue.test.ts b/src/permission/queue.test.ts new file mode 100644 index 000000000..ce843a8f9 --- /dev/null +++ b/src/permission/queue.test.ts @@ -0,0 +1,164 @@ +import { describe, test, expect } from "bun:test"; +import { EventEmitter } from "node:events"; +import { + createPermissionRequestQueue, + wirePermissionGrantReconciliation, +} from "./queue.js"; +import { isRequestCoveredByGrant } from "./gate.js"; +import { createPathRestriction } from "./path-restriction.js"; +import { createWorktreeRootsProvider } from "./worktree-roots.js"; +import type { Approval, ApprovalOutcome, PermissionRequest } from "./types.js"; + +function request(overrides: Partial = {}): PermissionRequest { + return { + tool: "run_shell", + action: "Run", + subject: "bun install", + scopes: [], + cwd: process.cwd(), + ...overrides, + }; +} + +// Mirrors the predicate PermissionGateOptions.onGrant hands callers: coverage +// judged with the gate's own path restriction and project workspace, not +// ones re-derived here. +function coversFor(approval: Approval, activeProviderModel?: string): (r: PermissionRequest) => boolean { + const cwd = process.cwd(); + const rootsProvider = createWorktreeRootsProvider(cwd); + const isRestricted = createPathRestriction(cwd, rootsProvider).isRestricted; + const workspace = { resolvedCwd: cwd, roots: rootsProvider() }; + return (r) => + isRequestCoveredByGrant(r, approval, activeProviderModel, isRestricted, workspace); +} + +describe("createPermissionRequestQueue", () => { + test("settle resolves the enqueued request and removes it", () => { + const queue = createPermissionRequestQueue(); + const outcomes: ApprovalOutcome[] = []; + const id = queue.enqueue(request(), (o) => outcomes.push(o)); + expect(queue.size()).toBe(1); + + expect(queue.settle(id, { allow: true })).toBe(true); + expect(outcomes).toEqual([{ allow: true }]); + expect(queue.size()).toBe(0); + }); + + test("settle is a no-op once an id has already settled", () => { + const queue = createPermissionRequestQueue(); + const outcomes: ApprovalOutcome[] = []; + const id = queue.enqueue(request(), (o) => outcomes.push(o)); + + expect(queue.settle(id, { allow: true })).toBe(true); + expect(queue.settle(id, { allow: false })).toBe(false); + expect(outcomes).toEqual([{ allow: true }]); + }); + + test("reconcile drains every queued request a grant now covers, in order", () => { + const queue = createPermissionRequestQueue(); + const outcomes: ApprovalOutcome[] = []; + for (let i = 0; i < 3; i++) { + queue.enqueue(request({ subject: "bun install" }), (o) => outcomes.push(o)); + } + // An unrelated request stays queued — the grant does not cover it. + queue.enqueue(request({ subject: "bun test" }), (o) => outcomes.push(o)); + + const covers = coversFor({ tool: "run_shell", pattern: "bun install" }); + const settledIds = queue.reconcile(covers); + + expect(settledIds).toHaveLength(3); + expect(outcomes).toEqual([{ allow: true }, { allow: true }, { allow: true }]); + expect(queue.size()).toBe(1); + expect(queue.list().map((e) => e.tool)).toEqual(["run_shell"]); + }); + + test("reconcile leaves requests from a different cwd queued for a project grant", () => { + const queue = createPermissionRequestQueue(); + const outcomes: ApprovalOutcome[] = []; + const cwd = process.cwd(); + queue.enqueue( + request({ subject: "bun install", cwd: `${cwd}/other-repo` }), + (o) => outcomes.push(o), + ); + + const covers = coversFor({ tool: "run_shell", pattern: "bun install", cwd }); + queue.reconcile(covers); + + expect(outcomes).toHaveLength(0); + expect(queue.size()).toBe(1); + }); + + test("reconcile is safe against settling mid-snapshot: no entry is skipped or double-visited", () => { + const queue = createPermissionRequestQueue(); + let calls = 0; + for (let i = 0; i < 5; i++) { + queue.enqueue(request({ subject: "bun install" }), () => { + calls++; + }); + } + queue.reconcile(coversFor({ tool: "run_shell", pattern: "bun install" })); + expect(calls).toBe(5); + expect(queue.size()).toBe(0); + }); + + test("drain denies everything still queued instead of leaving a resolve hanging", () => { + const queue = createPermissionRequestQueue(); + const outcomes: ApprovalOutcome[] = []; + queue.enqueue(request(), (o) => outcomes.push(o)); + queue.enqueue(request({ subject: "bun test" }), (o) => outcomes.push(o)); + + queue.drain(); + + expect(outcomes).toEqual([{ allow: false }, { allow: false }]); + expect(queue.size()).toBe(0); + }); +}); + +describe("wirePermissionGrantReconciliation", () => { + test("reconciles a queue against permission.grant events on the emitter", async () => { + const emitter = new EventEmitter(); + const queue = createPermissionRequestQueue(); + const dispose = wirePermissionGrantReconciliation(emitter, queue); + + const outcomes: ApprovalOutcome[] = []; + queue.enqueue(request({ subject: "bun install" }), (o) => outcomes.push(o)); + queue.enqueue(request({ subject: "bun install" }), (o) => outcomes.push(o)); + + const approval: Approval = { tool: "run_shell", pattern: "bun install" }; + emitter.emit("permission.grant", { approval, covers: coversFor(approval) }); + + expect(outcomes).toEqual([{ allow: true }, { allow: true }]); + expect(queue.size()).toBe(0); + + dispose(); + }); + + test("ignores a malformed grant payload instead of throwing", () => { + const emitter = new EventEmitter(); + const queue = createPermissionRequestQueue(); + const dispose = wirePermissionGrantReconciliation(emitter, queue); + + queue.enqueue(request(), () => { + throw new Error("must not settle on a malformed payload"); + }); + + expect(() => emitter.emit("permission.grant", { nope: true })).not.toThrow(); + expect(queue.size()).toBe(1); + + dispose(); + }); + + test("dispose stops further reconciliation", () => { + const emitter = new EventEmitter(); + const queue = createPermissionRequestQueue(); + const dispose = wirePermissionGrantReconciliation(emitter, queue); + dispose(); + + const outcomes: ApprovalOutcome[] = []; + queue.enqueue(request({ subject: "bun install" }), (o) => outcomes.push(o)); + const approval: Approval = { tool: "run_shell", pattern: "bun install" }; + emitter.emit("permission.grant", { approval, covers: coversFor(approval) }); + + expect(outcomes).toHaveLength(0); + }); +}); diff --git a/src/permission/queue.ts b/src/permission/queue.ts new file mode 100644 index 000000000..21eb34b1c --- /dev/null +++ b/src/permission/queue.ts @@ -0,0 +1,112 @@ +/** + * Headless queued-approval reconciliation. When a grant widens mid-run, the + * permission layer decides which already-queued requests it now covers and + * settles them without a prompt (see isRequestCoveredByGrant in gate.ts, + * which supplies the `covers` predicate this module drains against). A + * rendering surface only enqueues its pending requests and dispatches + * whatever settle calls come back — it never decides coverage itself. + */ + +import type { EventEmitter } from "node:events"; +import type { ApprovalOutcome, PermissionRequest } from "./types.js"; + +export type QueuedApprovalSummary = { + readonly id: number; + readonly tool: string; + readonly agentLabel?: string; +}; + +export type PermissionRequestQueue = { + /** Register a live request; the returned id is what settle/reconcile key on. */ + enqueue: (request: PermissionRequest, resolve: (outcome: ApprovalOutcome) => void) => number; + /** Settle one entry (accept, deny, timeout, or abort). False once already settled. */ + settle: (id: number, outcome: ApprovalOutcome) => boolean; + /** One line per still-queued request, for a queue-depth indicator. */ + list: () => readonly QueuedApprovalSummary[]; + /** + * Auto-settle every queued request a newly-minted grant now covers, without + * a prompt. Runs against a snapshot so settling mid-loop never skips or + * double-visits an entry. Returns the ids settled. + */ + reconcile: (covers: (request: PermissionRequest) => boolean) => readonly number[]; + /** Deny and remove everything still queued (session teardown) so no awaited resolve is left hanging. */ + drain: () => void; + size: () => number; +}; + +export function createPermissionRequestQueue(): PermissionRequestQueue { + const entries = new Map< + number, + { request: PermissionRequest; resolve: (outcome: ApprovalOutcome) => void } + >(); + let nextId = 1; + + const settle = (id: number, outcome: ApprovalOutcome): boolean => { + const entry = entries.get(id); + if (entry === undefined) return false; + entries.delete(id); + entry.resolve(outcome); + return true; + }; + + return { + enqueue: (request, resolve) => { + const id = nextId++; + entries.set(id, { request, resolve }); + return id; + }, + settle, + list: () => + [...entries.entries()].map(([id, entry]) => ({ + id, + tool: entry.request.tool, + ...(entry.request.agentLabel !== undefined ? { agentLabel: entry.request.agentLabel } : {}), + })), + reconcile: (covers) => { + const coveredIds = [...entries.entries()] + .filter(([, entry]) => covers(entry.request)) + .map(([id]) => id); + return coveredIds.filter((id) => settle(id, { allow: true })); + }, + drain: () => { + for (const id of [...entries.keys()]) settle(id, { allow: false }); + }, + size: () => entries.size, + }; +} + +export type PermissionGrantEvent = { + readonly approval: { readonly tool: string; readonly pattern: string }; + readonly covers: (request: PermissionRequest) => boolean; +}; + +// `covers` is a function, which arktype cannot express in a schema, so this +// stays a plain runtime guard rather than the usual declarative boundary +// validator — the shape is still checked field by field. +function isPermissionGrantEvent(raw: unknown): raw is PermissionGrantEvent { + if (raw === null || typeof raw !== "object") return false; + const approval = (raw as Record).approval; + const covers = (raw as Record).covers; + if (approval === null || typeof approval !== "object") return false; + const a = approval as Record; + return typeof a.tool === "string" && typeof a.pattern === "string" && typeof covers === "function"; +} + +/** + * Drain `queue` of any request a grant now covers whenever `permission.grant` + * fires (see PermissionGateOptions.onGrant for where that event originates). + * Any approval surface — TUI or headless — gets reconciliation for free by + * enqueuing its pending requests into a PermissionRequestQueue and calling + * this once, instead of reimplementing the walk. + */ +export function wirePermissionGrantReconciliation( + emitter: EventEmitter, + queue: PermissionRequestQueue, +): () => void { + const onGrant = (payload: unknown): void => { + if (!isPermissionGrantEvent(payload)) return; + queue.reconcile(payload.covers); + }; + emitter.on("permission.grant", onGrant); + return () => emitter.off("permission.grant", onGrant); +} From 167a5bc159a9f1e042f9778a93b51c4deff8ae9a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 08:07:35 -0700 Subject: [PATCH 2/6] Render permission gates over the reconciliation queue gate-wire.ts previously tracked settlement with a local boolean and never reacted to a grant that widened mid-queue, so an operator saw a prompt reconciliation should have skipped. It now enqueues each request into the shared permission queue and dispatches whatever settle call comes back, whether that is an operator's own choice, a timeout, a budget abort, or a grant draining the entry silently. Disposal drains anything still queued so teardown can never leave an evaluate() call hanging. --- src/tui-opentui/gate-wire.ts | 94 +++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 28 deletions(-) diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index 99926f138..c2b548550 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -25,6 +25,10 @@ import type { OperatorGateEvent, PermissionGateEvent, } from "../tui/gate-events.js" +import { + createPermissionRequestQueue, + wirePermissionGrantReconciliation, +} from "../permission/queue.js" /** Stable sentinel ids for the always-present deny / once rows. */ export const PERMISSION_DENY_ID = "__deny__" as const @@ -293,6 +297,14 @@ export function wireGates( // nothing on screen to answer — so a gate that arrives while another overlay // is up waits here and opens as soon as the host frees up. const pending: Array<() => void> = [] + // Owns queued-approval reconciliation (see src/permission/queue.ts): this + // host only enqueues requests and renders whatever settle calls the queue + // hands back — it never decides which grant covers which request. + const permissionQueue = createPermissionRequestQueue() + const disposeReconciliation = wirePermissionGrantReconciliation( + emitter, + permissionQueue, + ) function openOrQueue(open: () => void): void { if (shell.overlayList !== null) { @@ -302,6 +314,11 @@ export function wireGates( open() } + function unqueue(open: () => void): void { + const idx = pending.indexOf(open) + if (idx >= 0) pending.splice(idx, 1) + } + const disposeClosed = onOverlayClosed(shell, () => { const next = pending.shift() if (next) next() @@ -317,9 +334,31 @@ export function wireGates( const collapsedAnything = formatCommandForApproval(ev.request.subject).payloadCount > 0 let expanded = false - let settled = false let isOpen = false + // The queue is the single settle guard: once an id is removed (accept, + // cancel, timeout, abort, or a reconciled grant), a later call is a + // no-op instead of double-resolving. Its resolve callback settles + // through the onceClosed-wrapped `resolve` (not ev.resolve directly) so + // hooks.onGateClosed still fires exactly once regardless of which path + // drained this entry. settle's own return value tells a call site + // whether it was the one that actually settled, which is also how + // recordDecision below is guarded against firing twice — closing the + // overlay from inside this callback re-invokes the overlay's own + // onCancel (see shell.ts's closeInsetOverlay), and that reentrant call + // must find the id already gone. + const settle = (outcome: ApprovalOutcome): boolean => + permissionQueue.settle(id, outcome) + const id = permissionQueue.enqueue(ev.request, (outcome) => { + clearTimers() + if (isOpen) { + closeInsetOverlay(shell) + } else { + unqueue(open) + } + resolve(outcome) + }) + const onToggleExpand = (): void => { expanded = !expanded setOverlayBody( @@ -350,25 +389,28 @@ export function wireGates( echoChoice: false, ...(collapsedAnything ? { onToggleExpand } : {}), onAccept: (sel: OverlaySelection) => { - if (settled) return - settled = true - clearTimers() + // The shell already closed this overlay (and may have opened the + // next queued one) before invoking onAccept — settle must not + // closeInsetOverlay a second time and tear down that next overlay. + isOpen = false const gateSelection = { index: sel.index, ...(sel.id !== undefined ? { id: sel.id } : {}), } - recordDecision(shell, ev.request, choices, gateSelection) - resolve(approvalOutcomeFromSelection(choices, gateSelection)) + if (settle(approvalOutcomeFromSelection(choices, gateSelection))) { + recordDecision(shell, ev.request, 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. + // an unresolved gate hangs the run until the process is killed. The + // shell has already closed the overlay by the time onCancel runs, for + // the same reason noted in onAccept above. onCancel: () => { - if (settled) return - settled = true - clearTimers() + isOpen = false const gateSelection = { index: 0, id: PERMISSION_DENY_ID } - recordDecision(shell, ev.request, choices, gateSelection) - resolve(approvalOutcomeFromSelection(choices, gateSelection)) + if (settle(approvalOutcomeFromSelection(choices, gateSelection))) { + recordDecision(shell, ev.request, choices, gateSelection) + } }, }) } @@ -376,29 +418,21 @@ export function wireGates( // 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. + // 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. 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() - recordDecision(shell, ev.request, choices, { - index: 0, - id: PERMISSION_DENY_ID, - }) - if (isOpen) { - closeInsetOverlay(shell) - } else { - const idx = pending.indexOf(open) - if (idx >= 0) pending.splice(idx, 1) + if (settle({ allow: false, message })) { + recordDecision(shell, ev.request, choices, { + index: 0, + id: PERMISSION_DENY_ID, + }) } - resolve({ allow: false, message }) } function onAbort(): void { autoDeny("tool no longer running; permission request denied") @@ -470,7 +504,11 @@ export function wireGates( return () => { emitter.off("permission.gate", onPermission) emitter.off("operator.gate", onOperator) + disposeReconciliation() disposeClosed() pending.length = 0 + // Deny anything still queued so its awaited evaluate() call never hangs + // past session teardown. + permissionQueue.drain() } } From 690082b1e6eb76ed260c7dc888472269b210abdf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 7 Aug 2026 08:32:09 -0700 Subject: [PATCH 3/6] Verify overlay identity by generation instead of a flag The prior settle path tracked "is my overlay open" with a boolean the caller had to remember to clear at each settle site, which depended on knowing that the shell closes an accepted or cancelled overlay (and may open the next queued gate) before invoking that callback. A generation counter bumped at every open on the shared host makes that comparison explicit instead of assumed: a settle path checks whether its own captured generation still matches the current one before closing anything, so it can never tear down an overlay opened after its own. Closing an overlay from inside a settle path (autoDeny, or a grant-driven reconcile) re-invokes that overlay's own onCancel, since shell.ts's closeInsetOverlay calls onCancel after notifying close listeners. Recording the decision unconditionally at each call site meant that reentrant invocation wrote a second transcript row for the same request. permissionQueue.settle already reports whether an id was still live to settle, so gating the transcript write on that return value makes the reentrant call a no-op instead of a duplicate. --- src/tui-opentui/gate-wire.ts | 54 ++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index c2b548550..b8a415dc0 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -305,13 +305,29 @@ export function wireGates( emitter, permissionQueue, ) + // 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 + // accepted/cancelled overlay and may open the next queued gate before that + // gate's own settle callback runs — and closing blind would tear down that + // newer overlay instead of its own. Comparing the generation captured at + // open-time against the current one answers that directly, so correctness + // never rests on remembering shell.ts's close-before-callback ordering at + // each call site. openHost is the only place an overlay opens, so it is + // the only place this counter needs to change. + let overlayGeneration = 0 + + function openHost(open: () => void): void { + overlayGeneration++ + open() + } function openOrQueue(open: () => void): void { if (shell.overlayList !== null) { pending.push(open) return } - open() + openHost(open) } function unqueue(open: () => void): void { @@ -321,7 +337,7 @@ export function wireGates( const disposeClosed = onOverlayClosed(shell, () => { const next = pending.shift() - if (next) next() + if (next) openHost(next) }) function onPermission(ev: PermissionGateEvent): void { @@ -334,27 +350,30 @@ export function wireGates( const collapsedAnything = formatCommandForApproval(ev.request.subject).payloadCount > 0 let expanded = false - let isOpen = false + // Set only while this gate's own overlay is the one on screen — see + // overlayGeneration above for why the settle path checks it against the + // current generation instead of trusting this alone. + let openedGeneration: number | undefined // The queue is the single settle guard: once an id is removed (accept, // cancel, timeout, abort, or a reconciled grant), a later call is a // no-op instead of double-resolving. Its resolve callback settles // through the onceClosed-wrapped `resolve` (not ev.resolve directly) so // hooks.onGateClosed still fires exactly once regardless of which path - // drained this entry. settle's own return value tells a call site - // whether it was the one that actually settled, which is also how - // recordDecision below is guarded against firing twice — closing the - // overlay from inside this callback re-invokes the overlay's own - // onCancel (see shell.ts's closeInsetOverlay), and that reentrant call - // must find the id already gone. + // drained this entry. Closing the overlay from inside this callback + // re-invokes the overlay's own onCancel (see shell.ts's + // closeInsetOverlay, which fires onCancel after notifying close + // listeners) — settle's return value is how a call site tells that + // reentrant call apart from the original one, so recordDecision below + // fires exactly once per gate instead of once per reentry. const settle = (outcome: ApprovalOutcome): boolean => permissionQueue.settle(id, outcome) const id = permissionQueue.enqueue(ev.request, (outcome) => { clearTimers() - if (isOpen) { - closeInsetOverlay(shell) - } else { + if (openedGeneration === undefined) { unqueue(open) + } else if (openedGeneration === overlayGeneration) { + closeInsetOverlay(shell) } resolve(outcome) }) @@ -378,7 +397,7 @@ export function wireGates( } const open = (): void => { - isOpen = true + openedGeneration = overlayGeneration openPermissionsOverlay(shell, { items: choices.items, itemIds: choices.itemIds, @@ -389,10 +408,6 @@ export function wireGates( echoChoice: false, ...(collapsedAnything ? { onToggleExpand } : {}), onAccept: (sel: OverlaySelection) => { - // The shell already closed this overlay (and may have opened the - // next queued one) before invoking onAccept — settle must not - // closeInsetOverlay a second time and tear down that next overlay. - isOpen = false const gateSelection = { index: sel.index, ...(sel.id !== undefined ? { id: sel.id } : {}), @@ -402,11 +417,8 @@ export function wireGates( } }, // Esc must settle the awaited promise (as a deny), not abandon it — - // an unresolved gate hangs the run until the process is killed. The - // shell has already closed the overlay by the time onCancel runs, for - // the same reason noted in onAccept above. + // an unresolved gate hangs the run until the process is killed. onCancel: () => { - isOpen = false const gateSelection = { index: 0, id: PERMISSION_DENY_ID } if (settle(approvalOutcomeFromSelection(choices, gateSelection))) { recordDecision(shell, ev.request, choices, gateSelection) From d9e615bad51138318540085a138642e89dcea6d2 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 09:55:27 -0700 Subject: [PATCH 4/6] Test that a raced timeout and abort settle once and record once Reconciliation (queue.settle) and transcript recording (recordDecision) are two independent single-fire guards layered on the same terminal paths. Nothing exercised them together: a timeout and an abort racing the same request, or a timeout firing on a request that never opened an overlay. --- src/tui-opentui/gate-wire.test.ts | 73 +++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 8261b3d85..f1ef73369 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -599,6 +599,79 @@ describe("each gate decision appends exactly one transcript row", () => { } }) }) + + // The queue (settle-once guard) and the transcript recorder (record-once + // per decision) are two independent mechanisms layered on the same set of + // terminal paths. Racing a timeout against an abort on the same request + // exercises both at once: clearTimers must retire the loser before it can + // run autoDeny a second time, so ev.resolve fires exactly once and exactly + // one row lands, no matter which trigger wins. + test("a timeout and an abort racing the same request settle once and record once", 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 resolveCount = 0 + try { + wireGates(emitter, shell) + const before = shell.streamLog.length + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: () => { + resolveCount += 1 + }, + timeoutMs: 5, + signal: controller.signal, + }) + + await new Promise((r) => setTimeout(r, 20)) + // The timeout already fired and cleared the abort listener — this + // must be a no-op, not a second settle. + controller.abort() + + expect(resolveCount).toBe(1) + expect(shell.streamLog.length - before).toBe(1) + } finally { + shell.dispose() + } + }) + }) + + test("a queued gate's timeout settles once and records once without ever opening", 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("permission.gate", { + request: baseRequest(), + resolve: () => {}, + }) + const before = shell.streamLog.length + emitter.emit("permission.gate", { + request: baseRequest({ tool: "queued_tool" }), + resolve: () => { + resolveCount += 1 + }, + timeoutMs: 5, + }) + + await new Promise((r) => setTimeout(r, 20)) + + expect(resolveCount).toBe(1) + expect(shell.streamLog.length - before).toBe(1) + } finally { + shell.dispose() + } + }) + }) }) describe("permission.gate auto-deny", () => { From a15c0131cd85b0f6d9cb5ffae95fcc07a9560a42 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 10:11:53 -0700 Subject: [PATCH 5/6] Record a transcript row when a grant silently drains a queued request Every other terminal path (accept, Esc, timeout, abort) writes its own row at its own call site. reconcile() settles a queue entry directly, with no such call site, so a request that ran without ever being shown to the operator left no trace beyond the transient grant-minted flash. That is a gap on the highest-consequence path in the queue: the one where a request is approved without asking. --- src/tui-opentui/gate-wire.test.ts | 77 +++++++++++++++++++++++++++++++ src/tui-opentui/gate-wire.ts | 34 ++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index f1ef73369..070141cc7 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -672,6 +672,83 @@ describe("each gate decision appends exactly one transcript row", () => { } }) }) + + // reconcile() (src/permission/queue.ts) settles a queued request directly + // when a grant covers it, with no accept/cancel/autoDeny callback of its + // own to hang a row on — this is the one terminal path that has no natural + // call site, so it needs its own coverage. + test("a grant draining a queued request without ever displaying it", 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 resolved: unknown + try { + wireGates(emitter, shell) + // Occupies the overlay host so the second request queues instead of + // opening — the drain below must resolve it without ever opening it. + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: () => {}, + }) + const before = shell.streamLog.length + emitter.emit("permission.gate", { + request: baseRequest({ tool: "queued_tool" }), + resolve: (outcome: unknown) => { + resolveCount += 1 + resolved = outcome + }, + }) + + emitter.emit("permission.grant", { + approval: { tool: "queued_tool", pattern: "bun test" }, + covers: (r: { tool: string }) => r.tool === "queued_tool", + }) + + expect(resolveCount).toBe(1) + expect(resolved).toEqual({ allow: true }) + expect(shell.streamLog.length - before).toBe(1) + } finally { + shell.dispose() + } + }) + }) + + test("a grant draining the currently displayed request closes it and records once", 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) + const before = shell.streamLog.length + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: () => { + resolveCount += 1 + }, + }) + expect(shell.overlayKind).toBe("permissions") + + emitter.emit("permission.grant", { + approval: { tool: "run_shell", pattern: "bun test" }, + covers: () => true, + }) + + expect(resolveCount).toBe(1) + expect(shell.overlayList).toBeNull() + expect(shell.streamLog.length - before).toBe(1) + } finally { + shell.dispose() + } + }) + }) }) describe("permission.gate auto-deny", () => { diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index b8a415dc0..3af33fa64 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -231,6 +231,24 @@ function recordDecision( appendStreamRow(shell, { role: "system", text, meta: "permission" }) } +/** + * Write a row for a request a newly-minted grant drained without a prompt. + * Every other terminal path (accept, Esc, timeout, abort) writes its own row + * at its own call site; this one covers the path that has none — reconcile() + * settles the queue entry directly, with no accept/cancel/autoDeny callback + * to hang a record onto. Without this, the operator's only trace of the + * highest-consequence event in the queue (a request that ran without being + * shown) is the transient grant-recorded flash, gone once it scrolls off. + */ +function recordGrantDrain(shell: AppShell, request: PermissionRequest): void { + const body = middleEllipsis(permissionBodyFromRequest(request), 500) + appendStreamRow(shell, { + role: "system", + text: `${body}\n→ Auto-approved (already granted)`, + meta: "permission", + }) +} + /** * Write the operator's question and answer to the transcript, once decided. * Mirrors recordDecision: the overlay already shows this text while it is @@ -368,13 +386,26 @@ export function wireGates( // fires exactly once per gate instead of once per reentry. const settle = (outcome: ApprovalOutcome): boolean => permissionQueue.settle(id, outcome) + // Set immediately before every call to settle() from a known call site + // (accept, Esc, autoDeny), each of which writes its own row right after. + // reconcile() (src/permission/queue.ts) settles an entry directly, with + // no call site of its own — the resolve callback below falls back to + // recordGrantDrain whenever this is still false, so a request that ran + // without ever being shown still leaves a trace. + let recorded = false const id = permissionQueue.enqueue(ev.request, (outcome) => { clearTimers() + // Captured before closeInsetOverlay below, which — when this entry is + // the one on screen — reentrantly invokes this same overlay's onCancel + // (see the comment on `settle` above) and would otherwise set + // `recorded` out from under this check before it runs. + const needsGrantDrainRecord = !recorded if (openedGeneration === undefined) { unqueue(open) } else if (openedGeneration === overlayGeneration) { closeInsetOverlay(shell) } + if (needsGrantDrainRecord) recordGrantDrain(shell, ev.request) resolve(outcome) }) @@ -412,6 +443,7 @@ export function wireGates( index: sel.index, ...(sel.id !== undefined ? { id: sel.id } : {}), } + recorded = true if (settle(approvalOutcomeFromSelection(choices, gateSelection))) { recordDecision(shell, ev.request, choices, gateSelection) } @@ -420,6 +452,7 @@ export function wireGates( // an unresolved gate hangs the run until the process is killed. onCancel: () => { const gateSelection = { index: 0, id: PERMISSION_DENY_ID } + recorded = true if (settle(approvalOutcomeFromSelection(choices, gateSelection))) { recordDecision(shell, ev.request, choices, gateSelection) } @@ -439,6 +472,7 @@ export function wireGates( ev.signal?.removeEventListener("abort", onAbort) } const autoDeny = (message: string): void => { + recorded = true if (settle({ allow: false, message })) { recordDecision(shell, ev.request, choices, { index: 0, From 08c530d48f5dcb1378111639fd67665a0f194439 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 8 Aug 2026 10:22:02 -0700 Subject: [PATCH 6/6] Distinguish a teardown deny from a grant-driven approval in the record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recordGrantDrain hardcoded "Auto-approved (already granted)" for any request settled with no accept/cancel/autoDeny call site of its own — but drain() (session teardown) also settles that way, denying whatever is still queued. Every remaining request left the transcript mislabeled as approved on exit. The row now reads the settled outcome's allow flag to pick the right label. --- src/tui-opentui/gate-wire.test.ts | 56 +++++++++++++++++++++++++++++++ src/tui-opentui/gate-wire.ts | 40 +++++++++++++--------- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/src/tui-opentui/gate-wire.test.ts b/src/tui-opentui/gate-wire.test.ts index 070141cc7..575c20264 100644 --- a/src/tui-opentui/gate-wire.test.ts +++ b/src/tui-opentui/gate-wire.test.ts @@ -711,6 +711,9 @@ describe("each gate decision appends exactly one transcript row", () => { expect(resolveCount).toBe(1) expect(resolved).toEqual({ allow: true }) expect(shell.streamLog.length - before).toBe(1) + expect(shell.streamLog.at(-1)?.text).toContain( + "Auto-approved (already granted)", + ) } finally { shell.dispose() } @@ -744,11 +747,64 @@ describe("each gate decision appends exactly one transcript row", () => { expect(resolveCount).toBe(1) expect(shell.overlayList).toBeNull() expect(shell.streamLog.length - before).toBe(1) + expect(shell.streamLog.at(-1)?.text).toContain( + "Auto-approved (already granted)", + ) } finally { shell.dispose() } }) }) + + // drain() (src/permission/queue.ts) denies whatever is still queued on + // teardown — the same no-call-site path as a grant drain, but the + // opposite outcome. Mislabeling this "Auto-approved" would tell the + // operator a request ran when it was actually dropped unanswered. + test("disposing with a request still queued records it as denied, not approved", async () => { + await withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, { + terminal: { columns: 80, rows: 24 }, + run: "idle", + }) + const emitter = new EventEmitter() + // The currently-open request has no accept/cancel/autoDeny call site + // triggered before teardown either, so dispose must record it too — + // both entries go through the same no-call-site fallback as the + // queued one. + let openResolveCount = 0 + let queuedResolveCount = 0 + let queuedResolved: unknown + const dispose = wireGates(emitter, shell) + emitter.emit("permission.gate", { + request: baseRequest(), + resolve: () => { + openResolveCount += 1 + }, + }) + // Occupies the overlay host so this second request queues instead of + // opening — dispose must deny it without ever displaying it. + const before = shell.streamLog.length + emitter.emit("permission.gate", { + request: baseRequest({ tool: "queued_tool" }), + resolve: (outcome: unknown) => { + queuedResolveCount += 1 + queuedResolved = outcome + }, + }) + + dispose() + + expect(openResolveCount).toBe(1) + expect(queuedResolveCount).toBe(1) + expect(queuedResolved).toEqual({ allow: false }) + expect(shell.streamLog.length - before).toBe(2) + for (const row of shell.streamLog.slice(-2)) { + expect(row.text).toContain("Denied (session ended)") + expect(row.text).not.toContain("Auto-approved") + } + shell.dispose() + }) + }) }) describe("permission.gate auto-deny", () => { diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index 3af33fa64..253a30669 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -232,19 +232,28 @@ function recordDecision( } /** - * Write a row for a request a newly-minted grant drained without a prompt. - * Every other terminal path (accept, Esc, timeout, abort) writes its own row - * at its own call site; this one covers the path that has none — reconcile() - * settles the queue entry directly, with no accept/cancel/autoDeny callback - * to hang a record onto. Without this, the operator's only trace of the - * highest-consequence event in the queue (a request that ran without being - * shown) is the transient grant-recorded flash, gone once it scrolls off. + * Write a row for a request settled with no accept/cancel/autoDeny call site + * of its own to hang a record onto: reconcile() (a newly-minted grant + * covering this queued request) and drain() (session teardown denying + * whatever is still queued) both settle the queue entry directly. Every + * other terminal path (accept, Esc, timeout, abort) already writes its own + * row at its own call site. Without this, the operator's only trace of the + * highest-consequence event in the queue — a request that ran, or was + * dropped, without ever being shown — is the transient grant-recorded flash + * (nothing at all for teardown), gone once it scrolls off. */ -function recordGrantDrain(shell: AppShell, request: PermissionRequest): void { +function recordSilentSettle( + shell: AppShell, + request: PermissionRequest, + outcome: ApprovalOutcome, +): void { const body = middleEllipsis(permissionBodyFromRequest(request), 500) + const label = outcome.allow + ? "Auto-approved (already granted)" + : "Denied (session ended)" appendStreamRow(shell, { role: "system", - text: `${body}\n→ Auto-approved (already granted)`, + text: `${body}\n→ ${label}`, meta: "permission", }) } @@ -388,10 +397,11 @@ export function wireGates( permissionQueue.settle(id, outcome) // Set immediately before every call to settle() from a known call site // (accept, Esc, autoDeny), each of which writes its own row right after. - // reconcile() (src/permission/queue.ts) settles an entry directly, with - // no call site of its own — the resolve callback below falls back to - // recordGrantDrain whenever this is still false, so a request that ran - // without ever being shown still leaves a trace. + // reconcile() and drain() (src/permission/queue.ts) both settle an entry + // directly, with no call site of their own — the resolve callback below + // falls back to recordSilentSettle whenever this is still false, so a + // request that ran, or was dropped, without ever being shown still + // leaves a trace. let recorded = false const id = permissionQueue.enqueue(ev.request, (outcome) => { clearTimers() @@ -399,13 +409,13 @@ export function wireGates( // the one on screen — reentrantly invokes this same overlay's onCancel // (see the comment on `settle` above) and would otherwise set // `recorded` out from under this check before it runs. - const needsGrantDrainRecord = !recorded + const needsSilentSettleRecord = !recorded if (openedGeneration === undefined) { unqueue(open) } else if (openedGeneration === overlayGeneration) { closeInsetOverlay(shell) } - if (needsGrantDrainRecord) recordGrantDrain(shell, ev.request) + if (needsSilentSettleRecord) recordSilentSettle(shell, ev.request, outcome) resolve(outcome) })