Repository navigation
fix: resolve Gemini CLI 403 project-routing errors and prevent account bans - #868
christopher-s wants to merge 5 commits into
Conversation
…lation
- Remove x-goog-user-project header and executor-level project override
that caused 403 "Cloud Code Private API has not been used in project X"
- Add PROJECT_ROUTE_ERROR classifier type so project-routing 403s don't
permanently ban accounts (keeps accounts active, tracks the error)
- Fix Cloud Code envelope unwrapping for content accumulation in stream.ts
(Cloud Code wraps responses in { response: { candidates: [...] } })
- Extract unwrapGeminiChunk() into streamHelpers.ts with format guard
- Remove _currentModel singleton race condition from GeminiCLIExecutor
- Add handler for PROJECT_ROUTE_ERROR in chatCore.ts
- Add TODO in antigravity.ts about same stale-project risk
- Add 7 unit tests for error classifier and stream unwrap paths
…errors The stored projectId from OAuth connection time becomes stale because the Cloud Code API rotates free-tier projects. Native Gemini CLI refreshes the project every 30 seconds via loadCodeAssist — OmniRoute never did, causing 403 "has not been used in project X" errors that permanently banned accounts. - Add refreshProject() to GeminiCLIExecutor that calls loadCodeAssist API with 10s timeout and caches the result for 30s (matching native CLI) - transformRequest() replaces the stale projectId in the envelope before sending to the Cloud Code API, falling back to the stored ID on failure - Make transformRequest calls await-compatible in base executor and all subclasses (antigravity, cursor, kiro) so async overrides work
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a mechanism to refresh and cache Google Cloud project IDs for the Gemini CLI executor to prevent 403 'stale project' errors. It introduces a refreshProject method that calls the loadCodeAssist API, updates the error classifier to handle project routing errors without banning accounts, and adds utility functions to unwrap Cloud Code API response envelopes. Feedback focuses on improving the robustness of the refreshProject implementation, specifically addressing potential 'thundering herd' issues during cache misses, ensuring type safety when parsing project IDs, and strictly enforcing the cache size limit during eviction.
| async refreshProject(accessToken: string): Promise<string | null> { | ||
| // Check cache | ||
| const cached = projectCache.get(accessToken); | ||
| if (cached && cached.expiresAt > Date.now()) { | ||
| return cached.projectId; | ||
| } |
There was a problem hiding this comment.
This implementation is susceptible to a "thundering herd" issue. If multiple concurrent requests arrive for the same accessToken while the cache is empty or expired, they will all bypass the cache check and trigger separate fetch calls to the Google API. This can lead to unnecessary load and potential rate limiting. Consider deduplicating in-flight requests using a shared Map of promises.
| } else if (data.cloudaicompanionProject?.id) { | ||
| projectId = data.cloudaicompanionProject.id.trim(); | ||
| } |
There was a problem hiding this comment.
It's safer to verify that id is a string before calling .trim(). If the API returns a non-string value (e.g., a number), this will throw an error and crash the request processing.
| } else if (data.cloudaicompanionProject?.id) { | |
| projectId = data.cloudaicompanionProject.id.trim(); | |
| } | |
| } else if (typeof data.cloudaicompanionProject?.id === "string") { | |
| projectId = data.cloudaicompanionProject.id.trim(); | |
| } |
| if (projectCache.size >= MAX_CACHE_SIZE) { | ||
| const now = Date.now(); | ||
| for (const [key, val] of projectCache) { | ||
| if (val.expiresAt <= now) projectCache.delete(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current eviction logic only removes expired entries. If the cache is full of valid (non-expired) entries, it will continue to grow beyond MAX_CACHE_SIZE because projectCache.set is called regardless of whether any space was cleared. To strictly enforce the limit, you should remove the oldest entry if the cache is still full after the expiration cleanup.
| if (projectCache.size >= MAX_CACHE_SIZE) { | |
| const now = Date.now(); | |
| for (const [key, val] of projectCache) { | |
| if (val.expiresAt <= now) projectCache.delete(key); | |
| } | |
| } | |
| if (projectCache.size >= MAX_CACHE_SIZE) { | |
| const now = Date.now(); | |
| for (const [key, val] of projectCache) { | |
| if (val.expiresAt <= now) projectCache.delete(key); | |
| } | |
| // If still full, evict the oldest entry (Map maintains insertion order) | |
| if (projectCache.size >= MAX_CACHE_SIZE) { | |
| const firstKey = projectCache.keys().next().value; | |
| if (firstKey !== undefined) projectCache.delete(firstKey); | |
| } | |
| } |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| * The Cloud Code API wraps responses in { response: { candidates: [...] } } | ||
| * while standard Gemini returns { candidates: [...] } directly. | ||
| */ | ||
| export function unwrapGeminiChunk(parsed) { |
There was a problem hiding this comment.
WARNING: Incorrect unwrapping logic - handles edge case incorrectly
The ternary parsed.candidates ? parsed : parsed.response || parsed has a bug:
- When
parsed.responseexists but isnull(notundefined), the expressionparsed.response || parsedreturnsparsed(truthy check fails becausenullis falsy). - This means
unwrapGeminiChunk({ candidates: [...], response: null })incorrectly returnsparsed(with top-level candidates), whileunwrapGeminiChunk({ response: null })incorrectly returnsparsed(the input).
Correct logic: Only unwrap when candidates is NOT at top level AND response exists:
export function unwrapGeminiChunk(parsed) {
if (!parsed.candidates && parsed.response) {
return parsed.response;
}
return parsed;
}The test at line 96 (returns parsed when response exists but is null/undefined) should actually fail with the current implementation since null is falsy and would trigger the || parsed fallback.
| * while standard Gemini returns { candidates: [...] } directly. | ||
| */ | ||
| export function unwrapGeminiChunk(parsed) { | ||
| return parsed.candidates ? parsed : parsed.response || parsed; |
There was a problem hiding this comment.
SUGGESTION: Use explicit !== null check for null safety
Consider changing parsed.response || parsed to parsed.response !== undefined ? parsed.response : parsed for clarity and to handle the null case explicitly.
| }); | ||
|
|
||
| it("returns parsed when response exists but is null/undefined", () => { | ||
| const chunk = { response: null, other: "data" }; |
There was a problem hiding this comment.
WARNING: Test may pass incorrectly due to bug in unwrapGeminiChunk
The test returns parsed when response exists but null/undefined expects parsed to be returned when response: null. However, because the implementation uses parsed.response || parsed, the falsy null causes the fallback to parsed, making the test pass for the wrong reason.
If the implementation is fixed to if (!parsed.candidates && parsed.response), this test will fail because parsed.response (null) won't trigger the unwrap, and the function will correctly return parsed. The test should either be removed or updated to test response: undefined instead of response: null.
Code Review SummaryStatus: All Previous Issues Resolved | Recommendation: Merge Overview
Changes Reviewed (vs previous commit)All 3 issues from the previous review have been fixed in this commit:
New Changes Analysis
Files Reviewed (3 files)
Previous review issues have been resolved. No new issues found. |
- Deduplicate in-flight loadCodeAssist requests to prevent thundering herd - Add typeof guard on cloudaicompanionProject.id before calling .trim() - Evict oldest cache entry when all entries are still valid - Fix unwrapGeminiChunk to use explicit null-safe check - Update test description for null response case
CI Failure AnalysisAll failures are pre-existing on Lint (t11:any-budget) — FAIL
Unit Tests (test 214) — FAIL
Coverage / E2E — FAILCascading failures from the Unit Tests failure. Not independent issues. |
|
Merged manually into |
Summary
All Gemini CLI models were broken in production. The root cause was a stale
projectIdthat OmniRoute stores once at OAuth connection time and never refreshes. Native Gemini CLI refreshes the project every 30 seconds via theloadCodeAssistAPI, but OmniRoute sent the same stale project ID indefinitely, causing:FORBIDDEN(terminal), permanently disabling every Gemini CLI accountRoot Cause
Three compounding issues:
projectIdin Cloud Code envelopeopenai-to-gemini.ts→wrapInCloudCodeEnvelope()x-goog-user-projectheader sending stale projectgemini-cli.ts→buildHeaders()errorClassifier.ts→classifyProviderError()Changes
1. Refresh project ID via
loadCodeAssistAPI (open-sse/executors/gemini-cli.ts)The core fix. Added
refreshProject()toGeminiCLIExecutorthat:https://cloudcode-pa.googleapis.com/v1internal:loadCodeAssistwith the OAuth tokencloudaicompanionProjectfrom the response (matching native Gemini CLI behavior)projectIdif the refresh fails (best-effort)The
transformRequest()method callsrefreshProject()before each request and replaces the staleprojectIdin the Cloud Code envelope body.2. Remove stale project from headers (
open-sse/executors/gemini-cli.ts)x-goog-user-projectheader that was sending the stale project ID_currentModelinstance variable and project override fromtransformRequestUser-Agentheader (model-specific UA was unnecessary)3. Soften 403 error classification (
open-sse/services/errorClassifier.ts)Added a new error type
PROJECT_ROUTE_ERRORthat detects "has not been used in project" in 403 responses. This is classified as transient (not a ban) — the account stays active but the error is tracked. The handler inchatCore.tslogs a warning but does not disable the node.4. Handle Cloud Code envelope in streaming (
open-sse/utils/stream.ts,streamHelpers.ts)The Cloud Code API wraps streaming responses in
{ response: { candidates: [...] } }instead of the standard{ candidates: [...] }. AddedunwrapGeminiChunk()to normalize this format, guarded byisGeminiFormatcheck so it only applies to Gemini-family formats.5. Enable async
transformRequest(open-sse/executors/base.ts,antigravity.ts,cursor.ts,kiro.ts)Changed all
transformRequestcalls toawaitso that async overrides (like the newrefreshProjectin Gemini CLI) work correctly.awaiton a non-Promise is a no-op, so existing synchronoustransformRequestimplementations are unaffected.6. Update translator comment (
open-sse/translator/request/openai-to-gemini.ts)Updated comment in
wrapInCloudCodeEnvelopeto document that the executor'stransformRequestrefreshes the project ID.Files Changed
open-sse/executors/gemini-cli.tsrefreshProject()withloadCodeAssist, removed stale project injectionopen-sse/services/errorClassifier.tsPROJECT_ROUTE_ERRORtype for project-routing 403sopen-sse/handlers/chatCore.tsPROJECT_ROUTE_ERROR(no account ban)open-sse/executors/base.tsawait transformRequestfor async supportopen-sse/executors/antigravity.tsawait transformRequest+ TODO for same issueopen-sse/executors/cursor.tsawait transformRequestopen-sse/executors/kiro.tsawait transformRequestopen-sse/utils/stream.tsopen-sse/utils/streamHelpers.tsunwrapGeminiChunk()utilityopen-sse/translator/request/openai-to-gemini.tstests/unit/error-classifier.test.mjsPROJECT_ROUTE_ERRORtests/unit/streamHelpers.test.mjsunwrapGeminiChunkTesting
Tested against live Cloud Code API with a Gemini CLI OAuth account:
activeafter all tests (no false bans)loadCodeAssistrefresh works correctly, project is updated before each request