feat(antigravity): named-account pooling with quota failover - #604
jesusvillota wants to merge 4 commits into
Conversation
Store Google accounts as named files under ~/.shunt/accounts/antigravity/<name>.json (SHUNT_ANTIGRAVITY_ACCOUNTS_DIR override) via shunt login antigravity --name, reusing the flat StoredAuth schema so the existing refresh/discovery path reads them unchanged. antigravity_oauth accepts account/accounts like the other OAuth pools; an empty pool preserves the singleton ~/.shunt/antigravity-auth.json fallback. The Gemini adapter resolves credentials per pool candidate under the pool refresh lock and rotates on 429 RESOURCE_EXHAUSTED (classify_antigravity), rebuilding token, project id, and catalog per attempt and naming the serving account via x-shunt-account. Startup, check, and reload accept pool candidates without the singleton file.
There was a problem hiding this comment.
Code Review
This pull request introduces account-pooling and failover capabilities for the Antigravity provider, allowing multiple named accounts to be configured and rotated upon encountering rate limits. Key updates include new CLI options for named logins, configuration schema extensions, and failover routing logic in the Gemini adapter. A code review comment identifies a high-severity issue in the Gemini adapter where an empty HeaderMap is passed to the response classifier instead of the actual upstream response headers, which would prevent proper rate-limit and quota tracking during failover.
| let status = error.response.status(); | ||
| match crate::accounts::classify_antigravity(status, &HeaderMap::new()) { | ||
| crate::accounts::FailoverAction::Relay => return Err(error), | ||
| crate::accounts::FailoverAction::Rotate | ||
| | crate::accounts::FailoverAction::PauseSame | ||
| | crate::accounts::FailoverAction::RefreshRetry => { | ||
| state.accounts.cooldown( | ||
| &route.provider, | ||
| account, | ||
| std::time::Duration::from_secs( | ||
| if status == StatusCode::TOO_MANY_REQUESTS { | ||
| 60 | ||
| } else { | ||
| 30 | ||
| }, | ||
| ), | ||
| crate::accounts::rotation_reason(status, &HeaderMap::new()), | ||
| ); |
There was a problem hiding this comment.
Symptom: The code passes an empty HeaderMap::new() to classify_antigravity and rotation_reason instead of the actual headers from the failed upstream response (error.response.headers()).
Source: McConnell — Code Complete (Ch. 8: Defensive Programming)
Consequence: Any rate-limiting, quota, or retry-after headers returned by the Antigravity/Gemini backend are completely ignored. This prevents the failover and cooldown logic from utilizing accurate retry/backoff information, potentially leading to suboptimal routing or premature retries.
Remedy: Pass error.response.headers() instead of &HeaderMap::new() to both classify_antigravity and rotation_reason.
| let status = error.response.status(); | |
| match crate::accounts::classify_antigravity(status, &HeaderMap::new()) { | |
| crate::accounts::FailoverAction::Relay => return Err(error), | |
| crate::accounts::FailoverAction::Rotate | |
| | crate::accounts::FailoverAction::PauseSame | |
| | crate::accounts::FailoverAction::RefreshRetry => { | |
| state.accounts.cooldown( | |
| &route.provider, | |
| account, | |
| std::time::Duration::from_secs( | |
| if status == StatusCode::TOO_MANY_REQUESTS { | |
| 60 | |
| } else { | |
| 30 | |
| }, | |
| ), | |
| crate::accounts::rotation_reason(status, &HeaderMap::new()), | |
| ); | |
| let status = error.response.status(); | |
| let headers = error.response.headers(); | |
| match crate::accounts::classify_antigravity(status, headers) { | |
| crate::accounts::FailoverAction::Relay => return Err(error), | |
| crate::accounts::FailoverAction::Rotate | |
| | crate::accounts::FailoverAction::PauseSame | |
| | crate::accounts::FailoverAction::RefreshRetry => { | |
| state.accounts.cooldown( | |
| &route.provider, | |
| account, | |
| std::time::Duration::from_secs( | |
| if status == StatusCode::TOO_MANY_REQUESTS { | |
| 60 | |
| } else { | |
| 30 | |
| }, | |
| ), | |
| crate::accounts::rotation_reason(status, headers), | |
| ); |
References
- To ensure rate-limit and quota headers (which may only be present on error responses like 429 Too Many Requests) are correctly captured and recorded, perform quota parsing and recording on a common path before response status classification, rather than restricting it to successful response paths.
|
| AntigravityOauth { | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| account: Option<String>, | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| accounts: Option<Vec<AccountSelection>>, |
There was a problem hiding this comment.
Unsupported token source passes validation
Antigravity now accepts generic inline account entries containing token_env, but the runtime resolver always rejects that source because a bearer alone has no project context. A configuration such as accounts = [{ name = "primary", token_env = "AGY_TOKEN" }] therefore passes check and startup, then every request skips the candidate and a single-account pool returns 503. Reject this source during configuration validation or provide a supported way to supply the required project ID.
Knowledge Base Used: Authentication and account management
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/config/upstreams.rs
Line: 101-105
Comment:
**Unsupported token source passes validation**
Antigravity now accepts generic inline account entries containing `token_env`, but the runtime resolver always rejects that source because a bearer alone has no project context. A configuration such as `accounts = [{ name = "primary", token_env = "AGY_TOKEN" }]` therefore passes check and startup, then every request skips the candidate and a single-account pool returns 503. Reject this source during configuration validation or provide a supported way to supply the required project ID.
**Knowledge Base Used:** [Authentication and account management](https://app.greptile.com/passionfactory/-/custom-context/knowledge-base/pleaseai/shunt/-/docs/authentication.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 11.69%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | parse_body_to_value[200] |
3 ms | 3.4 ms | -11.76% |
| ❌ | parse_body_to_value[50] |
756.7 µs | 856.2 µs | -11.62% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing jesusvillota:feat/antigravity-account-pool (80ac0f1) with main (bd2cf38)
Four review findings on the Antigravity account-pool PR, all verified against the code before fixing: - Pool cooldown ignored the real upstream response: headers were already discarded one layer below the classification call (map_gemini_error), and the hardcoded 60s/30s cooldowns never honored Retry-After. Move classification into forward_single, where the raw reqwest::Response headers are still available, and honor Retry-After on 429 (clamped 1s..=1h, default 60s), mirroring forward_kimi_oauth. - A pooled 401 only cooled down and rotated, so a rejected-but-not-yet- locally-expired token kept getting reused. Add AntigravityAuthStore::force_refresh_if_access_token (with terminal vs. transient classification of the refresh-endpoint rejection) and a force-refresh-then-retry-same-account arm in the gemini pool loop, mirroring forward_claude_oauth. A terminal rejection marks the account needs_relogin instead of looping silently. - token_env on an antigravity_oauth account passed config validation but always failed at request time (no project id on a bearer). Reject it in Config::validate, scoped to Antigravity only (Kimi legitimately supports token_env). - store::store_oauth_tokens wrote a named account file without taking CREDENTIAL_FILE_LOCK, so a concurrent refresh/discovery writeback could clobber a fresh login. Route it through the same file lock write_stored already uses, via a new shared write_account_file_locked helper that keeps the born-private 0700/0600 semantics. Also adds SHUNT_ANTIGRAVITY_TOKEN_URL (mirroring the Claude/Codex stores) so the refresh path is testable, and covers all four fixes plus the Codecov-flagged gap with new unit and integration tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
get_valid and force_refresh_if_access_token read the stored credential, refreshed it over the network, then wrote the result straight back — with no re-read under CREDENTIAL_FILE_LOCK. A `shunt login antigravity --name` landing in that window (a different process, so the in-process REFRESH_LOCK can't see it) was silently clobbered by the stale refresh. Add persist_refresh: a compare-and-swap under CREDENTIAL_FILE_LOCK that re-reads the file, checks identity against the pre-refresh snapshot, and only writes when nothing else landed — folding the refreshed tokens onto the current on-disk email/project_id rather than the stale snapshot's. Same shape as project_id's writeback merge, sharing its test hook. Pins the race with force_refresh_serializes_against_a_concurrent_relogin, mirroring project_id_writeback_serializes_against_a_concurrent_relogin. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Design: pool Google accounts as named files under ~/.shunt/accounts/antigravity/.json (SHUNT_ANTIGRAVITY_ACCOUNTS_DIR override) via shunt login antigravity --name, reusing the flat StoredAuth schema so the existing refresh/discovery path reads them unchanged.
Tests: tests/antigravity_multi_account.rs (A-429 rotates to B with its own token/project; singleton still serves with no pool) plus store/config/readiness unit tests. Full suite green except pre-existing codex_websocket_fallback::websocket_rate_limits_event_records_account_quota failure, verified failing on clean main too. Docs updated in EN + ko/ja/zh-cn surfaces.
Summary by cubic
Adds named-account pooling to the Antigravity provider so requests rotate across multiple Google accounts when one hits a 429 quota limit.
shunt login antigravity --name <name>saves credentials to~/.shunt/accounts/antigravity/<name>.json(configurable viaSHUNT_ANTIGRAVITY_ACCOUNTS_DIR), leaving the singleton file untouched.antigravity_oauthnow acceptsaccount/accountsselection like the other OAuth pools; with no pool configured and an empty store, it falls back to the singleton~/.shunt/antigravity-auth.json.Retry-After, clamped 1s–1h) and tries the next, force-refreshes and retries the same account on a 401 (marking itneeds_reloginon terminal rejection), and adds anx-shunt-accountresponse header naming the serving account.shunt check, and reload now treat named pool candidates as satisfying the credential guard even when the singleton file is absent.token_envis rejected onantigravity_oauthaccounts, and named-account writes take the credential file lock so concurrent refreshes can't clobber a fresh login.Docs updated across all language surfaces; integration tests cover 429 failover, 401 recovery, and the singleton fallback.
Written for commit 80ac0f1. Summary will update on new commits.