Fix Claude never-prompt Keychain cache bypass (#1991)#2011
Conversation
When users choose Avoid Keychain prompts, skip CodexBar-owned oauth.claude cache and Claude keychain presence probes that could still trigger macOS Keychain UI during background refresh. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 2:11 PM ET / 18:11 UTC. Summary Reproducibility: yes. source-backed: the linked issue includes affected-machine diagnostics, and current main reads the CodexBar-owned Claude OAuth cache before file fallback even when prompt mode is Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the focused cache-bypass fix after redacted affected-app runtime proof or an explicit maintainer proof override confirms that Do we have a high-confidence way to reproduce the issue? Yes, source-backed: the linked issue includes affected-machine diagnostics, and current main reads the CodexBar-owned Claude OAuth cache before file fallback even when prompt mode is Is this the best way to solve the issue? Yes for the code shape: gating the Claude-specific CodexBar cache above AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b039d15e125. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a42ab76ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| private static func clearCacheKeychain() { | ||
| guard self.shouldUseCodexBarOAuthKeychainCache else { return } |
There was a problem hiding this comment.
When prompt mode is .never, invalidateCache() still clears memory but this guard makes the persistent oauth.claude cache survive explicit login/logout invalidation. If the user later switches back to onlyOnUserAction/always, loadRecord consults the CodexBar keychain cache before the credentials file, so a pre-logout/pre-reauth token can be resurrected. Avoid consuming the invalidation without either clearing the item when safe or recording a tombstone/pending clear so the cache cannot be used after prompt mode changes.
Useful? React with 👍 / 👎.
Do not save the file fingerprint while CodexBar OAuth Keychain cache work is skipped, so switching back to a prompt-enabled mode can clear stale oauth.claude entries before load. Co-authored-by: Cursor <cursoragent@cursor.com>
clawsweeper
Bot
removed
rating: 🧂 unranked krab
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f37327eabd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var cacheTemporarilyUnavailable = false | ||
|
|
||
| switch KeychainCacheStore.load(key: ClaudeOAuthCredentialsStore.cacheKey, as: CacheEntry.self) { | ||
| switch ClaudeOAuthCredentialsStore.loadCodexBarOAuthKeychainCache() { |
There was a problem hiding this comment.
The cache bypass added here is only applied once execution reaches KeychainCacheStore; loadRecord returns the 30-minute memory cache before this helper, and a prior non-never load from the CodexBar OAuth keychain cache populates that memory cache. If the user switches to .never while the app is running, subsequent loads can still return that cache-derived credential before checking the credentials file, so the never-mode bypass can serve stale keychain-cache data until the memory TTL expires or file invalidation happens. Consider clearing/ignoring memory cache when this predicate is false, or preserving provenance and rejecting cache-derived memory records.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| private static var shouldUseCodexBarOAuthKeychainCache: Bool { | ||
| ClaudeOAuthKeychainPromptPreference.storedMode() != .never |
There was a problem hiding this comment.
With this predicate returning false for .never, loadRecord no longer sees CodexBar OAuth keychain entries in that mode, but Tests/CodexBarTests/ClaudeOAuthCredentialsStoreCLIStorageOwnershipTests.swift still asserts a .never load returns a .cacheKeychain record at lines 481-496. In that test setup there is no credentials file and repair is disabled, so the call now throws .notFound and leaves the suite red; update the old expectation (for example to run under a non-never mode) alongside this behavior change.
Useful? React with 👍 / 👎.
| } else { | ||
| shouldClearKeychainCache = true | ||
| shouldSaveFileFingerprint = false |
There was a problem hiding this comment.
When prompt mode is .never, this path deliberately skips saving the new credentials-file fingerprint, but invalidateCacheIfCredentialsFileChanged() still returns true. UsageStore.captureClaudeRefreshAuthState treats that return value as a fresh credential change and clears Claude credential-derived state on each refresh (see UsageStore+Refresh.swift lines 445-447 and 225-226), so a user who starts in never mode or changes the credentials file while in never mode will repeatedly reset Claude state until they leave the mode. Preserve the pending keychain-cache invalidation separately from the public “changed” return value, or save enough fingerprint state to make subsequent calls return false while the file is unchanged.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Defer explicit invalidateCache keychain deletes while Keychain prompts are disabled, then flush the stale oauth.claude entry before the next prompt-enabled load. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a399deac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.clearPendingCodexBarOAuthKeychainCacheClear() | ||
| KeychainCacheStore.clear(key: self.cacheKey) |
There was a problem hiding this comment.
When a pending clear is flushed after leaving .never, this removes the tombstone before verifying that KeychainCacheStore.clear actually deleted the old cache item. If the delete is temporarily unavailable (for example errSecInteractionNotAllowed while the keychain is locked), the flag is gone but the stale pre-logout/pre-file-change OAuth cache remains, so a later unlocked read can resurrect it. Keep the pending marker until clearResult reports either removed or missing, and avoid reading the cache after a failed clear.
Useful? React with 👍 / 👎.
| guard !keychainAccessDisabled || self.isolatedSecurityCLIKeychainPath(environment: environment) != nil else { | ||
| return false | ||
| } | ||
| guard ClaudeOAuthKeychainPromptPreference.storedMode() != .never else { return false } |
There was a problem hiding this comment.
When the user has selected the experimental security(1) reader while their stored prompt preference is .never, ClaudeOAuthKeychainPromptPreference.current() intentionally treats that reader as .always and readRawClaudeKeychainPayloadViaSecurityCLIIfEnabled is non-interactive. This new stored-mode guard skips the MCP-only probe anyway, so background delegated refresh no longer takes the existing MCP-only short-circuit and instead runs the Claude CLI retry path for an MCP-only keychain. Gate only the Security.framework path here, or use the effective reader mode instead of the stored prompt preference.
Useful? React with 👍 / 👎.
Skip keychain-derived memory cache under never prompt, save credentials-file fingerprints while deferring keychain clears, and align ownership tests. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c340b40d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| historyOwnerIdentifier: String? = nil) | ||
| { | ||
| guard self.shouldUseCodexBarOAuthKeychainCache else { return } | ||
| self.clearPendingCodexBarOAuthKeychainCacheClear() |
There was a problem hiding this comment.
When a clear was deferred while prompt mode was .never, a later direct sync path such as syncFromClaudeKeychainWithoutPrompt can call this helper before any cache load flushes the pending clear. Removing the marker before KeychainCacheStore.store means a temporary store failure, for example while the CodexBar cache keychain is locked, leaves the stale pre-logout/pre-file-change cache item in place with no tombstone, so a subsequent load can reuse it. Clear the marker only after a successful replacement store, or delete/flush the stale item first and keep the marker on failure.
Useful? React with 👍 / 👎.
Retry tombstone flushes until keychain delete succeeds, fall back to the credentials file while pending, and keep MCP-only detection for the experimental security CLI reader under stored never prompt mode. Co-authored-by: Cursor <cursoragent@cursor.com>
clawsweeper
Bot
removed
status: 📣 needs proof
Flush deferred clears before writing, clear the tombstone only after storeResult succeeds, and re-mark pending when replacement fails. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed all seven @chatgpt-codex-connector P2 inline comments in commits Updated the PR body with redacted macOS behavior proof from isolated test-keychain runs (no live Keychain UI per @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Live macOS behavior proof (#1991 / #2025)Reproduced on a local machine against the installed Homebrew/Sparkle app Setup
/Applications/CodexBar.app/Contents/Helpers/CodexBarCLI usage --provider claude --source oauthResult
Repro helperAdded a small helper script on my branch: ./Scripts/repro_keychain_issues.sh 2025It seeds the cache, switches to Notes
|
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@clawsweeper re-review — added live macOS repro proof in the previous comment and pushed |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a41e5823d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| switch KeychainCacheStore.load(key: ClaudeOAuthCredentialsStore.cacheKey, as: CacheEntry.self) { | ||
| switch ClaudeOAuthCredentialsStore.loadCodexBarOAuthKeychainCache() { |
There was a problem hiding this comment.
When this wrapper returns .temporarilyUnavailable because flushPendingCodexBarOAuthKeychainCacheClearIfNeeded() failed, the cache is known stale from an explicit never-mode invalidation rather than merely unreadable, but the existing branch below still makes hasCachedCredentials return true. ClaudeUsageFetcher.OAuthExecutor.load uses that boolean to disable OAuth keychain bootstrap prompts, so after a user invalidates/logout in .never, switches back, and the first cache delete is temporarily unavailable, the app can treat Claude as cached even though loadRecord will not use the stale item. Consider distinguishing pending-clear failure from an ordinary keychain read outage here and returning false for the pending-clear case.
Useful? React with 👍 / 👎.
| else | ||
| rm -f "${HOME}/.claude/.credentials.json" | ||
| fi |
There was a problem hiding this comment.
When this helper is run as Scripts/repro_keychain_issues.sh 2024, repro_2025 never creates .credentials.json.repro-backup, but the EXIT trap still reaches this else branch and removes the user's real ~/.claude/.credentials.json. The cleanup should only remove the synthetic file when this invocation created it, or snapshot/restore the original credentials file for every mode.
Useful? React with 👍 / 👎.
| else | ||
| rm -f "${HOME}/.claude/.credentials.json" | ||
| fi | ||
| defaults delete com.steipete.codexbar claudeOAuthKeychainPromptMode 2>/dev/null || true |
There was a problem hiding this comment.
This cleanup unconditionally deletes the real claudeOAuthKeychainPromptMode default instead of restoring the value that was present before the repro. In 2024 mode the script never changes that setting, and in 2025/all it overwrites it with onlyOnUserAction/never, so running the helper can leave CodexBar with a different keychain prompt policy than the user configured.
Useful? React with 👍 / 👎.
| rm -f "${HOME}/.claude/.credentials.json" | ||
| fi | ||
| defaults delete com.steipete.codexbar claudeOAuthKeychainPromptMode 2>/dev/null || true | ||
| security delete-generic-password -s "com.steipete.codexbar.cache" -a "oauth.claude" 2>/dev/null || true |
There was a problem hiding this comment.
This cleanup always deletes the real CodexBar oauth.claude keychain item; in 2024 mode the script never seeds that item, so it removes whatever cache the user already had, and in 2025/all the seeded entry overwrites the old one without any backup. For users whose usable Claude OAuth state is only in CodexBar's cache, running the repro helper can force re-authentication; use an isolated service/keychain or save and restore the previous item.
Useful? React with 👍 / 👎.
Treat stale oauth.claude cache as absent when a never-mode pending clear flush fails, and make the live repro helper restore credentials, prompt mode, and keychain cache instead of deleting user state. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cb19cc030
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| security add-generic-password \ | ||
| -s "com.steipete.codexbar.cache" \ | ||
| -a "oauth.claude" \ | ||
| -l "CodexBar Cache" \ | ||
| -w "$(cat "$CACHE_BACKUP")" 2>/dev/null || true |
There was a problem hiding this comment.
Fresh evidence in this revision is that the cache is now backed up and re-added, but the restore path only writes the saved secret; security add-generic-password documents -T appPath as the way to specify apps that may access the item, and by default the creating app is trusted. When a user already had an oauth.claude cache and runs this repro, cleanup deletes it and recreates it without the CodexBar/CodexBarCLI trusted-app entries used by the seeding path, so subsequent app reads can prompt or fail even though the script claims to restore the cache.
Useful? React with 👍 / 👎.
| repro_2025 || true | ||
| repro_2024 || true |
There was a problem hiding this comment.
When the helper is run with its default all mode, these || true handlers discard each repro's failure status, so the script exits 0 after cleanup even if both commands finish early and wait_for_hang returns a miss. That makes automation or manual checks report a successful reproduction run when neither expected Keychain hang was observed; keep continuing to the second repro, but accumulate the statuses and return nonzero when the selected repros miss.
Useful? React with 👍 / 👎.
| set_prompt_mode() { | ||
| defaults write com.steipete.codexbar claudeOAuthKeychainPromptMode "$1" | ||
| PROMPT_MODE_CHANGED=1 |
There was a problem hiding this comment.
When reproducing #2025, this writes the app preferences domain, but the launched process is CodexBarCLI; the Claude prompt preference is resolved through ClaudeOAuthKeychainPromptPreference.storedMode() from UserDefaults.standard, and CLI startup does not switch standard defaults to com.steipete.codexbar (CLIHelpers explicitly opens that suite only for unrelated display options). On a normal packaged helper the CLI therefore keeps its default .onlyOnUserAction, so the never-mode cache bypass path being tested is not actually exercised unless the CLI domain happens to have the same value.
Useful? React with 👍 / 👎.
|
|
||
| repro_2024() { | ||
| log "=== #2024: browser Safe Storage via Claude web cookie import ===" | ||
| defaults delete com.steipete.codexbar browserCookieAccessDeniedUntil 2>/dev/null || true |
There was a problem hiding this comment.
When 2024 or the default all mode is selected, this deletes the real browserCookieAccessDeniedUntil default to force a Safe Storage attempt, but cleanup() never snapshots or restores it. If a user had recently denied browser cookie Keychain access, running the helper clears the six-hour suppression used by BrowserCookieAccessGate and the app can resume prompting immediately after the repro.
Useful? React with 👍 / 👎.
Write prompt mode to both app and CLI defaults domains, restore oauth cache ACLs and browser-cookie cooldown state, and propagate repro misses in all mode. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
never(Avoid Keychain prompts), CodexBar no longer reads/writes/clears the CodexBar-ownedoauth.claudeKeychain cache during OAuth load, invalidation, or cache presence checks.nevernow prefers~/.claude/.credentials.json(and existing Claude-keychain paths governed by the never policy) instead of repeatedly touching the CodexBar OAuth cache item that can surface macOS prompts.never, so a prior prompt-enabled load cannot keep serving staleoauth.claudecache data after the user switches modes.neverrecords the pending marker and flushes it once prompt-enabled cache access is safe again (retrying on locked-keychain failures).security(1)reader (effectiveMode(readStrategy:)), while Security.framework probes remain blocked under storednever.Claude Code-credentialsare skipped when stored prompt mode isnever.ClaudeOAuthCredentialsStoreNeverPromptCacheTestsplus updates to ownership / security-CLI suites.Fixes the bug reported in #1991 where periodic refresh could still trigger macOS Keychain UI even after choosing Avoid Keychain prompts. #1990 (manual-refresh pre-alert UX) is intentionally unchanged.
Codex review follow-ups
clearCacheKeychain()recordspendingCodexBarOAuthKeychainCacheClearundernever; flush on re-enable (28f7c02b, tests)shouldUseCodexBarOAuthKeychainCachegates memory + cache reads (3c340b40, tests)load record ignores codexbar cache in never prompt mode; prior assertion moved to.onlyOnUserAction(e5ca4498)shouldSaveFileFingerprintundernever+ idempotent secondinvalidateCacheIfCredentialsFileChanged()(3c340b40, tests)flushPendingCodexBarOAuthKeychainCacheClearIfNeeded()retains tombstone on.failed; load returns.temporarilyUnavailable(e5ca4498, tests)effectiveMode(readStrategy:)inisMcpOAuthOnlyClaudeKeychainPayloadPresent(e5ca4498, tests)saveToCacheKeychainclears tombstone only after successfulstoreResult(28f7c02b, tests)Behavior proof (redacted, no live Keychain UI)
Repo policy (
AGENTS.md) avoids live macOS Keychain prompt validation in CI/agents. Proof below is from isolated test-keychain runs on macOS 15 using per-testKeychainCacheStore.withServiceOverrideForTesting(no user Keychain prompts).Representative redacted logs from the never-prompt suite:
The second line is from the simulated
errSecInteractionNotAllowedretry path; underneverthe productionoauth.claudeitem is not consulted until the pending clear succeeds.Live macOS repro (installed app 0.41.0)
On a real setup against the installed
0.41.0 (100)app (not just isolated tests):oauth.claudeundercom.steipete.codexbar.cacheclaudeOAuthKeychainPromptMode = neverCodexBarCLI usage --provider claude --source oauthObserved: CLI hung for
>=12swaiting for macOS Keychain UI whilenevermode was active.Helper script (added in
4a41e582):Details: #2011 (comment)
Test plan
swift test --filter 'ClaudeOAuthCredentialsStoreNeverPromptCacheTests|ClaudeOAuthCredentialsStoreTests|ClaudeOAuthCredentialsStorePromptPolicyTests|ClaudeOAuthCredentialsStoreCLIStorageOwnershipTests|ClaudeOAuthCredentialsStoreIsolatedSecurityCLITests'make check(lint green on latest CI rollup)0.41.0app +Scripts/repro_keychain_issues.shMaintainer-requested live proof is in the PR comment above; isolated tests remain the default CI path per
AGENTS.md.