fix: Claude OAuth 429時に前回表示を保持 / Preserve Claude OAuth display on 429#1956
fix: Claude OAuth 429時に前回表示を保持 / Preserve Claude OAuth display on 429#1956ruushu wants to merge 3 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 1:30 PM ET / 17:30 UTC. Summary Reproducibility: yes. from source inspection: current main uses one Claude OAuth cooldown key and does not preserve OAuth 429 snapshots the way it preserves CLI rate-limit data. I did not run live OAuth or Keychain probes because AGENTS.md warns against prompt-prone provider validation without explicit request. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the credential-scoped Claude OAuth 429 preservation only after the owner accepts the auth/persistence boundary and the proof gate is satisfied or explicitly overridden. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main uses one Claude OAuth cooldown key and does not preserve OAuth 429 snapshots the way it preserves CLI rate-limit data. I did not run live OAuth or Keychain probes because AGENTS.md warns against prompt-prone provider validation without explicit request. Is this the best way to solve the issue? Mostly yes: the current head addresses the prior Linux hashing, stale-history, account-switch, and credential-edit blockers with focused tests. The remaining blocker is merge readiness for proof and owner acceptance, not a discrete code defect I can point to in the diff. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b039d15e125. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review 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: c5a95f2ab2
ℹ️ 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".
|
|
||
| private static func blockedUntilKey(accessToken: String) -> String { | ||
| let data = Data(accessToken.utf8) | ||
| let fingerprint = ClaudeOAuthCredentialsStore.sha256Prefix(data) ?? "unavailable" |
There was a problem hiding this comment.
On Linux CLI builds, ClaudeOAuthCredentialsStore.sha256Prefix returns nil because that helper only uses CryptoKit while the package's Linux dependency is the Crypto module, so this fallback makes every access token use the same claudeOAuthUsageRateLimitBlockedUntilV2.unavailable key. In multi-account OAuth usage, a 429 from one Claude account will therefore block background refreshes for all other accounts for the cooldown window, defeating the token-scoped gate and leaving valid accounts stale. Use the existing Crypto fallback path here instead of collapsing to a shared key.
Useful? React with 👍 / 👎.
| return ResolvedAccountOutcome( | ||
| snapshot: snapshot, | ||
| usage: priorUsage, | ||
| sourceLabel: priorSnapshot.sourceLabel) |
There was a problem hiding this comment.
When a stacked Claude token-account refresh hits an OAuth 429, this returns the prior cached snapshot as usage; refreshTokenAccounts treats every non-nil resolved.usage as a newly fetched sample and later records it with the current Date(). During a cooldown/outage, each retry can write stale usage points into plan-utilization history and reset detection as if they were real observations. Preserve the row via snapshot, but don't mark cached 429 data as a fetched usage sample.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
Co-authored-by: ruushu <nitasaka@gmail.com>
Co-authored-by: ruushu <nitasaka@gmail.com>
steipete
force-pushed
the
fix/claude-oauth-rate-limit-stale-cache
branch
from
c5a95f2 to
58fe37b
Compare
Summary
This keeps @ruushu's original resilience goal while fixing Linux hashing, account-switch leakage, stale-history recording, cooldown races, and credential-edit edge cases found during review.
Validation
Exact head:
58fe37bee29212c2bf27e0cfeaa0fd74947aa736swift test --filter ClaudeOAuthRateLimitResilienceTests: 13/13 passed.swift test --filter ClaudeOAuthTests: 39/39 passed.make check: passed; SwiftFormat and SwiftLint report zero violations.make test: all 50/50 shards passed.autoreview --mode local: clean; no actionable findings.codesign --verify --deep --strictpassed; Gatekeeper accepted the bundle as Developer ID; embedded commit58fe37be.CodexBar Debugmenu opened and the Claude pane rendered its current unauthenticated state.Public Model Identifier Gate: PASS (no model-bearing mutation).
Dependency freshness: PASS (no dependency changes).
Landing boundary
Code and local proof are ready. This changes credential-scoped cache/cooldown persistence behavior, so it remains unmerged pending explicit owner sign-off for that auth/persistence boundary.