fix(claude): reduce keychain prompts even after granting Always Allow #245
Conversation
CodexBar was prompting for keychain password repeatedly even after the user granted "Always Allow" access to Claude Code credentials. Changes: - Add `allowKeychainPrompt` parameter to `ClaudeOAuthCredentialsStore.load()` - Add `loadFromClaudeKeychainWithoutPrompt()` using LAContext to silently fail - Add `invalidateCacheIfClaudeKeychainChanged()` to detect credential changes - Skip keychain prompts during background refreshes and availability checks - Cache credentials in CodexBar's own keychain after first successful access - Re-migrate Claude credentials when they change to update accessibility
![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: e2fa96efbe
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #243 where CodexBar repeatedly prompts for keychain password despite "Always Allow" being selected. The fix implements a multi-layered caching strategy and silent keychain access to minimize user prompts.
Changes:
- Added
allowKeychainPromptparameter to control when keychain prompts are shown, preventing unexpected prompts during background operations - Implemented silent keychain access using
LAContext.interactionNotAllowedto read credentials without triggering prompts - Added credential caching in CodexBar's own keychain to reduce access frequency to Claude Code's keychain
- Implemented change detection for both file-based and keychain-based credentials to maintain cache validity
- Enhanced migration logic to re-migrate Claude credentials when they change
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift | Core implementation: added allowKeychainPrompt parameter, silent keychain access methods, cache invalidation logic, and file fingerprinting to detect credential changes |
| Sources/CodexBar/KeychainMigration.swift | Added logic to re-migrate Claude credentials when they change, tracking credentials via fingerprints to maintain proper accessibility settings |
| Sources/CodexBarCore/Providers/Claude/ClaudeUsageFetcher.swift | Updated to use allowKeychainPrompt: false during background refreshes to prevent prompts |
| Sources/CodexBarCore/Providers/Claude/ClaudeProviderDescriptor.swift | Updated availability check to use allowKeychainPrompt: false to prevent prompts during background checks |
| Sources/CodexBar/UsageStore.swift | Updated debug dump to use allowKeychainPrompt: false to prevent prompts during diagnostic operations |
| Sources/CodexBar/UsageStore+Refresh.swift | Added cache invalidation check after fetching to detect credential changes and trigger re-fetch if needed |
| Tests/CodexBarTests/ClaudeOAuthCredentialsStoreTests.swift | Added tests for cache invalidation when credentials file changes, with some test setup improvements |
| Tests/CodexBarTests/KeychainMigrationTests.swift | Added test entries for new keychain items and migration tracking reset functionality |
| CHANGELOG.md | Documented the bug fix with details of the implementation approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift
Outdated
Show resolved
Hide resolved
- Remove redundant fingerprint check in cached credentials (already validated by invalidateCacheIfCredentialsFileChanged) - Remove redundant KeychainAccessGate assignment in test - Add comment explaining cache re-store in test
|
ser, pls approve, it's annoying, compute is idling while waiting user to "always allow" every time |
|
While this PR significantly reduces keychain prompts, there's one scenario where prompts may still occur: When Claude Code refreshes expired OAuth tokens (typically every few hours/days), it updates the keychain item which can reset the Access Control List (ACL). This means:
Why this happens:
Mitigation in this PR:
Result: Prompts are reduced from "every access" to "occasionally after token refresh" - a significant improvement but not completely eliminated. Complete fix would require: Claude Code to preserve third-party app ACL entries when updating credentials (feature request to Anthropic). |
|
Does this address the scenario where the user does not wish to grant keychain access at all? I'm not sure why this is a mandatory requirement when just processing .codex/ folder data... |
|
@tylerseymour It wont address that you need permission to token to be able to get your usage from anthropic API what other way you will get your usage ? |
|
can we please merge this at least partial solution instead of waiting for perfect solution and address future issues in the future? |
|
Its not on my hand. Only project owner can merge it. |
|
I'm sorry. The internet is exploding rn. |
|
@elpinguinofrio ./Scripts/compile_and_run.sh |
5 participants
Fixes #243
Summary
Changes
allowKeychainPromptparameter toClaudeOAuthCredentialsStore.load()to control prompt behaviorloadFromClaudeKeychainWithoutPrompt()usingLAContext.interactionNotAllowedto silently failinvalidateCacheIfClaudeKeychainChanged()to detect credential changes without promptingTest plan