Skip to content

Commit 08c530d

Browse files
committed
Distinguish a teardown deny from a grant-driven approval in the record
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.
1 parent a15c013 commit 08c530d

2 files changed

Lines changed: 81 additions & 15 deletions

File tree

‎src/tui-opentui/gate-wire.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,9 @@ describe("each gate decision appends exactly one transcript row", () => {
711711
expect(resolveCount).toBe(1)
712712
expect(resolved).toEqual({ allow: true })
713713
expect(shell.streamLog.length - before).toBe(1)
714+
expect(shell.streamLog.at(-1)?.text).toContain(
715+
"Auto-approved (already granted)",
716+
)
714717
} finally {
715718
shell.dispose()
716719
}
@@ -744,11 +747,64 @@ describe("each gate decision appends exactly one transcript row", () => {
744747
expect(resolveCount).toBe(1)
745748
expect(shell.overlayList).toBeNull()
746749
expect(shell.streamLog.length - before).toBe(1)
750+
expect(shell.streamLog.at(-1)?.text).toContain(
751+
"Auto-approved (already granted)",
752+
)
747753
} finally {
748754
shell.dispose()
749755
}
750756
})
751757
})
758+
759+
// drain() (src/permission/queue.ts) denies whatever is still queued on
760+
// teardown — the same no-call-site path as a grant drain, but the
761+
// opposite outcome. Mislabeling this "Auto-approved" would tell the
762+
// operator a request ran when it was actually dropped unanswered.
763+
test("disposing with a request still queued records it as denied, not approved", async () => {
764+
await withTestRenderer(async (h) => {
765+
const shell = createAppShell(h.renderer, {
766+
terminal: { columns: 80, rows: 24 },
767+
run: "idle",
768+
})
769+
const emitter = new EventEmitter()
770+
// The currently-open request has no accept/cancel/autoDeny call site
771+
// triggered before teardown either, so dispose must record it too —
772+
// both entries go through the same no-call-site fallback as the
773+
// queued one.
774+
let openResolveCount = 0
775+
let queuedResolveCount = 0
776+
let queuedResolved: unknown
777+
const dispose = wireGates(emitter, shell)
778+
emitter.emit("permission.gate", {
779+
request: baseRequest(),
780+
resolve: () => {
781+
openResolveCount += 1
782+
},
783+
})
784+
// Occupies the overlay host so this second request queues instead of
785+
// opening — dispose must deny it without ever displaying it.
786+
const before = shell.streamLog.length
787+
emitter.emit("permission.gate", {
788+
request: baseRequest({ tool: "queued_tool" }),
789+
resolve: (outcome: unknown) => {
790+
queuedResolveCount += 1
791+
queuedResolved = outcome
792+
},
793+
})
794+
795+
dispose()
796+
797+
expect(openResolveCount).toBe(1)
798+
expect(queuedResolveCount).toBe(1)
799+
expect(queuedResolved).toEqual({ allow: false })
800+
expect(shell.streamLog.length - before).toBe(2)
801+
for (const row of shell.streamLog.slice(-2)) {
802+
expect(row.text).toContain("Denied (session ended)")
803+
expect(row.text).not.toContain("Auto-approved")
804+
}
805+
shell.dispose()
806+
})
807+
})
752808
})
753809

754810
describe("permission.gate auto-deny", () => {

‎src/tui-opentui/gate-wire.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -232,19 +232,28 @@ function recordDecision(
232232
}
233233

234234
/**
235-
* Write a row for a request a newly-minted grant drained without a prompt.
236-
* Every other terminal path (accept, Esc, timeout, abort) writes its own row
237-
* at its own call site; this one covers the path that has none — reconcile()
238-
* settles the queue entry directly, with no accept/cancel/autoDeny callback
239-
* to hang a record onto. Without this, the operator's only trace of the
240-
* highest-consequence event in the queue (a request that ran without being
241-
* shown) is the transient grant-recorded flash, gone once it scrolls off.
235+
* Write a row for a request settled with no accept/cancel/autoDeny call site
236+
* of its own to hang a record onto: reconcile() (a newly-minted grant
237+
* covering this queued request) and drain() (session teardown denying
238+
* whatever is still queued) both settle the queue entry directly. Every
239+
* other terminal path (accept, Esc, timeout, abort) already writes its own
240+
* row at its own call site. Without this, the operator's only trace of the
241+
* highest-consequence event in the queue — a request that ran, or was
242+
* dropped, without ever being shown — is the transient grant-recorded flash
243+
* (nothing at all for teardown), gone once it scrolls off.
242244
*/
243-
function recordGrantDrain(shell: AppShell, request: PermissionRequest): void {
245+
function recordSilentSettle(
246+
shell: AppShell,
247+
request: PermissionRequest,
248+
outcome: ApprovalOutcome,
249+
): void {
244250
const body = middleEllipsis(permissionBodyFromRequest(request), 500)
251+
const label = outcome.allow
252+
? "Auto-approved (already granted)"
253+
: "Denied (session ended)"
245254
appendStreamRow(shell, {
246255
role: "system",
247-
text: `${body}\n→ Auto-approved (already granted)`,
256+
text: `${body}\n→ ${label}`,
248257
meta: "permission",
249258
})
250259
}
@@ -388,24 +397,25 @@ export function wireGates(
388397
permissionQueue.settle(id, outcome)
389398
// Set immediately before every call to settle() from a known call site
390399
// (accept, Esc, autoDeny), each of which writes its own row right after.
391-
// reconcile() (src/permission/queue.ts) settles an entry directly, with
392-
// no call site of its own — the resolve callback below falls back to
393-
// recordGrantDrain whenever this is still false, so a request that ran
394-
// without ever being shown still leaves a trace.
400+
// reconcile() and drain() (src/permission/queue.ts) both settle an entry
401+
// directly, with no call site of their own — the resolve callback below
402+
// falls back to recordSilentSettle whenever this is still false, so a
403+
// request that ran, or was dropped, without ever being shown still
404+
// leaves a trace.
395405
let recorded = false
396406
const id = permissionQueue.enqueue(ev.request, (outcome) => {
397407
clearTimers()
398408
// Captured before closeInsetOverlay below, which — when this entry is
399409
// the one on screen — reentrantly invokes this same overlay's onCancel
400410
// (see the comment on `settle` above) and would otherwise set
401411
// `recorded` out from under this check before it runs.
402-
const needsGrantDrainRecord = !recorded
412+
const needsSilentSettleRecord = !recorded
403413
if (openedGeneration === undefined) {
404414
unqueue(open)
405415
} else if (openedGeneration === overlayGeneration) {
406416
closeInsetOverlay(shell)
407417
}
408-
if (needsGrantDrainRecord) recordGrantDrain(shell, ev.request)
418+
if (needsSilentSettleRecord) recordSilentSettle(shell, ev.request, outcome)
409419
resolve(outcome)
410420
})
411421

0 commit comments

Comments
 (0)