Skip to content

Commit cf41cb8

Browse files
committed
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.
1 parent 56a5d0c commit cf41cb8

1 file changed

Lines changed: 66 additions & 28 deletions

File tree

src/tui-opentui/gate-wire.ts

Lines changed: 66 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import type {
2525
OperatorGateEvent,
2626
PermissionGateEvent,
2727
} from "../tui/gate-events.js"
28+
import {
29+
createPermissionRequestQueue,
30+
wirePermissionGrantReconciliation,
31+
} from "../permission/queue.js"
2832

2933
/** Stable sentinel ids for the always-present deny / once rows. */
3034
export const PERMISSION_DENY_ID = "__deny__" as const
@@ -293,6 +297,14 @@ export function wireGates(
293297
// nothing on screen to answer — so a gate that arrives while another overlay
294298
// is up waits here and opens as soon as the host frees up.
295299
const pending: Array<() => void> = []
300+
// Owns queued-approval reconciliation (see src/permission/queue.ts): this
301+
// host only enqueues requests and renders whatever settle calls the queue
302+
// hands back — it never decides which grant covers which request.
303+
const permissionQueue = createPermissionRequestQueue()
304+
const disposeReconciliation = wirePermissionGrantReconciliation(
305+
emitter,
306+
permissionQueue,
307+
)
296308

297309
function openOrQueue(open: () => void): void {
298310
if (shell.overlayList !== null) {
@@ -302,6 +314,11 @@ export function wireGates(
302314
open()
303315
}
304316

317+
function unqueue(open: () => void): void {
318+
const idx = pending.indexOf(open)
319+
if (idx >= 0) pending.splice(idx, 1)
320+
}
321+
305322
const disposeClosed = onOverlayClosed(shell, () => {
306323
const next = pending.shift()
307324
if (next) next()
@@ -317,9 +334,31 @@ export function wireGates(
317334
const collapsedAnything =
318335
formatCommandForApproval(ev.request.subject).payloadCount > 0
319336
let expanded = false
320-
let settled = false
321337
let isOpen = false
322338

339+
// The queue is the single settle guard: once an id is removed (accept,
340+
// cancel, timeout, abort, or a reconciled grant), a later call is a
341+
// no-op instead of double-resolving. Its resolve callback settles
342+
// through the onceClosed-wrapped `resolve` (not ev.resolve directly) so
343+
// hooks.onGateClosed still fires exactly once regardless of which path
344+
// drained this entry. settle's own return value tells a call site
345+
// whether it was the one that actually settled, which is also how
346+
// recordDecision below is guarded against firing twice — closing the
347+
// overlay from inside this callback re-invokes the overlay's own
348+
// onCancel (see shell.ts's closeInsetOverlay), and that reentrant call
349+
// must find the id already gone.
350+
const settle = (outcome: ApprovalOutcome): boolean =>
351+
permissionQueue.settle(id, outcome)
352+
const id = permissionQueue.enqueue(ev.request, (outcome) => {
353+
clearTimers()
354+
if (isOpen) {
355+
closeInsetOverlay(shell)
356+
} else {
357+
unqueue(open)
358+
}
359+
resolve(outcome)
360+
})
361+
323362
const onToggleExpand = (): void => {
324363
expanded = !expanded
325364
setOverlayBody(
@@ -350,55 +389,50 @@ export function wireGates(
350389
echoChoice: false,
351390
...(collapsedAnything ? { onToggleExpand } : {}),
352391
onAccept: (sel: OverlaySelection) => {
353-
if (settled) return
354-
settled = true
355-
clearTimers()
392+
// The shell already closed this overlay (and may have opened the
393+
// next queued one) before invoking onAccept — settle must not
394+
// closeInsetOverlay a second time and tear down that next overlay.
395+
isOpen = false
356396
const gateSelection = {
357397
index: sel.index,
358398
...(sel.id !== undefined ? { id: sel.id } : {}),
359399
}
360-
recordDecision(shell, ev.request, choices, gateSelection)
361-
resolve(approvalOutcomeFromSelection(choices, gateSelection))
400+
if (settle(approvalOutcomeFromSelection(choices, gateSelection))) {
401+
recordDecision(shell, ev.request, choices, gateSelection)
402+
}
362403
},
363404
// Esc must settle the awaited promise (as a deny), not abandon it —
364-
// an unresolved gate hangs the run until the process is killed.
405+
// an unresolved gate hangs the run until the process is killed. The
406+
// shell has already closed the overlay by the time onCancel runs, for
407+
// the same reason noted in onAccept above.
365408
onCancel: () => {
366-
if (settled) return
367-
settled = true
368-
clearTimers()
409+
isOpen = false
369410
const gateSelection = { index: 0, id: PERMISSION_DENY_ID }
370-
recordDecision(shell, ev.request, choices, gateSelection)
371-
resolve(approvalOutcomeFromSelection(choices, gateSelection))
411+
if (settle(approvalOutcomeFromSelection(choices, gateSelection))) {
412+
recordDecision(shell, ev.request, choices, gateSelection)
413+
}
372414
},
373415
})
374416
}
375417

376418
// Watchdog abort (tool budget expired / parent run cancelled) and the
377419
// goal-mode timeout both race an operator who may never answer — each
378420
// must resolve the gate itself rather than leave the overlay (or the
379-
// queued open) parked forever. autoDeny wins the race exactly once:
380-
// whichever fires first tears down the other and, if the overlay is
381-
// already on screen for this gate, closes it so nothing stale lingers.
421+
// queued open) parked forever. Whichever fires first settles the queue
422+
// entry, which is itself the single-resolve guard, so the other side is
423+
// simply a no-op once it runs.
382424
let timer: ReturnType<typeof setTimeout> | undefined
383425
const clearTimers = (): void => {
384426
if (timer !== undefined) clearTimeout(timer)
385427
ev.signal?.removeEventListener("abort", onAbort)
386428
}
387429
const autoDeny = (message: string): void => {
388-
if (settled) return
389-
settled = true
390-
clearTimers()
391-
recordDecision(shell, ev.request, choices, {
392-
index: 0,
393-
id: PERMISSION_DENY_ID,
394-
})
395-
if (isOpen) {
396-
closeInsetOverlay(shell)
397-
} else {
398-
const idx = pending.indexOf(open)
399-
if (idx >= 0) pending.splice(idx, 1)
430+
if (settle({ allow: false, message })) {
431+
recordDecision(shell, ev.request, choices, {
432+
index: 0,
433+
id: PERMISSION_DENY_ID,
434+
})
400435
}
401-
resolve({ allow: false, message })
402436
}
403437
function onAbort(): void {
404438
autoDeny("tool no longer running; permission request denied")
@@ -470,7 +504,11 @@ export function wireGates(
470504
return () => {
471505
emitter.off("permission.gate", onPermission)
472506
emitter.off("operator.gate", onOperator)
507+
disposeReconciliation()
473508
disposeClosed()
474509
pending.length = 0
510+
// Deny anything still queued so its awaited evaluate() call never hangs
511+
// past session teardown.
512+
permissionQueue.drain()
475513
}
476514
}

0 commit comments

Comments
 (0)