Claude: consented direct keychain read + CLI usage fallback (#2634) - #2675
Merged
Conversation
![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: ec037bee64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+622
to
+623
| self.defaultsState.claudeOAuthDirectKeychainReadAllowed = newValue | ||
| self.userDefaults.set(newValue, forKey: ClaudeOAuthDirectKeychainReadConsent.userDefaultsKey) |
There was a problem hiding this comment.
When a user turns this setting back off after a successful OAuth read, the setter only flips the defaults flag. ClaudeOAuthCredentialsStore.loadRecord still returns valid in-memory or CodexBar keychain-cache entries before the keychainAccessAllowed-guarded freshness sync, so Claude OAuth usage can continue using the copied Claude Code token until the cache expires instead of immediately falling back to the CLI. Clear/invalidate the Claude OAuth credential cache when newValue is false.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 12:53 AM ET / 04:53 UTC. ClawSweeper reviewWhat this changesAdds a default-off consent setting for direct Claude Code Keychain reads and falls back to reduced-detail Claude CLI usage when OAuth credentials are unreadable. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open: the owner-authored PR now addresses the prior all-profile cache-revocation blocker, but remains pending normal owner validation of its intentional Keychain-access boundary. Priority: P1 Review scores
Verification
How this fits togetherCodexBar’s Claude provider obtains usage through OAuth credentials or the installed Claude CLI, then renders a usage snapshot in the menu bar. This change gates foreign-Keychain reads and changes the credential-cache and fallback path used before usage is fetched. flowchart LR
A[Claude refresh] --> B[Consent and prompt policy]
B -->|Consent granted| C[Claude Code Keychain]
B -->|Not consented or unreadable| D[Claude CLI usage probe]
C --> E[Credential cache]
E --> F[OAuth usage request]
D --> G[Reduced-detail usage snapshot]
F --> H[Claude menu card]
G --> H
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the default-off consent boundary and cross-profile revocation, then validate the documented signed-app scenarios before release. Do we have a high-confidence way to reproduce the issue? Yes for the source-level cache and fallback behavior: focused tests cover both profiles and routing, while the macOS Keychain prompt path requires the signed-app validation described in the PR. Is this the best way to solve the issue? Yes: the explicit default-off consent plus CLI fallback is a bounded recovery path, and the current head now prevents pre-revocation caches from surviving a profile switch. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 81ca9f66338c. LabelsLabel changes:
Label 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 (5 earlier review cycles)
|
…nted-read # Conflicts: # CHANGELOG.md
…consent is revoked
This was referenced Aug 8, 2026
This was referenced Aug 8, 2026
…-claude-consented-read
clawsweeper
Bot
added
rating: 🦪 silver shellfish
…nted-read # Conflicts: # Sources/CodexBarCore/Providers/Claude/ClaudeOAuth/ClaudeOAuthCredentials.swift # Tests/CodexBarTests/ClaudeOAuthCredentialsStoreCLIStorageOwnershipTests.swift # Tests/CodexBarTests/ClaudeOAuthRefreshChainOwnershipTests.swift
clawsweeper
Bot
added
rating: 🐚 platinum hermit
luisgonzaleznf
added a commit
to luisgonzaleznf/CodexBar
that referenced
this pull request
Aug 9, 2026
Addresses both P1 findings on the draft, now that steipete#2675 has landed. The feed carries only the 5h/7d windows. Publishing its result whole blanked identity, plan, model-scoped weekly, Daily Routines, extra usage and cost — the opposite of the "composes with, never replaces" constraint in the owner ruling. Its windows are now merged over the last polled Claude snapshot, and a window the observation omits means "no update" rather than "cleared". Composition also carries the account guard the feed cannot supply itself. Matching CLAUDE_CONFIG_DIR cannot distinguish an account switch within one profile, so an observation is discarded outright when the active account has changed since the snapshot it would compose over. Publishing it there would render one account's windows beneath another account's identity. Follows the existing shape of this apply path, which already reconciles results against previous state for Codex, CommandCode, DeepSeek and reset backfill. The helper lives in its own file and the call site replaces an existing binding, so the refresh function stays within its length budget. The card note keeps precedence over the dataConfidence note added by steipete#2675: a composed snapshot inherits its confidence from the previous poll, so a prior CLI scrape would otherwise label live statusLine windows as CLI-sourced.
luisgonzaleznf
added a commit
to luisgonzaleznf/CodexBar
that referenced
this pull request
Aug 12, 2026
Addresses both P1 findings on the draft, now that steipete#2675 has landed. The feed carries only the 5h/7d windows. Publishing its result whole blanked identity, plan, model-scoped weekly, Daily Routines, extra usage and cost — the opposite of the "composes with, never replaces" constraint in the owner ruling. Its windows are now merged over the last polled Claude snapshot, and a window the observation omits means "no update" rather than "cleared". Composition also carries the account guard the feed cannot supply itself. Matching CLAUDE_CONFIG_DIR cannot distinguish an account switch within one profile, so an observation is discarded outright when the active account has changed since the snapshot it would compose over. Publishing it there would render one account's windows beneath another account's identity. Follows the existing shape of this apply path, which already reconciles results against previous state for Codex, CommandCode, DeepSeek and reset backfill. The helper lives in its own file and the call site replaces an existing binding, so the refresh function stays within its length budget. The card note keeps precedence over the dataConfidence note added by steipete#2675: a composed snapshot inherits its confidence from the previous poll, so a prior CLI scrape would otherwise label live statusLine windows as CLI-sourced.
luisgonzaleznf
added a commit
to luisgonzaleznf/CodexBar
that referenced
this pull request
Aug 13, 2026
Addresses both P1 findings on the draft, now that steipete#2675 has landed. The feed carries only the 5h/7d windows. Publishing its result whole blanked identity, plan, model-scoped weekly, Daily Routines, extra usage and cost — the opposite of the "composes with, never replaces" constraint in the owner ruling. Its windows are now merged over the last polled Claude snapshot, and a window the observation omits means "no update" rather than "cleared". Composition also carries the account guard the feed cannot supply itself. Matching CLAUDE_CONFIG_DIR cannot distinguish an account switch within one profile, so an observation is discarded outright when the active account has changed since the snapshot it would compose over. Publishing it there would render one account's windows beneath another account's identity. Follows the existing shape of this apply path, which already reconciles results against previous state for Codex, CommandCode, DeepSeek and reset backfill. The helper lives in its own file and the call site replaces an existing binding, so the refresh function stays within its length budget. The card note keeps precedence over the dataConfidence note added by steipete#2675: a composed snapshot inherits its confidence from the previous poll, so a prior CLI scrape would otherwise label live statusLine windows as CLI-sourced.
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
2 participants
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
Implements the maintainer decision on #2634 (both paths): a consented direct keychain read as the primary recovery for Claude usage on 0.47+, and a CLI usage-probe fallback for users who decline. Root cause context:
e17ba24bdhard-disabledkeychainAccessAllowedin release, which silently blocked the direct read, the freshness sync, AND the delegated-refresh success verification — making.attemptedSucceededstructurally unreachable (analysis on the issue).Consent gate (primary)
claudeOAuthDirectKeychainReadAllowed) resolved through the same defaults-domain path as the existing prompt-mode preference, so app/CLI/helpers agree. No migration, no silent re-enable on upgrade.keychainAccessAllowed(the exact linee17ba24bdchanged) now resolves gate + consent, and remains the single choke point: direct read (both readers), freshness sync, and fingerprint verification open and close together. The existing prompt-policy setting layers on top unchanged.CLI fallback (degraded, honest)
No new scraper — reuses the existing hardened PTY
/usageprobe (ClaudeStatusProbe: shape validation, bounded timeouts, rate-limit gate, background-launch gating). The terminal unreadable state is now a typed error, so explicit-OAuth falls through to the CLI step; scraped snapshots carrydataConfidence: .percentOnlyand the card says "Usage via Claude CLI (limited detail)". Fail-soft preserved; background refresh keeps all existing no-PTY gates and cooldowns.Testing
ClaudeOAuthDirectKeychainReadConsentTests(9): choke-point flips with consent, kill-switch wins over consent, default-off / no-consent-no-read invariants, typed-error carve-outs, fallback routing per runtime/source-mode,.percentOnlypropagation.ClaudeCredentialOwnershipBoundaryTestsuntouched and passing — now proving the default posture.make testexit 0;make check0 violations. Codex autoreview (gpt-5.6-sol, high): clean, no findings, "patch is correct (0.98)".Live verification still needed (maintainer machine)
.attemptedSucceededobservable in logs.e17ba24bd).Open judgment calls (flagged, defaults chosen)
.percentOnly(PTY exposes rendered percentages) rather than.estimated.Fixes #2634.