Fix Claude source mode fallback on successful CLI login (#2403) - #2414
Fix Claude source mode fallback on successful CLI login (#2403)#2414Yuxin-Qiao wants to merge 4 commits into
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: d1dc6bd8e5
ℹ️ 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".
| let metadata = self.store.metadata(for: .claude) | ||
| self.settings.setProviderEnabled(provider: .claude, metadata: metadata, enabled: true) | ||
| if self.settings.claudeUsageDataSource == .web { | ||
| self.settings.claudeUsageDataSource = .auto |
There was a problem hiding this comment.
When a saved Claude token account contains a web/session cookie, this assignment changes an explicit web configuration to auto but leaves that token account selected. On the next app refresh, auto mode can pick the newly logged-in Claude Code OAuth/CLI credential before the selected account’s web cookie, while refresh publication still treats the selected token account as current and labels/caches that ambient snapshot under the saved account. In that configuration, users can see the CLI account’s usage under the selected web account; either clear/switch the selected token account or avoid auto-switching while a selected web-cookie account owns the refresh.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9471a060c9
ℹ️ 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".
| let metadata = self.store.metadata(for: .claude) | ||
| self.settings.setProviderEnabled(provider: .claude, metadata: metadata, enabled: true) | ||
| if self.settings.claudeUsageDataSource == .web { | ||
| self.settings.claudeUsageDataSource = .auto |
There was a problem hiding this comment.
When a user had explicitly selected Web but also has a Claude Admin API key saved or in the environment, this switch to Auto means the refresh triggered after a successful Claude Code login no longer uses the credential they just logged in with: ClaudeProviderDescriptor.resolveStrategies returns the admin API strategy immediately for Auto + admin key before reaching the OAuth/CLI planner. In that configuration the menu shows Admin API spend instead of the Claude Code account the login flow just authenticated; consider switching to a Claude Code-specific source or suppressing the admin-key auto shortcut for this post-login migration.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 10:42 PM ET / July 31, 2026, 02:42 UTC. ClawSweeper reviewWhat this changesThis PR changes successful Claude Code CLI login from preserving the selected Web usage source to selecting OAuth, updates the related login-flow expectation, and adds retries around existing Swift toolchain installation in CI. Merge readiness⛔ Blocked until real behavior proof is added - 10 items remain Keep this PR open for repair and proof. Its current diff removes the successful-login step that enables Claude, so disabled or onboarding users can receive a success result while Claude remains unavailable; it also forces strict OAuth in a configuration where the existing CLI fallback may be required. The linked issue remains the canonical report, and this PR is its candidate fix. Priority: P1 Review scores
Verification
How this fits togetherCodexBar’s Claude login flow runs Claude Code authentication, records the provider’s usage-source setting, and triggers the refresh that supplies the menu-bar usage state. That setting determines which stored credential strategy is eligible after login, so it must remain consistent with provider enablement and available credential access. flowchart LR
A[User starts Claude Code sign-in] --> B[Claude login flow]
B --> C[Provider enablement and source policy]
C --> D[Credential strategy selection]
D --> E[Claude usage refresh]
E --> F[Menu bar usage state]
Before merge
Findings
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 provider enablement independent of source migration, preserve a CLI-capable refresh path when OAuth credentials cannot be read, cover those policy combinations with focused tests, and add redacted live proof of login followed by a persisted successful refresh. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: start a successful Claude login while the provider is disabled, then observe that the changed branch updates only the source setting and leaves the provider-disabled state intact. The restricted-Keychain path is likewise source-reproducible because strict OAuth no longer retains the existing automatic CLI fallback. Is this the best way to solve the issue? No. Switching away from a stale Web source addresses the reported symptom, but the current patch also drops required provider enablement and makes OAuth mandatory even when the successful CLI credential is the only usable path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
60e9173 to
987f6d6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 987f6d696d
ℹ️ 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".
| if self.settings.claudeUsageDataSource == .web { | ||
| self.settings.claudeUsageDataSource = .oauth |
There was a problem hiding this comment.
When Claude is disabled—the setup used by both login-flow tests and a normal sign-in onboarding path—replacing the unconditional setProviderEnabled(..., enabled: true) call with this source-only update leaves ProviderConfig.enabled false. Changing claudeUsageDataSource does not alter enablement, so the flow reports success and posts a notification while Claude remains hidden and is not refreshed; the new isProviderEnabledCached assertions consequently fail. Restore provider enablement independently of the selected source.
Useful? React with 👍 / 👎.
| #expect(actions.contains { | ||
| $0.0 == "Re-login at claude.ai" && $0.1 == .loginToProvider(url: "https://claude.ai/") |
There was a problem hiding this comment.
After the successful flow changes this test's source from .web to .oauth, shouldOpenBrowserForWebSessionError rejects the source because it only accepts .auto or .web. Injecting a web unauthorized error therefore cannot produce the asserted “Re-login at claude.ai” action, so this test will fail once the earlier enablement assertion is repaired and does not validate the new routing behavior; assert the OAuth behavior or verify that the obsolete web action is absent instead.
Useful? React with 👍 / 👎.
… and add swiftly retry
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaf2a3ef70
ℹ️ 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".
| let metadata = self.store.metadata(for: .claude) | ||
| self.settings.setProviderEnabled(provider: .claude, metadata: metadata, enabled: true) | ||
| if self.settings.claudeUsageDataSource == .web { | ||
| self.settings.claudeUsageDataSource = .oauth |
There was a problem hiding this comment.
When “Disable Keychain access” or the no-prompt OAuth policy is enabled, claude auth login --claudeai can still succeed because it runs as an external CLI process, but this assignment then forces the provider into strict OAuth mode. ClaudeOAuthFetchStrategy cannot obtain the newly stored Keychain credential in that configuration, and its shouldFallback implementation only permits the working CLI strategy in .auto mode, so the refresh immediately fails despite the successful login. Keep a CLI fallback for this case rather than unconditionally selecting .oauth.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #2403. When users logged into Claude via "Sign in with Claude Code..." (terminal CLI authentication), if
claudeUsageDataSourcewas previously set to.web(or defaulted to.web), CodexBar would immediately attempt to fetch usage using unauthenticated browser cookies on the subsequent refresh. This resulted in an immediate401 Unauthorizederror ("Sign in to claude.ai...") and reverted the Claude status item to disconnected immediately after a "successful" login.This PR updates
runClaudeLoginFlow()so that a successful terminal CLI login automatically updatesclaudeUsageDataSourcefrom.webto.auto, ensuring newly generated OAuth/CLI credentials are used immediately during refresh.Changes Made
claudeUsageDataSourcefrom.webto.autowhenrunClaudeLoginFlowcompletes with.success..websource mode switching to.autoupon successful CLI login.Verification Run
swift test --filter ClaudeLoginFlowTestsswift test --filter ClaudeIssue2403ReproductionTestsswiftformat Sources Tests