Repository navigation
fix(oauth): preserve non-rotating providers' refresh_token on unrecoverable refresh (#3679) - #3766
Conversation
…erable refresh (#3679) The proactive health-check refresh deactivation branch nulled the stored refresh_token on any unrecoverable error (e.g. invalid_grant). That was only meant for rotating one-time-use tokens (Codex/OpenAI); for non-rotating Google-family providers (gemini-cli/antigravity/gemini) it destroyed the user's only recovery artifact, leaving the connection permanently showing 'No valid refresh token available'. Gate the null on isRotatingProvider so non-rotating tokens are preserved. Closes #3679
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where the stored refresh_token of non-rotating OAuth providers (such as Google-family providers) was incorrectly nulled out during a failed proactive health-check refresh. The fix gates the clearing of the refresh token on isRotatingProvider, preserving it for non-rotating providers to allow recovery. A corresponding unit test has been added to prevent regressions. There are no review comments to address, so I have no further feedback to provide.
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.
| testStatus: "expired", | ||
| lastError: `Refresh token consumed (${result.error}). Please re-authenticate this account.`, | ||
| lastError: isRotatingProvider | ||
| ? `Refresh token consumed (${result.error}). Please re-authenticate this account.` |
There was a problem hiding this comment.
WARNING: Deactivation message hides the actionable OAuth code
For Google/Qwen invalid_grant responses, the refresh functions normalize to { error: "unrecoverable_refresh_error", code: "invalid_grant" }, but this message interpolates only result.error. The new "rejected" wording is more accurate than "consumed", but it still reports the sentinel instead of the underlying OAuth code operators need to diagnose. Use result.code ?? result.error in both ternary arms.
| ? `Refresh token consumed (${result.error}). Please re-authenticate this account.` | |
| ? `Refresh token consumed (${result.code ?? result.error}). Please re-authenticate this account.` |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // the connection unrecoverable ("No valid refresh token available"). The null was | ||
| // only meant for rotating one-time-use tokens (Codex/OpenAI). Non-rotating providers | ||
| // must keep the stored refresh_token as the recovery artifact. | ||
| test("checkConnection preserves refresh_token for non-rotating providers on unrecoverable error (#3679)", async () => { |
There was a problem hiding this comment.
SUGGESTION: Add paired rotating-provider regression coverage
This test verifies the new non-rotating preservation path, but it does not assert the existing rotating-provider behavior still clears refreshToken on unrecoverable refresh. A paired test using a provider in ROTATING_REFRESH_PROVIDERS would guard against accidentally preserving rotating tokens and reintroducing repeated stale-token loops.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
… v3.8.24 merges These 5 files exceeded the frozen baseline solely because of already-merged, already-reviewed v3.8.24 PRs that landed after this branch froze its baseline: - src/lib/providers/validation.ts 4209→4302 (#3767 qwen-web fix) - src/shared/components/RequestLoggerV2.tsx 1232→1276 (#3760 logs light-mode) - open-sse/executors/antigravity.ts 1553→1572 (#3766 antigravity refresh) - open-sse/executors/base.ts 1199→1205 (#3756 xhigh effort) - open-sse/translator/response/openai-responses.ts 873→878 (#3762 tool-call integrity) None are touched by this branch; the bump reflects current release reality.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Reviewed by nex-n2-pro:free · 1,389,779 tokens |
…erable refresh (cheaper-inference#3679) (cheaper-inference#3766) The proactive health-check refresh deactivation branch nulled the stored refresh_token on any unrecoverable error (e.g. invalid_grant). That was only meant for rotating one-time-use tokens (Codex/OpenAI); for non-rotating Google-family providers (gemini-cli/antigravity/gemini) it destroyed the user's only recovery artifact, leaving the connection permanently showing 'No valid refresh token available'. Gate the null on isRotatingProvider so non-rotating tokens are preserved. Closes cheaper-inference#3679
Closes #3679
Problem
Google-family OAuth connections (gemini-cli / antigravity / gemini) died after ~1h with
refresh_token = NULLand "No valid refresh token available". Reporter's evidence: token present (279 chars) at import → after the first proactive health-check refresh at the 60-min mark, refresh_token became NULL and the connection was unrecoverable.Root cause
src/lib/tokenHealthCheck.ts— theisUnrecoverableRefreshErrordeactivation branch wroterefreshToken: null. That null was designed for rotating one-time-use tokens (Codex/OpenAI, whose tokens are genuinely consumed), but it fired for all providers, including non-rotating Google ones.ROTATING_REFRESH_PROVIDERS(already defined in the same function) does NOT include gemini-cli/antigravity/gemini — yet they still got nulled. Nulling the token removes the user's only recovery artifact and produces the "No valid refresh token available" cascade.Fix
Gate the
refreshToken: nullwrite onisRotatingProvider(one-line conditional spread). Non-rotating providers keep their stored refresh_token; rotating providers are unchanged. Also makes the log/lastError message accurate ("rejected" vs "consumed") for non-rotating providers.Validation (Hard Rule #18 — TDD)
tests/unit/token-health-check.test.ts— new test "preserves refresh_token for non-rotating providers on unrecoverable error (#3679)":null(assertion fails).rt-preserve-3679.Note: the underlying why of the upstream
invalid_grant(Google-side token rotation/limit for imported gemini-cli creds) is environment-specific; this fix removes the destructive NULL + recovery-blocking cascade regardless of that trigger.