From 1182a48ee90b3088751acb8c9bf565cd02ff970d Mon Sep 17 00:00:00 2001 From: axisrow Date: Sun, 9 Aug 2026 18:02:40 +0700 Subject: [PATCH 1/3] fix: gate CLI background usage fetch on confirmed-absent OAuth creds Add directCredentialIsMissingOverride TaskLocal for deterministic testing and thread an oauthCredentialsConfirmedAbsent check into ClaudeCLIBackgroundAvailability.allowsBackgroundAutoUsageFetch so the CLI background fallback only proceeds when direct OAuth credentials are confirmed absent. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z --- .../Claude/ClaudeProviderDescriptor.swift | 29 ++- .../ClaudeBaselineCharacterizationTests.swift | 73 +++++- ...ClaudeCLIBackgroundAvailabilityTests.swift | 244 ++++++++++-------- ...ClaudeOAuthUpgradeCompatibilityTests.swift | 47 ++-- 4 files changed, 252 insertions(+), 141 deletions(-) diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift index c150c5fc9b..dc55edd1d2 100644 --- a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift @@ -528,6 +528,7 @@ struct ClaudeOAuthFetchStrategy: ProviderFetchStrategy { #if DEBUG @TaskLocal static var nonInteractiveCredentialRecordOverride: ClaudeOAuthCredentialRecord? @TaskLocal static var claudeCLIAvailableOverride: Bool? + @TaskLocal static var directCredentialIsMissingOverride: Bool? #endif private func loadNonInteractiveCredentialRecord(environment: [String: String]) -> ClaudeOAuthCredentialRecord? { @@ -546,6 +547,9 @@ struct ClaudeOAuthFetchStrategy: ProviderFetchStrategy { func directCredentialIsMissing(environment: [String: String]) -> Bool { #if DEBUG + if let override = Self.directCredentialIsMissingOverride { + return override + } if Self.nonInteractiveCredentialRecordOverride != nil { return false } @@ -986,7 +990,10 @@ struct ClaudeCLIFetchStrategy: ProviderFetchStrategy { guard let binary = ClaudeCLIResolver.resolvedBinaryPath(environment: context.env) else { return false } return ClaudeCLIBackgroundAvailability.allowsBackgroundAutoUsageFetch( binary: binary, - environment: context.env) + environment: context.env, + oauthCredentialsConfirmedAbsent: { + ClaudeOAuthFetchStrategy().directCredentialIsMissing(environment: context.env) + }) } // The interactive Claude REPL can open browser OAuth when it starts logged out. CLI-runtime paths @@ -1100,10 +1107,26 @@ enum ClaudeCLIBackgroundAvailability { || ClaudeOAuthKeychainPromptPreference.storedMode() == .always } - static func allowsBackgroundAutoUsageFetch(binary: String, environment: [String: String]) -> Bool { + /// - Parameter oauthCredentialsConfirmedAbsent: A prompt-free, no-UI probe proving the OAuth step ahead + /// of this one is durably dead (not merely denied). Consulted lazily, only when no marker exists at + /// all for this profile — a marker that *is* established but denied by prompt policy or Keychain- + /// disable revocation is a deliberate, already-adjudicated gate that this never second-guesses. + static func allowsBackgroundAutoUsageFetch( + binary: String, + environment: [String: String], + oauthCredentialsConfirmedAbsent: () -> Bool = { false }) -> Bool + { guard ProviderInteractionContext.current == .background else { return true } guard KeychainAccessGate.isExplicitlyDisabled else { - return self.allowsOpaqueChildExecution(binary: binary, environment: environment) + if self.allowsOpaqueChildExecution(binary: binary, environment: environment) { + return true + } + guard !self.isEstablished(binary: binary, environment: environment) else { return false } + // The marker gate above never gets a chance to be set when the OAuth step ahead of this one + // is durably dead: it is only recorded by a prior *successful* user-initiated CLI fetch, and a + // scheduled refresh never reaches user-initiated status. Breaking that deadlock here mirrors + // explicit OAuth mode's own absence check (`ClaudeOAuthPlanningAvailability`). + return oauthCredentialsConfirmedAbsent() } // Disable Keychain explicitly permits one owner-CLI usage attempt on a cold profile. A failed attempt // records revocation below, preventing each background timer tick from retrying until a foreground success. diff --git a/Tests/CodexBarTests/ClaudeBaselineCharacterizationTests.swift b/Tests/CodexBarTests/ClaudeBaselineCharacterizationTests.swift index 965f818f74..7a88d82ae1 100644 --- a/Tests/CodexBarTests/ClaudeBaselineCharacterizationTests.swift +++ b/Tests/CodexBarTests/ClaudeBaselineCharacterizationTests.swift @@ -262,7 +262,9 @@ struct ClaudeBaselineCharacterizationTests { } @Test - func `app background auto does not start Claude CLI before foreground availability`() async throws { + func `app background auto does not start Claude CLI while OAuth credentials may still be readable`() + async throws + { let settings = ProviderSettingsSnapshot.make(claude: .init( usageDataSource: .auto, webExtrasEnabled: false, @@ -273,10 +275,12 @@ struct ClaudeBaselineCharacterizationTests { let stubCLIPath = try self.makeStubClaudeCLI(loggedIn: false, invocationLog: invocationLog) let env = ["CLAUDE_CLI_PATH": stubCLIPath] + // Without a durable "OAuth is dead" signal, background Auto never starts CLI here: no established + // marker exists (cold launch) and there is no confirmed credential absence yet. await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { await self.withBackgroundKeychainAccess { await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - await self.withNoOAuthCredentials { + await ClaudeOAuthFetchStrategy.$directCredentialIsMissingOverride.withValue(false) { let outcome = await self.fetchOutcome( runtime: .app, sourceMode: .auto, @@ -285,12 +289,47 @@ struct ClaudeBaselineCharacterizationTests { #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli", "claude.web"]) #expect(outcome.attempts.map(\.wasAvailable) == [true, false, false]) + #expect(!FileManager.default.fileExists(atPath: invocationLog.path)) } } } } + } - #expect(!FileManager.default.fileExists(atPath: invocationLog.path)) + @Test + func `app background auto starts Claude CLI once OAuth credentials are confirmed absent`() async throws { + let settings = ProviderSettingsSnapshot.make(claude: .init( + usageDataSource: .auto, + webExtrasEnabled: false, + cookieSource: .off, + manualCookieHeader: nil)) + let invocationLog = FileManager.default.temporaryDirectory + .appendingPathComponent("claude-invocations-\(UUID().uuidString).log") + let stubCLIPath = try self.makeStubClaudeCLI(loggedIn: false, invocationLog: invocationLog) + let env = ["CLAUDE_CLI_PATH": stubCLIPath] + + // A direct, non-interactive read confirming OAuth credentials are absent (not merely + // unreadable/denied) breaks the deadlock: this is the one background-Auto case where CLI must + // start even without a prior foreground-established marker, because there is otherwise no path + // out of a durably dead OAuth step. The stub here reports logged-out, so `loadViaAutoCLI` fails + // fast on the auth-status check — but it was tried, which is the point of this fallback. + await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { + await self.withBackgroundKeychainAccess { + await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { + await self.withNoOAuthCredentials { + let outcome = await self.fetchOutcome( + runtime: .app, + sourceMode: .auto, + env: env, + settings: settings) + + #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli"]) + #expect(outcome.attempts.map(\.wasAvailable) == [true, true]) + #expect(FileManager.default.fileExists(atPath: invocationLog.path)) + } + } + } + } } @Test @@ -313,17 +352,20 @@ struct ClaudeBaselineCharacterizationTests { sourceMode: .auto, env: env, settings: settings) - #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli", "claude.web"]) - #expect(outcome.attempts.map(\.wasAvailable) == [true, false, false]) + // OAuth credentials are confirmed absent here (`withNoOAuthCredentials`), so the + // deadlock-breaker starts CLI even without a prior foreground-established marker. + // The stub is logged in and returns valid usage, so the pipeline never reaches web. + #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli"]) + #expect(outcome.attempts.map(\.wasAvailable) == [true, true]) } } } - #expect(!FileManager.default.fileExists(atPath: invocationLog.path)) + #expect(FileManager.default.fileExists(atPath: invocationLog.path)) } @Test - func `app background auto falls back to web without probing Claude CLI`() async throws { + func `app background auto starts Claude CLI over web once OAuth credentials are confirmed absent`() async throws { let settings = ProviderSettingsSnapshot.make(claude: .init( usageDataSource: .auto, webExtrasEnabled: false, @@ -362,10 +404,13 @@ struct ClaudeBaselineCharacterizationTests { } let result = try outcome.result.get() - #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli", "claude.web"]) - #expect(outcome.attempts.map(\.wasAvailable) == [true, false, true]) - #expect(result.strategyID == "claude.web") - #expect(!FileManager.default.fileExists(atPath: invocationLog.path)) + // OAuth credentials are confirmed absent (`withNoOAuthCredentials`), so the deadlock-breaker + // starts CLI even without a prior foreground-established marker. The stub is logged in and + // returns valid usage, so the manual web cookie is never needed. + #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli"]) + #expect(outcome.attempts.map(\.wasAvailable) == [true, true]) + #expect(result.strategyID == "claude.cli") + #expect(FileManager.default.fileExists(atPath: invocationLog.path)) } @Test @@ -389,7 +434,11 @@ struct ClaudeBaselineCharacterizationTests { .securityCLIExperimental) { await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.onlyOnUserAction) { - await cli.isAvailable(context) + // Present-but-denied by policy here, not a durable OAuth absence, so the + // no-established-marker deadlock-breaker must not fire for this stored policy check. + await ClaudeOAuthFetchStrategy.$directCredentialIsMissingOverride.withValue(false) { + await cli.isAvailable(context) + } } } } diff --git a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift index c6ef578184..126729dd31 100644 --- a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift +++ b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift @@ -10,16 +10,8 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - await KeychainAccessGate.withTaskOverrideForTesting(true) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.never) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(context)) - } - } - } - } + await self.withBackgroundGates(keychainDisabled: true, promptMode: .never) { + #expect(await strategy.isAvailable(context)) } } @@ -30,16 +22,9 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - await KeychainAccessGate.withTaskOverrideForTesting(false) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await !strategy.isAvailable(context)) - } - } - } - } + // Credentials are present-but-denied here, not durably absent, so the deadlock-breaker must not fire. + await self.withBackgroundGates(keychainDisabled: false, promptMode: .always, oauthCredentialsMissing: false) { + #expect(await !strategy.isAvailable(context)) } } @@ -50,17 +35,13 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - ClaudeCLIBackgroundAvailability.establish(binary: "/bin/echo", environment: context.env) - await KeychainAccessGate.withTaskOverrideForTesting(true) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.never) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(context)) - } - } - } - } + await self.withBackgroundGates( + keychainDisabled: true, + promptMode: .never, + establishedBinary: "/bin/echo", + establishedEnvironment: context.env) + { + #expect(await strategy.isAvailable(context)) } } @@ -71,17 +52,13 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - ClaudeCLIBackgroundAvailability.establish(binary: "/bin/echo", environment: context.env) - await KeychainAccessGate.withTaskOverrideForTesting(false) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.onlyOnUserAction) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await !strategy.isAvailable(context)) - } - } - } - } + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .onlyOnUserAction, + establishedBinary: "/bin/echo", + establishedEnvironment: context.env) + { + #expect(await !strategy.isAvailable(context)) } } @@ -92,17 +69,13 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - ClaudeCLIBackgroundAvailability.establish(binary: "/bin/echo", environment: context.env) - await KeychainAccessGate.withTaskOverrideForTesting(false) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(context)) - } - } - } - } + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .always, + establishedBinary: "/bin/echo", + establishedEnvironment: context.env) + { + #expect(await strategy.isAvailable(context)) } } @@ -111,16 +84,8 @@ struct ClaudeCLIBackgroundAvailabilityTests { let strategy = self.makeStrategy() let context = self.makeContext(sourceMode: .oauth) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - await KeychainAccessGate.withTaskOverrideForTesting(true) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(promptMode) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await !strategy.isAvailable(context)) - } - } - } - } + await self.withBackgroundGates(keychainDisabled: true, promptMode: promptMode) { + #expect(await !strategy.isAvailable(context)) } } @@ -135,22 +100,16 @@ struct ClaudeCLIBackgroundAvailabilityTests { throw ExpectedFetchError.failed } - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - await KeychainAccessGate.withTaskOverrideForTesting(true) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.never) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(context)) - await #expect(throws: ExpectedFetchError.self) { - try await ClaudeStatusProbe.$fetchOverride.withValue(fetchOverride) { - try await strategy.fetch(context) - } - } - #expect(await !strategy.isAvailable(context)) - } - } + // Present-but-denied throughout: a revoked marker after a failed fetch is not the same as a + // durable OAuth absence, so the deadlock-breaker must not fire. + await self.withBackgroundGates(keychainDisabled: true, promptMode: .never, oauthCredentialsMissing: false) { + #expect(await strategy.isAvailable(context)) + await #expect(throws: ExpectedFetchError.self) { + try await ClaudeStatusProbe.$fetchOverride.withValue(fetchOverride) { + try await strategy.fetch(context) } } + #expect(await !strategy.isAvailable(context)) } } @@ -178,18 +137,58 @@ struct ClaudeCLIBackgroundAvailabilityTests { let contextA = self.makeContext(environment: profileA.environment) let contextB = self.makeContext(environment: profileB.environment) - await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - ClaudeCLIBackgroundAvailability.establish(binary: "/bin/echo", environment: contextA.env) - await KeychainAccessGate.withTaskOverrideForTesting(false) { - await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(contextA)) - #expect(await !strategy.isAvailable(contextB)) - } - } - } - } + // Profile B is present-but-denied here (a different account, not a durable OAuth absence), so + // the deadlock-breaker must not fire for it. + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .always, + establishedBinary: "/bin/echo", + establishedEnvironment: contextA.env, + oauthCredentialsMissing: false) + { + #expect(await strategy.isAvailable(contextA)) + #expect(await !strategy.isAvailable(contextB)) + } + } + + @Test + func `background Auto CLI falls back without an established marker when OAuth credentials are absent`() + async throws + { + let strategy = self.makeStrategy() + let profile = try self.makeProfile(accountID: "account-a") + defer { try? FileManager.default.removeItem(at: profile.root) } + let context = self.makeContext(environment: profile.environment) + + // Keychain access enabled and prompt policy not `.always`: `allowsBackgroundAutoUsageFetch` denies + // on its own (no established marker), so a `true` result here can only come from the + // deadlock-breaker below, not from the pre-existing cold-profile opt-in path. + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .onlyOnUserAction, + oauthCredentialsMissing: true) + { + #expect(await strategy.isAvailable(context)) + } + } + + @Test + func `background Auto CLI stays blocked without an established marker when OAuth credentials are merely denied`() + async throws + { + let strategy = self.makeStrategy() + let profile = try self.makeProfile(accountID: "account-a") + defer { try? FileManager.default.removeItem(at: profile.root) } + let context = self.makeContext(environment: profile.environment) + + // Credentials exist but are transiently unreadable/denied (not a durable absence) — the + // deadlock-breaker must not fire here, only for a confirmed "credentials not found" signal. + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .onlyOnUserAction, + oauthCredentialsMissing: false) + { + #expect(await !strategy.isAvailable(context)) } } @@ -200,22 +199,21 @@ struct ClaudeCLIBackgroundAvailabilityTests { defer { try? FileManager.default.removeItem(at: profile.root) } let context = self.makeContext(environment: profile.environment) - try await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { - ClaudeCLIBackgroundAvailability.establish(binary: "/bin/echo", environment: context.env) - try await KeychainAccessGate.withTaskOverrideForTesting(false) { - try await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - try await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting("/bin/echo") { - try await ProviderInteractionContext.$current.withValue(.background) { - #expect(await strategy.isAvailable(context)) - try Data(#"{"oauthAccount":{"accountUuid":"account-b"}}"#.utf8) - .write(to: profile.configURL, options: .atomic) - #expect(await !strategy.isAvailable(context)) - try FileManager.default.removeItem(at: profile.configURL) - #expect(await !strategy.isAvailable(context)) - } - } - } - } + // Credentials are present-but-denied once the active account no longer matches the established + // marker, not durably absent, so the deadlock-breaker must not fire here. + try await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .always, + establishedBinary: "/bin/echo", + establishedEnvironment: context.env, + oauthCredentialsMissing: false) + { + #expect(await strategy.isAvailable(context)) + try Data(#"{"oauthAccount":{"accountUuid":"account-b"}}"#.utf8) + .write(to: profile.configURL, options: .atomic) + #expect(await !strategy.isAvailable(context)) + try FileManager.default.removeItem(at: profile.configURL) + #expect(await !strategy.isAvailable(context)) } } @@ -267,4 +265,40 @@ struct ClaudeCLIBackgroundAvailabilityTests { configURL: configURL, environment: ["CLAUDE_CONFIG_DIR": root.path]) } + + /// Assembles the gate stack every test in this file needs: an isolated background-availability + /// store, the Keychain-disable/prompt-policy pair under test, the resolved CLI binary, a background + /// interaction context, and (optionally) a pre-established marker or a pinned + /// `directCredentialIsMissing` answer for the OAuth-absence deadlock-breaker. + /// - Parameter oauthCredentialsMissing: `nil` (the default) leaves `directCredentialIsMissingOverride` + /// unset, matching production when no test needs to pin that specific signal. + private func withBackgroundGates( + keychainDisabled: Bool, + promptMode: ClaudeOAuthKeychainPromptMode, + binary: String = "/bin/echo", + establishedBinary: String? = nil, + establishedEnvironment: [String: String]? = nil, + oauthCredentialsMissing: Bool? = nil, + operation: () async throws -> T) async rethrows -> T + { + try await ClaudeCLIBackgroundAvailability.withIsolatedStoreForTesting { + if let establishedBinary { + ClaudeCLIBackgroundAvailability.establish( + binary: establishedBinary, + environment: establishedEnvironment ?? [:]) + } + return try await KeychainAccessGate.withTaskOverrideForTesting(keychainDisabled) { + try await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(promptMode) { + try await ClaudeCLIResolver.withResolvedBinaryPathOverrideForTesting(binary) { + try await ProviderInteractionContext.$current.withValue(.background) { + try await ClaudeOAuthFetchStrategy.$directCredentialIsMissingOverride + .withValue(oauthCredentialsMissing) { + try await operation() + } + } + } + } + } + } + } } diff --git a/Tests/CodexBarTests/ClaudeOAuthUpgradeCompatibilityTests.swift b/Tests/CodexBarTests/ClaudeOAuthUpgradeCompatibilityTests.swift index a7572fcec6..c45d59ec2b 100644 --- a/Tests/CodexBarTests/ClaudeOAuthUpgradeCompatibilityTests.swift +++ b/Tests/CodexBarTests/ClaudeOAuthUpgradeCompatibilityTests.swift @@ -451,7 +451,7 @@ struct ClaudeOAuthUpgradeCompatibilityTests { } @Test - func `background Auto does not launch owner CLI before foreground establishment`() async throws { + func `background Auto launches owner CLI once OAuth credentials are confirmed absent`() async throws { let root = try Self.makeTemporaryDirectory() defer { try? FileManager.default.removeItem(at: root) } let cli = try Self.makeFakeClaudeCLI(in: root) @@ -469,13 +469,20 @@ struct ClaudeOAuthUpgradeCompatibilityTests { #expect(binary == cli.executable.path) return Self.makeCLIUsageSnapshot() } - let outcome = try await KeychainAccessGate.withTaskOverrideForTesting(false) { - try await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { - try await ClaudeStatusProbe.$fetchOverride.withValue(cliUsage) { - try await Self.withIsolatedCredentialState(credentialsURLOverride: missingCredentials) { - try await Self.withForeignKeychainTripwires(calls: calls) { - await Self.withWebTripwires(calls: calls) { - await descriptor.fetchOutcome(context: context) + // No direct-Keychain-read consent (#2634) is the real production gate here — model it explicitly + // instead of relying on the ambient test-shortcut that `hasTaskKeychainTestingOverride` grants once + // any Keychain fixture is installed. Without consent, the non-interactive absence probe used by the + // background-Auto deadlock-breaker resolves to `.notFound` from the missing local cache/file alone, + // and never reaches the foreign Keychain tripwires below. + let outcome = try await ClaudeOAuthDirectKeychainReadConsent.withTaskOverrideForTesting(false) { + try await KeychainAccessGate.withTaskOverrideForTesting(false) { + try await ClaudeOAuthKeychainPromptPreference.withTaskOverrideForTesting(.always) { + try await ClaudeStatusProbe.$fetchOverride.withValue(cliUsage) { + try await Self.withIsolatedCredentialState(credentialsURLOverride: missingCredentials) { + try await Self.withForeignKeychainTripwires(calls: calls) { + await Self.withWebTripwires(calls: calls) { + await descriptor.fetchOutcome(context: context) + } } } } @@ -483,22 +490,20 @@ struct ClaudeOAuthUpgradeCompatibilityTests { } } - #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli", "claude.web"]) - #expect(outcome.attempts.map(\.wasAvailable) == [true, false, false]) - switch outcome.result { - case let .failure(error as ClaudeOAuthCredentialsError): - guard case .notFound = error else { - Issue.record("Expected missing OAuth credentials, got \(error)") - return - } - case let .failure(error): - Issue.record("Expected missing OAuth credentials, got \(error)") - case let .success(result): - Issue.record("Background Auto unexpectedly produced \(result.strategyID)") - } + // Without a prior foreground-established marker, background Auto now starts the owner CLI once a + // direct, non-interactive, no-prompt read confirms OAuth credentials are absent (not merely + // unreadable/denied) — the same deadlock-breaker documented on `ClaudeCLIFetchStrategy.isAvailable`. + // The fixture's owner CLI is logged in and returns real usage, so web is never consulted. + #expect(outcome.attempts.map(\.strategyID) == ["claude.oauth", "claude.cli"]) + #expect(outcome.attempts.map(\.wasAvailable) == [true, true]) + let result = try outcome.result.get() + #expect(result.strategyID == "claude.cli") #expect(calls.recordedOAuthTokens.isEmpty) #expect(calls.recordedWebCalls.isEmpty) #expect(calls.recordedForeignKeychainReads == 0) + // `ClaudeCLIFetchStrategy.fetch()` calls `loadViaCLI` directly (PTY `/usage`), not the auth-status + // preflight `loadViaAutoCLI` uses — `cliUsage` intercepts at the `ClaudeStatusProbe.fetch()` level, + // so the fixture binary is never actually spawned here and its invocation log stays empty. #expect(Self.cliInvocations(at: cli.invocationLog).isEmpty) } From 5e929409a4e5e515ac4119183acc4c1dee56e181 Mon Sep 17 00:00:00 2001 From: axisrow Date: Sun, 9 Aug 2026 18:32:59 +0700 Subject: [PATCH 2/3] fix: honor CLI background revocation before the OAuth absence fallback allowsBackgroundAutoUsageFetch's deadlock-breaker only checked isEstablished(), which drops a marker as soon as it's revoked by a failed foreground fetch. That let the OAuth-absence probe re-permit a background CLI usage attempt on every tick after a failure, defeating the existing revocation/backoff guarantee. Check isRevoked() before falling through to oauthCredentialsConfirmedAbsent so a revoked marker stays denied until the next foreground success. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z --- .../Claude/ClaudeProviderDescriptor.swift | 10 ++++++ ...ClaudeCLIBackgroundAvailabilityTests.swift | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift index dc55edd1d2..3662f6bc53 100644 --- a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift @@ -1122,6 +1122,16 @@ enum ClaudeCLIBackgroundAvailability { return true } guard !self.isEstablished(binary: binary, environment: environment) else { return false } + // A marker that was established and then revoked by a failed foreground fetch is a deliberate, + // already-adjudicated "not available right now" outcome — `isEstablished` alone can't see it, + // since revocation removes the marker from the established set. The deadlock-breaker below + // exists only for profiles that never reached user-initiated status at all; a revoked profile + // already tried and must wait for the next foreground success, not be re-permitted here. + if let marker = self.captureMarker(binary: binary, environment: environment), + self.store.isRevoked(marker) + { + return false + } // The marker gate above never gets a chance to be set when the OAuth step ahead of this one // is durably dead: it is only recorded by a prior *successful* user-initiated CLI fetch, and a // scheduled refresh never reaches user-initiated status. Breaking that deadlock here mirrors diff --git a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift index 126729dd31..8d0cc7d506 100644 --- a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift +++ b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift @@ -113,6 +113,38 @@ struct ClaudeCLIBackgroundAvailabilityTests { } } + @Test + func `enabled Keychain revocation is not bypassed by the OAuth absence deadlock breaker`() async throws { + let strategy = self.makeStrategy() + let profile = try self.makeProfile(accountID: "account-a") + defer { try? FileManager.default.removeItem(at: profile.root) } + let context = self.makeContext(environment: profile.environment) + let fetchOverride: @Sendable (String, TimeInterval, Bool) async throws + -> ClaudeStatusSnapshot = { _, _, _ in + throw ExpectedFetchError.failed + } + + // A revoked marker is a deliberate, already-adjudicated "not available right now" outcome from a + // failed foreground fetch. It must not be second-guessed by the deadlock-breaker even when OAuth + // credentials are confirmed durably absent — that escape hatch exists only for profiles that never + // reached user-initiated status at all, not for ones that tried and failed. + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .always, + establishedBinary: "/bin/echo", + establishedEnvironment: context.env, + oauthCredentialsMissing: true) + { + #expect(await strategy.isAvailable(context)) + await #expect(throws: ExpectedFetchError.self) { + try await ClaudeStatusProbe.$fetchOverride.withValue(fetchOverride) { + try await strategy.fetch(context) + } + } + #expect(await !strategy.isAvailable(context)) + } + } + @Test func `user initiated explicit OAuth retains interactive CLI recovery`() async { let strategy = self.makeStrategy() From e8182603d67b2baa1030a078e42a9dd4a740eecc Mon Sep 17 00:00:00 2001 From: axisrow Date: Sun, 9 Aug 2026 18:49:11 +0700 Subject: [PATCH 3/3] fix: require an identified profile before the OAuth absence fallback allowsBackgroundAutoUsageFetch's deadlock-breaker could still fire for a profile ClaudeAccountProfile.identifiedSessionScope can't identify (missing/malformed/unreadable account config), since isEstablished and the round-1 isRevoked check both silently return false when captureMarker is nil. That let a background CLI fetch launch with no stable account binding, and a failed attempt could never be recorded as a revocation (revoke() needs a marker), so nothing would bound repeated launches. Require a non-nil marker before consulting oauthCredentialsConfirmedAbsent, matching the fail-closed contract identifiedSessionScope already documents for background work. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01HqBGFKF9M8etkRpgz2bk2Z --- .../Claude/ClaudeProviderDescriptor.swift | 9 ++++--- ...ClaudeCLIBackgroundAvailabilityTests.swift | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift index 3662f6bc53..ffcfb62b2a 100644 --- a/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift +++ b/Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift @@ -1122,14 +1122,17 @@ enum ClaudeCLIBackgroundAvailability { return true } guard !self.isEstablished(binary: binary, environment: environment) else { return false } + // The deadlock-breaker below requires a profile CodexBar can actually identify. Without one, + // a failed attempt could never be recorded via `revoke()` (which needs a marker), so nothing + // would ever bound repeated background launches — the same fail-closed contract + // `identifiedSessionScope` documents for background work in general. + guard let marker = self.captureMarker(binary: binary, environment: environment) else { return false } // A marker that was established and then revoked by a failed foreground fetch is a deliberate, // already-adjudicated "not available right now" outcome — `isEstablished` alone can't see it, // since revocation removes the marker from the established set. The deadlock-breaker below // exists only for profiles that never reached user-initiated status at all; a revoked profile // already tried and must wait for the next foreground success, not be re-permitted here. - if let marker = self.captureMarker(binary: binary, environment: environment), - self.store.isRevoked(marker) - { + if self.store.isRevoked(marker) { return false } // The marker gate above never gets a chance to be set when the OAuth step ahead of this one diff --git a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift index 8d0cc7d506..fb63bf7aca 100644 --- a/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift +++ b/Tests/CodexBarTests/ClaudeCLIBackgroundAvailabilityTests.swift @@ -145,6 +145,33 @@ struct ClaudeCLIBackgroundAvailabilityTests { } } + @Test + func `enabled Keychain does not fall back to the OAuth absence probe for an unidentified profile`() + async throws + { + let strategy = self.makeStrategy() + // No profile is created here: the config file is verifiably absent, so + // `ClaudeAccountProfile.identifiedSessionScope` returns nil and `captureMarker` can never produce a + // marker for this environment — there is nothing to establish, revoke, or bind a background attempt to. + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("codexbar-claude-unidentified-profile-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let context = self.makeContext(environment: ["CLAUDE_CONFIG_DIR": root.path]) + + // Keychain enabled, prompt policy not `.always`, no established marker: `allowsBackgroundAutoUsageFetch` + // would deny on its own. The deadlock-breaker only exists to unblock a profile CodexBar can identify + // but has never seen a successful foreground fetch for — it must not fire for a profile with no + // identity at all, since a failed attempt here could never be recorded as a revocation. + await self.withBackgroundGates( + keychainDisabled: false, + promptMode: .onlyOnUserAction, + oauthCredentialsMissing: true) + { + #expect(await !strategy.isAvailable(context)) + } + } + @Test func `user initiated explicit OAuth retains interactive CLI recovery`() async { let strategy = self.makeStrategy()