Claude: stop rotating CLI-owned refresh chains on keychain-only installs - #2745
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: 49ff8c9d97
ℹ️ 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".
| case .unavailable, .notApplicable: | ||
| return ClaudeAccountProfile.accountUuid(environment: environment) != nil |
There was a problem hiding this comment.
When the Keychain probe is unavailable—which is the normal release configuration—accountUuid returns nil not only when the CLI is logged out, but also when its config is unreadable, malformed, or uses an unrecognized schema. In those cases this marks the cached chain as CodexBar-owned, so an expired mirror is refreshed directly and its rotating token can invalidate the still-live Claude CLI chain. Preserve CLI ownership unless the plaintext config can positively establish that CLI storage is absent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3ce97e6: ownership evidence is now tri-state (signedIn / signedOut / indeterminate). Only a verifiably missing config or a cleanly parsed signed-out config releases the chain to CodexBar; unreadable, malformed, unrecognized-schema, and identity-less configs are indeterminate and stay CLI-owned. Regression tests cover all three indeterminate shapes.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed August 8, 2026, 9:15 PM ET / August 9, 2026, 01:15 UTC. ClawSweeper reviewWhat this changesThe branch conservatively delegates Claude OAuth refresh to the CLI when CLI ownership cannot be disproved, and ties terminal refresh blocks to the refresh-token lineage that failed. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until real behavior proof is added - 3 items remain The final source addresses the prior ownership and lineage findings, but this active, maintainer-engaged PR still needs real after-fix proof on the release-signed keychain-only path before merge. Priority: P1 Review scores
Verification
How this fits togetherCodexBar’s Claude provider decides whether a cached OAuth credential may be refreshed directly or must be handed to Claude CLI. That ownership decision feeds usage refresh and protects rotating CLI refresh tokens from being consumed by CodexBar. flowchart LR
A[Cached Claude credentials] --> B[Ownership evidence]
C[Claude config and Keychain state] --> B
B --> D{Refresh owner}
D -->|Claude CLI| E[Delegated CLI refresh]
D -->|CodexBar only| F[Direct OAuth refresh]
E --> G[Usage update]
F --> G
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: Land the conservative ownership and lineage behavior only after a redacted release-signed keychain-only trace demonstrates delegated ownership and successful post-reauth recovery without rotating the Claude CLI chain. Do we have a high-confidence way to reproduce the issue? No live reproduction was established in this review; the current source and focused tests provide a high-confidence source reproduction of the affected ownership and gate paths. Is this the best way to solve the issue? Yes for the code direction: retaining CLI ownership unless absence is positively proven is the narrowest safe response to rotating refresh tokens, pending real release-signed proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5e8797b14b6c. 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 (9 earlier review cycles; latest 8 shown)
|
|
Reviewed in depth — the ownership diagnosis is strong and this is a genuinely valuable find: on keychain-only installs the delegated touch rotates Claude Code's own refresh chain while CodexBar can never read the result, so every retry actively worsens the user's state. The design direction (stop rotating CLI-owned chains) is right, and this should land before #2675, which will rebase on top so its consent path composes with your ownership logic. Two gaps to close before merge:
Then rebase onto current main (the subsystem moved this week: #2650 terminal-state handling and the #2675 branch both touch |
49ff8c9 to
3ce97e6
Compare
|
Both gaps closed, and rebased onto current main (the CHANGELOG entry moved to the 0.48.2 unreleased section). 1. Indeterminate config evidence is now conservative. New tri-state 2. Superseded-lineage transient failures now transition into backoff. Regression tests — three config-evidence cases (malformed config, Verification after the rebase: One drive-by observation from triaging those flakes: when |
|
Checking in — the two asks from review (tri-state ownership evidence with indeterminate-treated-as-CLI-owned, and the legacy-lineage transient-backoff transition, both with regression tests) are the only things between this and merge, and #2675 is queued to rebase on top of you. If you're short on time this week, say so and we'll carry the fixes over the line with your authorship preserved — the ownership diagnosis is yours either way. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Landed. Verification: the ownership diagnosis was confirmed against the delegated-refresh coordinator (on keychain-only installs each touch rotated Claude Code's refresh chain while CodexBar could never read the result — actively worsening state per retry). The two review gaps were carried over the line with your authorship preserved: tri-state ownership evidence (indeterminate → treated as CLI-owned, never rotating an unproven chain) and the legacy-lineage transient-backoff transition, both regression-tested (ownership 11/11, consent 10/10, CLI-storage 9/9). #2675 is rebased on top so the consent path composes with this. CI green at merge after the runner-flake rerun. Thanks @avenoxai — the mechanism find here was the missing piece of the 0.47 recovery story. |
2 participants
Summary
Two surgical fixes in the Claude OAuth credential-ownership seam:
resolvedCacheOwnerdowngrades a.codexbar-owned cache entry to.claudeCLI(delegated refresh) unless there is positive evidence that Claude CLI storage is gone. Previously, "cannot see CLI storage" was treated as "CLI storage absent" — on keychain-only Claude Code installs (2.1.x, no~/.claude/.credentials.json, andkeychainAccessAllowed == falsein release sincee17ba24bd), that evidence could never be produced, so a persisted.codexbarentry stayed CodexBar-owned forever.invalid_grantblock to the token lineage that failed. The refresh-failure gate now records a SHA-256 hash of the refresh token that producedinvalid_grant, and a refresh attempt with a different refresh token is allowed through (re-latching with the new hash if it also fails). Previously the block could only clear via a file-fingerprint change — impossible on keychain-only installs, where the fingerprint is permanentlynil, so "blocked until auth changes. Runclaudeto re-authenticate." was unsatisfiable: re-auth could never clear it.Root cause
Anthropic refresh tokens are single-use and rotating. When an expired cache entry stays
.codexbar-owned, CodexBar POSTs the refresh itself, accepts the rotated refresh token, and stores it only in its own cache — Claude Code's stored token is now server-side invalid, so Claude Code's next refresh getsinvalid_grantand the user is logged out of Claude Code (the #1161 failure class). The self-renewing mirror also never expires, so it permanently shadows the real account state — the "CodexBar keeps reverting to my old Claude account" symptom (#2689).The decision helper
hasClaudeCLIStorageWithoutPromptonly accepted two proofs of CLI ownership: a credentials-file fingerprint, or a.matchedno-prompt keychain probe. On keychain-only installs in release, both are structurally impossible — the probe API already distinguishes.unavailable(can't look) from.absent(looked, nothing there), but the caller collapsed them.What changed
hasClaudeCLIStorageWithoutPromptnow implements an explicit decision table:.matched.mismatch(item exists, different token).absent(readable, no item).unavailable/ prompt modeNeverClaudeAccountProfile.accountUuidread of Claude's plaintext config — a logged-in Claude Code install means the CLI owns the lifecycleThe only scenario that keeps direct refresh alive is the one it was built for: Claude Code is genuinely gone or logged out, and CodexBar keeps its own mirrored chain running — where rotation can't hurt anyone.
The failure gate keeps its public API (new parameters are defaulted), stays monotonic, and persists the failed-lineage hash per profile alongside the existing keys. Raw tokens are never persisted or logged — hash only.
Composition with in-flight PRs
keychainAccessAllowed, the consent flag, or the fallback routing. When consent re-opens the no-prompt probe, the.matched/.mismatch/.absentrows of the table simply become reachable again — the two changes compose.ClaudeUsageFetcher.loadAfterDelegatedRefreshis untouched.Verification
swift test --filter ClaudeOAuthRefreshChainOwnershipTests— 6 new decision-table/delegation tests (probe.unavailable× config present/absent,.absent,.mismatch, file fingerprint, prompt-modeNeverfallback), all passing.swift test --filter ClaudeOAuthRefreshFailureGateTests— 3 new lineage tests (scoped block, legacy-block healing + re-latch, persistence round-trip), 16/16 passing.swift test --filter ClaudeOAuthCredentialsStoreCLIStorageOwnershipTests— 9/9 (one assertion updated:.mismatchnow proves CLI storage; fixtures isolated from the developer's ambient Claude config viaCLAUDE_CONFIG_DIR).swift test --filter ClaudeUsageTests(40/40) and--filter ClaudeOAuthFetchStrategyAvailabilityTests(22/22).make check— SwiftFormat clean, SwiftLint strict zero violations.make test— full sharded suite green except one pre-existing, environment-caused failure:MiniMaxMenuCardBillingTests."minimax billing history renders inline dashboard"expects"1,234"but gets"1.234"on a Turkish-locale machine (thousands-separator hardcoded). It fails identically on an unmodifiedorigin/mainworktree (both-trees solo runs), so it is unrelated to this change and should be green on CI's en-US locale.load with auto refresh expired codexbar owner uses direct refresh pathnow pins itsCLAUDE_CONFIG_DIRto an empty temp dir — it covers the CLI-absent path, and without isolation the developer's own logged-in~/.claude.jsonwould (correctly, under the new rule) hand the chain to the CLI.CLAUDE_CONFIG_DIRfixtures; no real SecItem reads, no live probes, no packaged-app launches.Fixes #2689. Refs #2634 (the unrecoverable-after-re-auth tail), #1161 (the original rotation-desync report), #2115/#2195 (prompt-loop context).