diff --git a/Sources/AskKeyAppKit/App/AgentApprovalPanelActions.swift b/Sources/AskKeyAppKit/App/AgentApprovalPanelActions.swift new file mode 100644 index 0000000..4fd89f5 --- /dev/null +++ b/Sources/AskKeyAppKit/App/AgentApprovalPanelActions.swift @@ -0,0 +1,6 @@ +/// Window operations kept separate from request selection and authorization. +@MainActor +struct AgentApprovalPanelActions { + let bringForward: () -> Void + let dismiss: () -> Void +} diff --git a/Sources/AskKeyAppKit/App/AgentApprovalPanelPlacement.swift b/Sources/AskKeyAppKit/App/AgentApprovalPanelPlacement.swift new file mode 100644 index 0000000..aeb6146 --- /dev/null +++ b/Sources/AskKeyAppKit/App/AgentApprovalPanelPlacement.swift @@ -0,0 +1,28 @@ +import AppKit + +enum AgentApprovalPanelPlacement { + static func screenFrame(pointer: NSPoint, screens: [NSRect], keyWindowScreen: NSRect?, mainScreen: NSRect?) -> NSRect? { + screens.first { $0.contains(pointer) } ?? keyWindowScreen ?? mainScreen + } + + static func centeredOrigin(size: NSSize, visibleFrame: NSRect) -> NSPoint { + NSPoint( + x: visibleFrame.midX - size.width / 2, + y: min(max(visibleFrame.midY - size.height / 2, visibleFrame.minY), visibleFrame.maxY - size.height) + ) + } + + @MainActor + static func bringForward(_ panel: NSPanel) { + let screens = NSScreen.screens + let frame = screenFrame( + pointer: NSEvent.mouseLocation, screens: screens.map(\.frame), + keyWindowScreen: NSApp.keyWindow?.screen?.frame, mainScreen: NSScreen.main?.frame + ) + let screen = screens.first { $0.frame == frame } + if let visible = screen?.visibleFrame { + panel.setFrameOrigin(centeredOrigin(size: panel.frame.size, visibleFrame: visible)) + } + panel.makeKeyAndOrderFront(nil) + } +} diff --git a/Sources/AskKeyAppKit/App/AgentApprovalPresentationCoordinator.swift b/Sources/AskKeyAppKit/App/AgentApprovalPresentationCoordinator.swift new file mode 100644 index 0000000..474f492 --- /dev/null +++ b/Sources/AskKeyAppKit/App/AgentApprovalPresentationCoordinator.swift @@ -0,0 +1,123 @@ +import Foundation +import AskKeyBroker + +/// Owns the presentation queue independently of AppKit window construction. +@MainActor +final class AgentApprovalPresentationCoordinator { + private let screenState: () -> AgentApprovalScreenState + private let loadPending: () -> [BrokerPendingApproval] + private let clock: () -> Date + private let refreshExpiration: () -> Void + private let lockedReminder: (String, String) -> Void + private let feedback: (String) -> Void + private let present: (BrokerPendingApproval, BrokerApprovalDecision?, @escaping (BrokerApprovalDecision?) -> Void) -> AgentApprovalPanelActions + private let applyDecision: (BrokerPendingApproval, BrokerApprovalDecision) -> Void + private var activeRequest: BrokerPendingApproval? + private var presentationID: UUID? + private var panel: AgentApprovalPanelActions? + private var applyingDecision = false + private var deferredRequestIDs: Set = [] + var isPresenting: Bool { presentationID != nil } + + init( + screenState: @escaping () -> AgentApprovalScreenState, + loadPending: @escaping () -> [BrokerPendingApproval], + clock: @escaping () -> Date = Date.init, + refreshExpiration: @escaping () -> Void, + lockedReminder: @escaping (String, String) -> Void, + feedback: @escaping (String) -> Void, + present: @escaping (BrokerPendingApproval, BrokerApprovalDecision?, @escaping (BrokerApprovalDecision?) -> Void) -> AgentApprovalPanelActions, + applyDecision: @escaping (BrokerPendingApproval, BrokerApprovalDecision) -> Void + ) { + self.screenState = screenState + self.loadPending = loadPending + self.clock = clock + self.refreshExpiration = refreshExpiration + self.lockedReminder = lockedReminder + self.feedback = feedback + self.present = present + self.applyDecision = applyDecision + } + + func presentPendingApproval( + operationID: String? = nil, + cancelledAuthenticationDecision: BrokerApprovalDecision? = nil, + userInitiated: Bool = false + ) { + let requests: [BrokerPendingApproval] + switch AgentApprovalPrivacyPolicy.gatedRequest(screenState: screenState(), load: loadPending) { + case .lockedReminder(let title, let body): + if !applyingDecision { dismissActivePanel() } + lockedReminder(title, body) + if userInitiated { feedback("Unlock your Mac to review a pending request.") } + return + case .detailed(let loaded): + requests = (loaded ?? []).filter { $0.expiresAt.map { $0 > clock() } != false } + } + // Authentication/commit may still be running after the panel closes. + guard !applyingDecision else { return } + deferredRequestIDs.formIntersection(Set(requests.map(\.requestID))) + if let activeRequest { + let stillPending = requests.contains { $0.requestID == activeRequest.requestID && $0.capability == activeRequest.capability } + if stillPending, operationID == nil || operationID == activeRequest.request.operationID { + if userInitiated { panel?.bringForward() } + return + } + // Invalidate the old completion before closing its panel. It must + // never apply a decision to a newly selected request. + dismissActivePanel() + } + let eligible = userInitiated || operationID != nil + ? requests + : requests.filter { !deferredRequestIDs.contains($0.requestID) } + guard let pending = AgentApprovalRequestSelection.select(eligible, operationID: operationID) else { + if userInitiated { feedback("This request is no longer pending.") } + return + } + deferredRequestIDs.remove(pending.requestID) + activeRequest = pending + let id = UUID() + presentationID = id + let actions = present(pending, cancelledAuthenticationDecision) { [weak self] decision in + self?.finish(decision, pending: pending, presentationID: id) + } + if presentationID == id, !applyingDecision { panel = actions } + } + + func decisionFinished() { + activeRequest = nil + presentationID = nil + panel = nil + applyingDecision = false + } + + func refreshAfterResume() { + // Refresh badges and terminal records even while the privacy gate is + // closed; no request details need to be loaded to expire them. + refreshExpiration() + presentPendingApproval() + } + + private func finish(_ decision: BrokerApprovalDecision?, pending: BrokerPendingApproval, presentationID id: UUID) { + guard presentationID == id, !applyingDecision else { return } + panel = nil + let valid = screenState() == .unlocked + && pending.expiresAt.map { $0 > clock() } != false + if let decision, valid, loadPending().contains(where: { + $0.requestID == pending.requestID && $0.capability == pending.capability + }) { + applyingDecision = true + applyDecision(pending, decision) + return + } + decisionFinished() + if valid { deferredRequestIDs.insert(pending.requestID) } + presentPendingApproval() + } + + private func dismissActivePanel() { + let previous = panel + decisionFinished() + previous?.dismiss() + } +} diff --git a/Sources/AskKeyAppKit/App/AppDelegate+Approval.swift b/Sources/AskKeyAppKit/App/AppDelegate+Approval.swift index f351519..eda0430 100644 --- a/Sources/AskKeyAppKit/App/AppDelegate+Approval.swift +++ b/Sources/AskKeyAppKit/App/AppDelegate+Approval.swift @@ -30,7 +30,7 @@ extension AppDelegate { forName: .presentNextAgentApproval, object: nil, queue: .main ) { [weak self] note in MainActor.assumeIsolated { - self?.presentPendingApproval(operationID: note.object as? String) + self?.presentPendingApproval(operationID: note.object as? String, userInitiated: true) } } screenUnlockObserver = DistributedNotificationCenter.default.addObserver( @@ -38,48 +38,55 @@ extension AppDelegate { ) { [weak self] _ in MainActor.assumeIsolated { self?.resetLockedApprovalReminder() - self?.presentPendingApproval() + self?.approvalPresentation.refreshAfterResume() } } + approvalWakeObserver = NSWorkspace.shared.notificationCenter.addObserver( + forName: NSWorkspace.didWakeNotification, object: nil, queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.approvalPresentation.refreshAfterResume() } + } } private func presentPendingApproval( operationID: String? = nil, - cancelledAuthenticationDecision: BrokerApprovalDecision? = nil + cancelledAuthenticationDecision: BrokerApprovalDecision? = nil, + userInitiated: Bool = false ) { - guard !presentingApproval else { return } - let pending: BrokerPendingApproval - switch AgentApprovalPrivacyPolicy.gatedRequest( - screenState: Self.screenState(), - load: { - AgentApprovalRequestSelection.select( - Vault.shared.approvalRequests.pendingRequests(), - operationID: operationID + approvalPresentation.presentPendingApproval( + operationID: operationID, + cancelledAuthenticationDecision: cancelledAuthenticationDecision, + userInitiated: userInitiated + ) + } + + func makeApprovalPresentationCoordinator() -> AgentApprovalPresentationCoordinator { + AgentApprovalPresentationCoordinator( + screenState: { Self.screenState() }, + loadPending: { Vault.shared.approvalRequests.pendingRequests() }, + refreshExpiration: { Vault.shared.approvalRequests.refreshExpiration() }, + lockedReminder: { [weak self] title, body in + self?.postLockedApprovalReminder(title: title, body: body) + }, + feedback: { [weak self] message in self?.vault.errorMessage = message }, + present: { [weak self] pending, cancelledDecision, completion in + guard let self else { return AgentApprovalPanelActions(bringForward: {}, dismiss: {}) } + self.resetLockedApprovalReminder() + return self.runFrozenApprovalPanel( + request: pending.request, + expiresAt: pending.expiresAt, + pending: pending, + cancelledAuthenticationDecision: cancelledDecision, + completion: completion ) + }, + applyDecision: { [weak self] pending, decision in + self?.applyApprovalDecision(pending: pending, decision: decision) } - ) { - case .lockedReminder(let title, let body): - postLockedApprovalReminder(title: title, body: body) - return - case .detailed(let loaded): - guard let loaded else { return } - pending = loaded - resetLockedApprovalReminder() - } - presentingApproval = true + ) + } - let request = pending.request - runFrozenApprovalPanel( - request: request, - expiresAt: pending.expiresAt, - pending: pending, - cancelledAuthenticationDecision: cancelledAuthenticationDecision - ) { [weak self] decision in - guard let self else { return } - guard let decision else { - self.presentingApproval = false - return - } + private func applyApprovalDecision(pending: BrokerPendingApproval, decision: BrokerApprovalDecision) { let fileWrites = self.fileWriteCoordinator let applyDecision: @Sendable () -> Void = { [weak self, fileWrites] in var didFail = false @@ -109,7 +116,7 @@ extension AppDelegate { let cancelled = authenticationCancelled Task { @MainActor [weak self] in guard let self else { return } - self.presentingApproval = false + self.approvalPresentation.decisionFinished() if cancelled { self.presentPendingApproval( operationID: pending.request.operationID, @@ -120,7 +127,7 @@ extension AppDelegate { } // The retried request may have expired meanwhile; never leave // other requests waiting without a prompt. - if !self.presentingApproval, !Vault.shared.approvalRequests.pendingRequests().isEmpty { + if !self.approvalPresentation.isPresenting, !Vault.shared.approvalRequests.pendingRequests().isEmpty { self.presentPendingApproval() } } @@ -130,7 +137,6 @@ extension AppDelegate { } else { DispatchQueue.global(qos: .userInitiated).async(execute: applyDecision) } - } } private func runFrozenApprovalPanel( @@ -139,7 +145,7 @@ extension AppDelegate { pending: BrokerPendingApproval? = nil, cancelledAuthenticationDecision: BrokerApprovalDecision? = nil, completion: @escaping @MainActor (BrokerApprovalDecision?) -> Void = { _ in } - ) { + ) -> AgentApprovalPanelActions { var finished = false var privacyTimer: Timer? var resizeObserver: NSObjectProtocol? @@ -238,7 +244,6 @@ extension AppDelegate { hosting.sizingOptions = [.preferredContentSize] panel.contentViewController = hosting panel.setContentSize(hosting.view.fittingSize) - panel.center() // Expanding Details makes the card taller; keep its buttons on screen. resizeObserver = NotificationCenter.default.addObserver( forName: NSWindow.didResizeNotification, object: panel, queue: .main @@ -255,13 +260,16 @@ extension AppDelegate { } ?? false if Self.screenState() != .unlocked || expiresAt.map({ $0 <= Date() }) == true || requestEnded { finish(nil) - if Self.screenState() == .unlocked { self.presentPendingApproval() } } } } if let privacyTimer { RunLoop.main.add(privacyTimer, forMode: .common) } panel.level = .modalPanel - panel.makeKeyAndOrderFront(nil) + AgentApprovalPanelPlacement.bringForward(panel) + return AgentApprovalPanelActions( + bringForward: { AgentApprovalPanelPlacement.bringForward(panel) }, + dismiss: { finish(nil) } + ) } /// Moves the panel just enough to fit the screen's visible frame, keeping diff --git a/Sources/AskKeyAppKit/App/AppDelegate+Termination.swift b/Sources/AskKeyAppKit/App/AppDelegate+Termination.swift index dfa6bee..95e71df 100644 --- a/Sources/AskKeyAppKit/App/AppDelegate+Termination.swift +++ b/Sources/AskKeyAppKit/App/AppDelegate+Termination.swift @@ -21,6 +21,9 @@ extension AppDelegate { if let screenUnlockObserver { DistributedNotificationCenter.default.removeObserver(screenUnlockObserver) } + if let approvalWakeObserver { + NSWorkspace.shared.notificationCenter.removeObserver(approvalWakeObserver) + } Vault.shared.cleanupRuntimeFileDeliveries() } } diff --git a/Sources/AskKeyAppKit/App/AppDelegate.swift b/Sources/AskKeyAppKit/App/AppDelegate.swift index 7972c07..109a197 100644 --- a/Sources/AskKeyAppKit/App/AppDelegate.swift +++ b/Sources/AskKeyAppKit/App/AppDelegate.swift @@ -21,6 +21,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, ObservableObject { var statusItemMenuMonitor: Any? var approvalPresentationObserver: NSObjectProtocol? var screenUnlockObserver: NSObjectProtocol? + var approvalWakeObserver: NSObjectProtocol? var recycleBinCleanupTimer: Timer? var lockedApprovalReminderPosted = false var lockedApprovalReminderAttempt: UUID? @@ -30,7 +31,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, ObservableObject { var brokerServer: BrokerSocketServer? var fileWriteCoordinator: BrokerFileWriteCoordinator? @Published internal(set) var pendingApprovalCount = 0 - var presentingApproval = false + lazy var approvalPresentation = makeApprovalPresentationCoordinator() var launchSource = AppLaunchSource.active var managementDockPolicy = ManagementDockPolicy() var dockPolicyApplicationScheduled = false diff --git a/Sources/AskKeyAppKit/AppLanguage.swift b/Sources/AskKeyAppKit/AppLanguage.swift index 3575d41..9640be7 100644 --- a/Sources/AskKeyAppKit/AppLanguage.swift +++ b/Sources/AskKeyAppKit/AppLanguage.swift @@ -162,6 +162,8 @@ enum AppLanguage { "Keep Launch at Login", "Ask Key has pending requests", "Unlock your Mac to review a pending request.", + "This request is no longer pending.", + "Expired without decision", "Open Ask Key to review pending requests.", "Disable system authentication for read approvals", "Ask Key could not save an access record. Credential operations continue, and this warning will remain until recording succeeds.", diff --git a/Sources/AskKeyAppKit/Resources/Localizable.xcstrings b/Sources/AskKeyAppKit/Resources/Localizable.xcstrings index d0eeb93..16a9124 100644 --- a/Sources/AskKeyAppKit/Resources/Localizable.xcstrings +++ b/Sources/AskKeyAppKit/Resources/Localizable.xcstrings @@ -6429,6 +6429,52 @@ } } }, + "Expired without decision": { + "extractionState": "extracted_with_value", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Expired without decision" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "过期未处理" + } + }, + "qps-ploc": { + "stringUnit": { + "state": "translated", + "value": "[!!Expired without decision!!]" + } + } + } + }, + "This request is no longer pending.": { + "extractionState": "extracted_with_value", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "This request is no longer pending." + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "这个请求已结束" + } + }, + "qps-ploc": { + "stringUnit": { + "state": "translated", + "value": "[!!This request is no longer pending.!!]" + } + } + } + }, "Denied": { "extractionState": "extracted_with_value", "localizations": { diff --git a/Sources/AskKeyAppKit/Views/AccessRecords/AccessRecordPresentation.swift b/Sources/AskKeyAppKit/Views/AccessRecords/AccessRecordPresentation.swift index 9a31922..8cbd3a6 100644 --- a/Sources/AskKeyAppKit/Views/AccessRecords/AccessRecordPresentation.swift +++ b/Sources/AskKeyAppKit/Views/AccessRecords/AccessRecordPresentation.swift @@ -126,6 +126,7 @@ struct AccessRecordPresentation: Equatable { let isWrite = [.create, .modify, .delete].contains(event.operation) return (isWrite ? appLocalized("Approved") : appLocalized("Allowed"), .accent) case .denied: return (appLocalized("Denied"), .warning) + case .expired: return (appLocalized("Expired without decision"), .neutral) case .failed: return (appLocalized("Failed"), .warning) case .hiddenNameRejected: return (appLocalized("Hidden"), .neutral) } diff --git a/Sources/AskKeyAppKit/Views/VaultPopover.swift b/Sources/AskKeyAppKit/Views/VaultPopover.swift index 2b5d23b..acd113b 100644 --- a/Sources/AskKeyAppKit/Views/VaultPopover.swift +++ b/Sources/AskKeyAppKit/Views/VaultPopover.swift @@ -38,8 +38,9 @@ struct VaultPopover: View { systemImage: "tray", count: vault.pendingApprovalCount ) { + vault.errorMessage = nil NotificationCenter.default.post(name: .presentNextAgentApproval, object: nil) - closePopover() + if vault.errorMessage == nil { closePopover() } } Divider().padding(.horizontal, 10) menuEntry( diff --git a/Sources/AskKeyBroker/BrokerApprovalStateMachine+Expiration.swift b/Sources/AskKeyBroker/BrokerApprovalStateMachine+Expiration.swift index 0d91b68..cabc6b8 100644 --- a/Sources/AskKeyBroker/BrokerApprovalStateMachine+Expiration.swift +++ b/Sources/AskKeyBroker/BrokerApprovalStateMachine+Expiration.swift @@ -30,6 +30,12 @@ extension BrokerApprovalStateMachine { } } + /// Reconcile wall-clock deadlines after wake or screen unlock without + /// exposing pending request details or extending any authorization. + public func refreshExpiration() { + expireScheduledRequests() + } + static func validDuration(_ value: TimeInterval) -> Bool { value.isFinite && value > 0 } @@ -48,7 +54,7 @@ extension BrokerApprovalStateMachine { hasScheduledExpiration = true let delay = max(0, nextExpiration.timeIntervalSince(now)) expirationTimer.schedule( - deadline: .now() + delay, + wallDeadline: .now() + delay, repeating: .never, leeway: .milliseconds(10) ) diff --git a/Sources/AskKeyVault/Models/CredentialAccessEvent.swift b/Sources/AskKeyVault/Models/CredentialAccessEvent.swift index 023cf72..8a19329 100644 --- a/Sources/AskKeyVault/Models/CredentialAccessEvent.swift +++ b/Sources/AskKeyVault/Models/CredentialAccessEvent.swift @@ -11,7 +11,7 @@ public struct CredentialAccessEvent: Codable, Equatable, Sendable { } public enum Result: String, Codable, Sendable { - case allowed, denied, failed, hiddenNameRejected + case allowed, denied, expired, failed, hiddenNameRejected } public let timestamp: Date diff --git a/Sources/AskKeyVault/Vault.swift b/Sources/AskKeyVault/Vault.swift index 956f1bf..9d191ef 100644 --- a/Sources/AskKeyVault/Vault.swift +++ b/Sources/AskKeyVault/Vault.swift @@ -151,7 +151,7 @@ public final class Vault { if request?.operation == .organize { self.agentOrganizations.remove(operationID: operationID) if let request, state == .denied || state == .cancelled || state == .expired { - self.recordAgentOrganizationAccess(request, result: .denied) + self.recordAgentOrganizationAccess(request, result: state == .expired ? .expired : .denied) } return } @@ -171,7 +171,7 @@ public final class Vault { timestamp: self.currentDate, credentialID: approvalRequest.credentialID, operation: operation, - result: .denied, + result: state == .expired ? .expired : .denied, callerHint: approvalRequest.callerName, declaredPurpose: approvalRequest.callerPurpose, executableBasename: operation == .runtimeRead diff --git a/Tests/AskKeyAppTests/AgentApprovalPanelPlacementTests.swift b/Tests/AskKeyAppTests/AgentApprovalPanelPlacementTests.swift new file mode 100644 index 0000000..282818c --- /dev/null +++ b/Tests/AskKeyAppTests/AgentApprovalPanelPlacementTests.swift @@ -0,0 +1,34 @@ +import AppKit +import XCTest +@testable import AskKeyAppKit + +final class AgentApprovalPanelPlacementTests: XCTestCase { + private let main = NSRect(x: 0, y: 0, width: 1440, height: 900) + private let other = NSRect(x: -1920, y: 0, width: 1920, height: 1080) + + func testPointerDisplayWinsOverKeyWindowAndMainDisplay() { + XCTAssertEqual(AgentApprovalPanelPlacement.screenFrame( + pointer: NSPoint(x: -100, y: 500), screens: [main, other], keyWindowScreen: main, mainScreen: main + ), other) + } + + func testScreenSelectionFallsBackToKeyWindowThenMain() { + let pointer = NSPoint(x: 10_000, y: 10_000) + XCTAssertEqual(AgentApprovalPanelPlacement.screenFrame( + pointer: pointer, screens: [main, other], keyWindowScreen: other, mainScreen: main + ), other) + XCTAssertEqual(AgentApprovalPanelPlacement.screenFrame( + pointer: pointer, screens: [main, other], keyWindowScreen: nil, mainScreen: main + ), main) + XCTAssertNil(AgentApprovalPanelPlacement.screenFrame( + pointer: pointer, screens: [], keyWindowScreen: nil, mainScreen: nil + )) + } + + func testPanelCentersOnSelectedDisplayAndKeepsTallPanelTopVisible() { + XCTAssertEqual(AgentApprovalPanelPlacement.centeredOrigin(size: NSSize(width: 400, height: 500), visibleFrame: other), + NSPoint(x: -1160, y: 290)) + let tall = AgentApprovalPanelPlacement.centeredOrigin(size: NSSize(width: 400, height: 1200), visibleFrame: other) + XCTAssertEqual(tall.y + 1200, other.maxY) + } +} diff --git a/Tests/AskKeyAppTests/AgentApprovalPresentationCoordinatorTests.swift b/Tests/AskKeyAppTests/AgentApprovalPresentationCoordinatorTests.swift new file mode 100644 index 0000000..be1e645 --- /dev/null +++ b/Tests/AskKeyAppTests/AgentApprovalPresentationCoordinatorTests.swift @@ -0,0 +1,244 @@ +import Foundation +import XCTest +import AskKeyBroker +@testable import AskKeyAppKit + +@MainActor +final class AgentApprovalPresentationCoordinatorTests: XCTestCase { + func testLockedManagementActionBringsOpenPanelForwardOnCurrentDisplay() { + let harness = Harness() + harness.pending = [request("first")] + harness.coordinator.presentPendingApproval() + harness.currentDisplay = "other" + harness.panelCovered = true + harness.coordinator.presentPendingApproval(userInitiated: true) + XCTAssertEqual(harness.presented, ["first"]) + XCTAssertEqual(harness.panelDisplay, "other") + XCTAssertFalse(harness.panelCovered) + XCTAssertEqual(harness.focusCount, 1) + XCTAssertTrue(harness.applied.isEmpty) + } + + func testDismissalPresentsNextRequestAndDoesNotReopenDeferredRequests() { + let harness = Harness() + harness.pending = [request("first"), request("second")] + harness.coordinator.presentPendingApproval() + harness.finish?(nil) + XCTAssertTrue(harness.coordinator.isPresenting) + XCTAssertEqual(harness.presented, ["first", "second"]) + harness.finish?(nil) + XCTAssertFalse(harness.coordinator.isPresenting) + XCTAssertEqual(harness.presented, ["first", "second"]) + harness.coordinator.presentPendingApproval(userInitiated: true) + XCTAssertEqual(harness.presented, ["first", "second", "first"]) + } + + func testExpiredSelectionGivesGenericFeedback() { + let harness = Harness() + harness.pending = [request("expired", deadline: harness.now)] + harness.coordinator.presentPendingApproval(operationID: "expired", userInitiated: true) + XCTAssertEqual(harness.presented, []) + XCTAssertEqual(harness.reminders, 0) + XCTAssertEqual(harness.messages, ["This request is no longer pending."]) + } + + func testAutomaticUpdatesDoNotMoveOrFocusAnOpenPanel() { + let harness = Harness() + harness.pending = [request("first")] + harness.coordinator.presentPendingApproval() + harness.currentDisplay = "other" + harness.panelCovered = true + harness.pending.append(request("second")) + harness.coordinator.presentPendingApproval() + XCTAssertEqual(harness.presented, ["first"]) + XCTAssertEqual(harness.panelDisplay, "main") + XCTAssertTrue(harness.panelCovered) + XCTAssertEqual(harness.focusCount, 0) + } + + func testInitialAutomaticPresentationUsesCurrentDisplay() { + let harness = Harness() + harness.currentDisplay = "other" + harness.pending = [request("first")] + harness.coordinator.presentPendingApproval() + XCTAssertEqual(harness.panelDisplay, "other") + } + + func testSelectedRequestReplacesPanelAndOldCompletionCannotApproveEitherRequest() { + let harness = Harness() + harness.pending = [request("first"), request("second")] + harness.coordinator.presentPendingApproval() + let staleFinish = harness.finish + harness.coordinator.presentPendingApproval(operationID: "second", userInitiated: true) + staleFinish?(.once) + XCTAssertEqual(harness.dismissCount, 1) + XCTAssertEqual(harness.presented, ["first", "second"]) + XCTAssertTrue(harness.applied.isEmpty) + harness.finish?(.deny) + XCTAssertEqual(harness.applied, ["second"]) + } + + func testOldCompletionCannotApproveSameRequestAfterReopening() { + let harness = Harness() + harness.pending = [request("first")] + harness.coordinator.presentPendingApproval() + let staleFinish = harness.finish + harness.finish?(nil) + harness.coordinator.presentPendingApproval(userInitiated: true) + staleFinish?(.once) + XCTAssertTrue(harness.applied.isEmpty) + harness.finish?(.deny) + XCTAssertEqual(harness.applied, ["first"]) + } + + func testExpiryCompletionAdvancesToNextLiveRequest() { + let harness = Harness() + harness.pending = [request("first", deadline: harness.now.addingTimeInterval(30)), request("second")] + harness.coordinator.presentPendingApproval() + harness.now.addTimeInterval(30) + harness.finish?(nil) + XCTAssertEqual(harness.presented, ["first", "second"]) + XCTAssertTrue(harness.applied.isEmpty) + } + + func testManualActionDiscardsExpiredOpenPanelAndReportsItEnded() { + let harness = Harness() + harness.pending = [request("first", deadline: harness.now.addingTimeInterval(30))] + harness.coordinator.presentPendingApproval() + harness.now.addTimeInterval(30) + harness.coordinator.presentPendingApproval(userInitiated: true) + XCTAssertEqual(harness.dismissCount, 1) + XCTAssertFalse(harness.coordinator.isPresenting) + XCTAssertEqual(harness.messages, ["This request is no longer pending."]) + XCTAssertEqual(harness.focusCount, 0) + } + + func testLateDecisionAfterExpiryOrCancellationDoesNotApply() { + for cancelled in [false, true] { + let harness = Harness() + harness.pending = [request("first")] + harness.coordinator.presentPendingApproval() + if cancelled { harness.pending = [] } else { harness.now.addTimeInterval(301) } + harness.finish?(.once) + XCTAssertTrue(harness.applied.isEmpty) + XCTAssertFalse(harness.coordinator.isPresenting) + } + } + + func testPrivacyChangeClosesOpenPanelWithoutFocusOrDetailLoadsAndReopensOnUnlock() { + for state in [AgentApprovalScreenState.locked, .unknown] { + let harness = Harness() + harness.pending = [request("private")] + harness.coordinator.presentPendingApproval() + let loads = harness.loads + harness.screenState = state + harness.coordinator.presentPendingApproval(userInitiated: true) + XCTAssertEqual(harness.loads, loads) + XCTAssertEqual(harness.dismissCount, 1) + XCTAssertEqual(harness.focusCount, 0) + XCTAssertEqual(harness.messages, ["Unlock your Mac to review a pending request."]) + harness.screenState = .unlocked + harness.coordinator.refreshAfterResume() + XCTAssertEqual(harness.presented, ["private", "private"]) + } + } + + func testPrivacyTimerCompletionWaitsForUnlockWithoutDeferringRequest() { + let harness = Harness() + harness.pending = [request("private")] + harness.coordinator.presentPendingApproval() + let loads = harness.loads + harness.screenState = .locked + harness.finish?(.once) + XCTAssertEqual(harness.loads, loads) + XCTAssertTrue(harness.applied.isEmpty) + harness.screenState = .unlocked + harness.coordinator.refreshAfterResume() + XCTAssertEqual(harness.presented, ["private", "private"]) + } + + func testResumeRefreshRunsBeforePrivacyGateWithoutLoadingDetails() { + let harness = Harness() + harness.screenState = .locked + harness.pending = [request("private")] + harness.now.addTimeInterval(301) + harness.coordinator.refreshAfterResume() + XCTAssertEqual(harness.refreshes, 1) + XCTAssertTrue(harness.pending.isEmpty) + XCTAssertEqual(harness.loads, 0) + XCTAssertEqual(harness.reminders, 1) + } + + func testDecisionInFlightBlocksDuplicatePanelAndStaleCompletion() { + let harness = Harness() + harness.pending = [request("first"), request("second")] + harness.coordinator.presentPendingApproval() + let finish = harness.finish + finish?(.once) + finish?(.deny) + harness.coordinator.presentPendingApproval(operationID: "second", userInitiated: true) + XCTAssertEqual(harness.presented, ["first"]) + XCTAssertEqual(harness.applied, ["first"]) + } + + func testLockedAndUnknownScreensNeverLoadDetails() { + for state in [AgentApprovalScreenState.locked, .unknown] { + let harness = Harness() + harness.screenState = state + harness.pending = [request("private")] + harness.coordinator.presentPendingApproval() + XCTAssertEqual(harness.loads, 0) + XCTAssertEqual(harness.reminders, 1) + XCTAssertEqual(harness.presented, []) + } + } + + private func request(_ id: String, deadline: Date = Date(timeIntervalSince1970: 2_000_000_300)) -> BrokerPendingApproval { + .init(requestID: id, capability: "synthetic-\(id)", request: .init( + operationID: id, credentialID: id, targetID: id, + operation: .create, payloadDigest: String(repeating: "a", count: 64) + ), expiresAt: deadline) + } + + @MainActor + private final class Harness { + var screenState = AgentApprovalScreenState.unlocked + var now = Date(timeIntervalSince1970: 2_000_000_000) + var pending: [BrokerPendingApproval] = [] + var currentDisplay = "main" + var panelDisplay: String? + var panelCovered = false + var presented: [String] = [] + var finish: ((BrokerApprovalDecision?) -> Void)? + var loads = 0 + var reminders = 0 + var focusCount = 0 + var dismissCount = 0 + var refreshes = 0 + var messages: [String] = [] + var applied: [String] = [] + lazy var coordinator = AgentApprovalPresentationCoordinator( + screenState: { self.screenState }, + loadPending: { self.loads += 1; return self.pending }, + clock: { self.now }, + refreshExpiration: { + self.refreshes += 1 + self.pending.removeAll { $0.expiresAt.map { $0 <= self.now } == true } + }, + lockedReminder: { _, _ in self.reminders += 1 }, + feedback: { self.messages.append($0) }, + present: { pending, _, finish in + self.presented.append(pending.request.operationID) + self.panelDisplay = self.currentDisplay + self.panelCovered = false + self.finish = finish + return AgentApprovalPanelActions(bringForward: { + self.focusCount += 1 + self.panelDisplay = self.currentDisplay + self.panelCovered = false + }, dismiss: { self.dismissCount += 1; finish(nil) }) + }, + applyDecision: { pending, _ in self.applied.append(pending.request.operationID) } + ) + } +} diff --git a/Tests/AskKeyAppTests/AskKeyAppSource.swift b/Tests/AskKeyAppTests/AskKeyAppSource.swift index c8c9343..0d836ff 100644 --- a/Tests/AskKeyAppTests/AskKeyAppSource.swift +++ b/Tests/AskKeyAppTests/AskKeyAppSource.swift @@ -4,6 +4,9 @@ enum AskKeyAppSource { static let paths = [ "Sources/AskKeyAppKit/App/AgentApprovalGatedRequest.swift", "Sources/AskKeyAppKit/App/AgentApprovalPresentationPlan.swift", + "Sources/AskKeyAppKit/App/AgentApprovalPresentationCoordinator.swift", + "Sources/AskKeyAppKit/App/AgentApprovalPanelActions.swift", + "Sources/AskKeyAppKit/App/AgentApprovalPanelPlacement.swift", "Sources/AskKeyAppKit/App/AgentApprovalPrivacyPolicy.swift", "Sources/AskKeyAppKit/App/AgentApprovalRequestSelection.swift", "Sources/AskKeyAppKit/App/AgentApprovalScreenState.swift", diff --git a/Tests/AskKeyAppTests/ScreenPresentationTests.swift b/Tests/AskKeyAppTests/ScreenPresentationTests.swift index 8a1806f..97afba3 100644 --- a/Tests/AskKeyAppTests/ScreenPresentationTests.swift +++ b/Tests/AskKeyAppTests/ScreenPresentationTests.swift @@ -118,4 +118,18 @@ final class ScreenPresentationTests: WorkspaceVisualContractTestSupport { FrozenTimedAllowanceSettingsPresentation.offTag )) } + + func testExpiredAccessRecordHasItsOwnShortBilingualResult() { + let now = Date(timeIntervalSince1970: 2_000_000_000) + let event = CredentialAccessEvent(timestamp: now, credentialID: "synthetic", operation: .create, + result: .expired, callerHint: "Synthetic Agent", declaredPurpose: nil) + for (language, expected) in [("en", "Expired without decision"), ("zh-Hans", "过期未处理")] { // i18n-literal: Expected Simplified Chinese catalog value. + AppLanguage.current = language + let row = AccessRecordPresentation(records: [event], credentialName: { _ in "Synthetic" }, now: now).sections[0].rows[0] + XCTAssertEqual(row.resultTitle, expected) + XCTAssertEqual(row.resultRole, .neutral) + XCTAssertEqual(displayedUserMessage("This request is no longer pending.", language: language), + language == "en" ? "This request is no longer pending." : "这个请求已结束") // i18n-literal: Expected Simplified Chinese catalog value. + } + } } diff --git a/Tests/AskKeyBrokerTests/ApprovalAwayExpirationTests.swift b/Tests/AskKeyBrokerTests/ApprovalAwayExpirationTests.swift new file mode 100644 index 0000000..0844076 --- /dev/null +++ b/Tests/AskKeyBrokerTests/ApprovalAwayExpirationTests.swift @@ -0,0 +1,50 @@ +import Foundation +import XCTest +@testable import AskKeyBroker + +final class ApprovalAwayExpirationTests: ApprovalStateMachineTestCase { + func testWakeRefreshExpiresPausedTimerRequestsAndUpdatesObservers() throws { + let clock = Clock() + let counts = Counts() + let machine = BrokerApprovalStateMachine(clock: { clock.now }, pendingCountChanged: { counts.append($0) }) + machine.expirationTimer.suspend() + defer { machine.expirationTimer.resume() } + let ticket = try machine.submit(request(operationID: "away")) + machine.flushObservers() + XCTAssertEqual(counts.values, [1]) + clock.advance(301) + machine.flushObservers() + XCTAssertEqual(counts.values, [1], "No timer event or broker call refreshed the badge") + machine.refreshExpiration() + machine.flushObservers() + XCTAssertEqual(counts.values, [1, 0]) + XCTAssertTrue(machine.pendingRequests().isEmpty) + XCTAssertEqual(try machine.status(requestID: ticket.requestID, capability: ticket.capability), .expired) + XCTAssertThrowsError(try machine.decide(requestID: ticket.requestID, capability: ticket.capability, decision: .once)) + } + + func testResumeBeforeDeadlineDoesNotExtendOrEndRequest() throws { + let clock = Clock() + let machine = BrokerApprovalStateMachine(clock: { clock.now }) + let ticket = try machine.submit(request(operationID: "brief-away")) + clock.advance(30) + machine.refreshExpiration() + XCTAssertEqual(try machine.status(requestID: ticket.requestID, capability: ticket.capability), .pending) + clock.advance(270) + machine.refreshExpiration() + XCTAssertEqual(try machine.status(requestID: ticket.requestID, capability: ticket.capability), .expired) + } + + private final class Clock: @unchecked Sendable { + private let lock = NSLock() + private var value = Date(timeIntervalSince1970: 2_000_000_000) + var now: Date { lock.lock(); defer { lock.unlock() }; return value } + func advance(_ seconds: TimeInterval) { lock.lock(); defer { lock.unlock() }; value.addTimeInterval(seconds) } + } + private final class Counts: @unchecked Sendable { + private let lock = NSLock() + private var storage: [Int] = [] + var values: [Int] { lock.lock(); defer { lock.unlock() }; return storage } + func append(_ value: Int) { lock.lock(); defer { lock.unlock() }; storage.append(value) } + } +} diff --git a/Tests/AskKeyVaultTests/AgentApprovalExpiryRecordTests.swift b/Tests/AskKeyVaultTests/AgentApprovalExpiryRecordTests.swift new file mode 100644 index 0000000..d81b04f --- /dev/null +++ b/Tests/AskKeyVaultTests/AgentApprovalExpiryRecordTests.swift @@ -0,0 +1,58 @@ +import XCTest +import AskKeyBroker +@testable import AskKeyVault + +final class AgentApprovalExpiryRecordTests: AgentOrganizationTestSupport { + func testExpiredCreateIsRecordedAsExpiredWithoutCreatingCredential() throws { + let clock = MutableClock(Date(timeIntervalSince1970: 2_000_000_000)) + let machine = BrokerApprovalStateMachine(clock: { clock.now }) + let harness = try makeHarness(now: { clock.now }, approvalRequests: machine, authenticate: { _ in true }) + _ = try submitted(harness.vault.requestAgentTextWrite(.init( + operationID: "away-create", action: .create(name: "Synthetic", value: "synthetic-value") + ))) + clock.now.addTimeInterval(301) + XCTAssertTrue(machine.pendingRequests().isEmpty) + try harness.vault.beginManagementSession(using: .allow) + let records = try harness.vault.listCredentialAccessRecords() + XCTAssertEqual(records.count, 1) + XCTAssertEqual(records.first?.result, .expired) + XCTAssertTrue(try harness.vault.listTextCredentials().isEmpty) + } + + func testExpiryDenialAndCancellationKeepDistinctTerminalRecords() throws { + for state in [BrokerRequestState.expired, .denied, .cancelled] { + let clock = MutableClock(Date(timeIntervalSince1970: 2_000_000_000)) + let machine = BrokerApprovalStateMachine(clock: { clock.now }) + let harness = try makeHarness(now: { clock.now }, approvalRequests: machine, authenticate: { _ in true }) + let credential = try credential("Synthetic", group: "Original", harness: harness) + let operation = request([.renameGroup(from: "Original", to: "Changed")], id: "away-organize") + let ticket = try submitted(harness.vault.requestAgentTextWrite(operation)) + switch state { + case .expired: + clock.now.addTimeInterval(301) + machine.refreshExpiration() + try harness.vault.beginManagementSession(using: .allow) + case .denied: + _ = try machine.decide(requestID: ticket.requestID, capability: ticket.capability, decision: .deny) + case .cancelled: + _ = try harness.vault.cancelAgentTextWrite(operationID: operation.operationID, requestID: ticket.requestID, capability: ticket.capability) + default: XCTFail("Unexpected state") + } + let records = try harness.vault.listCredentialAccessRecords() + XCTAssertEqual(records.count, 1) + XCTAssertEqual(records.first?.result, state == .expired ? .expired : .denied) + XCTAssertEqual(try group(credential.id, harness: harness), "Original") + XCTAssertThrowsError(try commit(operation, harness: harness, ticket: ticket)) + machine.refreshExpiration() + XCTAssertEqual(try harness.vault.listCredentialAccessRecords().count, 1) + } + } + + func testExpiredResultRoundTripsAndOldDeniedRecordsStillDecode() throws { + for result in [CredentialAccessEvent.Result.denied, .expired] { + let event = CredentialAccessEvent(timestamp: Date(), credentialID: "synthetic", operation: .create, + result: result, callerHint: nil, declaredPurpose: nil) + XCTAssertEqual(try JSONDecoder().decode(CredentialAccessEvent.self, from: JSONEncoder().encode(event)), event) + } + } +} diff --git a/Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift b/Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift index 0e807ca..22bbf28 100644 --- a/Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift +++ b/Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift @@ -149,7 +149,8 @@ final class AgentOrganizationConcurrencyTests: AgentOrganizationTestSupport { XCTAssertThrowsError(try commit(batch, harness: harness, ticket: ticket)) XCTAssertEqual(try group(a.id, harness: harness), "Old") try harness.vault.beginManagementSession(using: .allow) - XCTAssertEqual(try harness.vault.listCredentialAccessRecords().filter { $0.operation == .modify && $0.result == .denied }.map(\.credentialID), [a.id]) + let expectedResult: CredentialAccessEvent.Result = state == .expired ? .expired : .denied + XCTAssertEqual(try harness.vault.listCredentialAccessRecords().filter { $0.operation == .modify && $0.result == expectedResult }.map(\.credentialID), [a.id]) } }