Conversation
Enforce per-account proxy routing across auth/request/quota flows with fail-closed behavior
Add regression coverage for proxy propagation, interception hardening, and legacy storage backfill
Make e2e helper scripts work on Windows opencode binary
WalkthroughThis pull request introduces comprehensive proxy support for network requests across the application. It adds two new dependencies (fetch-socks and undici) for SOCKS and HTTP proxy handling. The changes extend public function signatures in authentication, quota checking, project management, and search to accept optional proxy configurations and account indices. A new proxy management module implements failover logic, cooldown tracking, and dispatcher caching. CLI and UI flows gain interactive proxy configuration options. Per-account proxy state is persisted in storage and integrated into token refresh, OAuth exchange, and API calls. Windows compatibility is improved in test scripts. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds per-account HTTP/SOCKS proxy support with fail-closed routing semantics to prevent account IP leakage. All account-scoped external API calls (OAuth, token refresh, project context, quota, search, generation) are routed through
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as AI SDK / OpenCode
participant Plugin as plugin.ts (fetch interceptor)
participant Proxy as proxy.ts (fetchWithProxy)
participant ProxyServer as Proxy Server (HTTP/SOCKS)
participant API as Google APIs
Client->>Plugin: fetch(request, init)
Plugin->>Plugin: isGenerativeLanguageRequest(input)
alt Not a target request
Plugin->>API: fetch(input, init) [direct]
else Target request
Plugin->>Plugin: Resolve account (proxies, accountIndex)
Plugin->>Proxy: fetchWithProxy(url, init, proxies, accountIndex)
alt No proxies configured
Proxy->>API: fetch(url, init) [direct]
else Proxies configured
Proxy->>Proxy: Filter enabled & available proxies
alt No available proxies
Proxy-->>Plugin: throw ProxyExhaustedError (fail-closed)
else Available proxy found
Proxy->>ProxyServer: fetch via dispatcher (ProxyAgent/SOCKS)
alt Proxy connection succeeds
ProxyServer->>API: Proxied request
API-->>ProxyServer: Response
ProxyServer-->>Proxy: Response
Proxy->>Proxy: markSuccess (clear cooldown)
Proxy-->>Plugin: Response
else Proxy connection fails
Proxy->>Proxy: markFailed (set cooldown)
Proxy->>Proxy: Try next available proxy
Note over Proxy: Exhausted all proxies → ProxyExhaustedError
end
end
end
end
Plugin-->>Client: Response / Error
Last reviewed commit: 96db1ea |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function isProxyConnectionError(error: unknown): boolean { | ||
| if (!(error instanceof Error)) return true |
There was a problem hiding this comment.
Non-Error values default to proxy error
isProxyConnectionError returns true for any non-Error value (line 165). This means if fetch rejects with an unexpected non-Error value (e.g., a string, number, or plain object), it will be silently classified as a proxy connection error, triggering failover and marking the proxy as failed — even if the real issue is unrelated to the proxy.
While the fail-closed approach is reasonable as a safety default, consider logging a warning when hitting this branch so operators can diagnose unexpected error shapes:
| function isProxyConnectionError(error: unknown): boolean { | |
| if (!(error instanceof Error)) return true | |
| function isProxyConnectionError(error: unknown): boolean { | |
| if (!(error instanceof Error)) { | |
| log.warn("Non-Error value treated as proxy error for failover", { error: String(error) }) | |
| return true | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/proxy.ts
Line: 164:165
Comment:
**Non-Error values default to proxy error**
`isProxyConnectionError` returns `true` for any non-`Error` value (line 165). This means if `fetch` rejects with an unexpected non-Error value (e.g., a string, number, or plain object), it will be silently classified as a proxy connection error, triggering failover and marking the proxy as failed — even if the real issue is unrelated to the proxy.
While the fail-closed approach is reasonable as a safety default, consider logging a warning when hitting this branch so operators can diagnose unexpected error shapes:
```suggestion
function isProxyConnectionError(error: unknown): boolean {
if (!(error instanceof Error)) {
log.warn("Non-Error value treated as proxy error for failover", { error: String(error) })
return true
}
```
How can I resolve this? If you propose a fix, please make it concise.| const SUPPORTED_PROXY_PROTOCOLS = new Set([ | ||
| "http:", | ||
| "https:", | ||
| "socks4:", | ||
| "socks4a:", | ||
| "socks5:", | ||
| "socks5h:", | ||
| ]); | ||
|
|
There was a problem hiding this comment.
Duplicated SUPPORTED_PROXY_PROTOCOLS constant
This exact same Set is also defined in src/plugin/storage.ts (lines 240-247). If a new protocol is added to one but not the other, validation in the CLI will diverge from normalization in storage — e.g., a user could add a proxy URL via the CLI that storage then silently drops during normalization, or vice versa.
Consider extracting this into a shared constant (e.g., in storage.ts as an export) and importing it here.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/cli.ts
Line: 13:21
Comment:
**Duplicated `SUPPORTED_PROXY_PROTOCOLS` constant**
This exact same `Set` is also defined in `src/plugin/storage.ts` (lines 240-247). If a new protocol is added to one but not the other, validation in the CLI will diverge from normalization in storage — e.g., a user could add a proxy URL via the CLI that storage then silently drops during normalization, or vice versa.
Consider extracting this into a shared constant (e.g., in `storage.ts` as an export) and importing it here.
How can I resolve this? If you propose a fix, please make it concise.| const matchesUrl = (value: string): boolean => { | ||
| try { | ||
| return new URL(value).hostname.toLowerCase() === targetHost; | ||
| } catch { | ||
| return value.toLowerCase().includes(targetHost); | ||
| } |
There was a problem hiding this comment.
includes fallback weakens host-matching for malformed strings
When new URL(value) throws (line 601), the catch branch falls back to value.toLowerCase().includes(targetHost). This means a malformed string like "not-a-url-but-mentions-generativelanguage.googleapis.com" would match, whereas the URL-based hostname check would not.
In practice this is unlikely to cause problems since the fetch interceptor only receives real URL strings, and this maintains backward compatibility with the old implementation. However, if strictness is the goal of this hardening, the catch branch could return false instead of using includes:
| const matchesUrl = (value: string): boolean => { | |
| try { | |
| return new URL(value).hostname.toLowerCase() === targetHost; | |
| } catch { | |
| return value.toLowerCase().includes(targetHost); | |
| } | |
| const matchesUrl = (value: string): boolean => { | |
| try { | |
| return new URL(value).hostname.toLowerCase() === targetHost; | |
| } catch { | |
| return false; | |
| } | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/request.ts
Line: 599:604
Comment:
**`includes` fallback weakens host-matching for malformed strings**
When `new URL(value)` throws (line 601), the catch branch falls back to `value.toLowerCase().includes(targetHost)`. This means a malformed string like `"not-a-url-but-mentions-generativelanguage.googleapis.com"` would match, whereas the URL-based hostname check would not.
In practice this is unlikely to cause problems since the fetch interceptor only receives real URL strings, and this maintains backward compatibility with the old implementation. However, if strictness is the goal of this hardening, the catch branch could return `false` instead of using `includes`:
```suggestion
const matchesUrl = (value: string): boolean => {
try {
return new URL(value).hostname.toLowerCase() === targetHost;
} catch {
return false;
}
};
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/test-regression.ts (1)
312-314:⚠️ Potential issue | 🟡 MinorMinor:
SIGTERMmay not propagate throughshell: trueon Windows.When
shell: trueis set,proc.kill("SIGTERM")sends the signal to the shell wrapper, not the child process. On Windows this is a no-op (SIGTERM isn't supported). The process may not actually be killed on timeout. Consider usingSIGKILL(astest-models.tsdoes) orproc.kill()without a signal argument for Windows, which callsTerminateProcess.
🤖 Fix all issues with AI agents
In `@src/plugin/cli.ts`:
- Around line 89-97: printProxyList currently logs the full proxy.url which can
contain sensitive credentials; update printProxyList to redact credentials
before printing by parsing proxy.url (e.g., detect user:pass@host or
URL.username/URL.password) and replacing them with a mask like *** or
<redacted>, then use that sanitized string in the console.log alongside
proxyStatus(proxy). Ensure the redaction logic is implemented as a small helper
used by printProxyList so any ProxyConfig with embedded credentials will never
be printed in clear text.
In `@src/plugin/proxy.ts`:
- Around line 164-189: The function isProxyConnectionError currently treats
non-Error throwables as proxy errors; change the initial check in
isProxyConnectionError to return false when error is not an instance of Error so
unknown throwables are re-thrown, not swallowed. Keep the loop over
getErrorChain(...) using getErrorName(...), getErrorCode(...)/PROXY_ERROR_CODES
and getErrorMessage(...)/hasProxyErrorMessage(...) unchanged. Remove the
redundant final TypeError branch (or fold it into the single final return false)
so the function ends with a single return false when no proxy indicators are
found.
- Around line 65-88: The dispatcherCache created/used by getOrCreateDispatcher
holds Dispatcher instances (ProxyAgent and socksDispatcher) that must be closed
to avoid socket leaks; update resetProxyState to iterate over
dispatcherCache.values(), call and await each dispatcher's close()
(Dispatcher.close(): Promise<void>) with try/catch to log/ignore errors, then
clear the map; ensure resetProxyState is async (or returns a Promise) so callers
await the closures and prevent resource leaks on proxy rotations or config
resets.
🧹 Nitpick comments (5)
src/antigravity/oauth.test.ts (1)
66-106: Clarify intent: userinfo 500 is expected to be non-fatal.The second test mocks a
500for the userinfo call (line 78) yet assertsresult.type === "success". If the intent is thatexchangeAntigravitysilently tolerates a userinfo failure and proceeds to project discovery, a brief comment would help future readers understand this is deliberate, not an accidental mock ordering.src/plugin/quota.test.ts (1)
33-33: Nit:providerIdvalue doesn't match the production constant.The test uses
"antigravity"whileANTIGRAVITY_PROVIDER_IDis"google"(persrc/constants.ts:152). SincerefreshAccessTokenis mocked and doesn't inspect the value, it's functionally irrelevant, but using the actual constant (astoken-proxy.test.tsdoes) would be more self-documenting.src/plugin/request.ts (1)
685-690: Theinput instanceof URLbranch is unreachable given the current type signature.
prepareAntigravityRequestdeclaresinput: RequestInfo(i.e.,string | Request), so theinstanceof URLcheck at line 687 can never be true in type-safe callers. It's harmless as defensive code, but if URL inputs are expected at runtime, consider widening the parameter type toRequestInfo | URLfor consistency withisGenerativeLanguageRequest.src/plugin/storage.ts (1)
726-758: Backfill write on every cold load when any account lacksproxies.The
shouldBackfillProxiescheck at line 727 triggers asaveAccountscall on every initial load until all accounts haveproxiespersisted. SincesaveAccountsacquires a file lock and does a read-merge-write cycle, this adds I/O overhead on startup.In practice this self-heals after the first successful write, so the overhead is transient. Just noting that
saveAccountsusesmergeAccountStorage, which does a generic spread (...acc) without specialproxiesmerge logic — this means a concurrent writer that sets non-empty proxies between the initial read and the backfill save could have those proxies overwritten with[]. The race window is extremely narrow and only on first run, so this is low risk.src/plugin/proxy.ts (1)
97-112:calculateCooldownMsis computed twice with the same argument.Store the result in a local variable to avoid the redundant call on Line 110.
♻️ Proposed fix
function markFailed(accountIndex: number, proxy: ProxyConfig): void { const key = stateKey(accountIndex, proxy.url) const existing = proxyStates.get(key) const failCount = (existing?.failCount ?? 0) + 1 const now = Date.now() + const cooldownMs = calculateCooldownMs(failCount) proxyStates.set(key, { failCount, lastFailTime: now, - cooldownUntil: now + calculateCooldownMs(failCount), + cooldownUntil: now + cooldownMs, }) log.warn("Proxy marked failed", { proxy: redactUrl(proxy.url), failCount, - cooldownMs: calculateCooldownMs(failCount), + cooldownMs, }) }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
package.jsonscript/test-models.tsscript/test-regression.tssrc/antigravity/oauth.test.tssrc/antigravity/oauth.tssrc/plugin.tssrc/plugin/accounts.test.tssrc/plugin/accounts.tssrc/plugin/cli.test.tssrc/plugin/cli.tssrc/plugin/project.test.tssrc/plugin/project.tssrc/plugin/proxy.test.tssrc/plugin/proxy.tssrc/plugin/quota-fallback.test.tssrc/plugin/quota.test.tssrc/plugin/quota.tssrc/plugin/refresh-queue.tssrc/plugin/request.test.tssrc/plugin/request.tssrc/plugin/search.tssrc/plugin/storage.test.tssrc/plugin/storage.tssrc/plugin/token-proxy.test.tssrc/plugin/token.tssrc/plugin/ui/auth-menu.ts
🧰 Additional context used
🧬 Code graph analysis (16)
src/plugin/storage.test.ts (1)
src/plugin/storage.ts (1)
loadAccounts(652-774)
src/plugin/request.test.ts (1)
src/plugin/request.ts (1)
isGenerativeLanguageRequest(596-617)
src/plugin/token.ts (2)
src/plugin/storage.ts (1)
ProxyConfig(182-185)src/plugin/proxy.ts (1)
fetchWithProxy(204-248)
src/plugin/quota.ts (5)
src/plugin/storage.ts (1)
ProxyConfig(182-185)scripts/check-quota.mjs (3)
accountIndex(23-23)accounts(155-155)disabled(170-170)src/plugin/proxy.ts (1)
fetchWithProxy(204-248)src/plugin/token.ts (1)
refreshAccessToken(87-173)src/plugin/project.ts (1)
ensureProjectContext(235-341)
src/plugin/accounts.test.ts (1)
src/plugin/storage.ts (1)
AccountStorageV4(224-232)
src/plugin/proxy.test.ts (2)
src/plugin/storage.ts (1)
ProxyConfig(182-185)src/plugin/proxy.ts (3)
resetProxyState(250-253)ProxyExhaustedError(19-36)isProxyConnectionError(191-191)
src/plugin/project.ts (3)
src/plugin/storage.ts (1)
ProxyConfig(182-185)src/plugin/proxy.ts (1)
fetchWithProxy(204-248)src/plugin/types.ts (2)
OAuthAuthDetails(4-9)ProjectContextResult(108-111)
src/plugin/token-proxy.test.ts (3)
src/plugin/types.ts (2)
PluginClient(42-42)OAuthAuthDetails(4-9)src/plugin/token.ts (1)
refreshAccessToken(87-173)src/constants.ts (1)
ANTIGRAVITY_PROVIDER_ID(153-153)
src/plugin/project.test.ts (1)
src/plugin/project.ts (2)
loadManagedProject(123-169)onboardManagedProject(175-230)
src/plugin/quota.test.ts (4)
scripts/check-quota.mjs (2)
accountIndex(23-23)accounts(155-155)src/plugin/types.ts (2)
OAuthAuthDetails(4-9)PluginClient(42-42)src/plugin/storage.ts (1)
AccountMetadataV3(187-212)src/plugin/quota.ts (1)
checkAccountsQuota(315-402)
src/plugin/proxy.ts (2)
src/plugin/logger.ts (1)
createLogger(64-106)src/plugin/storage.ts (1)
ProxyConfig(182-185)
src/plugin/cli.ts (1)
src/plugin/storage.ts (1)
ProxyConfig(182-185)
src/plugin.ts (7)
src/plugin/storage.ts (1)
ProxyConfig(182-185)src/plugin/token.ts (1)
refreshAccessToken(87-173)src/plugin/proxy.ts (2)
fetchWithProxy(204-248)ProxyExhaustedError(19-36)src/antigravity/oauth.ts (2)
exchangeAntigravity(205-289)AntigravityTokenExchangeResult(53-55)src/plugin/project.ts (1)
ensureProjectContext(235-341)src/plugin/cli.ts (3)
promptAccountProxyConfiguration(273-286)promptOAuthProxyConfiguration(245-271)isTTY(435-435)src/plugin/ui/ansi.ts (1)
isTTY(55-57)
src/plugin/refresh-queue.ts (1)
src/plugin/token.ts (1)
refreshAccessToken(87-173)
src/plugin/cli.test.ts (2)
src/plugin/ui/auth-menu.ts (2)
showAuthMenu(54-104)showAccountDetails(106-142)src/plugin/cli.ts (3)
promptOAuthProxyConfiguration(245-271)promptAccountProxyConfiguration(273-286)promptLoginMode(363-433)
src/plugin/search.ts (2)
src/plugin/storage.ts (1)
ProxyConfig(182-185)src/plugin/proxy.ts (1)
fetchWithProxy(204-248)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🔇 Additional comments (73)
script/test-models.ts (1)
36-36: LGTM — clean cross-platform spawn handling.The
OPENCODE_BINconstant and conditionalshell: trueare the standard pattern for spawning.cmdwrappers on Windows. Consistent with the sibling script.Also applies to: 48-51
script/test-regression.ts (1)
59-59: LGTM — consistent cross-platform changes.Same
OPENCODE_BIN+shell: truepattern applied correctly to bothrunTurnanddeleteSessionspawn calls.Also applies to: 295-297, 350-352
src/plugin/ui/auth-menu.ts (1)
27-27: LGTM!The
'proxy'variant is cleanly added to theAccountActionunion, and the menu item follows existing conventions (cyan for non-destructive actions, no confirmation gate since it opens a sub-menu).Also applies to: 121-121
src/plugin/request.test.ts (1)
65-100: Good interception hardening coverage.The expanded tests properly validate
string,Request, andURLinput types, mixed-case handling, non-target host rejection, and the anti-bypass check for query-string injection. This aligns well with the hardened implementation inrequest.ts.src/plugin/quota-fallback.test.ts (1)
46-46: LGTM!Reasonable timeout increase for a
beforeAllthat performs dynamic imports with module mocking.src/plugin/refresh-queue.ts (1)
227-227: LGTM!Correctly propagates
account.proxiesandaccount.indextorefreshAccessToken, aligning with the expanded signature intoken.ts. Both parameters are optional, so backward compatibility is maintained.src/plugin/accounts.test.ts (1)
42-58: LGTM!Good coverage for both the proxy backfill default (
[]) and thesetAccountProxiesupdate path. The test correctly validates that legacy accounts without aproxiesfield are normalized.src/plugin/storage.test.ts (1)
433-509: LGTM!Both tests effectively validate the proxy backfill and normalization paths:
- The backfill test confirms legacy accounts without
proxiesreceive[]and that a persistence write is triggered.- The deduplication test confirms URL-based dedup keeps the latest entry and that URL normalization (trailing slash) is applied consistently.
Good use of the established
.tmpfile inspection pattern for verifying persistence.src/plugin/project.test.ts (1)
1-76: LGTM!Clean and focused test file. Good use of
vi.hoistedfor mock initialization before imports, and both tests properly validate thatfetchWithProxyreceives the correct proxy arguments alongside the expected URL fragments and HTTP method. Settingattempts=1, delayMs=0foronboardManagedProjectkeeps the test fast.package.json (1)
63-65: Bothfetch-socksandundiciversions are current and appropriately ranged.
fetch-sockswith^1.3.0allows up to 1.3.2 (latest), andundiciwith^7.0.0allows up to 7.16.0 (latest as of 2025). The caret ranges are appropriate for semver flexibility and both packages are actively maintained.src/plugin/token.ts (1)
87-112: LGTM — clean proxy threading through token refresh.The optional
proxiesandaccountIndexparameters are correctly appended to the existing signature and forwarded positionally tofetchWithProxy. Backward compatibility is preserved sincefetchWithProxyfalls back to plainfetchwhenproxiesisundefinedor empty.src/antigravity/oauth.test.ts (1)
1-106: LGTM — proxy routing assertions are thorough.Both tests correctly validate that
proxies(positional arg 2) andaccountIndex(positional arg 3) are forwarded through token, userinfo, and project discovery calls. Thevi.hoisted+vi.mockpattern is idiomatic for Vitest module mocking.src/plugin/token-proxy.test.ts (1)
1-65: LGTM — focused test for proxy propagation in token refresh.The test correctly validates that
proxiesandaccountIndexare forwarded tofetchWithProxywith the expected URL and HTTP method. The mock response properly exercises the success path.Minor note:
storeCachedAuth,invalidateProjectContextCache, etc. from sibling modules are not mocked, so the test has side effects on those caches. This is fine for a proxy-routing-focused test, but worth keeping in mind if tests become flaky due to shared state.src/plugin/quota.test.ts (1)
30-121: LGTM — comprehensive end-to-end proxy routing test for quota flow.The test correctly validates that
proxiesandaccountIndexpropagate through the entire chain: token refresh → project context → quota fetch calls. The mock wiring and assertions are thorough.src/plugin/search.ts (1)
225-297: LGTM — proxy support cleanly integrated into search execution.The optional
proxiesandaccountIndexparameters are correctly forwarded tofetchWithProxy. The existing error handling (lines 312-315) gracefully catchesProxyExhaustedErrorand surfaces it as a user-friendly search error message, which is appropriate behavior for the search tool.src/plugin/cli.test.ts (1)
1-168: LGTM — thorough test coverage for CLI proxy management flows.The test suite covers a good range of scenarios: reuse, add/merge, invalid input re-prompt, decline, cancel, delete, toggle, TUI account-level proxy action, and non-TTY fallback. The shared
state.answersqueue pattern effectively simulates interactive user input sequences.src/plugin/request.ts (1)
596-617: LGTM —isGenerativeLanguageRequestcorrectly handlesstring,URL, andRequestinputs.The three-branch dispatch properly prevents the bypass where a non-string
RequestInfo(e.g., aRequestobject) could skip interception. ThematchesUrlhelper's try/catch withincludesfallback is a reasonable safeguard for edge-case inputs.src/plugin/storage.ts (3)
182-185: LGTM —ProxyConfiginterface is minimal and well-placed alongsideAccountMetadataV3.The interface is clean with just
urland optionalenabled. Omittingenableddefaulting to "enabled" is a sensible convention that reduces storage noise.Also applies to: 210-211
240-286: LGTM —normalizeProxyConfigsis solid defensive normalization.Good design: validates type, parses URL, checks protocol allowlist, deduplicates by canonical URL, and only persists
enabled: falseexplicitly. TheSUPPORTED_PROXY_PROTOCOLSset correctly covers HTTP and all SOCKS variants.
840-866: LGTM —loadAccountsUnsafeconsistently normalizes across all migration paths.Each version branch deduplicates by email and then normalizes proxy configs, ensuring all accounts have well-formed
proxiesarrays regardless of the storage version encountered.src/antigravity/oauth.ts (4)
15-16: Proxy imports look clean.
120-131: Timeout helper now routes through proxy as intended.
136-163: Project ID discovery now honors per-account proxy routing.
205-271: OAuth exchange and userinfo fetch propagate proxy context consistently.src/plugin/quota.ts (5)
9-13: Proxy types and helper import wired in cleanly.
176-181: Timeout helper now proxies fetches as expected.
186-205: Available-models fetch now honors per-account proxy routing.
220-244: Gemini CLI quota fetch now honors per-account proxy routing.
315-397: Effective account index propagation is consistent across refresh, context, and logging.src/plugin/proxy.test.ts (7)
1-83: Test scaffolding and helpers are well-structured.
84-374: Core fetchWithProxy behaviors are comprehensively covered.
376-454: Integration scenarios validate account isolation and proxy chains.
457-502: isProxyConnectionError coverage looks solid.
504-559: Cooldown progression tests match expected tiering.
561-573: redactUrl edge cases are covered.
575-646: Dispatcher factory tests cover HTTP/SOCKS and caching behavior.src/plugin/project.ts (4)
9-10: Proxy helper imports are wired in cleanly.
123-155: Managed-project load now uses proxy routing.
175-206: Onboarding flow now uses proxy routing.
235-301: Project context resolution forwards proxy context end-to-end.src/plugin/accounts.ts (6)
127-156: ManagedAccount now carries proxies — good for per-account routing.
331-372: Stored accounts default proxies to an empty list.
393-436: New account creation paths initialize proxies consistently.
813-821: setAccountProxies provides a clean update path with persistence.
989-1016: Proxies are persisted to disk alongside account state.
1169-1179: Quota checks now receive per-account proxy configuration.src/plugin/cli.ts (7)
13-83: Proxy parsing/normalization utilities are clear and robust.
113-222: Proxy manager flow looks solid and user-friendly.
245-271: OAuth proxy configuration prompt is well integrated.
273-286: Account proxy configuration hook looks good.
300-309: LoginMenuResult now carries proxy management intent.
321-357: CLI fallback menu adds proxy management cleanly.
396-409: TUI account details now support proxy management.src/plugin.ts (14)
14-56: Proxy-related imports and types are wired in cleanly.
139-173: Quota refresh now preserves the effective account index mapping.
452-534: Verification probes now respect per-account proxy routing.
752-769: Manual OAuth flow passes proxy context through exchange.
779-858: Proxy configurations are persisted for new and updated accounts.
1382-1414: Search tool now reuses the account’s proxy context.
1739-1818: Token refresh and project context now forward proxies consistently.
1883-1890: Warmup requests now flow through proxies.
2050-2055: Primary request path now uses proxy-aware fetch.
2489-2494: Proxy exhaustion toast improves user feedback.
2507-2519: Recovery response now surfaces original error context.
2797-2822: Proxy management is integrated into account management flow.
3104-3270: CLI OAuth flow captures and persists per-account proxy settings.
3330-3457: TUI OAuth flow applies proxy settings and reminders consistently.src/plugin/proxy.ts (6)
1-17: LGTM!Imports are well-organized and the interfaces are clean and appropriately typed.
19-36: LGTM!Clear error messages distinguishing between "all proxies failed" and "no enabled proxies" — good for diagnostics.
60-63: LGTM!Progression logic is sound; the nullish coalescing fallback to
300_000is a safe guard.
204-248: Fail-closed routing logic is well-structured.The three-tier check (no proxies → direct, none enabled → error, all in cooldown → error, all fail → error) correctly enforces fail-closed semantics. The fallback to direct
fetchonly when no proxies are configured is the right design.One note: Line 235 relies on the Node.js
fetchaccepting undici'sdispatcheroption via the cast. This is correct for Node.js ≥ 18 but would silently ignore the dispatcher in non-Node environments.
193-202: LGTM!Solid credential redaction with a safe fallback for malformed URLs.
255-261: LGTM!Standard pattern for exposing internals to tests without polluting the public API.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| function printProxyList(proxies: ProxyConfig[]): void { | ||
| if (proxies.length === 0) { | ||
| console.log(" (no proxies configured)"); | ||
| return; | ||
| } | ||
|
|
||
| for (const [index, proxy] of proxies.entries()) { | ||
| console.log(` ${index + 1}. ${proxy.url} [${proxyStatus(proxy)}]`); | ||
| } |
There was a problem hiding this comment.
Redact proxy credentials when listing proxies.
printProxyList prints the full proxy URL, which can include usernames/passwords. Consider masking credentials to avoid leaking secrets in terminal logs or screen shares.
🔒 Suggested redaction helper
+function redactProxyUrl(value: string): string {
+ try {
+ const url = new URL(value);
+ if (url.username || url.password) {
+ url.username = "***";
+ url.password = "***";
+ }
+ return url.toString();
+ } catch {
+ return value;
+ }
+}
+
function printProxyList(proxies: ProxyConfig[]): void {
if (proxies.length === 0) {
console.log(" (no proxies configured)");
return;
}
for (const [index, proxy] of proxies.entries()) {
- console.log(` ${index + 1}. ${proxy.url} [${proxyStatus(proxy)}]`);
+ console.log(` ${index + 1}. ${redactProxyUrl(proxy.url)} [${proxyStatus(proxy)}]`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function printProxyList(proxies: ProxyConfig[]): void { | |
| if (proxies.length === 0) { | |
| console.log(" (no proxies configured)"); | |
| return; | |
| } | |
| for (const [index, proxy] of proxies.entries()) { | |
| console.log(` ${index + 1}. ${proxy.url} [${proxyStatus(proxy)}]`); | |
| } | |
| function redactProxyUrl(value: string): string { | |
| try { | |
| const url = new URL(value); | |
| if (url.username || url.password) { | |
| url.username = "***"; | |
| url.password = "***"; | |
| } | |
| return url.toString(); | |
| } catch { | |
| return value; | |
| } | |
| } | |
| function printProxyList(proxies: ProxyConfig[]): void { | |
| if (proxies.length === 0) { | |
| console.log(" (no proxies configured)"); | |
| return; | |
| } | |
| for (const [index, proxy] of proxies.entries()) { | |
| console.log(` ${index + 1}. ${redactProxyUrl(proxy.url)} [${proxyStatus(proxy)}]`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/plugin/cli.ts` around lines 89 - 97, printProxyList currently logs the
full proxy.url which can contain sensitive credentials; update printProxyList to
redact credentials before printing by parsing proxy.url (e.g., detect
user:pass@host or URL.username/URL.password) and replacing them with a mask like
*** or <redacted>, then use that sanitized string in the console.log alongside
proxyStatus(proxy). Ensure the redaction logic is implemented as a small helper
used by printProxyList so any ProxyConfig with embedded credentials will never
be printed in clear text.
| function getOrCreateDispatcher(proxyUrl: string): Dispatcher { | ||
| const cached = dispatcherCache.get(proxyUrl) | ||
| if (cached) return cached | ||
|
|
||
| const url = new URL(proxyUrl) | ||
| const protocol = url.protocol.replace(":", "") | ||
| let dispatcher: Dispatcher | ||
|
|
||
| if (protocol === "socks5" || protocol === "socks4" || | ||
| protocol === "socks5h" || protocol === "socks4a") { | ||
| dispatcher = socksDispatcher({ | ||
| type: protocol.startsWith("socks5") ? 5 : 4, | ||
| host: url.hostname, | ||
| port: parseInt(url.port, 10) || 1080, | ||
| userId: url.username ? decodeURIComponent(url.username) : undefined, | ||
| password: url.password ? decodeURIComponent(url.password) : undefined, | ||
| }) | ||
| } else { | ||
| dispatcher = new ProxyAgent({ uri: proxyUrl }) | ||
| } | ||
|
|
||
| dispatcherCache.set(proxyUrl, dispatcher) | ||
| return dispatcher | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
undici ProxyAgent close destroy method API
💡 Result:
ProxyAgent is a Dispatcher, so it inherits the standard Dispatcher.close() / Dispatcher.destroy() lifecycle API. ProxyAgent exposes:
proxyAgent.close(): Promise<void>– graceful shutdown: stops accepting new work and waits for enqueued/in-flight requests to finish before resolving. [1]proxyAgent.destroy(err?: Error): Promise<void>– abrupt shutdown: aborts pending + running requests (they error), and tears down resources immediately (optionally associating an error). [1]
The ProxyAgent class surface (including close() / destroy()) is documented alongside other connection managers. [2]
Sources
[1] Undici Dispatcher API docs (close/destroy semantics) (app.unpkg.com)
[2] Undici connection management docs showing ProxyAgent methods (tessl.io)
Citations:
- 1: https://app.unpkg.com/undici%407.11.0/files/docs/docs/api/Dispatcher.md
- 2: https://tessl.io/registry/tessl/npm-undici/7.15.0/files/docs/connection-management.md
🏁 Script executed:
# First, let's check the actual code in the file to see the current state
head -100 src/plugin/proxy.ts | cat -nRepository: NoeFabris/opencode-antigravity-auth
Length of output: 3824
🏁 Script executed:
# Let's also check if there's a resetProxyState function and see its implementation
rg -A 10 "export function resetProxyState" src/plugin/proxy.tsRepository: NoeFabris/opencode-antigravity-auth
Length of output: 298
🏁 Script executed:
# Check imports and types used for dispatchers
head -50 src/plugin/proxy.ts | cat -nRepository: NoeFabris/opencode-antigravity-auth
Length of output: 1765
Cached dispatchers must be closed before cache reset to prevent connection leaks.
resetProxyState() clears the dispatcherCache map without closing the ProxyAgent and SOCKS dispatchers. Since both implement the Dispatcher interface with close(): Promise<void>, leaving them unclosed leaks their underlying socket connections. In a long-running plugin host, proxy URL rotations or config resets cause cumulative resource exhaustion.
♻️ Proposed fix in resetProxyState
export function resetProxyState(): void {
proxyStates.clear()
+ for (const dispatcher of dispatcherCache.values()) {
+ dispatcher.close?.()
+ }
dispatcherCache.clear()
}🤖 Prompt for AI Agents
In `@src/plugin/proxy.ts` around lines 65 - 88, The dispatcherCache created/used
by getOrCreateDispatcher holds Dispatcher instances (ProxyAgent and
socksDispatcher) that must be closed to avoid socket leaks; update
resetProxyState to iterate over dispatcherCache.values(), call and await each
dispatcher's close() (Dispatcher.close(): Promise<void>) with try/catch to
log/ignore errors, then clear the map; ensure resetProxyState is async (or
returns a Promise) so callers await the closures and prevent resource leaks on
proxy rotations or config resets.
| function isProxyConnectionError(error: unknown): boolean { | ||
| if (!(error instanceof Error)) return true | ||
|
|
||
| for (const entry of getErrorChain(error)) { | ||
| const name = getErrorName(entry) | ||
| if (name === "AbortError") { | ||
| return false | ||
| } | ||
|
|
||
| const code = getErrorCode(entry) | ||
| if (code && PROXY_ERROR_CODES.includes(code as (typeof PROXY_ERROR_CODES)[number])) { | ||
| return true | ||
| } | ||
|
|
||
| const message = getErrorMessage(entry) | ||
| if (hasProxyErrorMessage(message)) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| if (error instanceof TypeError) { | ||
| return false | ||
| } | ||
|
|
||
| return false | ||
| } |
There was a problem hiding this comment.
Non-Error values are silently classified as proxy errors — likely inverted.
Line 165: when error is not an Error instance (e.g., a thrown string, null, or plain object without prototype), the function returns true, treating it as a proxy connection error. In fetchWithProxy, this means such values are swallowed (the proxy is marked failed and iteration continues) rather than re-thrown.
The safer default for an unrecognizable throwable is false (re-throw it), since you can't confirm it's proxy-related.
Additionally, Lines 184-188 are redundant — both the TypeError branch and the default return false.
🐛 Proposed fix
function isProxyConnectionError(error: unknown): boolean {
- if (!(error instanceof Error)) return true
+ if (!(error instanceof Error)) return false
for (const entry of getErrorChain(error)) {
const name = getErrorName(entry)
if (name === "AbortError") {
return false
}
const code = getErrorCode(entry)
if (code && PROXY_ERROR_CODES.includes(code as (typeof PROXY_ERROR_CODES)[number])) {
return true
}
const message = getErrorMessage(entry)
if (hasProxyErrorMessage(message)) {
return true
}
}
- if (error instanceof TypeError) {
- return false
- }
-
return false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isProxyConnectionError(error: unknown): boolean { | |
| if (!(error instanceof Error)) return true | |
| for (const entry of getErrorChain(error)) { | |
| const name = getErrorName(entry) | |
| if (name === "AbortError") { | |
| return false | |
| } | |
| const code = getErrorCode(entry) | |
| if (code && PROXY_ERROR_CODES.includes(code as (typeof PROXY_ERROR_CODES)[number])) { | |
| return true | |
| } | |
| const message = getErrorMessage(entry) | |
| if (hasProxyErrorMessage(message)) { | |
| return true | |
| } | |
| } | |
| if (error instanceof TypeError) { | |
| return false | |
| } | |
| return false | |
| } | |
| function isProxyConnectionError(error: unknown): boolean { | |
| if (!(error instanceof Error)) return false | |
| for (const entry of getErrorChain(error)) { | |
| const name = getErrorName(entry) | |
| if (name === "AbortError") { | |
| return false | |
| } | |
| const code = getErrorCode(entry) | |
| if (code && PROXY_ERROR_CODES.includes(code as (typeof PROXY_ERROR_CODES)[number])) { | |
| return true | |
| } | |
| const message = getErrorMessage(entry) | |
| if (hasProxyErrorMessage(message)) { | |
| return true | |
| } | |
| } | |
| return false | |
| } |
🤖 Prompt for AI Agents
In `@src/plugin/proxy.ts` around lines 164 - 189, The function
isProxyConnectionError currently treats non-Error throwables as proxy errors;
change the initial check in isProxyConnectionError to return false when error is
not an instance of Error so unknown throwables are re-thrown, not swallowed.
Keep the loop over getErrorChain(...) using getErrorName(...),
getErrorCode(...)/PROXY_ERROR_CODES and
getErrorMessage(...)/hasProxyErrorMessage(...) unchanged. Remove the redundant
final TypeError branch (or fold it into the single final return false) so the
function ends with a single return false when no proxy indicators are found.
|
Dang, I just realized somebody submitted the same feature already :O No worries, I'm currently reviewing Pull #301, then I'll think about what I should do next. The Next: Working on it. |
|
Proxies are insecure and will almost certainly disable all your Google accounts for using them. |
this is now the 2nd time you added this with 0 context or understanding of how proxies work. |
Thanks for the feedback~ I agree that proxies can be risky in some situations, especially if you don’t trust the operator (e.g., shared/http proxies), and they can indeed increase latency. You might trigger Google's 'Unusual Traffic' flag regardless of whether you are using a proxy. However, using a proxy isn't just about "bypassing" rate-limits, quota-limits or avoiding bans. In some enterprise/corporate environments, users have limited access to direct outbound traffic. For them, this is a strict requirement for connectivity. The goal of this PR is network compatibility and safety for users who already need an egress proxy. It also provides per-account egress isolation (or Risk Isolation for "some situation") to a certain level. Of course, I can't guarantee it reduces (or increases) risk in every environment. Nobody knows exactly how Google's WAF works, as it is constantly updating. But at least we know some enterprise/corporate environments will need it, and some regions' direct connections from local ISPs might carry HIGHER risk scores than high-quality proxies. "Unnecessary" is a matter of perspective. I would say this is a necessary feature because I need it, and I want to share to the people who need it too. As the project README states:
Users have to think about whether they should add a proxy. You own the risks. If you don't clearly understand it, I recommend not using it. (It's off by default if you didn't add a proxy to your account.) But once you decide to use it, you should only use trusted proxies (never free/shared/unknown ones) :) |
4 participants
Feature - Per-account Proxy Support
per-account proxy enforcement with fail-closed routing, OAuth proxy UX, and full regression coverage
This PR completes per-account proxy support for Antigravity OAuth and request execution paths, with explicit fail-closed behavior to prevent account IP leakage when proxies are configured but unavailable.
It also adds proxy management in the auth/account CLI UX, ensures backward compatibility for legacy account storage without
proxies, and expands regression tests to lock in expected behavior.Motivation
We need account-isolated proxy routing to:
antigravity-accounts.json.What Changed
Proxy routing and safety model
fetchWithProxy(...).proxies + accountIndexthrough:Interception hardening
isGenerativeLanguageRequest(...)to correctly handlestring,Request, andURLinputs.RequestInfocould skip interception/proxy routing.OAuth / account UX
Backward compatibility
proxiesare normalized and backfilled to[].antigravity-accounts.json.Test coverage
Added/updated tests for:
Request/URL/host matching),Validation
npm run typecheckpassednpm testpassed (32 files,933 testspassed)npm run buildpassednpx tsx script/test-regression.ts --dry-runpassednpx tsx script/test-models.ts --dry-runpassedSecurity Notes
Risk / Rollback