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

Sources/CodeIsland/AppState.swift

@@ -1287,9 +1287,6 @@ final class AppState {
             return
         }
 
-        // New incoming permission request means session needs user decision again.
-        dismissedPermissionSessionIds.remove(sessionId)
-
         // Clear any pending questions for THIS session (mutually exclusive within a session)
         drainQuestions(forSession: sessionId, reason: "newPermissionRequest")
 
@@ -1310,16 +1307,64 @@ final class AppState {
             return
         }
 
+        // A genuinely new request means this session needs a user decision again,
+        // so it stops being dismissed. This must come AFTER the replay-dedup
+        // return above: a replay is the same decision arriving twice, not a new
+        // one, and un-dismissing on a replay resurrects the request the user
+        // hid — which then takes the card the arriving session should have got
+        // and, counting as a burst already in progress, silences its sound too.
+        // (A same-id request with different tool inputs is a distinct request,
+        // not a replay — merge returns false for those, so they still land here.)
+        dismissedPermissionSessionIds.remove(sessionId)
+
+        // Dismissing hides a request but deliberately leaves it queued, so the
+        // CLI stays blocked and the prompt stays recoverable. Gating on
+        // `permissionQueue.count == 1` therefore swallowed every later request —
+        // from any session — for as long as a dismissed one sat in the queue.
+        //
+        // The gate's real question is "is an approval card on screen", so ask
+        // the surface. Queue-derived proxies do not survive the un-dismiss
+        // above: a session's own next request clears its dismissal, which makes
+        // its still-queued earlier request count as visible again while nothing
+        // is displayed — silencing every later request all over again. (#309)
+        //
+        // ponytail: a card suppressed by Smart Suppress also leaves a visible
+        // request undisplayed, so a second session's request still waits behind
+        // it. That is pre-existing (`main` behaves the same) and needs
+        // showNextPending to skip un-openable entries; tracked separately.
+        //
+        // The surface alone is not enough either: `drainPermissions` empties one
+        // SESSION's requests without clearing `surface`, so a card can be left
+        // pointing at a session that has nothing queued. Ask per session, not
+        // per queue — a whole-queue test (`!permissionQueue.isEmpty`) reads as
+        // "a card is up" whenever some other session is still waiting, which
+        // leaves the panel showing a card for a session with no pending request
+        // while later requests queue silently behind it.
+        let approvalCardOnScreen: Bool
+        if case .approvalCard(let shownSessionId) = surface,
+           permissionQueue.contains(where: { ($0.event.sessionId ?? "default") == shownSessionId }) {
+            approvalCardOnScreen = true
+        } else {
+            approvalCardOnScreen = false
+        }
+
+        // Card and sound answer different questions and must not share a gate.
+        // The sound marks the start of a burst of approvals, which is what
+        // `count == 1` used to approximate; within a burst it stays quiet, and
+        // a dismissed request sitting in the queue must not count as a burst
+        // already in progress.
+        let burstAlreadyInProgress = nextVisiblePermissionIndex() != nil
         permissionQueue.append(request)
 
-        // Show UI only if this is the first (or only) queued item
-        if permissionQueue.count == 1 {
-            activeSessionId = sessionId
-            // If user is already browsing the session list, keep them there and
-            // let inline controls handle approval without stealing focus.
-            if surface != .sessionList, shouldAutoOpenPendingSurface(for: sessionId) {
-                surface = .approvalCard(sessionId: sessionId)
-            }
+        // Show UI only when no approval card is already up to be stolen from.
+        // showNextPending picks the first *visible* request, promotes it to the
+        // head and applies the session-list / Smart Suppress rules — pointing
+        // the card at this session by hand would show the dismissed request's
+        // content whenever a dismissed entry still leads the queue.
+        if !approvalCardOnScreen {
+            showNextPending()
+        }
+        if !burstAlreadyInProgress {
             SoundManager.shared.handleEvent("PermissionRequest")
         }
         refreshDerivedState()

Tests/CodeIslandTests/AppStatePermissionFlowTests.swift

@@ -532,6 +532,203 @@ final class AppStatePermissionFlowTests: XCTestCase {
         try String(contentsOf: codeIslandRulesPath(in: codexHome), encoding: .utf8)
     }
 
+    /// #309 — a dismissed request stays queued so the CLI stays blocked, which
+    /// used to make `permissionQueue.count == 1` false forever and swallow every
+    /// later request, from every session, with no card and no sound.
+    func testDismissedPermissionDoesNotSilenceALaterSessionsRequest() async throws {
+        let appState = AppState()
+        let dismissed = try makePermissionRequestEvent(sessionId: "s-dismissed", toolName: "Bash")
+        let later = try makePermissionRequestEvent(sessionId: "s-later", toolName: "Edit")
+
+        let dismissedTask = Task<Data, Never> {
+            await withCheckedContinuation { continuation in
+                appState.handlePermissionRequest(dismissed, continuation: continuation)
+            }
+        }
+        await Task.yield()
+        XCTAssertEqual(appState.surface, .approvalCard(sessionId: "s-dismissed"))
+
+        appState.dismissPermissionPrompt()
+        XCTAssertEqual(appState.surface, .collapsed)
+        XCTAssertEqual(appState.permissionQueue.count, 1, "dismiss must keep the request queued")
+
+        let laterTask = Task<Data, Never> {
+            await withCheckedContinuation { continuation in
+                appState.handlePermissionRequest(later, continuation: continuation)
+            }
+        }
+        await Task.yield()
+
+        XCTAssertEqual(
+            appState.surface,
+            .approvalCard(sessionId: "s-later"),
+            "a different session's approval must still raise a card while a dismissed one sits in the queue"
+        )
+        // Stop here on failure: under the bug, approvePermission() below resolves
+        // the dismissed request instead, so `await laterTask.value` would hang
+        // and the test would report as a timeout rather than by name. The head
+        // check matters as much as the surface one — an implementation that
+        // points the card at this session by hand shows "s-later" while the
+        // dismissed request still leads the queue, so the surface assertion
+        // alone passes and the await still hangs.
+        XCTAssertEqual(
+            appState.permissionQueue.first?.event.sessionId,
+            "s-later",
+            "the card on screen must be backed by the head of the queue, which is what approve resolves"
+        )
+        guard appState.surface == .approvalCard(sessionId: "s-later"),
+              appState.permissionQueue.first?.event.sessionId == "s-later" else {
+            appState.handlePeerDisconnect(sessionId: "s-dismissed")
+            appState.handlePeerDisconnect(sessionId: "s-later")
+            _ = await dismissedTask.value
+            _ = await laterTask.value
+            return
+        }
+
+        appState.approvePermission()
+        let laterResponse = await laterTask.value
+        XCTAssertEqual(try extractPermissionBehavior(from: laterResponse), "allow")
+
+        await assertTaskNotResolved(dismissedTask)
+        appState.handlePeerDisconnect(sessionId: "s-dismissed")
+        _ = await dismissedTask.value
+    }
+
+    /// A dismissal is cleared by that session's NEXT request arriving
+    /// (`handlePermissionRequest` removes it from `dismissedPermissionSessionIds`
+    /// on entry — "session needs user decision again"), not by the dismissed
+    /// request resolving. So the session's next request must bring its card back.
+    ///
+    /// This is also the state that silenced everything else: the un-dismiss makes
+    /// the still-queued earlier request count as visible while nothing is on
+    /// screen, so a queue-derived "is a card showing" proxy reads true forever.
+    func testDismissedSessionsNextRequestReRaisesItsCard() async throws {
+        let appState = AppState()
+        let first = try makePermissionRequestEvent(sessionId: "s-same", toolName: "Bash")
+        let second = try makePermissionRequestEvent(sessionId: "s-same", toolName: "Edit")
+
+        let firstTask = Task<Data, Never> {
+            await withCheckedContinuation { continuation in
+                appState.handlePermissionRequest(first, continuation: continuation)
+            }
+        }
+        await Task.yield()
+        appState.dismissPermissionPrompt()
+        XCTAssertEqual(appState.surface, .collapsed)
+
+        let secondTask = Task<Data, Never> {
+            await withCheckedContinuation { continuation in
+                appState.handlePermissionRequest(second, continuation: continuation)
+            }
+        }
+        await Task.yield()
+
+        XCTAssertEqual(
+            appState.surface,
+            .approvalCard(sessionId: "s-same"),
+            "the session un-dismissed itself by asking again, so its card must come back"
+        )
+        XCTAssertEqual(appState.permissionQueue.count, 2)
+        // Pins queue order, not the promotion mechanism: the card renders the
+        // head, so the user is asked about the earlier request rather than the
+        // one that just arrived. Both requests here are from one session, so no
+        // assertion at this level can tell showNextPending's promotion from a
+        // hand-pointed surface — the cross-session test's head check is what
+        // covers that.
+        XCTAssertEqual(
+            appState.pendingPermission?.event.toolName,
+            "Bash",
+            "the card must show the earlier queued request, not the one that just arrived"
+        )
+
+        appState.handlePeerDisconnect(sessionId: "s-same")
+        _ = await firstTask.value
+        _ = await secondTask.value
+    }
+
+    /// `drainPermissions` (process exit, or a question arriving for the session)
+    /// empties the queue without clearing `surface`, and the card renders nothing
+    /// when there is no head request. A gate that trusts `.approvalCard` alone
+    /// would block on that phantom card and swallow the next request.
+    func testRequestArrivingUnderAStaleApprovalSurfaceStillRaisesACard() async throws {
+        let appState = AppState()
+
+        // The phantom state itself: an .approvalCard surface with an empty queue.
+        // Production reaches it through the drainPermissions callers that do not
+        // touch `surface` (process exit; a question arriving for the session).
+        // Set here directly because those callers are private.
+        appState.surface = .approvalCard(sessionId: "s-gone")
+        XCTAssertTrue(appState.permissionQueue.isEmpty)
+
+        let next = try makePermissionRequestEvent(sessionId: "s-next", toolName: "Read")
+        let nextTask = Task<Data, Never> {
+            await withCheckedContinuation { appState.handlePermissionRequest(next, continuation: $0) }
+        }
+        await Task.yield()
+
+        XCTAssertEqual(
+            appState.surface,
+            .approvalCard(sessionId: "s-next"),
+            "a phantom card must not block the next request from being shown"
+        )
+
+        appState.approvePermission()
+        let response = await nextTask.value
+        XCTAssertEqual(try extractPermissionBehavior(from: response), "allow")
+    }
+
+    /// The state F1 described: a dismissed session asking again must not leave
+    /// the panel silent for everyone else.
+    func testDismissedSessionAskingAgainDoesNotSilenceOtherSessions() async throws {
+        let appState = AppState()
+        let firstA = try makePermissionRequestEvent(sessionId: "s-a", toolName: "Bash")
+        let secondA = try makePermissionRequestEvent(sessionId: "s-a", toolName: "Edit")
+        let fromB = try makePermissionRequestEvent(sessionId: "s-b", toolName: "Read")
+
+        let firstATask = Task<Data, Never> {
+            await withCheckedContinuation { appState.handlePermissionRequest(firstA, continuation: $0) }
+        }
+        await Task.yield()
+        appState.dismissPermissionPrompt()
+
+        let secondATask = Task<Data, Never> {
+            await withCheckedContinuation { appState.handlePermissionRequest(secondA, continuation: $0) }
+        }
+        await Task.yield()
+
+        let fromBTask = Task<Data, Never> {
+            await withCheckedContinuation { appState.handlePermissionRequest(fromB, continuation: $0) }
+        }
+        await Task.yield()
+
+        // The panel must be showing *something* — under the incomplete gate the
+        // un-dismiss left A's card unopened and B arrived to a silent, collapsed
+        // panel. (Without this the rest of the test passes either way, because
+        // resolving A's requests surfaces B regardless.)
+        XCTAssertEqual(
+            appState.surface,
+            .approvalCard(sessionId: "s-a"),
+            "A's card must be up — a silent collapsed panel is the bug, and it must be A's card since A's request leads the queue"
+        )
+
+        // B queues behind A's card rather than stealing it — but it must not be
+        // lost: as A's requests clear, B's card has to come up.
+        XCTAssertEqual(appState.permissionQueue.count, 3)
+        appState.approvePermission()
+        appState.approvePermission()
+        _ = await firstATask.value
+        _ = await secondATask.value
+
+        XCTAssertEqual(
+            appState.surface,
+            .approvalCard(sessionId: "s-b"),
+            "B's request must surface once A's are resolved, not sit silently forever"
+        )
+        appState.approvePermission()
+        let bResponse = await fromBTask.value
+        XCTAssertEqual(try extractPermissionBehavior(from: bResponse), "allow")
+    }
+
     private func makePermissionRequestEvent(
         sessionId: String,
         toolName: String,