diff --git a/CHANGELOG.md b/CHANGELOG.md index 811d62722a..337802ea7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Settings: split provider pane "Settings" sections into "Menu bar" and "Connection" so metric pickers and auth/cookie/source controls are grouped by topic. ### Fixed +- Claude OAuth: remember an acknowledged CodexBar Keychain explanation for six hours without suppressing macOS authorization or either Keychain opt-out (#1990). Thanks @harjothkhara! - Claude: prevent CodexBar's passive CLI probes from starting background Claude Code updates, avoiding repeated partial downloads when a probe exits before an update completes. Thanks @PG2047! - Codex cost history: bound malformed session-metadata lines and release read chunks promptly, preventing metadata pre-scans from retaining memory in proportion to oversized JSONL records. Thanks @Yuxin-Qiao! - Widgets: add Cursor to configurable and switcher widgets with accurate legacy Requests and current Total, Auto, and API quota labels (#2040). Thanks @Zihao-Qi! diff --git a/Sources/CodexBarCore/KeychainAccessPreflight.swift b/Sources/CodexBarCore/KeychainAccessPreflight.swift index d9523abf6f..e203a5ed3b 100644 --- a/Sources/CodexBarCore/KeychainAccessPreflight.swift +++ b/Sources/CodexBarCore/KeychainAccessPreflight.swift @@ -46,11 +46,18 @@ public enum KeychainPromptHandler { public nonisolated(unsafe) static var handler: ((KeychainPromptContext) -> Void)? public static func notify(_ context: KeychainPromptContext) { + _ = self.notifyIfHandled(context) + } + + @discardableResult + static func notifyIfHandled(_ context: KeychainPromptContext) -> Bool { if let taskHandlerStore { taskHandlerStore.handler(context) - return + return true } - self.handler?(context) + guard let handler else { return false } + handler(context) + return true } #if DEBUG diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials+TestingOverrides.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials+TestingOverrides.swift index 1a38f38aaf..4510bab8a2 100644 --- a/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials+TestingOverrides.swift +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials+TestingOverrides.swift @@ -105,15 +105,21 @@ extension ClaudeOAuthCredentialsStore { static func withIsolatedMemoryCacheForTesting(operation: () throws -> T) rethrows -> T { let store = MemoryCacheStore() - return try self.$taskMemoryCacheStoreOverride.withValue(store) { - try operation() + let preAlertStore = ClaudeOAuthKeychainPreAlertGate.StateStore() + return try ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(preAlertStore) { + try self.$taskMemoryCacheStoreOverride.withValue(store) { + try operation() + } } } static func withIsolatedMemoryCacheForTesting(operation: () async throws -> T) async rethrows -> T { let store = MemoryCacheStore() - return try await self.$taskMemoryCacheStoreOverride.withValue(store) { - try await operation() + let preAlertStore = ClaudeOAuthKeychainPreAlertGate.StateStore() + return try await ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(preAlertStore) { + try await self.$taskMemoryCacheStoreOverride.withValue(store) { + try await operation() + } } } diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift index c34f5008b5..bef352fe69 100644 --- a/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift @@ -423,11 +423,13 @@ public enum ClaudeOAuthCredentialsStore { } if ClaudeOAuthCredentialsStore.shouldNotifyClaudeKeychainPreAlert() { - KeychainPromptHandler.notify( - KeychainPromptContext( - kind: .claudeOAuth, - service: ClaudeOAuthCredentialsStore.claudeKeychainService, - account: nil)) + ClaudeOAuthKeychainPreAlertGate.presentIfNeeded { + KeychainPromptHandler.notifyIfHandled( + KeychainPromptContext( + kind: .claudeOAuth, + service: ClaudeOAuthCredentialsStore.claudeKeychainService, + account: nil)) + } } let keychainData: Data = if shouldPreferSecurityCLIKeychainRead { try ClaudeOAuthCredentialsStore.loadFromClaudeKeychainUsingSecurityFramework( diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthKeychainPreAlertGate.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthKeychainPreAlertGate.swift new file mode 100644 index 0000000000..285947a604 --- /dev/null +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthKeychainPreAlertGate.swift @@ -0,0 +1,169 @@ +import Foundation + +#if os(macOS) +import os.lock + +enum ClaudeOAuthKeychainPreAlertGate { + fileprivate struct State { + var loaded = false + var acknowledgedUntil: Date? + var presentationInFlight = false + } + + private static let lock = OSAllocatedUnfairLock(initialState: State()) + private static let defaultsKey = "claudeOAuthKeychainPreAlertAcknowledgedUntilV1" + static let cooldownInterval: TimeInterval = 60 * 60 * 6 + + #if DEBUG + final class StateStore: @unchecked Sendable { + fileprivate let lock = OSAllocatedUnfairLock(initialState: State(loaded: true)) + } + + @TaskLocal private static var taskStateStoreOverrideForTesting: StateStore? + #endif + + /// Presents at most one explanatory alert and starts the cooldown only when it reaches a handler. + @discardableResult + static func presentIfNeeded( + now: Date = Date(), + completedAt: Date? = nil, + present: () -> Bool) -> Bool + { + guard self.beginPresentation(now: now) else { return false } + let wasPresented = present() + self.finishPresentation(wasPresented: wasPresented, now: completedAt ?? Date()) + return wasPresented + } + + private static func beginPresentation(now: Date) -> Bool { + #if DEBUG + if let store = self.taskStateStoreOverrideForTesting { + return store.lock.withLock { state in + self.reservePresentation(state: &state, now: now) + } + } + #endif + return self.lock.withLock { state in + self.loadIfNeeded(&state) + guard self.reservePresentation(state: &state, now: now) else { return false } + self.persist(state) + return true + } + } + + private static func finishPresentation(wasPresented: Bool, now: Date) { + #if DEBUG + if let store = self.taskStateStoreOverrideForTesting { + store.lock.withLock { state in + self.completePresentation(state: &state, wasPresented: wasPresented, now: now) + } + return + } + #endif + self.lock.withLock { state in + self.loadIfNeeded(&state) + self.completePresentation(state: &state, wasPresented: wasPresented, now: now) + self.persist(state) + } + } + + #if DEBUG + static func withStateStoreOverrideForTesting( + _ store: StateStore?, + operation: () throws -> T) rethrows -> T + { + try self.$taskStateStoreOverrideForTesting.withValue(store) { + try operation() + } + } + + static func withStateStoreOverrideForTesting( + _ store: StateStore?, + operation: () async throws -> T) async rethrows -> T + { + try await self.$taskStateStoreOverrideForTesting.withValue(store) { + try await operation() + } + } + + static func resetForTesting() { + self.lock.withLock { state in + state = State(loaded: true) + UserDefaults.standard.removeObject(forKey: self.defaultsKey) + } + } + + static func resetInMemoryForTesting() { + self.lock.withLock { state in + state = State() + } + } + #endif + + private static func loadIfNeeded(_ state: inout State) { + guard !state.loaded else { return } + state.loaded = true + if let raw = UserDefaults.standard.object(forKey: self.defaultsKey) as? Double { + state.acknowledgedUntil = Date(timeIntervalSince1970: raw) + } + } + + private static func reservePresentation(state: inout State, now: Date) -> Bool { + guard !state.presentationInFlight else { return false } + if let acknowledgedUntil = state.acknowledgedUntil, acknowledgedUntil > now { + return false + } + state.acknowledgedUntil = nil + state.presentationInFlight = true + return true + } + + private static func completePresentation(state: inout State, wasPresented: Bool, now: Date) { + state.presentationInFlight = false + if wasPresented { + state.acknowledgedUntil = now.addingTimeInterval(self.cooldownInterval) + } + } + + private static func persist(_ state: State) { + if let acknowledgedUntil = state.acknowledgedUntil { + UserDefaults.standard.set(acknowledgedUntil.timeIntervalSince1970, forKey: self.defaultsKey) + } else { + UserDefaults.standard.removeObject(forKey: self.defaultsKey) + } + } +} +#else +enum ClaudeOAuthKeychainPreAlertGate { + static let cooldownInterval: TimeInterval = 60 * 60 * 6 + + #if DEBUG + final class StateStore: @unchecked Sendable {} + #endif + + @discardableResult + static func presentIfNeeded( + now _: Date = Date(), + completedAt _: Date? = nil, + present _: () -> Bool) -> Bool + { + false + } + + #if DEBUG + static func withStateStoreOverrideForTesting( + _: StateStore?, + operation: () throws -> T) rethrows -> T + { + try operation() + } + + static func withStateStoreOverrideForTesting( + _: StateStore?, + operation: () async throws -> T) async rethrows -> T + { + try await operation() + } + #endif +} +#endif diff --git a/Tests/CodexBarTests/ClaudeOAuthCredentialsStorePromptPolicyTests.swift b/Tests/CodexBarTests/ClaudeOAuthCredentialsStorePromptPolicyTests.swift index 8fbb5e8e53..118f0c3694 100644 --- a/Tests/CodexBarTests/ClaudeOAuthCredentialsStorePromptPolicyTests.swift +++ b/Tests/CodexBarTests/ClaudeOAuthCredentialsStorePromptPolicyTests.swift @@ -4,6 +4,12 @@ import Testing @Suite(.serialized) struct ClaudeOAuthCredentialsStorePromptPolicyTests { + @Test + func `keychain prompt notify preserves its void function signature`() { + let notify: (KeychainPromptContext) -> Void = KeychainPromptHandler.notify + _ = notify + } + private func makeCredentialsData(accessToken: String, expiresAt: Date, refreshToken: String? = nil) -> Data { let millis = Int(expiresAt.timeIntervalSince1970 * 1000) let refreshField: String = { @@ -127,7 +133,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { } @Test - func `user initiated claude keychain read shows pre alert even when preflight allows`() throws { + func `user initiated claude keychain reads respect pre alert acknowledgement cooldown`() throws { let service = "com.steipete.codexbar.cache.tests.\(UUID().uuidString)" try KeychainCacheStore.withServiceOverrideForTesting(service) { try KeychainAccessGate.withTaskOverrideForTesting(false) { @@ -158,7 +164,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { let promptHandler: (KeychainPromptContext) -> Void = { _ in preAlertHits += 1 } - let creds = try KeychainAccessPreflight.withCheckGenericPasswordOverrideForTesting( + let credentials = try KeychainAccessPreflight.withCheckGenericPasswordOverrideForTesting( preflightOverride, operation: { try KeychainPromptHandler.withHandlerForTesting(promptHandler, operation: { @@ -170,17 +176,23 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { data: keychainData, fingerprint: nil) { - try ClaudeOAuthCredentialsStore.load( + let first = try ClaudeOAuthCredentialsStore.load( + environment: [:], + allowKeychainPrompt: true) + ClaudeOAuthCredentialsStore.invalidateCache() + let second = try ClaudeOAuthCredentialsStore.load( environment: [:], allowKeychainPrompt: true) + return (first, second) } } } }) }) - #expect(creds.accessToken == "keychain-token") - #expect(preAlertHits >= 1) + #expect(credentials.0.accessToken == "keychain-token") + #expect(credentials.1.accessToken == "keychain-token") + #expect(preAlertHits == 1) } } } @@ -241,9 +253,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { }) #expect(creds.accessToken == "keychain-token") - // TODO: tighten this to `== 1` once keychain pre-alert delivery is deduplicated/scoped. - // This path can currently emit more than one pre-alert during a single load attempt. - #expect(preAlertHits >= 1) + #expect(preAlertHits == 1) } } } @@ -304,9 +314,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { }) #expect(creds.accessToken == "keychain-token") - // TODO: tighten this to `== 1` once keychain pre-alert delivery is deduplicated/scoped. - // This path can currently emit more than one pre-alert during a single load attempt. - #expect(preAlertHits >= 1) + #expect(preAlertHits == 1) } } } @@ -434,7 +442,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { }) #expect(creds.accessToken == "fallback-token") - #expect(preAlertHits >= 1) + #expect(preAlertHits == 1) } } } @@ -725,7 +733,7 @@ struct ClaudeOAuthCredentialsStorePromptPolicyTests { }) #expect(creds.accessToken == "fallback-token") - #expect(preAlertHits >= 1) + #expect(preAlertHits == 1) } } } diff --git a/Tests/CodexBarTests/ClaudeOAuthKeychainPreAlertGateTests.swift b/Tests/CodexBarTests/ClaudeOAuthKeychainPreAlertGateTests.swift new file mode 100644 index 0000000000..4c25ebfa89 --- /dev/null +++ b/Tests/CodexBarTests/ClaudeOAuthKeychainPreAlertGateTests.swift @@ -0,0 +1,96 @@ +import Foundation +import Testing +@testable import CodexBarCore + +#if os(macOS) +@Suite(.serialized) +struct ClaudeOAuthKeychainPreAlertGateTests { + @Test + func `acknowledgement suppresses repeated presentation until cooldown expires`() { + let store = ClaudeOAuthKeychainPreAlertGate.StateStore() + ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(store) { + let now = Date(timeIntervalSince1970: 1000) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now, completedAt: now) { true }) + + #expect( + ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: now.addingTimeInterval(ClaudeOAuthKeychainPreAlertGate.cooldownInterval - 1), + completedAt: now, + present: { true }) == false) + #expect( + ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: now.addingTimeInterval(ClaudeOAuthKeychainPreAlertGate.cooldownInterval + 1), + completedAt: now, + present: { true })) + } + } + + @Test + func `cooldown starts when presentation completes`() { + let store = ClaudeOAuthKeychainPreAlertGate.StateStore() + ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(store) { + let startedAt = Date(timeIntervalSince1970: 1000) + let completedAt = Date(timeIntervalSince1970: 2000) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: startedAt, + completedAt: completedAt, + present: { true })) + + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: startedAt.addingTimeInterval(ClaudeOAuthKeychainPreAlertGate.cooldownInterval + 1), + completedAt: completedAt, + present: { true }) == false) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: completedAt.addingTimeInterval(ClaudeOAuthKeychainPreAlertGate.cooldownInterval + 1), + completedAt: completedAt, + present: { true })) + } + } + + @Test + func `missing prompt handler does not consume acknowledgement cooldown`() { + let store = ClaudeOAuthKeychainPreAlertGate.StateStore() + ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(store) { + let now = Date(timeIntervalSince1970: 2000) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now, completedAt: now) { false } == false) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now, completedAt: now) { true }) + } + } + + @Test + func `duplicate while presentation is in flight is suppressed`() { + let store = ClaudeOAuthKeychainPreAlertGate.StateStore() + ClaudeOAuthKeychainPreAlertGate.withStateStoreOverrideForTesting(store) { + let now = Date(timeIntervalSince1970: 3000) + var nestedPresentationRan = false + var nestedResult: Bool? + let outerResult = ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now, completedAt: now) { + nestedResult = ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now) { + nestedPresentationRan = true + return true + } + return true + } + #expect(outerResult) + #expect(nestedResult == false) + #expect(nestedPresentationRan == false) + } + } + + @Test + func `acknowledgement persists across in memory reset`() { + ClaudeOAuthKeychainPreAlertGate.resetForTesting() + defer { ClaudeOAuthKeychainPreAlertGate.resetForTesting() } + + let now = Date(timeIntervalSince1970: 4000) + #expect(ClaudeOAuthKeychainPreAlertGate.presentIfNeeded(now: now, completedAt: now) { true }) + ClaudeOAuthKeychainPreAlertGate.resetInMemoryForTesting() + + #expect( + ClaudeOAuthKeychainPreAlertGate.presentIfNeeded( + now: now.addingTimeInterval(ClaudeOAuthKeychainPreAlertGate.cooldownInterval - 1), + completedAt: now, + present: { true }) == false) + } +} +#endif diff --git a/docs/keychain-prompts.md b/docs/keychain-prompts.md index 3b9c57d248..60f4d17f1c 100644 --- a/docs/keychain-prompts.md +++ b/docs/keychain-prompts.md @@ -20,6 +20,10 @@ Before a Keychain read that may require interaction, CodexBar shows an explanati **Learn More** opens this page without dismissing that explanation or starting the macOS prompt. Choose **OK** only when you are ready to continue, or use the opt-out below. +After you acknowledge the Claude OAuth explanation, CodexBar does not repeat that explanation for six hours. This +cooldown only applies to CodexBar's explanatory alert: macOS can still show its own Keychain authorization prompt, +and the Claude **Never prompt** and global **Disable Keychain access** settings remain in effect. + ## If the prompt appears after uninstalling CodexBar Deleting `CodexBar.app` prevents a new process from launching from that bundle, but it does not terminate a process