Repository navigation
Conversation
…ling - Fix Claude OAuth token refresh to use form-urlencoded format (standard OAuth2) - Add anthropic-beta header required by Claude OAuth API - Switch Antigravity quota to use retrieveUserQuota API (same as Gemini CLI) - Parse quota reset time for all providers (not just Antigravity) - Add quota reset keywords to error classifier - Cap maximum retry time at 24 hours to prevent infinite wait Closes diegosouzapw#836, diegosouzapw#857, diegosouzapw#858, diegosouzapw#832
There was a problem hiding this comment.
Code Review Summary
Status: No Issues Found | Recommendation: Merge
Overview
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 0 |
| SUGGESTION | 0 |
Verification Performed
All 4 changed files were reviewed:
-
open-sse/services/accountFallback.ts(lines 244-245)- Added "quota will reset" and "your quota will reset" to quota exhaustion detection
- Pattern follows existing code style using
includes()
-
open-sse/services/tokenRefresh.ts(lines 210-224)- Changed to form-urlencoded format as required by Anthropic OAuth spec
- Added required
anthropic-beta: oauth-2025-04-20header PROVIDERS.claude.clientIdcorrectly sourced from REGISTRY.oauth.clientIdDefault
-
open-sse/services/usage.ts(lines 668-728)- Switched to
retrieveUserQuotaAPI for accurate quota across Free/Pro tiers - Added projectId validation with early return
- Added AbortSignal timeout (10s) for reliability
- Proper null handling for bucket.remainingFraction
- Correct Math.max(0, total - remaining) to prevent negative values
- Switched to
-
open-sse/utils/error.ts(lines 125-134)- Extended retry time parsing to all providers (not just Antigravity)
- Added 24-hour cap to prevent infinite wait scenarios
Files Reviewed (4 files)
open-sse/services/accountFallback.tsopen-sse/services/tokenRefresh.tsopen-sse/services/usage.tsopen-sse/utils/error.ts
There was a problem hiding this comment.
Code Review
This pull request updates error classification for quota resets, switches Claude OAuth token refresh to form-urlencoded with a beta header, and migrates Antigravity usage tracking to the retrieveUserQuota API. It also introduces a 24-hour cap on retry times. Feedback includes recommendations to avoid hardcoding API endpoints in favor of existing configuration patterns, safer error handling in catch blocks, and merging redundant logic for parsing retry times.
| const response = await fetch( | ||
| "https://cloudcode-pa.googleapis.com/v1internal:retrieveUserQuota", |
There was a problem hiding this comment.
The API endpoint for Antigravity quota retrieval is hardcoded here, which is inconsistent with the existing configuration pattern in this file. It is recommended to update ANTIGRAVITY_CONFIG.quotaApiUrl (line 16) to the new retrieveUserQuota endpoint and reference it here instead.
const response = await fetch(ANTIGRAVITY_CONFIG.quotaApiUrl, {| }; | ||
| } catch (error) { | ||
| return { message: `Antigravity error: ${error.message}` }; | ||
| return { message: `Antigravity error: ${(error as Error).message}` }; |
There was a problem hiding this comment.
Casting error directly to Error in a catch block can be unsafe if the thrown value is not an instance of Error (e.g., a string or a plain object). A safer approach is to check if it's an instance of Error or use a utility to extract the message.
| return { message: `Antigravity error: ${(error as Error).message}` }; | |
| return { message: `Antigravity error: ${error instanceof Error ? error.message : String(error)}` }; |
| if (response.status === 429 && !retryAfterMs) { | ||
| retryAfterMs = parseAntigravityRetryTime(messageStr); | ||
| } |
There was a problem hiding this comment.
This block is redundant for the antigravity provider because the previous block (lines 121-123) already calls parseAntigravityRetryTime for 429 status codes. Since the parsing logic is now intended to be applied globally for all providers on 429 errors, these blocks should be merged for better maintainability.
| if (response.status === 429 && !retryAfterMs) { | |
| retryAfterMs = parseAntigravityRetryTime(messageStr); | |
| } | |
| // Parse retry time for providers with "quota will reset after XhYmZs" format | |
| if (response.status === 429 && !retryAfterMs) { | |
| retryAfterMs = parseAntigravityRetryTime(messageStr); | |
| } |
|
Integrated into release/v3.4.0. Released as part of v3.4.0. Thank you! |
…ta, and 429 rate-limit handling
…ta, and 429 rate-limit handling
Summary
Fixes 3 bugs in OmniRoute:
Verification