Skip to content

fix(panel): route answers to the card's session, not the queue head #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
halindrome wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(panel): route answers to the card's session, not the queue head #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f20530
fix(panel): route answers to the card's session, not the queue head
shanemccarron-maker Aug 12, 2026
eb5ef93
fix: address QA round 1
shanemccarron-maker Aug 12, 2026
c65b9d9
fix: address QA round 2
shanemccarron-maker Aug 12, 2026
f40cee5
test: strengthen the two assertions QA round 3 found weak
shanemccarron-maker Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Sources/CodeIsland/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,16 @@ class AppDelegate: NSObject, NSApplicationDelegate {
}
}
}
// Shortcuts act on the card currently on screen, so they target that
// card's session rather than the head of the queue. (#308)
case .approve:
appState.approvePermission()
appState.approvePermission(expectedSessionId: appState.surface.approvalSessionId)
case .approveAlways:
appState.approvePermission(always: true)
appState.approvePermission(always: true, expectedSessionId: appState.surface.approvalSessionId)
case .deny:
appState.denyPermission()
appState.denyPermission(expectedSessionId: appState.surface.approvalSessionId)
case .skipQuestion:
appState.skipQuestion()
appState.skipQuestion(expectedSessionId: appState.surface.questionSessionId)
case .jumpToTerminal:
if let id = appState.activeSessionId, let session = appState.sessions[id] {
TerminalActivator.activate(session: session, sessionId: id)
Expand Down
154 changes: 131 additions & 23 deletions Sources/CodeIsland/AppState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,27 @@ final class AppState {
var pendingPermission: PermissionRequest? { permissionQueue.first }
/// Computed: first item in question queue
var pendingQuestion: QuestionRequest? { questionQueue.first }

/// The queued request belonging to a specific session. A card is addressed
/// by session, so it must render (and resolve) that session's request
/// rather than whatever currently sits at the head of the queue. (#308)
func pendingPermission(forSession sessionId: String) -> PermissionRequest? {
permissionQueue.first { ($0.event.sessionId ?? "default") == sessionId }
}

func pendingQuestion(forSession sessionId: String) -> QuestionRequest? {
questionQueue.first { ($0.event.sessionId ?? "default") == sessionId }
}

/// 1-based position for a card's "N of M" label. The card may be showing a
/// request that is not the head, so the position has to be looked up. (#308)
func permissionQueuePosition(forSession sessionId: String) -> Int {
(permissionQueue.firstIndex { ($0.event.sessionId ?? "default") == sessionId } ?? 0) + 1
}

func questionQueuePosition(forSession sessionId: String) -> Int {
(questionQueue.firstIndex { ($0.event.sessionId ?? "default") == sessionId } ?? 0) + 1
}
/// Preview-only: mock question payload for DebugHarness (no continuation needed)
var previewQuestionPayload: QuestionPayload?
var surface: IslandSurface = .collapsed {
Expand Down Expand Up @@ -1325,9 +1346,44 @@ final class AppState {
refreshDerivedState()
}

func approvePermission(always: Bool = false) {
guard !permissionQueue.isEmpty else { return }
let pending = permissionQueue.removeFirst()
/// Index of the queued request the user actually acted on.
///
/// The card on screen is identified by its session, but the answer used to
/// be applied to `queue.removeFirst()`. Anything that mutates the head
/// while a card is open — a peer disconnect draining another session, a
/// stale tool-use eviction, the reorder in `showNextPending()` — would then
/// resolve whichever request happened to be first, delivering the answer to
/// the wrong CLI. Callers that know which session the card belongs to pass
/// it in; `nil` keeps the head-of-queue behaviour for surfaces that only
/// ever mirror the head (keyboard shortcuts, iPhone/Watch Buddy). (#308)
private func permissionIndex(expecting expected: String?) -> Int? {
guard let expected else { return permissionQueue.isEmpty ? nil : 0 }
return permissionQueue.firstIndex { ($0.event.sessionId ?? "default") == expected }
}

/// Question-queue counterpart of `permissionIndex(expecting:)`. (#308)
private func questionIndex(expecting expected: String?) -> Int? {
guard let expected else { return questionQueue.isEmpty ? nil : 0 }
return questionQueue.firstIndex { ($0.event.sessionId ?? "default") == expected }
}

/// The request the card was showing is no longer queued (answered in the
/// terminal, drained on disconnect). `showNextPending()` drops the dead card
/// and re-opens whatever is genuinely waiting. (#308)
private func discardStalePanelAction(expected: String, kind: String) {
log.notice("⚠️ ignored \(kind, privacy: .public) for session=\(expected, privacy: .public) — request no longer queued")
showNextPending()
refreshDerivedState()
}

func approvePermission(always: Bool = false, expectedSessionId: String? = nil) {
guard let index = permissionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "approve")
}
return
}
let pending = permissionQueue.remove(at: index)
let sessionId = pending.event.sessionId ?? "default"
dismissedPermissionSessionIds.remove(sessionId)
let responseData: Data
Expand Down Expand Up @@ -1520,9 +1576,14 @@ final class AppState {
})?.key
}

func denyPermission() {
guard !permissionQueue.isEmpty else { return }
let pending = permissionQueue.removeFirst()
func denyPermission(expectedSessionId: String? = nil) {
guard let index = permissionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "deny")
}
return
}
let pending = permissionQueue.remove(at: index)
let sessionId = pending.event.sessionId ?? "default"
dismissedPermissionSessionIds.remove(sessionId)
let response = #"{"hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"deny"}}}"#
Expand All @@ -1544,8 +1605,14 @@ final class AppState {
refreshDerivedState()
}

func dismissPermissionPrompt() {
guard let pending = permissionQueue.first else { return }
func dismissPermissionPrompt(expectedSessionId: String? = nil) {
guard let index = permissionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "dismiss")
}
return
}
let pending = permissionQueue[index]

let sessionId = pending.event.sessionId ?? "default"
dismissedPermissionSessionIds.insert(sessionId)
Expand Down Expand Up @@ -1729,17 +1796,22 @@ final class AppState {
refreshDerivedState()
}

func answerQuestion(_ answer: String) {
guard !questionQueue.isEmpty else { return }
func answerQuestion(_ answer: String, expectedSessionId: String? = nil) {
guard let index = questionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "answer")
}
return
}
// Multi-question wizards (AskUserQuestion, Codex app-server) use the batch
// path — direct single answers are not processed.
if questionQueue[0].askUserQuestionState != nil,
(questionQueue[0].isFromPermission || questionQueue[0].isCodexAppServer) {
if questionQueue[index].askUserQuestionState != nil,
(questionQueue[index].isFromPermission || questionQueue[index].isCodexAppServer) {
return
}
// Codex app-server questions reply over the JSON-RPC client, not a hook.
if questionQueue[0].isCodexAppServer {
let pending = questionQueue.removeFirst()
if questionQueue[index].isCodexAppServer {
let pending = questionQueue.remove(at: index)
let answerKey = pending.askUserQuestionState?.items.first?.answerKey
?? pending.question.header ?? "answer"
pending.resolveCodexAppServer([answerKey: [answer]])
Expand All @@ -1749,7 +1821,7 @@ final class AppState {
refreshDerivedState()
return
}
let pending = questionQueue.removeFirst()
let pending = questionQueue.remove(at: index)
let responseData: Data
if pending.isFromPermission {
let answerKey = pending.question.header ?? "answer"
Expand Down Expand Up @@ -1792,11 +1864,19 @@ final class AppState {
refreshDerivedState()
}

func answerQuestionMulti(_ answers: [(question: String, answer: String)]) {
guard !questionQueue.isEmpty else { return }
func answerQuestionMulti(
_ answers: [(question: String, answer: String)],
expectedSessionId: String? = nil
) {
guard let index = questionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "answer")
}
return
}
// Codex app-server questions reply over the JSON-RPC client, not a hook.
if questionQueue[0].isCodexAppServer {
let pending = questionQueue.removeFirst()
if questionQueue[index].isCodexAppServer {
let pending = questionQueue.remove(at: index)
var answersByKey: [String: [String]] = [:]
if let askState = pending.askUserQuestionState {
// Match by position — the wizard collects answers in item order.
Expand All @@ -1814,7 +1894,7 @@ final class AppState {
refreshDerivedState()
return
}
let pending = questionQueue.removeFirst()
let pending = questionQueue.remove(at: index)
let responseData: Data
if pending.isFromPermission {
var answersDict: [String: String] = [:]
Expand Down Expand Up @@ -1888,9 +1968,14 @@ final class AppState {
return updatedInput
}

func skipQuestion() {
guard !questionQueue.isEmpty else { return }
let pending = questionQueue.removeFirst()
func skipQuestion(expectedSessionId: String? = nil) {
guard let index = questionIndex(expecting: expectedSessionId) else {
if let expectedSessionId {
discardStalePanelAction(expected: expectedSessionId, kind: "skip")
}
return
}
let pending = questionQueue.remove(at: index)
if pending.isCodexAppServer {
// No "skip" verb in the Codex protocol — abandon the request so the
// server stops waiting (it will re-prompt or fall back to its TUI).
Expand Down Expand Up @@ -1967,9 +2052,32 @@ final class AppState {
}
}

/// A card the user can no longer act on must never stay on screen: the panel
/// would sit expanded showing a request that is gone or dismissed, and any
/// click landing on it can only be discarded. Auto-open suppression decides
/// whether to open a *new* card, not whether to keep a dead one, so this
/// runs unconditionally. (#308)
///
/// "Dead" is the same predicate `nextVisiblePermissionIndex()` applies:
/// dismissed counts as not visible. Testing queue membership alone would
/// keep a dismissed card up, because dismissing hides without dequeuing.
private func collapseStaleCardSurface() {
switch surface {
case .approvalCard(let sid)
where pendingPermission(forSession: sid) == nil
|| dismissedPermissionSessionIds.contains(sid):
surface = .collapsed
case .questionCard(let sid) where pendingQuestion(forSession: sid) == nil:
surface = .collapsed
default:
break
}
}

/// After dequeuing, show next pending item or collapse
@discardableResult
func showNextPending() -> Bool {
collapseStaleCardSurface()
if let idx = nextVisiblePermissionIndex() {
let next = permissionQueue.remove(at: idx)
permissionQueue.insert(next, at: 0)
Expand Down
13 changes: 13 additions & 0 deletions Sources/CodeIsland/IslandSurface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,17 @@ enum IslandSurface: Equatable {
case .approvalCard(let id), .questionCard(let id), .completionCard(let id): return id
}
}

/// Session of the surface only when it is the matching card kind. A
/// permission shortcut fired while a question card is up must not address
/// that session's (non-existent) approval and discard the live card. (#308)
var approvalSessionId: String? {
if case .approvalCard(let id) = self { return id }
return nil
}

var questionSessionId: String? {
if case .questionCard(let id) = self { return id }
return nil
}
}
30 changes: 16 additions & 14 deletions Sources/CodeIsland/NotchPanelView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,38 +204,40 @@ struct NotchPanelView: View {

switch appState.surface {
case .approvalCard(let sid):
if let pending = appState.pendingPermission {
// Card is addressed by session — render that session's
// request, not whatever is at the head of the queue. (#308)
if let pending = appState.pendingPermission(forSession: sid) {
let session = appState.sessions[sid]
ApprovalBar(
tool: pending.event.toolName ?? "Unknown",
toolInput: pending.event.toolInput,
queuePosition: 1,
queuePosition: appState.permissionQueuePosition(forSession: sid),
queueTotal: appState.permissionQueue.count,
session: session,
sessionId: sid,
appState: appState,
onAllow: { appState.approvePermission(always: false) },
onAlwaysAllow: { appState.approvePermission(always: true) },
onDeny: { appState.denyPermission() },
onDismiss: { appState.dismissPermissionPrompt() }
onAllow: { appState.approvePermission(always: false, expectedSessionId: sid) },
onAlwaysAllow: { appState.approvePermission(always: true, expectedSessionId: sid) },
onDeny: { appState.denyPermission(expectedSessionId: sid) },
onDismiss: { appState.dismissPermissionPrompt(expectedSessionId: sid) }
)
.transition(.blurFade.combined(with: .scale(scale: 0.96, anchor: .top)))
}
case .questionCard(let sid):
let session = appState.sessions[sid]
if let q = appState.pendingQuestion {
if let q = appState.pendingQuestion(forSession: sid) {
QuestionBar(
question: q.question.question,
options: q.question.options,
descriptions: q.question.descriptions,
allQuestions: q.askUserQuestionState?.items ?? [],
sessionSource: session?.source,
sessionContext: session?.cwd,
queuePosition: 1,
queuePosition: appState.questionQueuePosition(forSession: sid),
queueTotal: appState.questionQueue.count,
onAnswer: { appState.answerQuestion($0) },
onAnswerMulti: { appState.answerQuestionMulti($0) },
onSkip: { appState.skipQuestion() }
onAnswer: { appState.answerQuestion($0, expectedSessionId: sid) },
onAnswerMulti: { appState.answerQuestionMulti($0, expectedSessionId: sid) },
onSkip: { appState.skipQuestion(expectedSessionId: sid) }
)
.transition(.blurFade.combined(with: .scale(scale: 0.96, anchor: .top)))
} else if let preview = appState.previewQuestionPayload {
Expand Down Expand Up @@ -2279,21 +2281,21 @@ private struct SessionCard: View {
fg: .white,
bg: Color(red: 0.25, green: 0.65, blue: 0.35),
enabled: isActiveApproval,
action: { appState.approvePermission(always: false) }
action: { appState.approvePermission(always: false, expectedSessionId: sessionId) }
)
inlineActionButton(
L10n.shared["always"],
fg: .white,
bg: Color(red: 0.25, green: 0.55, blue: 0.85),
enabled: isActiveApproval,
action: { appState.approvePermission(always: true) }
action: { appState.approvePermission(always: true, expectedSessionId: sessionId) }
)
inlineActionButton(
L10n.shared["deny"],
fg: .white,
bg: Color(red: 0.85, green: 0.3, blue: 0.3),
enabled: isActiveApproval,
action: { appState.denyPermission() }
action: { appState.denyPermission(expectedSessionId: sessionId) }
)
}

Expand Down
Loading