fix(panel): don't let a dismissed approval silence later requests (#309)

[halindrome]

Fixes #309.

Cause

Dismissing an approval hides it but deliberately leaves it queued, so the CLI stays blocked and the prompt stays recoverable. The card-and-sound trigger in handlePermissionRequest was gated on the queue having been empty:

permissionQueue.append(request)
if permissionQueue.count == 1 { ...show card...; SoundManager.shared.handleEvent("PermissionRequest") }

A dismissed-but-queued entry keeps that count at ≥ 1 forever, so every later request — from any session — landed on count >= 2 and showed nothing, silently, until the dismissed one was resolved some other way.

Queue size was standing in for "is a card already showing". Dismissal is exactly the state where those two stop agreeing.

Change

Gate on the same predicate the display path already uses — nextVisiblePermissionIndex(), which skips dismissed sessions — evaluated before the append.

Showing the card through showNextPending() rather than pointing the surface at this session by hand matters: the approval card renders the head of the queue, so with a dismissed entry still leading it, a hand-set surface would show the dismissed request's content under the new session's name. showNextPending() promotes the first visible request and applies the session-list and Smart Suppress rules that were previously duplicated here.

Deliberately unchanged

The dismissed session's own next request stays hidden; dismissal is per-session and clears when that session's request resolves. That is existing behaviour, and it is now pinned by a test so this fix is not read as altering it. If it should instead re-show, that is a separate call worth its own change.

Tests

Two added to AppStatePermissionFlowTests: a later session's request still raises a card while a dismissed one sits in the queue, and the same-session case stays hidden. The regression test was confirmed to fail against the old gate, and it stops on that failure rather than continuing into an await that would hang.

swift test: 689 tests, 2 skipped, 0 failures.

Branches from main @ 9e3a1eb and is independent of #310.

[halindrome]

Heads-up on the history of this branch, in case you looked at it before now: the version I first pushed did not actually fix #309. It has since been corrected over three review rounds. Current head is c86fe89.

What was wrong with the first version

It gated on nextVisiblePermissionIndex() != nil — "a non-dismissed request is queued". That is still not "a card is on screen". handlePermissionRequest clears the session's dismissal on entry ("session needs user decision again"), so a dismissed session's next request un-dismisses it, its still-queued earlier request counts as visible again, nothing opens, and the silencing resumes exactly as reported. The bug moved one step; it did not go away.

Worse, the test I shipped alongside it asserted that broken state was correct: I had believed a dismissal was cleared when the dismissed request resolved, when in fact it is cleared when the session's next request arrives. That test is now inverted — a dismissed session's next request must bring its card back.

The second version gated on the surface alone, which had its own hole: drainPermissions (process exit, or a question arriving for that session) empties the queue without clearing surface, and the card renders nothing when there is no head request — so it blocked on a phantom card. The gate now requires both an .approvalCard surface and a non-empty queue.

Card and sound also no longer share a gate. They answer different questions — "is a card already up" versus "does this request start a new burst" — and folding them together silently changed the sound from once-per-burst to once-per-request whenever no card was open.

Each guard was verified by mutating the production condition it names and confirming the test fails without it.

Known behaviour change, flagged rather than buried

An arriving permission can now replace an open question card in states where main would have shown nothing at all. That is the same steal main already performs when the queue is empty, so it is consistent — but it is a real behaviour change in this PR, not a no-op.

One variant is genuinely surprising and I have left it unfixed deliberately: if an older permission is queued-but-undisplayed (Smart Suppress), the card that appears belongs to that older session rather than to the request that triggered it. It is self-healing and nothing is lost. The fix would be another condition on the card gate — which is the exact change shape that produced the defects in both earlier rounds — so I would rather it be a considered follow-up than a fourth patch to the same if.

Also unfixed, and filed separately

The new burst-sound predicate has no test coverage, because SoundManager.shared has no seam to assert against. Adding one is a larger change than this fix, so it is #312 rather than more unreviewed code here.

Happy to squash the four commits if you would prefer a single one.

[halindrome]

Updated to 2e497e6 with two more fixes. Both were found by reviewing this change merged with #310 (the #308 answer-routing fix) — but both are defects on this branch alone, so they belong here rather than in that merge.

1. The gate asked a whole-queue question about per-session state.

drainPermissions empties one session's requests without clearing surface — a question arriving for that session does exactly this. So a card can be left pointing at a session that has nothing queued while other sessions still wait. !permissionQueue.isEmpty reads that as "a card is up", and every later request then queues silently behind a card for a session with no pending request. The gate now asks whether that session still has a queued request.

2. A replayed tool_use_id un-dismissed the session.

dismissedPermissionSessionIds.remove(sessionId) ran before the replay-dedup early return, so replaying a request for a dismissed session resurrected the one the user had hidden — taking the card the arriving session should have received, and counting as a burst already in progress so that session lost its sound too. A replay is the same decision arriving twice, not a new one; the un-dismiss now happens only on the path that actually enqueues.

This second one is latent on main independently of this PR: a resurrected request leads nextVisiblePermissionIndex(), so any later showNextPending() can raise the card the user dismissed. It is fixed here because this is the PR that touches dismissal behaviour, but it is not caused by it.

A same-id request with differing tool inputs is a distinct request rather than a replay (#169), so it still clears the dismissal — that path is pinned by its own test.

AppStatePermissionGateTests covers all three cases. Both fixes were verified by mutating the production line each one names and confirming the test fails without it; 694 tests, 0 failures.

Note for reviewing order: #310 and this PR are independent and each stands alone, but the two interact once both are applied. I have a local merge of the pair that has been through two further review rounds — happy to share what that turned up if you plan to take both.