From 94139744e10b67e9a17f48a32764e53562181d8a Mon Sep 17 00:00:00 2001 From: Hugo THIEAUT Date: Sat, 3 Oct 2026 00:24:42 +0200 Subject: [PATCH] Queue pending approvals instead of displacing them Each PermissionRequest keeps its own fd, disconnect source and 115 s timeout. FIFO with a cap of 5 (the newest gets ask beyond it). The card shows '1 of N' when several are waiting. Decisions only apply to the request shown on the card, and dead sockets are dropped before display. Co-Authored-By: Claude Sonnet 5.5 --- .../NotchBuddy.xcodeproj/project.pbxproj | 6 + NotchBuddy/Sources/App/ApprovalQueue.swift | 82 +++++ NotchBuddy/Sources/App/HookServer.swift | 297 ++++++++++-------- NotchBuddy/Sources/App/IslandTypes.swift | 7 + .../Sources/App/IslandViewContent.swift | 19 +- scripts/test-approval-queue.sh | 8 + tests/ApprovalQueueTests.swift | 68 ++++ 7 files changed, 355 insertions(+), 132 deletions(-) create mode 100644 NotchBuddy/Sources/App/ApprovalQueue.swift create mode 100755 scripts/test-approval-queue.sh create mode 100644 tests/ApprovalQueueTests.swift diff --git a/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj b/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj index 4803a9de1..0b1d1c9d0 100644 --- a/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj +++ b/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 0325A44358904B44C2237CCE /* ApprovalQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */; }; 0329E76BA4A8FEEA8A6D76E7 /* ClaudeService.swift in Sources */ = {isa = PBXBuildFile; fileRef = A83D5410BD030B243ADDCFF4 /* ClaudeService.swift */; }; 065FCE817315454DE3F85F25 /* IslandViewContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA02B08886939FF6FE0475DD /* IslandViewContent.swift */; }; 06BF5FD95E03010CAFDBF2D0 /* NotchBuddyApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = F1BD8F3CAA60CF37640759E1 /* NotchBuddyApp.swift */; }; @@ -45,6 +46,7 @@ 78FCEDC8F78B683AB10E6529 /* NotionPoller.swift in Sources */ = {isa = PBXBuildFile; fileRef = BE03E03D1804B0B92C5C2DAD /* NotionPoller.swift */; }; 878924960FC0F2B3524D48AC /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 192844E7299DEED5C2372067 /* AppDelegate.swift */; }; 8C32A0C3EAF536D3836F61D5 /* GreetingCanvasView.swift in Sources */ = {isa = PBXBuildFile; fileRef = C581F7B1A9F14B281B60063E /* GreetingCanvasView.swift */; }; + 8C5CF7C82640DD6A0685A0D5 /* ApprovalQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */; }; 8D1C01F283FA7806119A1C85 /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C1A7D5CBE2AC084A10C5FF3 /* SettingsView.swift */; }; 967B525F7D3837F31D740D18 /* ResendPoller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61B5FDDE03F5F711146937E2 /* ResendPoller.swift */; }; 9DD56C95ECABDDCE23652D50 /* HookServer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 79D8DD79C3DD9BBC584B3CB2 /* HookServer.swift */; }; @@ -76,6 +78,7 @@ /* Begin PBXFileReference section */ 15A1A8206F73376F4CAF2057 /* IslandRootView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandRootView.swift; sourceTree = ""; }; 192844E7299DEED5C2372067 /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = ""; }; + 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApprovalQueue.swift; sourceTree = ""; }; 1BE21C2B315D37AA70ED45E9 /* SoundEngine.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoundEngine.swift; sourceTree = ""; }; 2EF78999D68D56ADE4CEFBE2 /* StripePoller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StripePoller.swift; sourceTree = ""; }; 3104222B3F17BA5E8DEA5277 /* CalcomPoller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalcomPoller.swift; sourceTree = ""; }; @@ -116,6 +119,7 @@ children = ( 192844E7299DEED5C2372067 /* AppDelegate.swift */, 9F6D7015D6A77AA59323C0E7 /* AppLog.swift */, + 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */, C2E4F18AA940EFF7E047B9F7 /* AppState.swift */, A29F4C0B02BE91654D02AA0C /* BotCanvasView.swift */, D4540F630F8BA04AFFFA3979 /* BotEngine.swift */, @@ -289,6 +293,7 @@ C940B651C6CA952D99388E04 /* AppDelegate.swift in Sources */, 4B6AC00F22EC75E90E0157D5 /* AppLog.swift in Sources */, 1F11EC4E6F604C612A1ADAD1 /* AppState.swift in Sources */, + 0325A44358904B44C2237CCE /* ApprovalQueue.swift in Sources */, CD8DECA66B9E09C97E469DB9 /* BotCanvasView.swift in Sources */, C24D863967D8D32D828ED4B1 /* BotEngine.swift in Sources */, 4F1E0B3C838DA3CD6F864BC0 /* CalcomPoller.swift in Sources */, @@ -326,6 +331,7 @@ 878924960FC0F2B3524D48AC /* AppDelegate.swift in Sources */, 61CB7049490F61CFA1B36B64 /* AppLog.swift in Sources */, 61F73B27A7E3AFF584B2CC22 /* AppState.swift in Sources */, + 8C5CF7C82640DD6A0685A0D5 /* ApprovalQueue.swift in Sources */, 651FA3F09608DE5D02D61CCE /* BotCanvasView.swift in Sources */, 0E9CE475EC53C67DAB00F8E5 /* BotEngine.swift in Sources */, 1969590AFEE6CB21404346F1 /* CalcomPoller.swift in Sources */, diff --git a/NotchBuddy/Sources/App/ApprovalQueue.swift b/NotchBuddy/Sources/App/ApprovalQueue.swift new file mode 100644 index 000000000..bdc33be93 --- /dev/null +++ b/NotchBuddy/Sources/App/ApprovalQueue.swift @@ -0,0 +1,82 @@ +import Foundation + +// MARK: - ApprovalQueue +// Pure FIFO queue of pending PermissionRequests. No AppKit, no SwiftUI, no I/O: +// HookServer owns the sockets, timers and UI, and mutates this value on the main actor only. +// The head is the request currently shown on the approval card. + +struct QueuedApproval: Equatable, Sendable { + let id: UUID + /// Client socket, held open until the user decides. Closed by the owner's dispatch source. + let fd: Int32 + let sessionId: String + /// "integration_claude", "agent_cursor" or "agent_codex". + let pillId: String + let projectName: String + let cwd: String + let tool: String + let command: String + /// tool_input serialized with sortedKeys, "" if absent. Used to match PostToolUse. + let inputKey: String +} + +struct ApprovalQueue: Sendable { + /// Upper bound of simultaneously pending requests. Each one holds a socket and a 115 s timer + /// and the card only shows one at a time, so beyond this the newest falls back to its terminal. + static let maxPending = 5 + + enum EnqueueResult: Equatable, Sendable { case accepted, rejectedFull } + + private(set) var entries: [QueuedApproval] = [] + + var head: QueuedApproval? { entries.first } + var count: Int { entries.count } + var isEmpty: Bool { entries.isEmpty } + + func contains(id: UUID) -> Bool { entries.contains { $0.id == id } } + + /// 1-based position of the entry in the queue, nil if absent. + func position(of id: UUID) -> Int? { + entries.firstIndex { $0.id == id }.map { $0 + 1 } + } + + /// True when the queued requests do not all come from the same session and agent. + var hasMixedSources: Bool { + guard let first = entries.first else { return false } + return entries.contains { $0.sessionId != first.sessionId || $0.pillId != first.pillId } + } + + func hasEntries(forPill pillId: String) -> Bool { + entries.contains { $0.pillId == pillId } + } + + mutating func enqueue(_ entry: QueuedApproval, maxPending: Int = ApprovalQueue.maxPending) -> EnqueueResult { + guard entries.count < maxPending else { return .rejectedFull } + entries.append(entry) + return .accepted + } + + /// Removes only the entry with this id. Other entries keep their order. + @discardableResult + mutating func remove(id: UUID) -> QueuedApproval? { + guard let idx = entries.firstIndex(where: { $0.id == id }) else { return nil } + return entries.remove(at: idx) + } + + /// Entries made moot by a later hook event of the same agent. Matching is per request + /// (session + agent, plus tool and input for PostToolUse), never by pill alone. + func resolved(byEvent name: String, sessionId: String, pillId: String, + tool: String, inputKey: String) -> [QueuedApproval] { + switch name { + case "PostToolUse", "PostToolUseFailure": + return entries.filter { + $0.pillId == pillId && $0.sessionId == sessionId + && $0.tool == tool && $0.inputKey == inputKey + } + case "Stop", "StopFailure", "UserPromptSubmit", "SessionEnd", "Interrupt": + return entries.filter { $0.pillId == pillId && $0.sessionId == sessionId } + default: + return [] + } + } +} diff --git a/NotchBuddy/Sources/App/HookServer.swift b/NotchBuddy/Sources/App/HookServer.swift index 066876623..3f47f0523 100644 --- a/NotchBuddy/Sources/App/HookServer.swift +++ b/NotchBuddy/Sources/App/HookServer.swift @@ -38,8 +38,9 @@ final class HookServer: @unchecked Sendable { private var serverFD: Int32 = -1 private let connectionLock = NSLock() private var connectionCount = 0 - private var pendingApprovalFD: Int32 = -1 // held open while user decides - private var approvalFDSource: (any DispatchSourceRead)? = nil // monitors pendingApprovalFD + // Pending approvals, FIFO. Main actor only. Each entry owns its fd, disconnect source and timeout. + private var approvalQueue = ApprovalQueue() + private var approvalResources: [UUID: ApprovalResources] = [:] private var activeSessionId: String? = nil // current Claude Code session private var focusBeforeApproval: String? = nil // saved focus to restore after approval @@ -47,38 +48,124 @@ final class HookServer: @unchecked Sendable { // MARK: - Approval fd helpers + private struct ApprovalResources { + let source: any DispatchSourceRead + let timeout: DispatchWorkItem + } + + private static func handledNote(for pillId: String) -> String { + switch pillId { + case "agent_cursor": return "Handled in Cursor." + case "agent_codex": return "Handled in Codex." + default: return "Handled in VS Code." + } + } + + private static func waitingNote(for pillId: String) -> String { + switch pillId { + case "agent_cursor": return "Still waiting in Cursor." + case "agent_codex": return "Still waiting in Codex." + default: return "Still waiting in VS Code." + } + } + + private static func agentLabel(for pillId: String) -> String { + switch pillId { + case "agent_cursor": return "Cursor" + case "agent_codex": return "Codex" + default: return "Claude" + } + } + + /// True while the peer has not closed its end. A request whose socket is dead must never be shown. + private static func isAlive(fd: Int32) -> Bool { + var byte: UInt8 = 0 + let n = recv(fd, &byte, 1, MSG_PEEK | MSG_DONTWAIT) + if n == 0 { return false } + if n < 0 { return errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR } + return true + } + + /// Removes one entry from the queue and tears down its timeout and source. + /// The source's cancel handler closes the fd — never close it here. + /// Returns the removed entry and its source so the caller can still write a decision first. + @MainActor + private func detachApproval(id: UUID) -> (entry: QueuedApproval, source: (any DispatchSourceRead)?)? { + guard let entry = approvalQueue.remove(id: id) else { return nil } + let res = approvalResources.removeValue(forKey: id) + res?.timeout.cancel() + return (entry, res?.source) + } + + /// Drops these entries only (client gone, timed out, or handled elsewhere). Silent for the others. + /// `note` is shown only when the queue becomes empty. @MainActor - private func cancelApprovalFDSource() { - approvalFDSource?.cancel() - approvalFDSource = nil + private func dismissApprovals(ids: [UUID], note: String) { + var lastPill: String? + for id in ids { + guard let removed = detachApproval(id: id) else { continue } + removed.source?.cancel() + lastPill = removed.entry.pillId + } + guard let pillId = lastPill else { return } + refreshApprovalCard(releasedPill: pillId, emptyNote: note) } - /// Cancels the approval fd source (which closes the fd via its cancel handler), shows a - /// 3-second note, clears approval state, then collapses the island. + /// Brings the card and pill states in line with the queue after any removal. + /// Non-empty: the next live request becomes the card. Empty: release the island as before. @MainActor - private func dismissApprovalCard(note: String) { - // cancelApprovalFDSource() triggers the cancel handler which closes the fd. - // Never close the fd here directly — Apple requires it to happen in the cancel handler. - cancelApprovalFDSource() - pendingApprovalFD = -1 + private func refreshApprovalCard(releasedPill: String, emptyNote: String?) { let state = AppState.shared - let pillId = state.pendingApproval?.pillId ?? "integration_claude" - state.pendingApproval = nil - state.isPinned = false - state.updateTask(id: pillId, state: .working) - clearPillBadge(id: pillId) - // Restore focus to the pill that was focused before the approval card appeared. - if let prev = focusBeforeApproval { - focusBeforeApproval = nil - if state.focusId == pillId, state.tasks.contains(where: { $0.id == prev }) { - withAnimation(.spring(response: 0.5, dampingFraction: 0.72)) { state.focusId = prev } + // Never show a request whose socket is already dead: drop it and move on. + while let head = approvalQueue.head, !Self.isAlive(fd: head.fd) { + nbLog("Dropped dead approval \(head.tool) [\(head.pillId)]") + if let removed = detachApproval(id: head.id) { removed.source?.cancel() } + } + // A pill with no request left goes back to work. + if !approvalQueue.hasEntries(forPill: releasedPill) { + state.updateTask(id: releasedPill, state: .working) + clearPillBadge(id: releasedPill) + } + guard let head = approvalQueue.head else { + state.pendingApproval = nil + state.isPinned = false + // Restore focus to the pill that was focused before the approval card appeared. + if let prev = focusBeforeApproval { + focusBeforeApproval = nil + if state.focusId == releasedPill, state.tasks.contains(where: { $0.id == prev }) { + withAnimation(.spring(response: 0.5, dampingFraction: 0.72)) { state.focusId = prev } + } + } + if let note = emptyNote { + state.noteMessage = note + state.view = .note + DispatchQueue.main.asyncAfter(deadline: .now() + 3) { + NotificationCenter.default.post(name: .islandCollapse, object: nil) + } + } else { + state.view = state.tasks.isEmpty ? .empty : .overview } + return } - state.noteMessage = note - state.view = .note - DispatchQueue.main.asyncAfter(deadline: .now() + 3) { - NotificationCenter.default.post(name: .islandCollapse, object: nil) + showApproval(head) + } + + /// Makes `entry` (the queue head) the card on screen. + @MainActor + private func showApproval(_ entry: QueuedApproval) { + let state = AppState.shared + upsertWorkspaceTask(id: entry.pillId, projectName: entry.projectName, cwd: entry.cwd) + state.updateTask(id: entry.pillId, state: .approval) + state.pendingApproval = ApprovalInfo( + sessionId: entry.sessionId, tool: entry.tool, command: entry.command, + inputKey: entry.inputKey, pillId: entry.pillId, id: entry.id, + position: 1, total: approvalQueue.count, + sourceLabel: approvalQueue.hasMixedSources ? Self.agentLabel(for: entry.pillId) : nil) + state.isPinned = true + if state.focusId != entry.pillId { + withAnimation(.spring(response: 0.5, dampingFraction: 0.72)) { state.focusId = entry.pillId } } + if state.mode == .expanded { state.view = .approval } } /// Returns the tool_input serialized as sorted-keys JSON, "" if absent or empty. @@ -262,35 +349,16 @@ final class HookServer: @unchecked Sendable { let focused = state.focusId == agentId - // While a permission request is pending, dismiss when the resolving event arrives, - // then continue normal processing. Only skip normal processing when unresolved. - if let pending = state.pendingApproval, agentId == pending.pillId { - let handledNote: String - switch pending.pillId { - case "agent_cursor": handledNote = "Handled in Cursor." - case "agent_codex": handledNote = "Handled in Codex." - default: handledNote = "Handled in VS Code." - } - var resolved = false - switch name { - case "PostToolUse", "PostToolUseFailure": - // Only dismiss when this exact tool call finished — same session, tool and input. - // Other parallel tools finishing must not close the card. - if sessionId == pending.sessionId, - (payload["tool_name"] as? String ?? "") == pending.tool, - Self.approvalInputKey(payload["tool_input"] as? [String: Any] ?? [:]) == pending.inputKey { - dismissApprovalCard(note: handledNote) - resolved = true - } - case "Stop", "StopFailure", "UserPromptSubmit", "SessionEnd", "Interrupt": - // Turn ended or session interrupted — the permission is moot. - if sessionId == pending.sessionId { - dismissApprovalCard(note: handledNote) - resolved = true - } - default: break - } - if !resolved { return } + // While permission requests are pending for this agent, dismiss the ones this event + // resolves (same session and agent, and same tool + input for PostToolUse), then continue + // normal processing. Only skip normal processing when nothing was resolved. + if approvalQueue.hasEntries(forPill: agentId) { + let tool = payload["tool_name"] as? String ?? "" + let inputKey = Self.approvalInputKey(payload["tool_input"] as? [String: Any] ?? [:]) + let resolved = approvalQueue.resolved(byEvent: name, sessionId: sessionId, pillId: agentId, + tool: tool, inputKey: inputKey) + if resolved.isEmpty { return } + dismissApprovals(ids: resolved.map(\.id), note: Self.handledNote(for: agentId)) // Approval dismissed — fall through so the resolving event updates state normally. } @@ -439,7 +507,7 @@ final class HookServer: @unchecked Sendable { // Approval always wins; other alerts are blocked while a card is showing if view == .approval { state.view = view - } else if isAlert && state.pendingApproval == nil { + } else if isAlert && approvalQueue.isEmpty { state.view = view } } else if isAlert { @@ -507,80 +575,65 @@ final class HookServer: @unchecked Sendable { let tool = payload["tool_name"] as? String ?? "Tool" let toolInput = payload["tool_input"] as? [String: Any] ?? [:] - var command = toolInput["command"] as? String ?? tool + let command = toolInput["command"] as? String ?? tool let inputKey = Self.approvalInputKey(toolInput) nbLog("PermissionRequest \(tool) [\(pillId)]") - if pendingApprovalFD >= 0 { - // Displace the previous request: write "ask" then cancel its source. - // The cancel handler closes the old fd — never close it directly. - let old = pendingApprovalFD - let oldSource = approvalFDSource - approvalFDSource = nil + let entry = QueuedApproval(id: UUID(), fd: fd, sessionId: sessionId, pillId: pillId, + projectName: projectName, cwd: cwd, tool: tool, + command: command, inputKey: inputKey) + guard approvalQueue.enqueue(entry) == .accepted else { + // Queue full: the newest request falls back to its terminal ("ask"), as displacement did. + nbLog("Approval queue full (\(ApprovalQueue.maxPending)): ask for \(tool) [\(pillId)]") Task.detached { [weak self] in - // "ask" → nb-hook outputs nothing → Claude Code re-asks - self?.sendLine(fd: old, text: #"{"permissionDecision":"ask"}"#) - DispatchQueue.main.async { oldSource?.cancel() } + self?.sendLine(fd: fd, text: #"{"permissionDecision":"ask"}"#) + close(fd) } + return } - pendingApprovalFD = fd activeSessionId = sessionId - upsertWorkspaceTask(id: pillId, projectName: projectName, cwd: cwd) - state.updateTask(id: pillId, state: .approval) - state.pendingApproval = ApprovalInfo(sessionId: sessionId, tool: tool, - command: command, inputKey: inputKey, pillId: pillId) - state.isPinned = true - SoundEngine.shared.play("approval") - - // Approval always forces the island open — user must be able to respond. - // Save current focus so we can restore it when the card is dismissed. - if focusBeforeApproval == nil { focusBeforeApproval = state.focusId } - withAnimation(.spring(response: 0.5, dampingFraction: 0.72)) { state.focusId = pillId } - expandIfNeeded(to: .approval) - - // Monitor fd: if the editor closes the connection (handled externally), dismiss the card. - // The cancel handler closes the fd — never close it anywhere else. - let capturedPillId = pillId + // Monitor fd: if the client closes the connection (handled in the editor), drop only this + // entry. The cancel handler closes the fd — never close it anywhere else. + let id = entry.id let source = DispatchSource.makeReadSource(fileDescriptor: fd, queue: .main) source.setEventHandler { [weak self] in - guard let self, self.pendingApprovalFD == fd else { return } - let note: String - switch capturedPillId { - case "agent_cursor": note = "Handled in Cursor." - case "agent_codex": note = "Handled in Codex." - default: note = "Handled in VS Code." - } - self.dismissApprovalCard(note: note) + guard let self, self.approvalQueue.contains(id: id) else { return } + self.dismissApprovals(ids: [id], note: Self.handledNote(for: pillId)) } source.setCancelHandler { close(fd) } source.resume() - approvalFDSource = source - - // 115s safety timeout — show a note and cancel without sending a decision. - // nb-hook reads EOF from the cancel handler's close and exits; Claude Code / Codex re-asks. - let captured = fd - DispatchQueue.main.asyncAfter(deadline: .now() + 115) { [weak self] in - guard let self, self.pendingApprovalFD == captured else { return } - let note: String - switch capturedPillId { - case "agent_cursor": note = "Still waiting in Cursor." - case "agent_codex": note = "Still waiting in Codex." - default: note = "Still waiting in VS Code." - } - self.dismissApprovalCard(note: note) + + // 115s safety timeout, on this request's own clock (it was sent at arrival time): + // cancel without sending a decision. The relay reads EOF and exits; the agent re-asks. + let timeout = DispatchWorkItem { [weak self] in + guard let self, self.approvalQueue.contains(id: id) else { return } + self.dismissApprovals(ids: [id], note: Self.waitingNote(for: pillId)) + } + DispatchQueue.main.asyncAfter(deadline: .now() + 115, execute: timeout) + approvalResources[id] = ApprovalResources(source: source, timeout: timeout) + + nbLog("Approval queued \(approvalQueue.count)/\(ApprovalQueue.maxPending) [\(pillId)]") + SoundEngine.shared.play("approval") + // First request: take over the card. Later ones wait their turn (FIFO) and only bump the counter. + if approvalQueue.count == 1 { + // Approval always forces the island open — user must be able to respond. + // Save current focus so we can restore it when the card is dismissed. + if focusBeforeApproval == nil { focusBeforeApproval = state.focusId } + showApproval(entry) + expandIfNeeded(to: .approval) + } else if let head = approvalQueue.head { + showApproval(head) } } /// Called by ApprovalView buttons. Writes the decision to the waiting nb-hook and cleans up. + /// `id` is the request the card showed: a click is never applied to a different request. @MainActor - func sendApprovalDecision(_ decision: String) { - let fd = pendingApprovalFD - pendingApprovalFD = -1 - // Capture source before nulling — we send the decision first, then cancel the source. - // The cancel handler closes the fd; never close it directly. - let source = approvalFDSource - approvalFDSource = nil + func sendApprovalDecision(_ decision: String, for id: UUID) { + guard approvalQueue.head?.id == id, let removed = detachApproval(id: id) else { return } + let fd = removed.entry.fd + let source = removed.source let json: String switch decision { @@ -590,30 +643,18 @@ final class HookServer: @unchecked Sendable { default: json = #"{"permissionDecision":"deny"}"# } - if fd >= 0 { + if Self.isAlive(fd: fd) { Task.detached { [weak self] in // Write decision while fd is still valid, then cancel source → cancel handler closes fd self?.sendLine(fd: fd, text: json) DispatchQueue.main.async { source?.cancel() } } } else { + // Client already gone: nothing to answer. Drop it. + nbLog("Dropped dead approval on decision \(removed.entry.tool) [\(removed.entry.pillId)]") source?.cancel() } - - let state = AppState.shared - let pillId = state.pendingApproval?.pillId ?? "integration_claude" - state.pendingApproval = nil - state.isPinned = false - state.updateTask(id: pillId, state: .working) - clearPillBadge(id: pillId) - // Restore focus to the pill that was focused before the approval card appeared. - if let prev = focusBeforeApproval { - focusBeforeApproval = nil - if state.focusId == pillId, state.tasks.contains(where: { $0.id == prev }) { - withAnimation(.spring(response: 0.5, dampingFraction: 0.72)) { state.focusId = prev } - } - } - state.view = state.tasks.isEmpty ? .empty : .overview + refreshApprovalCard(releasedPill: removed.entry.pillId, emptyNote: nil) } /// Updates or transiently creates a workspace pill (VS Code or Cursor) task. diff --git a/NotchBuddy/Sources/App/IslandTypes.swift b/NotchBuddy/Sources/App/IslandTypes.swift index 109a2457c..ba3a96dd6 100644 --- a/NotchBuddy/Sources/App/IslandTypes.swift +++ b/NotchBuddy/Sources/App/IslandTypes.swift @@ -38,6 +38,13 @@ struct ApprovalInfo: Sendable { var inputKey: String /// Pill that owns this approval: "integration_claude", "agent_cursor", or "agent_codex". var pillId: String + /// Queue entry this card answers. A decision is only accepted for this id. + var id: UUID = UUID() + /// 1-based position and size of the approval queue, shown as "1 of N" when total > 1. + var position: Int = 1 + var total: Int = 1 + /// Agent name, set only when queued requests come from different sessions or agents. + var sourceLabel: String? = nil } // MARK: - Pill badge (shown on pill edge when non-focused task has an alert) diff --git a/NotchBuddy/Sources/App/IslandViewContent.swift b/NotchBuddy/Sources/App/IslandViewContent.swift index 87d1d1d33..d88ddb216 100644 --- a/NotchBuddy/Sources/App/IslandViewContent.swift +++ b/NotchBuddy/Sources/App/IslandViewContent.swift @@ -230,23 +230,34 @@ struct ApprovalView: View { var approval: ApprovalInfo? { state.pendingApproval } + /// "needs permission", with "1 of N" (and the agent when sources differ) only while others are queued. + private var approvalLabel: String { + guard let a = approval, a.total > 1 else { return "needs permission" } + var parts = ["needs permission", "\(a.position) of \(a.total)"] + if let source = a.sourceLabel { parts.append(source) } + return parts.joined(separator: " · ") + } + var body: some View { + // Captured when the card is drawn: a click answers the request that was on screen, + // and is ignored if the queue has moved on since. + let shownId = approval?.id ZStack { CardBackground(wash: .amber) VStack(alignment: .leading, spacing: 5) { - AgentWho(task: state.focusTask, label: "needs permission") + AgentWho(task: state.focusTask, label: approvalLabel) CodeBlock(text: approval?.command ?? approval?.tool ?? "…") HStack(spacing: 8) { SecondaryButton("Deny") { - HookServer.shared.sendApprovalDecision("deny") + if let id = shownId { HookServer.shared.sendApprovalDecision("deny", for: id) } } PrimaryButton("Allow") { - HookServer.shared.sendApprovalDecision("allow") + if let id = shownId { HookServer.shared.sendApprovalDecision("allow", for: id) } } // Codex rejects updatedPermissions, so "Always" is not offered if approval?.pillId != "agent_codex" { SecondaryButton("Always") { - HookServer.shared.sendApprovalDecision("always") + if let id = shownId { HookServer.shared.sendApprovalDecision("always", for: id) } } } } diff --git a/scripts/test-approval-queue.sh b/scripts/test-approval-queue.sh new file mode 100755 index 000000000..b100510a3 --- /dev/null +++ b/scripts/test-approval-queue.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "$0")/.." +TEST_DIR="$(mktemp -d "${TMPDIR:-/tmp}/coucou-approval-queue.XXXXXX")" +trap 'rm -rf "$TEST_DIR"' EXIT +swiftc NotchBuddy/Sources/App/ApprovalQueue.swift \ + tests/ApprovalQueueTests.swift -o "$TEST_DIR/approval-queue-tests" +"$TEST_DIR/approval-queue-tests" diff --git a/tests/ApprovalQueueTests.swift b/tests/ApprovalQueueTests.swift new file mode 100644 index 000000000..d85d13eff --- /dev/null +++ b/tests/ApprovalQueueTests.swift @@ -0,0 +1,68 @@ +import Foundation + +@main +enum ApprovalQueueTests { + static func entry(_ n: Int, session: String = "s1", pill: String = "integration_claude", + tool: String = "Bash", key: String = "{}") -> QueuedApproval { + QueuedApproval(id: UUID(), fd: Int32(n), sessionId: session, pillId: pill, + projectName: "p", cwd: "/", tool: tool, command: "cmd\(n)", inputKey: key) + } + + static func main() { + // FIFO: head is the oldest, removal of the head promotes the next + var q = ApprovalQueue() + let a = entry(1), b = entry(2), c = entry(3) + for e in [a, b, c] { precondition(q.enqueue(e) == .accepted) } + precondition(q.count == 3 && q.head == a) + precondition(q.position(of: a.id) == 1 && q.position(of: c.id) == 3) + precondition(q.remove(id: a.id) == a) + precondition(q.head == b && q.count == 2) + + // Removal on disconnect / timeout of a non-head entry leaves the others untouched + precondition(q.remove(id: c.id) == c) + precondition(q.head == b && q.count == 1) + precondition(q.remove(id: c.id) == nil) // already gone: no-op + precondition(q.remove(id: b.id) == b && q.isEmpty && q.head == nil) + + // Cap: the newest is rejected, queue unchanged + var full = ApprovalQueue() + let all = (1...ApprovalQueue.maxPending).map { entry($0) } + for e in all { precondition(full.enqueue(e) == .accepted) } + precondition(full.enqueue(entry(99)) == .rejectedFull) + precondition(full.entries == all) + // Room again after a removal + _ = full.remove(id: all[0].id) + precondition(full.enqueue(entry(97), maxPending: ApprovalQueue.maxPending) == .accepted) + + // Event matching is per request: session + agent, never the pill alone + var m = ApprovalQueue() + let s1 = entry(1, session: "s1", tool: "Bash", key: "{\"command\":\"ls\"}") + let s2 = entry(2, session: "s2", tool: "Bash", key: "{\"command\":\"ls\"}") + let cur = entry(3, session: "s1", pill: "agent_cursor", tool: "Bash", key: "{\"command\":\"ls\"}") + for e in [s1, s2, cur] { _ = m.enqueue(e) } + let pt = m.resolved(byEvent: "PostToolUse", sessionId: "s1", pillId: "integration_claude", + tool: "Bash", inputKey: "{\"command\":\"ls\"}") + precondition(pt == [s1]) + precondition(m.resolved(byEvent: "PostToolUse", sessionId: "s1", pillId: "integration_claude", + tool: "Bash", inputKey: "{\"command\":\"pwd\"}").isEmpty) + precondition(m.resolved(byEvent: "PostToolUse", sessionId: "s1", pillId: "integration_claude", + tool: "Read", inputKey: "{\"command\":\"ls\"}").isEmpty) + precondition(m.resolved(byEvent: "Stop", sessionId: "s2", pillId: "integration_claude", + tool: "", inputKey: "") == [s2]) + precondition(m.resolved(byEvent: "Stop", sessionId: "s1", pillId: "agent_cursor", + tool: "", inputKey: "") == [cur]) + precondition(m.resolved(byEvent: "Stop", sessionId: "other", pillId: "integration_claude", + tool: "", inputKey: "").isEmpty) + precondition(m.resolved(byEvent: "PreToolUse", sessionId: "s1", pillId: "integration_claude", + tool: "Bash", inputKey: "{\"command\":\"ls\"}").isEmpty) + + // Mixed sources and per-pill lookup + precondition(m.hasMixedSources) + precondition(m.hasEntries(forPill: "agent_cursor") && !m.hasEntries(forPill: "agent_codex")) + var same = ApprovalQueue() + _ = same.enqueue(entry(1)); _ = same.enqueue(entry(2, tool: "Edit")) + precondition(!same.hasMixedSources) + + print("pass") + } +}