feat: add HTTP/HTTPS proxy support for all API traffic #301
Conversation
- Add fetchWithProxy() utility using https-proxy-agent - Thread proxyUrl through all fetch call sites (token refresh, project context, quota, search) - Save proxy URL from ANTIGRAVITY_LOGIN_PROXY env during login - Store proxyUrl per-account in antigravity-accounts.json - Add docs/PROXY.md with configuration guide - Update README with proxy quick start section Supports: http://host:port, http://user:pass@host:port, https://host:port Note: SOCKS5 proxies are NOT supported
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds proxy support end-to-end: new src/plugin/proxy.ts (ProxyAgent cache, sanitization, dispose, fetchWithProxy) and tests in src/plugin/proxy.test.ts; adds undici dependency. Threads an optional proxyUrl through OAuth, token refresh, project load/onboard, quota checks, search, streaming, refresh queue, and account persistence. AccountMetadataV3 and ManagedAccount gain optional proxyUrl. Documentation updated with two Proxy sections in README.md and a new docs/PROXY.md describing proxy behavior and configuration. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
To use Codex here, create an environment for this repo. |
Greptile OverviewGreptile SummaryThis PR adds comprehensive HTTP/HTTPS proxy support for all API traffic in the Antigravity authentication plugin. The implementation introduces a Key changes:
Architecture: Issue identified: Confidence Score: 4/5
Important Files Changed
|
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugin/accounts.ts
Line: 762:775
Comment:
`proxyUrl` is missing from the account mapping in `saveToDisk()`. The proxy configuration will not persist across application restarts, even though it's saved during login in `plugin.ts:345`. Add `proxyUrl: a.proxyUrl` to the mapped account object.
How can I resolve this? If you propose a fix, please make it concise. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f2fa57280
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
proxyUrl was missing from saveToDisk() mapping and constructor hydration, causing proxy config to be lost on restart and all API calls to bypass proxy.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Aaron Sewall <[email protected]>
Request objects were accepted but properties (method, headers, body) were silently lost. All call sites pass strings, so narrowing the type prevents future misuse. Removed test for Request input since it's no longer valid.
Resolve conflicts to keep both proxy support and new quota caching features. Added proxyUrl parameter to fetchGeminiCliQuota for parallel quota fetching.
Signed-off-by: Aaron Sewall <[email protected]>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Aaron Sewall <[email protected]>
Additional Comments (1)
Both Add Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugin/quota.ts
Line: 329:336
Comment:
proxy not used for token refresh and project context during quota checks
Both `refreshAccessToken()` and `ensureProjectContext()` are missing the `account.proxyUrl` parameter. This means quota checks will bypass the configured proxy for authentication and project discovery, even though the actual quota API calls (lines 345-347) correctly use the proxy.
Add `account.proxyUrl` as the 4th parameter to `refreshAccessToken()` on line 329, and as the 2nd parameter to `ensureProjectContext()` on line 336.
How can I resolve this? If you propose a fix, please make it concise. |
Signed-off-by: Aaron Sewall <[email protected]>
- Add MAX_AGENT_CACHE_SIZE (50) with LRU eviction to prevent unbounded growth - Add disposeProxyAgent() for explicit cleanup when accounts are removed - Fix proxyUrl clearing logic: use explicit undefined check instead of ?? (allows removing proxy by setting empty string, not treated as falsy) - Pass account.proxyUrl to refreshAccessToken() and ensureProjectContext() in quota checks to ensure proxy is used for all auth operations
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| refreshToken: parts.refreshToken, | ||
| projectId: parts.projectId ?? existing.projectId, | ||
| managedProjectId: parts.managedProjectId ?? existing.managedProjectId, | ||
| proxyUrl: proxyUrl !== undefined ? proxyUrl : existing.proxyUrl, |
There was a problem hiding this comment.
proxy cannot be cleared once set - undefined falls back to existing proxy
if user logs in without ANTIGRAVITY_LOGIN_PROXY after previously setting a proxy, the old proxy persists. empty string ("") would also be falsy and fall back.
to enable proxy clearing, distinguish between "not provided" vs "explicitly cleared":
| proxyUrl: proxyUrl !== undefined ? proxyUrl : existing.proxyUrl, | |
| proxyUrl: proxyUrl !== undefined ? proxyUrl : existing.proxyUrl, |
could become:
// Use null to explicitly clear proxy
proxyUrl: proxyUrl !== undefined ? (proxyUrl || null) : existing.proxyUrl,
then users could clear with ANTIGRAVITY_LOGIN_PROXY="" opencode auth login
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 423:423
Comment:
proxy cannot be cleared once set - `undefined` falls back to existing proxy
if user logs in without `ANTIGRAVITY_LOGIN_PROXY` after previously setting a proxy, the old proxy persists. empty string (`""`) would also be falsy and fall back.
to enable proxy clearing, distinguish between "not provided" vs "explicitly cleared":
```suggestion
proxyUrl: proxyUrl !== undefined ? proxyUrl : existing.proxyUrl,
```
could become:
```
// Use null to explicitly clear proxy
proxyUrl: proxyUrl !== undefined ? (proxyUrl || null) : existing.proxyUrl,
```
then users could clear with `ANTIGRAVITY_LOGIN_PROXY="" opencode auth login`
How can I resolve this? If you propose a fix, please make it concise.
|
@aaronsewall do you intend on moving this forward? |
…proxy Merge origin/dev which adds account verification, config updater, quota fallback tests, refresh queue, model resolver updates, and CLI/UI improvements. Conflict resolution: - plugin.ts: combined saveAccountsReplace + AccountMetadataV3 imports - storage.ts: kept both proxyUrl and verification fields on AccountMetadataV3 Proxy coverage fixes: - verifyAccountAccess: use fetchWithProxy for Antigravity API probe - verifyAccountAccess: pass proxyUrl to refreshAccessToken - Google Search tool: resolve account before token refresh for proxyUrl - ProactiveRefreshQueue: pass account.proxyUrl to refreshAccessToken
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugin.ts
Line: 402:406
Comment:
**Proxy breaks warmup fetch**
`fetchWithProxy` only accepts `string | URL`, but later this file calls it with `resolvedUrl` which is not defined in this scope (the original code used `fetch(prepared.request, prepared.init)`). As written, this should fail to compile or will reference an undefined variable at runtime. This needs to be updated to pass the same request URL that was previously used (e.g. `prepared.request`), and ensure the argument type matches `fetchWithProxy`.
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugin/accounts.ts (1)
142-166:⚠️ Potential issue | 🟠 MajorPropagate
proxyUrlinto quota-refresh metadata.
checkAccountsQuota()now honorsaccount.proxyUrl, butAccountManager.getAccountsForQuotaCheck()still strips it, so background quota refresh can bypass the proxy. That breaks the “all traffic via proxy” guarantee.🔧 Proposed fix
getAccountsForQuotaCheck(): AccountMetadataV3[] { return this.accounts.map((a) => ({ email: a.email, refreshToken: a.parts.refreshToken, projectId: a.parts.projectId, managedProjectId: a.parts.managedProjectId, + proxyUrl: a.proxyUrl, addedAt: a.addedAt, lastUsed: a.lastUsed, enabled: a.enabled, })); }
🤖 Fix all issues with AI agents
In `@docs/PROXY.md`:
- Around line 112-126: The fenced code blocks containing the error examples lack
language identifiers and trigger MD040; update the two blocks that include the
exact strings "Error: Failed to create proxy agent for http://proxy:8080:
connect ECONNREFUSED" and "Error: Invalid proxy URL format:
http://***:***@:invalid" to use a language fence (e.g., ```text) so each fenced
block starts with ```text and ends with ``` to satisfy markdownlint.
🧹 Nitpick comments (6)
src/plugin/proxy.ts (3)
24-40: Comment says "LRU eviction" but this is actually FIFO.The cache uses
Mapinsertion order for eviction (oldest-inserted entries are removed first), but a re-used agent (cache hit on line 58) is never moved to the end of the map. True LRU would re-insert on access. With a max of 50 and typical usage of 1–5 proxies this is functionally fine, but the comment is misleading.
77-94: Mixed static and dynamic import ofundici.
ProxyAgentis statically imported at line 1 (always loaded), whilefetchis dynamically imported at line 88. If the intent of the dynamic import is to avoid loading undici when no proxy is configured, the static import on line 1 defeats that purpose. Consider making both imports dynamic, or both static for consistency.If you want both to be static (simpler):
-import { ProxyAgent } from 'undici'; +import { ProxyAgent, fetch as undiciFetch } from 'undici';Then in
fetchWithProxy, remove the dynamic import:- const { fetch: undiciFetch } = await import('undici');
88-93: Type incompatibility between globalRequestInitand undici'sRequestInitis suppressed via@ts-ignore.The
initparameter is typed as the globalRequestInit, but undici'sfetchexpects its ownRequestInit(fromundici). Properties likesignal,body, andheadershave subtly different types. The@ts-ignore+as unknowncast hides potential runtime issues. This works today for the simple cases in this PR (plain objects withmethod,headers,body,signal), but could break if callers pass more exotic RequestInit properties.Consider typing
initas a narrower interface with only the properties actually used, or importing undici'sRequestInitand using a union type.src/plugin/proxy.test.ts (2)
16-22: Agent cache leaks between tests, making test order significant.Since
agentCacheis module-private and never cleared between tests, agents created in earlier tests persist. Each test works around this by using unique URLs, but this is fragile — adding a test that reuses a URL from another test will produce confusing failures.Consider exporting a
clearAgentCache()test helper (or usingresetModulesinbeforeEach) to ensure isolation.
92-114: Test coverage gaps for proxy utilities.The
fetchWithProxytests cover the happy paths, but there's no coverage for:
disposeProxyAgent(exported, untested)- Unsupported protocol rejection (e.g.,
socks5://...)- LRU/FIFO eviction when cache exceeds
MAX_AGENT_CACHE_SIZEfetchWithProxywith aURLobject as input (the.hrefbranch on line 90 ofproxy.ts)These aren't blockers but would strengthen confidence in the module.
src/antigravity/oauth.ts (1)
119-135: Passingundefinedas positional placeholder fortimeoutMsis a code smell.
fetchWithTimeoutnow has two trailing optional parameters (timeoutMs,proxyUrl), forcing callers to passundefinedexplicitly to reach the proxy parameter (lines 165, 234, 252). Consider using an options object for the optional parameters.♻️ Suggested refactor
+interface FetchWithTimeoutOptions extends RequestInit { + timeoutMs?: number; + proxyUrl?: string; +} async function fetchWithTimeout( url: string, - options: RequestInit, - timeoutMs = FETCH_TIMEOUT_MS, - proxyUrl?: string, + options: FetchWithTimeoutOptions, ): Promise<Response> { + const { timeoutMs = FETCH_TIMEOUT_MS, proxyUrl, ...fetchOptions } = options; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); try { if (proxyUrl) { - return await fetchWithProxy(url, { ...options, signal: controller.signal }, proxyUrl); + return await fetchWithProxy(url, { ...fetchOptions, signal: controller.signal }, proxyUrl); } - return await fetch(url, { ...options, signal: controller.signal }); + return await fetch(url, { ...fetchOptions, signal: controller.signal }); } finally { clearTimeout(timeout); } }
📜 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 (14)
README.mddocs/PROXY.mdpackage.jsonsrc/antigravity/oauth.tssrc/plugin.tssrc/plugin/accounts.tssrc/plugin/project.tssrc/plugin/proxy.test.tssrc/plugin/proxy.tssrc/plugin/quota.tssrc/plugin/refresh-queue.tssrc/plugin/search.tssrc/plugin/storage.tssrc/plugin/token.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/plugin/token.ts (2)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)src/constants.ts (2)
ANTIGRAVITY_CLIENT_ID(4-4)ANTIGRAVITY_CLIENT_SECRET(9-9)
src/plugin/refresh-queue.ts (1)
src/plugin/token.ts (1)
refreshAccessToken(86-175)
src/plugin/search.ts (1)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)
src/antigravity/oauth.ts (1)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)
src/plugin/quota.ts (3)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)src/plugin/token.ts (1)
refreshAccessToken(86-175)src/plugin/project.ts (1)
ensureProjectContext(230-326)
src/plugin/proxy.test.ts (1)
src/plugin/proxy.ts (2)
getProxyAgent(42-75)fetchWithProxy(77-94)
🪛 markdownlint-cli2 (0.20.0)
docs/PROXY.md
[warning] 112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (19)
package.json (1)
64-64: LGTM —undicidependency addition is appropriate.ProxyAgent isn't available through Node's built-in global fetch, so the explicit dependency is justified. The
^6.23.0range is compatible with thenode >= 20engine requirement.README.md (1)
263-299: LGTM — Proxy documentation section is clear and well-structured.Covers the key scenarios (env var setup, supported formats, security warning). The link to
docs/PROXY.mdfor advanced configuration is a good approach.src/antigravity/oauth.ts (2)
137-165: LGTM — proxy threading throughfetchProjectIDis correct.The env var read is appropriate here since this runs during the login flow before a persisted account exists.
208-254: LGTM — proxy threading throughexchangeAntigravityis correct.Same login-time env var usage pattern. Token exchange and userinfo fetch both correctly route through the proxy.
src/plugin/refresh-queue.ts (1)
227-227: LGTM — correctly passes per-account proxy URL to token refresh.src/plugin/storage.ts (1)
196-196: LGTM —proxyUrladdition toAccountMetadataV3is clean.The optional field integrates naturally with the existing merge logic (spread in
mergeAccountStorage) and migration paths (V2→V3 won't set it, which is correct for legacy accounts).src/plugin/accounts.ts (2)
378-383: LGTM: proxyUrl is hydrated from storage.Cleanly carries per-account proxy settings into the in-memory model.
1001-1016: LGTM: proxyUrl is persisted.Including proxyUrl in AccountStorage keeps behavior consistent across restarts.
src/plugin/token.ts (1)
6-6: Proxy-aware token refresh looks good.Cleanly threads
proxyUrland keeps existing error handling intact.Also applies to: 86-114
docs/PROXY.md (1)
1-106: Docs look solid and comprehensive.Covers login proxy, manual config, supported formats, and troubleshooting clearly.
src/plugin/search.ts (1)
17-17: LGTM: search requests are now proxy-aware.Signature and call-site changes are consistent and preserve existing behavior when unset.
Also applies to: 224-295
src/plugin/quota.ts (2)
7-7: LGTM: quota fetches are proxy-aware.fetchWithTimeout + proxyUrl propagation is clean and keeps timeouts intact.
Also applies to: 176-205, 219-242
327-348: LGTM: quota checks now threadaccount.proxyUrl.Consistent with the per-account proxy model.
src/plugin/project.ts (1)
9-9: LGTM: project context flows now respect proxyUrl.Consistent proxy threading across load, onboard, and ensure context.
Also applies to: 122-152, 172-200, 230-286
src/plugin.ts (5)
34-34: LGTM: verification flow is proxy-aware end to end.Proxy is passed through refresh and probe fetches cleanly.
Also applies to: 43-43, 438-514
757-835: LGTM: login proxy is persisted per account.Clear propagation from environment into new/updated account records.
1356-1386: LGTM: google_search now threads proxyUrl.Early account resolution makes proxy usage consistent with token refresh.
1715-1715: LGTM: proxy wiring in main request path.Token refresh, project context, warmup, and content fetches all respect per-account proxy.
Also applies to: 1789-1789, 1855-1860, 2017-2018
3151-3152: LGTM: proxyUrl preserved on account refresh updates.Ensures refreshed accounts keep their proxy association.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Yes, just pushed updates. I'll respond to PR feedback as it comes in. It's been ready a few times. |
|
Looking forward to using this live rather than a fork I'm itterating on what I am calling the the "Silent Jailer" Protocol: Transparent Proxy Sandboxing |
no need to fix all the pr comments, sometimes these bots are wrong :) But if you could have a look at the conflicts I could merge it for the next release |
Conflict resolution: - accounts.ts: use dev's fingerprint init, keep proxyUrl hydration, add fingerprintHistory - storage.ts: keep both proxyUrl and fingerprintHistory on AccountMetadataV3 New fetch in version.ts (public URL, no account) — no proxy needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/plugin.ts`:
- Around line 1360-1364: Replace the on-demand disk read by using the in-memory
activeAccountManager: instead of calling AccountManager.loadFromDisk(auth) and
assigning to accountManager, use the module-level activeAccountManager instance
to call getAccounts() and then find the matching account (the existing
accounts.find(a => a.parts.refreshToken === parts.refreshToken)). Update
references to accountManager.getAccounts() to activeAccountManager.getAccounts()
so you avoid repeated disk I/O from AccountManager.loadFromDisk(auth).
In `@src/plugin/storage.ts`:
- Line 196: mergeAccountStorage currently uses a shallow spread ({
...existingAcc, ...acc }) that allows incoming acc.proxyUrl === undefined to
overwrite and drop an existing proxyUrl; update mergeAccountStorage to
explicitly preserve proxyUrl when acc.proxyUrl is undefined (similar to how
projectId and managedProjectId are handled). In the mergeAccountStorage
function, replace or augment the spread merge so proxyUrl = acc.proxyUrl ??
existingAcc.proxyUrl (or set proxyUrl after the spread) to ensure
existingAcc.proxyUrl is retained unless acc provides a defined value.
🧹 Nitpick comments (1)
src/plugin/quota.ts (1)
176-184: Consider using an options object forfetchWithTimeoutto avoid positional parameter ambiguity.Adding
proxyUrlafter the defaultedtimeoutMsparameter means callers must always explicitly passFETCH_TIMEOUT_MSto reachproxyUrl. This works today since both call sites pass it explicitly, but it's fragile for future callers.This is a minor ergonomic nit — not blocking.
📜 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 selected for processing (7)
src/antigravity/oauth.tssrc/plugin.tssrc/plugin/accounts.tssrc/plugin/project.tssrc/plugin/quota.tssrc/plugin/search.tssrc/plugin/storage.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/antigravity/oauth.ts
- src/plugin/accounts.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/plugin/search.ts (1)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)
src/plugin.ts (5)
src/plugin/token.ts (1)
refreshAccessToken(86-175)src/plugin/proxy.ts (1)
fetchWithProxy(77-94)src/constants.ts (1)
ANTIGRAVITY_ENDPOINT_PROD(34-34)src/plugin/accounts.ts (1)
AccountManager(299-1232)src/plugin/project.ts (1)
ensureProjectContext(230-326)
src/plugin/project.ts (2)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)src/plugin/types.ts (2)
OAuthAuthDetails(4-9)ProjectContextResult(108-111)
src/plugin/quota.ts (3)
src/plugin/proxy.ts (1)
fetchWithProxy(77-94)src/plugin/token.ts (1)
refreshAccessToken(86-175)src/plugin/project.ts (1)
ensureProjectContext(230-326)
🔇 Additional comments (7)
src/plugin/search.ts (1)
17-17: LGTM!The proxy threading is clean:
proxyUrlis added as an optional trailing parameter toexecuteSearch, and thefetchWithProxycall correctly passes it through. WhenproxyUrlisundefined,fetchWithProxyfalls back to nativefetchper the proxy module's implementation.Also applies to: 224-295
src/plugin/quota.ts (1)
313-398: Proxy threading throughcheckAccountsQuotalooks correct.
account.proxyUrlis consistently forwarded torefreshAccessToken,ensureProjectContext,fetchAvailableModels, andfetchGeminiCliQuota.src/plugin/project.ts (1)
9-9: LGTM!Proxy support is cleanly threaded through
loadManagedProject,onboardManagedProject, andensureProjectContext. The endpoint fallback loops inloadManagedProjectand retry logic inonboardManagedProjectboth correctly passproxyUrltofetchWithProxy.Also applies to: 122-166, 172-225, 230-286
src/plugin.ts (4)
439-515: LGTM —verifyAccountAccessproxy wiring is correct.The
proxyUrlfield is added to the account parameter type, forwarded torefreshAccessToken, and used in thefetchWithProxycall for the verification probe. The pattern is consistent with other call sites.
758-862:persistAccountPoolproxy handling looks correct.The logic at line 834 (
proxyUrl: proxyUrl !== undefined ? proxyUrl : existing.proxyUrl) correctly distinguishes between "env var not set" (keep existing) and "env var explicitly set" (update). New accounts at line 812 receive the current env var value.
1715-1793: Proxy correctly threaded through the main request loop.
account.proxyUrlis consistently forwarded torefreshAccessToken(line 1719),ensureProjectContext(line 1793), the warmup fetch (line 1861), and the main API fetch (line 2021). This ensures all HTTP traffic for a given account uses its configured proxy.
3148-3158: LGTM — account refresh path correctly preservesproxyUrl.Line 3155 uses
process.env.ANTIGRAVITY_LOGIN_PROXY ?? updatedAccounts[refreshAccountIndex]?.proxyUrl, which correctly updates the proxy if the env var is currently set, or preserves the existing value otherwise. Consistent withpersistAccountPoollogic.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Replace AccountManager.loadFromDisk() with activeAccountManager in search tool (avoids redundant disk read) - Preserve proxyUrl in mergeAccountStorage (same pattern as projectId/managedProjectId)
| refreshToken: parts.refreshToken, | ||
| projectId: parts.projectId ?? updatedAccounts[refreshAccountIndex]?.projectId, | ||
| managedProjectId: parts.managedProjectId ?? updatedAccounts[refreshAccountIndex]?.managedProjectId, | ||
| proxyUrl: process.env.ANTIGRAVITY_LOGIN_PROXY ?? updatedAccounts[refreshAccountIndex]?.proxyUrl, | ||
| addedAt: updatedAccounts[refreshAccountIndex]?.addedAt ?? Date.now(), |
There was a problem hiding this comment.
Proxy clearing blocked
In the refresh-account update branch, proxyUrl: process.env.ANTIGRAVITY_LOGIN_PROXY ?? updatedAccounts[refreshAccountIndex]?.proxyUrl makes it impossible to clear an existing proxy by logging in without ANTIGRAVITY_LOGIN_PROXY (it will always fall back to the stored value). This reintroduces the “proxy can’t be cleared once set” behavior even though other paths now use proxyUrl !== undefined ? proxyUrl : existing.proxyUrl.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 3151:3155
Comment:
**Proxy clearing blocked**
In the refresh-account update branch, `proxyUrl: process.env.ANTIGRAVITY_LOGIN_PROXY ?? updatedAccounts[refreshAccountIndex]?.proxyUrl` makes it impossible to clear an existing proxy by logging in without `ANTIGRAVITY_LOGIN_PROXY` (it will always fall back to the stored value). This reintroduces the “proxy can’t be cleared once set” behavior even though other paths now use `proxyUrl !== undefined ? proxyUrl : existing.proxyUrl`.
How can I resolve this? If you propose a fix, please make it concise.
Use !== undefined check (matching persistAccountPool) so setting ANTIGRAVITY_LOGIN_PROXY='' clears the proxy instead of falling back.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/plugin.ts`:
- Line 43: Remove the unused type import AccountMetadataV3 from the import
statement in plugin.ts (the line that currently imports clearAccounts,
loadAccounts, saveAccounts, saveAccountsReplace, type AccountMetadataV3 from
"./plugin/storage"); simply delete the ", type AccountMetadataV3" portion so
only the used functions remain imported.
📜 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 selected for processing (1)
src/plugin.ts
🔇 Additional comments (5)
src/plugin.ts (5)
439-515: Proxy threading inverifyAccountAccesslooks correct.The
proxyUrlis cleanly added as an optional field and consistently passed to bothrefreshAccessTokenandfetchWithProxy.
1360-1389: Past review feedback addressed —activeAccountManagerused instead of disk I/O.The proxy URL is correctly threaded via optional chaining through both
refreshAccessTokenandexecuteSearch.
1718-1718: Proxy support correctly threaded through the main request lifecycle.Token refresh (Line 1718), project context (Line 1792), warmup requests (Line 1860), and the primary API call (Line 2020) all consistently pass
account.proxyUrl.Also applies to: 1792-1792, 1860-1860, 2020-2020
3154-3156: Proxy update pattern is consistent withpersistAccountPool.The
!== undefinedguard correctly distinguishes between "env var not set" (preserve existing) and "env var set to empty" (clear proxy). Same empty-string concern noted in thepersistAccountPoolcomment applies here.
758-834: No action needed. The code already handles empty-stringproxyUrlcorrectly.getProxyAgent()checksif (!proxyUrl?.trim())which returnsundefinedfor both empty strings and falsy values, causingfetchWithProxy()to fall back to regularfetch()without a proxy agent. Empty stringproxyUrlis safely treated the same asundefined.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/plugin.ts
Outdated
| import { AntigravityTokenRefreshError, refreshAccessToken } from "./plugin/token"; | ||
| import { startOAuthListener, type OAuthListener } from "./plugin/server"; | ||
| import { clearAccounts, loadAccounts, saveAccounts, saveAccountsReplace } from "./plugin/storage"; | ||
| import { clearAccounts, loadAccounts, saveAccounts, saveAccountsReplace, type AccountMetadataV3 } from "./plugin/storage"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all usages of AccountMetadataV3 in the entire codebase
echo "=== All usages of AccountMetadataV3 ==="
rg -n 'AccountMetadataV3' --type=ts
echo ""
echo "=== Check for re-exports of AccountMetadataV3 in src/plugin.ts ==="
rg -n 'export.*AccountMetadataV3' src/plugin.ts
echo ""
echo "=== Full context around line 43 in src/plugin.ts ==="
sed -n '40,50p' src/plugin.tsRepository: NoeFabris/opencode-antigravity-auth
Length of output: 2908
Remove unused AccountMetadataV3 import from line 43.
The type is imported but not used anywhere in this file and is not re-exported. Other modules import AccountMetadataV3 directly from ./plugin/storage as needed. This import can be safely removed.
🤖 Prompt for AI Agents
In `@src/plugin.ts` at line 43, Remove the unused type import AccountMetadataV3
from the import statement in plugin.ts (the line that currently imports
clearAccounts, loadAccounts, saveAccounts, saveAccountsReplace, type
AccountMetadataV3 from "./plugin/storage"); simply delete the ", type
AccountMetadataV3" portion so only the used functions remain imported.
| # - POST to Antigravity loadCodeAssist endpoints | ||
|
|
||
| # Make a test request | ||
| opencode run "Hello" --model=google/claude-sonnet-4-5 |
There was a problem hiding this comment.
model name should be google/antigravity-claude-sonnet-4-5 to match README standards (the antigravity- prefix is used throughout the documentation)
| opencode run "Hello" --model=google/claude-sonnet-4-5 | |
| opencode run "Hello" --model=google/antigravity-claude-sonnet-4-5 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/PROXY.md
Line: 103:103
Comment:
model name should be `google/antigravity-claude-sonnet-4-5` to match README standards (the `antigravity-` prefix is used throughout the documentation)
```suggestion
opencode run "Hello" --model=google/antigravity-claude-sonnet-4-5
```
How can I resolve this? If you propose a fix, please make it concise.3 participants
Supports: http://host:port, http://user:pass@host:port, https://host:port
Note: SOCKS5 proxies are NOT supported