Add Claude setting to suppress Keychain access explanation - #2718
Draft
MGidnian wants to merge 4 commits into
Draft
Add Claude setting to suppress Keychain access explanation#2718MGidnian wants to merge 4 commits into
MGidnian wants to merge 4 commits into
Conversation
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed August 12, 2026, 10:09 AM ET / 14:09 UTC. ClawSweeper reviewWhat this changesAdds a persisted Claude setting to suppress CodexBar’s Keychain explanation while retaining native macOS authorization and OAuth behavior. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 5 items remain The feature remains distinct from current main, but needs maintainer UX direction, a rebase, and real behavior proof before merge. Priority: P2 Review scores
Verification
How this fits togetherClaude usage refreshes can read OAuth credentials from macOS Keychain. CodexBar shows an explanatory alert before a possible native prompt, then reads credentials to fetch usage. flowchart LR
A[Claude usage refresh] --> B[Keychain access preflight]
B --> C[CodexBar explanation gate]
C --> D[Native macOS Keychain prompt]
D --> E[Claude OAuth credential read]
E --> F[Usage result]
Decision needed
Why: This is a new security-education preference, not a repair to a defined broken contract; source proves it is distinct but cannot establish the desired UX policy. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the explanation enabled by default, add the scoped opt-out only if its UX is accepted, and merge a rebased branch with redacted bundled-app proof for both states. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new opt-out preference, though current source clearly identifies the explanation path it would change. Is this the best way to solve the issue? Unclear: the design preserves defaults and native authorization, but maintainers must choose whether a separate setting is preferable to an inline acknowledgement or current policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a90dfed5c264. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (18 earlier review cycles; latest 8 shown)
|
Author
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Author
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Author
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
rating: 🦪 silver shellfish
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Show Keychain access explanation, with an explicit opt-out.without changing the public
KeychainPromptHandler.handlercallback signature.Motivation
When refreshing Claude usage, CodexBar can show its explanatory alert before macOS asks for access to the Claude OAuth Keychain item. After the user has already granted access with “Always Allow”, seeing the explanatory alert again is cumbersome. The existing cooldown is temporary and does not provide a durable preference for an already-authorized setup.
This change makes the common already-authorized flow quiet after the user disables the explanation in Claude settings,
while preserving the current explanation-by-default behavior for upgrades. That avoids silently changing an existing
installation's security/education flow; new or existing users can opt out explicitly once they understand the prompt.
Scope and safety
This only suppresses CodexBar's explanatory alert for the
.claudeOAuthprompt context. It does not disable Keychainaccess, change
ClaudeOAuthKeychainPromptMode, bypass native macOS authorization, or alter OAuth refresh behavior.The native macOS authorization prompt remains controlled by macOS. The additive result callback keeps the existing
six-hour cooldown semantics correct: only a displayed CodexBar explanation acknowledges the cooldown, while existing
CodexBarCore clients can continue assigning the public
Voidcallback.Follow-up UX option
If maintainers prefer an inline acknowledgement, the same preference could later be exposed directly in this alert with an option such as “Don't show this again.” I kept this PR settings-first so the preference is explicit, discoverable, and consistent with the existing Claude authentication controls.
Verification
make checkmake test(823 selections, 69 groups, all passed)swift test --filter 'KeychainPromptCoordinatorTests|SettingsStoreCoverageTests|ClaudeOAuthKeychainPreAlertGateTests'(59 tests, all passed)
used by suppression, plus precedence for legacy public handler overrides; the full suite covers the unchanged native
Keychain integration paths.
./Scripts/compile_and_run.sh, but the separate Widget extensionxcodebuildstalled in the locked CI-like localenvironment; no Keychain credentials or native authorization prompts were touched.
Keychain reads were performed.
Screenshots
Not included: this reuses the existing provider settings UI and changes the visibility of the existing explanatory flow
rather than adding a new visual component. The focused and full test commands above are the available redacted terminal
evidence for both preference states.
Related
Related to #2588 and #2668.