fix(auth): cooldown Antigravity credentials on invalid_grant instead of silent retry loop#4130
Conversation
…trying them indefinitely Antigravity OAuth credentials that return HTTP 400 with invalid_grant (revoked/banned accounts) were never cooled down because: 1. isRequestInvalidError matched the outer invalid_request_error wrapper, causing the conductor to return immediately without trying other creds 2. MarkResult's switch on statusCode fell into the default case for 400, applying no cooldown — the bad credential was reselected immediately Now: - isRequestInvalidError returns false when the message contains invalid_grant, so the conductor falls through to try other credentials - MarkResult and applyAuthFailureState treat invalid_grant like 401 (30-minute cooldown + model suspension) Closes router-for-me#4120
|
This pull request targeted The base branch has been automatically changed to |
There was a problem hiding this comment.
Code Review
This pull request introduces handling for OAuth invalid_grant errors, treating them as credential-level issues that trigger a 30-minute cooldown rather than standard invalid request payloads. It updates error classification, state management, and adds corresponding unit tests. The review feedback suggests making the invalid_grant substring check case-insensitive, simplifying the error message check using err.Error(), and explicitly clearing auth.NextRetryAfter when disableCooling is enabled to avoid preserving stale cooldowns.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func isOAuthInvalidGrantMessage(msg string) bool { | ||
| return strings.Contains(msg, "invalid_grant") | ||
| } |
There was a problem hiding this comment.
We should perform a case-insensitive check when looking for the "invalid_grant" substring. Upstream OAuth providers or intermediate API wrappers might return error codes or messages in uppercase (e.g., "INVALID_GRANT") or mixed case. Converting the message to lowercase first ensures robust detection.
| func isOAuthInvalidGrantMessage(msg string) bool { | |
| return strings.Contains(msg, "invalid_grant") | |
| } | |
| func isOAuthInvalidGrantMessage(msg string) bool { | |
| return strings.Contains(strings.ToLower(msg), "invalid_grant") | |
| } |
| func isOAuthInvalidGrantResultError(err *Error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| return isOAuthInvalidGrantMessage(err.Message) || isOAuthInvalidGrantMessage(err.Code) | ||
| } |
There was a problem hiding this comment.
Instead of manually checking both err.Message and err.Code separately, we can leverage the err.Error() method. This is cleaner, more maintainable, and consistent with how we check errors in other parts of the codebase (such as in applyAuthFailureState).
| func isOAuthInvalidGrantResultError(err *Error) bool { | |
| if err == nil { | |
| return false | |
| } | |
| return isOAuthInvalidGrantMessage(err.Message) || isOAuthInvalidGrantMessage(err.Code) | |
| } | |
| func isOAuthInvalidGrantResultError(err *Error) bool { | |
| if err == nil { | |
| return false | |
| } | |
| return isOAuthInvalidGrantMessage(err.Error()) | |
| } |
| if resultErr != nil && isOAuthInvalidGrantMessage(resultErr.Error()) { | ||
| auth.StatusMessage = "unauthorized" | ||
| if !disableCooling { | ||
| auth.NextRetryAfter = now.Add(30 * time.Minute) | ||
| } |
There was a problem hiding this comment.
When disableCooling is true, auth.NextRetryAfter is not explicitly cleared, which means any pre-existing cooldown timestamp might be preserved. To be consistent with other status code cases (like 401, 402, 403, 404, etc.) and to ensure cooling is correctly disabled, we should explicitly set auth.NextRetryAfter = time.Time{} when disableCooling is true.
| if resultErr != nil && isOAuthInvalidGrantMessage(resultErr.Error()) { | |
| auth.StatusMessage = "unauthorized" | |
| if !disableCooling { | |
| auth.NextRetryAfter = now.Add(30 * time.Minute) | |
| } | |
| if resultErr != nil && isOAuthInvalidGrantMessage(resultErr.Error()) { | |
| auth.StatusMessage = "unauthorized" | |
| if disableCooling { | |
| auth.NextRetryAfter = time.Time{} | |
| } else { | |
| auth.NextRetryAfter = now.Add(30 * time.Minute) | |
| } | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fd4df3b6b
ℹ️ 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".
| next := now.Add(30 * time.Minute) | ||
| state.NextRetryAfter = next | ||
| suspendReason = "unauthorized" | ||
| shouldSuspendModel = true |
There was a problem hiding this comment.
Cool down the whole invalid_grant credential
When a revoked Antigravity refresh token returns invalid_grant for one model, this only records the cooldown on that model state and suspends that single auth/model pair. isAuthBlockedForModel treats the same auth as available for a different requested model when that model has no state, so the banned credential can still be selected for every other model and recreate the retry/failover loop per model. Since invalid_grant is credential-level, block the auth across models rather than only result.Model.
Useful? React with 👍 / 👎.
| if !disableCooling { | ||
| auth.NextRetryAfter = now.Add(30 * time.Minute) |
There was a problem hiding this comment.
Clear stale retry time when cooling is disabled
When disable_cooling is true and an auth-level invalid_grant is recorded after the auth already has a retry deadline, this branch leaves auth.NextRetryAfter unchanged because it only assigns in the !disableCooling path. The other auth-level cooldown cases above explicitly zero this field when cooling is disabled, otherwise selector checks can keep blocking the auth on a stale deadline despite cooling being disabled.
Useful? React with 👍 / 👎.
- Case-insensitive invalid_grant check (strings.ToLower)
- Simplify isOAuthInvalidGrantResultError to use err.Error()
- Explicit auth.NextRetryAfter = time.Time{} when disableCooling
- Set auth-level NextRetryAfter alongside model-level so the banned
credential is blocked across all models, not just result.Model
Summary
invalid_grant(banned/revoked accounts) were never cooled down, causing the conductor to repeatedly select the bad credentialisRequestInvalidErrornow excludesinvalid_granterrors so the conductor tries other credentials instead of returning immediatelyMarkResultandapplyAuthFailureStatenow treatinvalid_grantlike 401 unauthorized (30-minute cooldown + model suspension)Root cause
Two independent bugs conspire:
Bug 1 —
isRequestInvalidErrorfalse positive. The Antigravity API wrapsinvalid_grantinside aninvalid_request_errorenvelope.isRequestInvalidErrormatched the outer wrapper, returningtrue— this told the conductor to stop immediately without trying other credentials in the pool.Bug 2 — no cooldown in
MarkResult. When the error did reachMarkResult, thedefaultcase in thestatusCodeswitch applied no cooldown (NextRetryAfter = time.Time{}). The bad credential was immediately eligible for reselection on the next request.Combined effect: with multiple credentials, one valid and one banned, the banned credential kept being selected and blocking requests.
Changes
conductor.goisOAuthInvalidGrantMessage+isOAuthInvalidGrantResultErrorhelpersconductor.go:4155isRequestInvalidErrorreturnsfalseearly when message containsinvalid_grantconductor.go:3736MarkResultdefault case checks forinvalid_grantand applies 401-like cooldownconductor.go:4272applyAuthFailureStatedefault case applies same cooldown forinvalid_grantcooldown_backoff_test.goTests
go test -run "Cooldown\|Backoff\|InvalidGrant\|IsRequestInvalid" ./sdk/cliproxy/auth/— all 22 tests passgo build -o test-output ./cmd/server— compiles cleanlyCloses #4120