Fix Claude Auto fallback after OAuth rotation - #2981
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 auth-provider
|
Codex review: needs changes before merge. Reviewed August 16, 2026, 1:00 PM ET / 17:00 UTC. ClawSweeper reviewWhat this changesThis PR changes Claude Auto to reuse recent CLI usage after OAuth Keychain access is lost, show recovery guidance, and retain stale quota history when consumer sources fail. Merge readinessKeep open: the latest head still retains both prior P2 blockers around background Claude CLI safety and revoked-access classification. Priority: P2 Review scores
Verification
How this fits togetherClaude Auto selects OAuth, CLI, or web usage sources and publishes the resulting quota snapshot to the menu bar. The patch changes its OAuth-to-CLI fallback and the stale-data behavior when no live consumer source succeeds. flowchart LR
A[Claude Auto refresh] --> B[OAuth credential read]
B --> C{Credential usable?}
C -->|No| D[CLI or web fallback]
C -->|Revoked| E[Recovery guidance]
D --> F[Usage snapshot]
D -->|All fail| G[Stored quota history]
G --> F
F --> H[Menu bar display]
Before merge
Findings
Agent review detailsSecurityNeeds attention: The cached availability change can bypass the background Keychain no-UI safety boundary during owner-only recovery. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Restrict cached CLI availability to non-recovery Auto contexts, require both prior-grant and active-profile evidence for revoked-access guidance, and cover both cases with focused regressions. Do we have a high-confidence way to reproduce the issue? Yes, from source: seed a cached CLI result, enter background Auto with owner-only recovery, and the availability shortcut admits a fetch that intentionally bypasses that cache; separately, set the denial cooldown without grant/profile evidence and observe revoked-access classification. Is this the best way to solve the issue? No: the central fallback approach is appropriate, but the two guards above must be tightened to preserve the existing no-UI and authentication-state contracts. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 355f8443a55c. LabelsLabel justifications:
EvidenceSecurity concerns:
Acceptance criteria:
What 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 (4 earlier review cycles)
|
![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: 2b807974d3
ℹ️ 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".
| if let throttleKey = self.throttleKey(binary: binary, context: context), | ||
| ClaudeCLIUsageSpawnThrottle.cachedResult(for: throttleKey) != nil | ||
| { | ||
| return true |
There was a problem hiding this comment.
When a background Auto refresh enters claudeOwnerCLIRecoveryOnly while a recent cached CLI result exists, this returns true without consulting ClaudeCLIBackgroundAvailability, but fetch deliberately refuses that cached result for recovery passes and launches Claude instead. Thus an account-mismatch recovery can bypass the background opaque-child/Keychain-prompt gate and spawn the owner CLI during a scheduled refresh; only use this shortcut when !context.claudeOwnerCLIRecoveryOnly, otherwise retain the existing availability check.
Useful? React with 👍 / 👎.
| guard directReadConsentGranted, | ||
| !keychainAccessDisabled, | ||
| keychainAccessDenied || (previousKeychainGrantRecorded && loggedInProfilePresent) |
There was a problem hiding this comment.
When the Keychain denial cooldown is active, this condition classifies missing credentials as keychainAccessRevoked solely because keychainAccessDenied is true, even if there is no recorded prior grant and identifiedSessionScope confirms no logged-in profile. This occurs after logout or credential removal during the six-hour cooldown and incorrectly tells the user that token rotation revoked access and that Refresh can re-grant it, rather than reporting that authentication is absent; the denial branch should not bypass the prior-grant/profile evidence.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
clawsweeper
Bot
added
rating: 🦐 gold shrimp
1 participant
Summary
Fixes #2516 by making Claude Auto resilient when Claude Code rotates its Keychain credential and replaces CodexBar's ACL grant.
The source fallback loop was already capable of continuing from OAuth to CLI and Web. The failure happened in the CLI availability gate: a background Auto refresh treated an unreadable OAuth credential as neither usable nor conclusively absent, then skipped the CLI even when an account-scoped CLI result had succeeded moments earlier. With no usable Web session, the OAuth error became the visible result.
What changed
history/claude.jsonbucket, retain the capture timestamp, and show the capture age as stale data.Deliberately not changed
This does not attempt to preserve or rewrite the ACL on Claude Code's
Claude Code-credentialsitem. Claude Code owns that item and replaces its ACL during token rotation; CodexBar cannot fix that behavior. Manual Refresh remains the explicit re-grant path.Proof
swift test --filter ClaudeOAuthRotationErrorTestsswift test --filter ClaudeCLIUsageSpawnThrottleTestsswift test --filter ClaudeResilienceTestsmake checkmake test