diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 531877cb4..7c1f3d888 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -54,6 +54,9 @@ jobs: - name: Test ask question run: bash scripts/test-ask-question.sh + - name: Test approval queue + run: bash scripts/test-approval-queue.sh + - name: Diff engine tests run: bash scripts/test-diff-engine.sh diff --git a/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj b/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj index dcaa83f00..21fe665ea 100644 --- a/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj +++ b/NotchBuddy/NotchBuddy.xcodeproj/project.pbxproj @@ -52,6 +52,7 @@ 2875610F701376BD65573386 /* WindowContextCapture.swift in Sources */ = {isa = PBXBuildFile; fileRef = E91B733ADC638E14E9BB6EFA /* WindowContextCapture.swift */; }; 295F8FE5A9E2E8F1A6389DEE /* HomeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B53E4CAD2BCC77B280689B0F /* HomeView.swift */; }; 2A30FA2F046492E22A1C5719 /* ServiceDetail.swift in Sources */ = {isa = PBXBuildFile; fileRef = 65F64746B2740D5FE26B7E5A /* ServiceDetail.swift */; }; + 0325A44358904B44C2237CCE /* ApprovalQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */; }; 2C798C3837A63B9BB561A1A0 /* HookServer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 79D8DD79C3DD9BBC584B3CB2 /* HookServer.swift */; }; 2CC2E3B688B15BBEB3AE3BCE /* WindowContextCapture.swift in Sources */ = {isa = PBXBuildFile; fileRef = E91B733ADC638E14E9BB6EFA /* WindowContextCapture.swift */; }; 2DEB459FBD0BA3CEAAA5FDE9 /* ClaudeService.swift in Sources */ = {isa = PBXBuildFile; fileRef = A83D5410BD030B243ADDCFF4 /* ClaudeService.swift */; }; @@ -156,6 +157,7 @@ 9AEFD05D0A39D4AF3C091B3A /* QuestionRelay.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB78996E9F9A2E8A80D45E54 /* QuestionRelay.swift */; }; 9BCD79FD8CF211DC8FBD2971 /* SearchAndSpotlight.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE60A834934474CDAE022FF8 /* SearchAndSpotlight.swift */; }; 9C653B8291EFFA414565E8A8 /* MusicController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F142FB3952B5F0639383047 /* MusicController.swift */; }; + 8C5CF7C82640DD6A0685A0D5 /* ApprovalQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */; }; 9DD56C95ECABDDCE23652D50 /* HookServer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 79D8DD79C3DD9BBC584B3CB2 /* HookServer.swift */; }; 9E34044A001B01C3A514D20A /* IslandDisplayChoice.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A09E73F1E58019EF6EA2A62 /* IslandDisplayChoice.swift */; }; 9ECEE890D4486B6E83BD5B7F /* ColorHex.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ACB4350729C6513824D1ADF /* ColorHex.swift */; }; @@ -343,6 +345,7 @@ 73FA4DF1BE5246CCFFB72E5C /* CodexPlanGauge.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CodexPlanGauge.swift; sourceTree = ""; }; 78FB882D49084DEA6A10DB4D /* RecapStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecapStore.swift; sourceTree = ""; }; 79A7807ADD0963FF260D1FAD /* MochiWardrobe.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MochiWardrobe.swift; sourceTree = ""; }; + 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApprovalQueue.swift; sourceTree = ""; }; 79D8DD79C3DD9BBC584B3CB2 /* HookServer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HookServer.swift; sourceTree = ""; }; 7C1A7D5CBE2AC084A10C5FF3 /* SettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsView.swift; sourceTree = ""; }; 824E56D2255CA86DAC350DBF /* SessionPublisher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionPublisher.swift; sourceTree = ""; }; @@ -449,6 +452,7 @@ 8CB8B6E5E9DA6164F979E88F /* GithubPoller.swift */, F8D3AD61802A32C4833F3EDA /* GitHubPulse.swift */, C581F7B1A9F14B281B60063E /* GreetingCanvasView.swift */, + 1BAE224C1C435C9FFD05BF65 /* ApprovalQueue.swift */, 79D8DD79C3DD9BBC584B3CB2 /* HookServer.swift */, 1218B86E249691AF44F819AC /* HotKeyCenter.swift */, 4A09E73F1E58019EF6EA2A62 /* IslandDisplayChoice.swift */, @@ -832,6 +836,7 @@ 862CE6CB220D55E46F646DA9 /* GitHubPulse.swift in Sources */, C808B654C84D9F71DBF8493A /* GithubPoller.swift in Sources */, 8C32A0C3EAF536D3836F61D5 /* GreetingCanvasView.swift in Sources */, + 0325A44358904B44C2237CCE /* ApprovalQueue.swift in Sources */, 2C798C3837A63B9BB561A1A0 /* HookServer.swift in Sources */, B733A76DD197F6D6D949284C /* HotKeyCenter.swift in Sources */, 658C579523CF5F8D67B31369 /* InstructionRunner.swift in Sources */, @@ -988,6 +993,7 @@ 38EEB2D3A6EC538958D92575 /* GitHubPulse.swift in Sources */, C18227D148D99FE6EE3CC51A /* GithubPoller.swift in Sources */, C0E18D0F6F5F6D131E6066B8 /* GreetingCanvasView.swift in Sources */, + 8C5CF7C82640DD6A0685A0D5 /* ApprovalQueue.swift in Sources */, 9DD56C95ECABDDCE23652D50 /* HookServer.swift in Sources */, 530443F46F896F037A56F663 /* HotKeyCenter.swift in Sources */, 52F0DF1FBCB522951910390E /* InstructionRunner.swift in Sources */, diff --git a/NotchBuddy/Sources/App/ApprovalQueue.swift b/NotchBuddy/Sources/App/ApprovalQueue.swift new file mode 100644 index 000000000..e5c02353d --- /dev/null +++ b/NotchBuddy/Sources/App/ApprovalQueue.swift @@ -0,0 +1,85 @@ +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 + /// Host app / bundle id of the session, so the card's pill keeps showing where it runs. + var hostApp: String? = nil + var bundleId: 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 a1c537f2c..e1ea94297 100644 --- a/NotchBuddy/Sources/App/HookServer.swift +++ b/NotchBuddy/Sources/App/HookServer.swift @@ -38,13 +38,15 @@ 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 private var pendingQuestionFD: Int32 = -1 // held open while user answers AskUserQuestion private var questionFDSource: (any DispatchSourceRead)? = nil // monitors pendingQuestionFD - /// True when a real nb-hook connection is holding the approval fd open. - @MainActor var hasRealPendingApproval: Bool { pendingApprovalFD >= 0 } + // Pending approvals, FIFO. Main actor only. Each entry owns its fd, disconnect source and timeout. + private var approvalQueue = ApprovalQueue() + private var approvalResources: [UUID: ApprovalResources] = [:] + + /// True when a real nb-hook connection is holding an approval fd open. + @MainActor var hasRealPendingApproval: Bool { !approvalQueue.isEmpty } /// True when a real nb-hook connection is holding the question fd open. @MainActor var hasRealPendingQuestion: Bool { pendingQuestionFD >= 0 } private var questionPillId: String = "" // pill that owns the pending question @@ -57,38 +59,123 @@ final class HookServer: @unchecked Sendable { // MARK: - Approval fd helpers + private struct ApprovalResources { + let source: any DispatchSourceRead + let timeout: DispatchWorkItem + } + + /// Where the user is expected to answer for this pill ("Cursor", "Codex", the Claude host…). + @MainActor + private func agentLabel(for pillId: String) -> String { + switch pillId { + case "agent_cursor": return "Cursor" + case "agent_codex": return "Codex" + case "agent_copilot": return "Copilot CLI" + case "agent_muse": return "Muse Code" + case "agent_hermes": return "Hermes" + default: return claudeHostName + } + } + + @MainActor + private func handledNote(for pillId: String) -> String { "Handled in \(agentLabel(for: pillId))." } + + @MainActor + private func waitingNote(for pillId: String) -> String { "Still waiting in \(agentLabel(for: pillId))." } + + /// 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. + // The dropped request may belong to another pill than `releasedPill`: release that one too. + var affectedPills: [String] = [releasedPill] + while let head = approvalQueue.head, !Self.isAlive(fd: head.fd) { + nbLog("Dropped dead approval \(head.tool) [\(head.pillId)]") + if !affectedPills.contains(head.pillId) { affectedPills.append(head.pillId) } + if let removed = detachApproval(id: head.id) { removed.source?.cancel() } + } + // A pill with no request left goes back to work. + for pillId in affectedPills where !approvalQueue.hasEntries(forPill: pillId) { + state.updateTask(id: pillId, state: .working) + clearPillBadge(id: pillId) + } + 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, + hostApp: entry.hostApp, bundleId: entry.bundleId) + 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 ? 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 } } // MARK: - Question fd helpers @@ -410,39 +497,16 @@ final class HookServer: @unchecked Sendable { if !isExternalAgent { TurnRecorder.shared.record(event: name, payload: payload, pillId: agentId) } #endif - // 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, - pending.sessionId != "demo_session" { - let handledNote: String - switch pending.pillId { - case "agent_cursor": handledNote = "Handled in Cursor." - case "agent_codex": handledNote = "Handled in Codex." - case "agent_copilot": handledNote = "Handled in Copilot CLI." - case "agent_muse": handledNote = "Handled in Muse Code." - case "agent_hermes": handledNote = "Handled in Hermes." - default: handledNote = "Handled in \(claudeHostName)." - } - 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: handledNote(for: agentId)) // Approval dismissed — fall through so the resolving event updates state normally. } @@ -632,7 +696,7 @@ final class HookServer: @unchecked Sendable { // Approval and question always win; other alerts are blocked while a card is showing if view == .approval || view == .question { state.view = view - } else if isAlert && state.pendingApproval == nil { + } else if isAlert && approvalQueue.isEmpty { state.view = view } } else if isAlert { @@ -730,6 +794,7 @@ final class HookServer: @unchecked Sendable { let tool = payload["tool_name"] as? String ?? "Tool" let toolInput = payload["tool_input"] as? [String: Any] ?? [:] + let command = toolInput["command"] as? String ?? tool let inputKey = Self.approvalInputKey(toolInput) nbLog("PermissionRequest \(tool) [\(pillId)]") @@ -744,84 +809,65 @@ final class HookServer: @unchecked Sendable { return } - let command = toolInput["command"] as? String ?? tool - - 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, + hostApp: terminalHost?.bundleId, bundleId: bundleId) + 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, hostApp: terminalHost?.bundleId, bundleId: bundleId) - 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." - case "agent_copilot": note = "Handled in Copilot CLI." - case "agent_muse": note = "Handled in Muse Code." - case "agent_hermes": note = "Handled in Hermes." - default: note = "Handled in \(self.claudeHostName)." - } - 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 - // 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. + // 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. // Copilot/Muse use 110s (their relay waits 118s but their hook timeout is 120s, leaving little margin). let waitTimeout: Double = (isCopilotRequest || isMuseRequest) ? 110 : 115 - let captured = fd - DispatchQueue.main.asyncAfter(deadline: .now() + waitTimeout) { [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." - case "agent_copilot": note = "Still waiting in Copilot CLI." - case "agent_muse": note = "Still waiting in Muse Code." - case "agent_hermes": note = "Still waiting in Hermes." - default: note = "Still waiting in \(self.claudeHostName)." - } - self.dismissApprovalCard(note: note) + 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() + waitTimeout, 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) { - // Only intercept a demo card — a real card has a live fd (pendingApprovalFD >= 0). + func sendApprovalDecision(_ decision: String, for id: UUID) { + // Only intercept a demo card — a real card has a queue entry behind it. if DemoEngine.shared.isActive, AppState.shared.pendingApproval?.sessionId == "demo_session", - pendingApprovalFD < 0 { + approvalQueue.isEmpty { let s = AppState.shared s.pendingApproval = nil s.isPinned = false @@ -829,12 +875,9 @@ final class HookServer: @unchecked Sendable { DemoEngine.shared.handleApprovalDecision(decision) return } - 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 + 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 { @@ -844,31 +887,19 @@ 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 - RecapStore.shared.recordDecision(pillId: pillId, decision: decision) - 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 + RecapStore.shared.recordDecision(pillId: removed.entry.pillId, decision: decision) + refreshApprovalCard(releasedPill: removed.entry.pillId, emptyNote: nil) } // MARK: - Question request diff --git a/NotchBuddy/Sources/App/IslandViewContent.swift b/NotchBuddy/Sources/App/IslandViewContent.swift index 3d161ae57..39cab4a89 100644 --- a/NotchBuddy/Sources/App/IslandViewContent.swift +++ b/NotchBuddy/Sources/App/IslandViewContent.swift @@ -299,18 +299,29 @@ 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, Copilot CLI and Muse Code do not support updatedPermissions let hideAlways = approval?.pillId == "agent_codex" @@ -318,7 +329,7 @@ struct ApprovalView: View { || approval?.pillId == "agent_muse" if !hideAlways { SecondaryButton("Always") { - HookServer.shared.sendApprovalDecision("always") + if let id = shownId { HookServer.shared.sendApprovalDecision("always", for: id) } } } } diff --git a/NotchBuddy/Sources/App/PhoneLink/ApprovalRelay.swift b/NotchBuddy/Sources/App/PhoneLink/ApprovalRelay.swift index 392cd720e..7c49288ea 100644 --- a/NotchBuddy/Sources/App/PhoneLink/ApprovalRelay.swift +++ b/NotchBuddy/Sources/App/PhoneLink/ApprovalRelay.swift @@ -149,10 +149,10 @@ final class ApprovalRelay { switch decision { case "allow": log("allowed from the iPhone: \(pending.tool)") - HookServer.shared.sendApprovalDecision("allow") + HookServer.shared.sendApprovalDecision("allow", for: pending.id) case "deny": log("denied from the iPhone: \(pending.tool)") - HookServer.shared.sendApprovalDecision("deny") + HookServer.shared.sendApprovalDecision("deny", for: pending.id) default: log("unknown decision '\(decision)', ignored") } diff --git a/NotchBuddy/Sources/CoucouKit/IslandTypes.swift b/NotchBuddy/Sources/CoucouKit/IslandTypes.swift index 26217856f..d26b45e1f 100644 --- a/NotchBuddy/Sources/CoucouKit/IslandTypes.swift +++ b/NotchBuddy/Sources/CoucouKit/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/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") + } +}