Repository navigation
Align OAuth pool with Claude CLI: Retry-After, accountId, scope validation, User-Agent, login_hint - #4
Merged
Conversation
…ation, User-Agent, login_hint - Parse Retry-After header (seconds or HTTP-date) on 429/529 responses and use it as the pool cooldown duration instead of the fixed 5-minute default. Implemented in PoolManager::markRateLimited(), a new markRateLimitedFromResponse() helper, and AnthropicMaxTextGenerationModel::handleRateLimitResponse(). - Store accountId (account.uuid from token response) alongside each pool entry. Exposed in listAccounts() and healthCheck() output for diagnostics. - Validate user:inference scope after token exchange. Returns a scope_error result when the granted scope list is missing the required scope; the REST API surfaces this as HTTP 403 with a clear message before the account is added to the pool. - Update User-Agent to claude-cli/2.1.80 (wordpress-plugin) to align with Claude CLI format on token exchange and refresh requests. - Add alignment comments for REDIRECT_URI fallback (platform.claude.com) and AUTHORIZE_URL fallback (claude.com/cai/oauth/authorize). - Add login_hint, login_method, and org_uuid params to startOAuthFlow() and the /authorize REST endpoint, matching Claude CLI optional authorize params.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds optional OAuth authorize query params ( Changes
Sequence Diagram(s)sequenceDiagram
participant Model as AnthropicMaxTextGenerationModel
participant Auth as AnthropicOAuthRequestAuthentication
participant PM as PoolManager
participant API as Anthropic API
Model->>Auth: authenticateRequest()
Auth->>PM: getActiveTokenWithEmail()
PM-->>Auth: {token, email}
Auth->>Auth: cache email
Auth-->>Model: Authorization header (Bearer token)
Model->>API: send request with token
API-->>Model: 429/529 + Retry-After
Model->>Model: handleRateLimitResponse($auth, $response)
Model->>Auth: getActiveEmail()
Auth-->>Model: email
Model->>Model: parseRetryAfterHeader($response)
Model->>PM: markRateLimited(email, retry_after_secs)
PM->>PM: update account cooldown
sequenceDiagram
participant Client as Client/Browser
participant REST as REST API Handler
participant PM as PoolManager
participant OAuth as OAuth Provider
Client->>REST: GET /authorize?login_hint=...&login_method=...&org_uuid=...
REST->>PM: startOAuthFlow(login_hint, login_method, org_uuid)
PM->>PM: build auth URL with provided params
PM-->>REST: redirect URL
REST-->>Client: redirect to OAuth provider
Client->>OAuth: user authenticates
OAuth-->>Client: authorization code
Client->>REST: POST /authorize (code)
REST->>PM: exchangeCode(code)
PM->>OAuth: exchange code for token
OAuth-->>PM: {access_token, scope, account.uuid, email?}
PM->>PM: validate scope contains REQUIRED_SCOPE
alt scope missing
PM-->>REST: {scope_error, granted_scopes}
REST-->>Client: 403 insufficient_scope
else scope ok
PM->>PM: addAccount(token, accountId)
PM-->>REST: success
REST-->>Client: success response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/Authentication/AnthropicOAuthRequestAuthentication.php (1)
74-83: ResetactiveEmailbefore each lookup.If
getActiveTokenWithEmail()returnsnull, the previous request's email stays cached andgetActiveEmail()can report the wrong account until the next successful authentication.Suggested change
public function authenticateRequest(Request $request): Request { + $this->activeEmail = null; $result = $this->pool->getActiveTokenWithEmail(); if ($result === null || empty($result['token'])) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Authentication/AnthropicOAuthRequestAuthentication.php` around lines 74 - 83, Reset the cached active email before attempting to fetch a token: in AnthropicOAuthRequestAuthentication (the method calling $this->pool->getActiveTokenWithEmail()), set $this->activeEmail = null prior to calling getActiveTokenWithEmail() so that a null result does not leave the previous email cached; if getActiveTokenWithEmail() returns a result, then set $this->activeEmail = $result['email'] as you already do, otherwise keep it null so getActiveEmail() returns the correct state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/OAuthPool/PoolManager.php`:
- Around line 190-229: The selection loop in PoolManager is choosing the oldest
account regardless of expiry, so an already-expired account can be picked and a
failed TokenRefresher.refresh causes an immediate null return; change selection
to prefer healthy, non-expired active tokens first (check $account['expires']
against $now_ms and $account['status'] === 'active' and non-empty access) and
only if none exist choose a refreshable candidate (has non-empty 'refresh' and
expired); implement two-pass selection or track both $best_active and
$best_refreshable; when a refresh attempt on $best_refreshable fails, mark its
status ('refresh-failed') and continue scanning/try the next candidate instead
of returning null so a healthy token (if any) can still be returned.
- Around line 442-446: The current check treats Retry-After: 0 as no header and
falls back to DEFAULT_COOLDOWN_MS; change the condition so any non-null
$retry_after_secs (including 0) takes precedence: if ($retry_after_secs !==
null) set $cooldown_ms = $retry_after_secs * 1000; otherwise if ($cooldown_ms
=== null) set DEFAULT_COOLDOWN_MS. Update the logic around the variables
$retry_after_secs, $cooldown_ms and use self::DEFAULT_COOLDOWN_MS only when
$retry_after_secs is null.
- Around line 637-646: Remove the unsupported organization query parameter by
deleting the conditional that sets $params['orgUUID'] based on $org_uuid;
specifically, remove the block that checks if ($org_uuid !== null && $org_uuid
!== '') and the assignment $params['orgUUID'] = $org_uuid so the authorize URL
is not sent an orgUUID parameter. Keep the other optional param blocks
(login_hint, login_method) unchanged and ensure no other code references
$params['orgUUID'] or $org_uuid for the authorization request.
---
Nitpick comments:
In `@src/Authentication/AnthropicOAuthRequestAuthentication.php`:
- Around line 74-83: Reset the cached active email before attempting to fetch a
token: in AnthropicOAuthRequestAuthentication (the method calling
$this->pool->getActiveTokenWithEmail()), set $this->activeEmail = null prior to
calling getActiveTokenWithEmail() so that a null result does not leave the
previous email cached; if getActiveTokenWithEmail() returns a result, then set
$this->activeEmail = $result['email'] as you already do, otherwise keep it null
so getActiveEmail() returns the correct state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e63aee8-b201-4758-83d3-f5175e6151e8
📒 Files selected for processing (5)
inc/rest-api.phpsrc/Authentication/AnthropicOAuthRequestAuthentication.phpsrc/Models/AnthropicMaxTextGenerationModel.phpsrc/OAuthPool/PoolManager.phpsrc/OAuthPool/TokenRefresher.php
Comment on lines
+190
to
+229
| foreach ($pool['accounts'] as $index => $account) { | ||
| $status = $account['status'] ?? 'idle'; | ||
| if ($status === 'rate-limited') { | ||
| continue; | ||
| } | ||
| if (empty($account['access'])) { | ||
| continue; | ||
| } | ||
| if ($best === null || ($account['lastUsed'] ?? '') < ($best['lastUsed'] ?? '')) { | ||
| $best = $account; | ||
| $best_index = $index; | ||
| } | ||
| } | ||
|
|
||
| if ($best === null) { | ||
| return null; | ||
| } | ||
|
|
||
| $expires = $best['expires'] ?? 0; | ||
| if ($expires > 0 && $expires <= $now_ms && !empty($best['refresh'])) { | ||
| $refresher = new TokenRefresher(); | ||
| $refreshed = $refresher->refresh($best['refresh']); | ||
|
|
||
| if ($refreshed !== null) { | ||
| $pool['accounts'][$best_index]['access'] = $refreshed['access_token']; | ||
| $pool['accounts'][$best_index]['expires'] = $now_ms + ($refreshed['expires_in'] * 1000); | ||
| $pool['accounts'][$best_index]['status'] = 'active'; | ||
| $pool['accounts'][$best_index]['lastUsed'] = gmdate('Y-m-d\TH:i:s\Z'); | ||
|
|
||
| if (!empty($refreshed['refresh_token'])) { | ||
| $pool['accounts'][$best_index]['refresh'] = $refreshed['refresh_token']; | ||
| } | ||
|
|
||
| $this->savePool($pool); | ||
| return ['token' => $refreshed['access_token'], 'email' => $best['email'] ?? '']; | ||
| } | ||
|
|
||
| $pool['accounts'][$best_index]['status'] = 'refresh-failed'; | ||
| $this->savePool($pool); | ||
| return null; |
There was a problem hiding this comment.
Prefer healthy tokens before refresh candidates.
This scan ignores expiry when picking the “best” account, so an older expired entry can win over a later valid token. If its refresh fails, the method returns null immediately and the new auth path never reaches the healthy account.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/OAuthPool/PoolManager.php` around lines 190 - 229, The selection loop in
PoolManager is choosing the oldest account regardless of expiry, so an
already-expired account can be picked and a failed TokenRefresher.refresh causes
an immediate null return; change selection to prefer healthy, non-expired active
tokens first (check $account['expires'] against $now_ms and $account['status']
=== 'active' and non-empty access) and only if none exist choose a refreshable
candidate (has non-empty 'refresh' and expired); implement two-pass selection or
track both $best_active and $best_refreshable; when a refresh attempt on
$best_refreshable fails, mark its status ('refresh-failed') and continue
scanning/try the next candidate instead of returning null so a healthy token (if
any) can still be returned.
- AnthropicOAuthRequestAuthentication::authenticateRequest(): reset $this->activeEmail = null before getActiveTokenWithEmail() to prevent stale email cache when the result is null. - PoolManager::markRateLimited(): treat Retry-After=0 as authoritative; only fall back to DEFAULT_COOLDOWN_MS when $retry_after_secs is null (previously the > 0 guard incorrectly skipped the 0 case). - PoolManager::startOAuthFlow(): remove unsupported orgUUID parameter from the authorize URL params; the claude.ai endpoint does not accept this parameter. $org_uuid is retained in the method signature for API compatibility but is no longer appended to the URL.
Contributor
Author
CodeRabbit review fixes (commit 6bdd5b1)All three actionable issues from the CodeRabbit review have been addressed: 1. Stale email cache in 2. Retry-After=0 edge case in 3. Unsupported Both modified files pass aidevops.sh v3.6.31 plugin for OpenCode v1.3.0 with claude-sonnet-4-6 Overall, 24m since this issue was created. |
Contributor
Author
|
Completed via PR #4, merged to main. Merged by deterministic merge pass (pulse-wrapper.sh). No worker summary was available — the worker either crashed before writing one or this PR predates the merge summary convention.aidevops.sh v3.6.31 spent 8m on this as a headless bash routine. |
This was referenced Apr 8, 2026
This was referenced May 16, 2026
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
1 participant
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
Incorporates six alignment improvements identified by comparing the plugin against the aidevops
oauth-pool-helper.shreference implementation.Changes
High priority
Retry-After header support (429/529)
PoolManager::markRateLimited()now accepts a$retry_after_secsparameter that overrides the fixed 5-minute default cooldown.PoolManager::markRateLimitedFromResponse()helper parsesRetry-After(integer seconds or HTTP-date per RFC 7231) from awp_remote_*response.AnthropicMaxTextGenerationModel::generateTextResult()intercepts 429/529 responses before throwing, callshandleRateLimitResponse()to mark the active account with the server-specified cooldown.AnthropicOAuthRequestAuthenticationnow tracks the active account email viagetActiveEmail()so the model can identify which account to mark.PoolManager::getActiveTokenWithEmail()returns{token, email}together to avoid a second pool lookup.Medium priority
accountId storage
PoolManager::addAccount()accepts an optional$account_idparameter (theaccount.uuidfrom the token response).exchangeCode()extractsaccount.uuidfrom theaccountobject in the token response and passes it toaddAccount().listAccounts()andhealthCheck()exposeaccountIdin their output for diagnostics.user:inference scope validation
exchangeCode()checks thescopefield in the token response againstuser:inferencebefore adding the account to the pool.['scope_error' => true, 'granted_scopes' => [...]]on failure (account is NOT added).rest_exchange_code()maps this to HTTP 403 with a clear message prompting re-authorization.Low priority
User-Agent alignment
claude-cli/2.1.80 (wordpress-plugin)viaPoolManager::USER_AGENT, matching the Claude CLI format.TokenRefresherreferencesPoolManager::USER_AGENTinstead of a hardcoded string.Fallback URL comments
PoolManagerconstants block now documents theplatform.claude.com/oauth/code/callbackfallback redirect URI and theclaude.com/cai/oauth/authorizefallback authorize URL, matching the notes inoauth-pool-helper.sh.login_hint / login_method / org_uuid
PoolManager::startOAuthFlow()accepts optional$login_hint,$login_method, and$org_uuidparameters, appending them to the authorize URL when provided./authorizeREST endpoint exposes these as optional query params with validation.Testing
All five modified files pass
php -lsyntax check. Functional testing requires a WordPress environment with a Claude Max account.Summary by CodeRabbit
New Features
Bug Fixes