Skip to content

feat(accounts): add Antigravity CLI provider with quota display and remove Gemini - #20388

Closed
bluetomlee wants to merge 4 commits into
stablyai:mainfrom
bluetomlee:feat/antigravity-cli-accounts
Closed

bluetomlee wants to merge 4 commits into
stablyai:mainfrom
bluetomlee:feat/antigravity-cli-accounts

Conversation

@bluetomlee

@bluetomlee bluetomlee commented Sep 12, 2026 •

Copy link
Copy Markdown

Summary

This PR adds support for Antigravity CLI under AI Provider Accounts with live quota and rate limit display, and removes deprecated Gemini provider references.

Changes Included

  1. Antigravity CLI Account Status & Settings:

    • Added Antigravity account status detector checking local credentials in ~/.gemini/google_accounts.json, ~/.antigravity/google_accounts.json, oauth_creds.json, and OpenCode auth.json.
    • Added IPC handler antigravityAccounts:getStatus and wired up desktop/web preload bridges.
    • Added AntigravityAccountsSection in Settings -> Accounts pane showing sign-in identity, OAuth status, and a manual quota refresh button.
    • Replaced geminiCliOAuthEnabled with antigravityCliOAuthEnabled across settings schemas, defaults, and telemetry.
  2. Antigravity Quota & Rate Limit Fetching:

    • Implemented antigravity-usage-fetcher and antigravity-oauth-sources to query quota via v1internal:fetchAvailableModels with the required Antigravity User-Agent (the contract the agy CLI and sub2api follow).
    • Credential precedence: Orca-managed accounts → agy keychain token (macOS go-keyring) → OpenCode auth.json → legacy oauth_creds files.
    • Parses per-model quotaInfo (remainingFraction/resetTime), displays used percentages and reset timestamps.
    • Updated rate-limit orchestration and polling services to track Antigravity and removed Gemini tracking.
  3. Live Reset Countdown (941f084):

    • Model buckets and the 5-hour session window now render a ticking Resets in Xh Ym label, reusing the shared boundary-scheduled useResetCountdownClock + formatResetCountdown from the status bar (no per-second polling).
  4. Quota 403 fix (d3f35ab):

    • The legacy retrieveUserQuota endpoint and Gemini-CLI-era tokens both 403; switched to fetchAvailableModels + Antigravity UA + the agy keychain credential (verified live against the service).
  5. Multi-account OAuth (d5340dd, sub2api-style):

    • Full browser sign-in flow (PKCE + loopback callback) with the public Gemini-CLI OAuth client; refresh tokens stored in a safeStorage-encrypted vault, settings keep only account metadata.
    • antigravityAccounts:addAccount / removeAccount / getManagedUsage IPC; accounts pane lists each Google account with live used% + reset countdown.
    • Status-bar snapshot aggregates all managed accounts to the most-constrained window.
  6. Gemini Deprecation & Removal:

    • Removed Gemini CLI settings section, search entries, status bar gating, and visibility toggle menu items.
  7. Localization:

    • Localized the {{value0}}% used badges and all AntigravityAccountsSection strings; en.json / en-runtime-required.json kept in sync so all four localization gates pass.

Testing & Quality

  • Targeted suites passing: rate-limits (incl. new managed-account + quota-client tests), AntigravityAccountsSection (8 cases incl. managed rows/add/remove), countdown clock, localization gates.
  • typecheck (3 tsc projects), oxlint (incl. type-aware + child_process boundary), reliability gates, and ratchets pass.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The Antigravity enable path is split across two setting keys: the service ORs the legacy geminiCliOAuthEnabled, while the new toggle and status-bar durability check read only antigravityCliOAuthEnabled. Existing Gemini-opt-in users fetch quota while the toggle shows OFF and cannot turn it off. Also, the reset-countdown row never renders against real data.

Reviewed changes

  • Antigravity account status — new main-process detector over ~/.gemini, ~/.antigravity, and OpenCode auth.json, IPC antigravityAccounts:getStatus, preload and web-preload bridges, and the AntigravityAccountStatus type.
  • Antigravity quota fetching — antigravity-usage-fetcher.ts / antigravity-oauth-sources.ts query cloudcode-pa.googleapis.com/v1internal:retrieveUserQuota via loadCodeAssist, reusing the Gemini bucket formatter and OAuth extractor, and replace the Gemini slot in the fetch cycle.
  • Settings UI — new AntigravityAccountsSection (sign-in card, manual quota refresh, OAuth toggle, per-model buckets) plus accounts search entries and the accounts-antigravity settings target.
  • Gemini removal — status-bar menu, gating, controller, tooltip, and usage-provider visibility no longer surface Gemini; antigravity-usage-mirror.ts deleted; telemetry whitelist swapped.

⚠️ Empty-usage CTA still advertises Gemini and omits Antigravity

StatusBarUsageEmptyCta.tsx:104 still renders GeminiIcon in its "Supports:" row after this PR removes Gemini from the status-bar menu, gating, settings, and usage visibility. Newly supported Antigravity is absent from the same row, so a first-run user is shown the wrong provider set.

Technical details
# Empty-usage CTA provider list is stale after the gemini → antigravity swap

## Affected sites
- `src/renderer/src/components/status-bar/StatusBarUsageEmptyCta.tsx:8,104` — imports and renders `GeminiIcon`; no Antigravity icon.
- `src/renderer/src/components/settings/usage-provider-settings-target.ts:19-22` — Gemini now maps to `null`, so there is no Gemini section to reach.

## Required outcome
The "Supports:" row names providers the accounts pane actually configures.

## Suggested approach
Replace `GeminiIcon` with `AgentIcon agent="antigravity"` (the icon already used in `AntigravityAccountsSection`).

ℹ️ Gemini leftovers that are now unreachable

Several Gemini artifacts survive the removal with no production entry point — harmless, but they keep the surface confusing and continue to be maintained by tests.

Technical details
# Dead Gemini code after the removal sweep

## Affected sites
- `src/main/rate-limits/gemini-usage-fetcher.ts` — `fetchGeminiRateLimits` has no production caller once `service-full-cycle-preparation.ts` uses Antigravity; only tests import it.
- `src/main/rate-limits/service/service-configuration.ts:55` — `setGeminiCliOAuthEnabledResolver` now delegates to the Antigravity resolver and has no callers.
- `src/main/antigravity-accounts/index.ts` — unused; the IPC module imports `./status` directly.
- `src/shared/rate-limit-types.ts:128` — `RateLimitState.gemini` is still written `fetching` at `service-full-cycle-preparation.ts:115` and nulled at `service-full-cycle-application.ts:176` every cycle.
- `src/main/rate-limits/service-antigravity-usage.test.ts:26` — leftover `vi.mock('./gemini-usage-fetcher')`.

## Required outcome
If Gemini is truly removed, delete the fetcher and the resolver shim, drop `RateLimitState.gemini` (and its per-cycle churn), and remove the stale mock. Otherwise keep the fetcher and stop treating it as a live provider in tests.

ℹ️ Nitpicks

  • src/main/antigravity-accounts/status.ts:70-77 re-enumerates the OpenCode auth.json candidates that readAuthJson already owns (antigravity-oauth-sources.ts:33); reuse it rather than maintaining a second copy of the platform paths.
  • Stale comments after the swap: status-bar-provider-visibility.ts:50 still cites "Gemini OAuth off", and rate-limit-types.ts:66 still describes buckets as "(Gemini only)" although Antigravity now uses them.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +130 to +134
state.rateLimits.setAntigravityCliOAuthEnabledResolver(
() =>
store.getSettings().antigravityCliOAuthEnabled ||
store.getSettings().geminiCliOAuthEnabled === true
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolver ORs the legacy geminiCliOAuthEnabled into Antigravity tracking, but the new toggle (AntigravityAccountsSection.tsx:208) and the status-bar durability check (status-bar-provider-visibility.ts:75,101) read only antigravityCliOAuthEnabled. An existing profile with the old Gemini opt-in therefore fetches quota while the toggle renders OFF, and turning it off can never stop the fetch because the legacy key is never cleared or migrated.

Technical details
# Legacy Gemini opt-in keeps Antigravity enabled after the toggle is turned off

## Affected sites
- `src/main/startup/main-process-account-services.ts:130-134` — resolver ORs `geminiCliOAuthEnabled === true`.
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:208` — toggle reads `antigravityCliOAuthEnabled`.
- `src/renderer/src/components/status-bar/status-bar-provider-visibility.ts:75,101` — durable checks read `antigravityCliOAuthEnabled`.
- `src/shared/global-settings-types.ts:374-375` — legacy key retained, but no migration writes the new key (`normalize-loaded-global-settings.ts`).

## Required outcome
The value the user sees and toggles must match the value the service honors, and an existing opt-in user must be able to disable tracking.

## Suggested approach
Add a one-shot migration in `normalizeLoadedGlobalSettings` copying stored `geminiCliOAuthEnabled === true` into `antigravityCliOAuthEnabled`, then drop the OR from the resolver. Alternatively have the toggle and visibility checks read the same effective predicate.

Comment thread src/renderer/src/components/settings/AntigravityAccountsSection.tsx Outdated
@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

The change adds Antigravity account detection, OAuth credential handling, encrypted token storage, quota retrieval, account-management IPC APIs, and preload APIs. Rate-limit services now fetch Antigravity data directly instead of deriving it from Gemini data. The settings UI displays Antigravity account and quota information. Gemini status-bar and account settings paths are removed or replaced with Antigravity equivalents. Tests cover main-process, service, and renderer behavior.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to d5340

Multi-account refreshes can lose credentials, quota and account state can be incorrect, and several OAuth workflows remain unreliable or unsafe. These issues should be resolved before merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 51 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description explains the main implementation and testing scope, but it does not follow the required template. It omits the required Linked Issue, Visual Proof, Review, Notes, checklist, and other … Rewrite the description using the repository template. Add an issue reference under Linked Issue, attach before-and-after visual proof or state N/A with a reason, complete the Testing checkboxes with platforms and verification steps, comple…
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary changes: adding the Antigravity CLI provider with quota display and removing Gemini support.
Full details: Docstring Coverage

Explanation

Docstring coverage is 13.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 51 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description explains the main implementation and testing scope, but it does not follow the required template. It omits the required Linked Issue, Visual Proof, Review, Notes, checklist, and other required sections.

Resolution

Rewrite the description using the repository template. Add an issue reference under Linked Issue, attach before-and-after visual proof or state N/A with a reason, complete the Testing checkboxes with platforms and verification steps, complete AI Disclosure when applicable, address Review and Agent skill upstream boundary, add Notes for security and compatibility considerations, and complete the Checklist.

  • Fix all pre-merge checks with AI

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/renderer/src/components/settings/accounts-search.ts (1)

121-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use an intent-named ID for the new Antigravity quota keyword.

c759741d77 was introduced after the repository’s grandfather cutoff for hash IDs. Rename this new reference to antigravityQuota, then run pnpm run sync:localization-catalog and pnpm run verify:localization-catalog.

Proposed change
-      ...translateSearchKeyword('auto.components.settings.accounts.search.c759741d77', 'quota'),
+      ...translateSearchKeyword(
+        'auto.components.settings.accounts.search.antigravityQuota',
+        'quota'
+      ),

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: aa30daef-c812-4b99-aad2-f186ec14deca

📥 Commits

Reviewing files that changed from the base of the PR and between 403b62a and deeb3c9.

📒 Files selected for processing (51)
  • src/main/antigravity-accounts/index.ts
  • src/main/antigravity-accounts/status.test.ts
  • src/main/antigravity-accounts/status.ts
  • src/main/ipc/antigravity-accounts.ts
  • src/main/ipc/register-core-handlers/register-core-handlers.test.ts
  • src/main/ipc/register-core-handlers/register-core-handlers.ts
  • src/main/rate-limits/antigravity-oauth-sources.ts
  • src/main/rate-limits/antigravity-usage-fetcher.test.ts
  • src/main/rate-limits/antigravity-usage-fetcher.ts
  • src/main/rate-limits/antigravity-usage-mirror.test.ts
  • src/main/rate-limits/antigravity-usage-mirror.ts
  • src/main/rate-limits/codex-fetcher-pty-settle.test.ts
  • src/main/rate-limits/rate-limit-service-test-harness.ts
  • src/main/rate-limits/service-antigravity-usage.test.ts
  • src/main/rate-limits/service-refresh-orchestration.test.ts
  • src/main/rate-limits/service-window-activation.test.ts
  • src/main/rate-limits/service/service-configuration.ts
  • src/main/rate-limits/service/service-full-cycle-application.ts
  • src/main/rate-limits/service/service-full-cycle-preparation.ts
  • src/main/rate-limits/service/service-polling.ts
  • src/main/rate-limits/service/service-state.ts
  • src/main/rate-limits/service/service-types.ts
  • src/main/startup/main-process-account-services.ts
  • src/preload/api-types.ts
  • src/preload/api/agent-account-api.ts
  • src/preload/api/antigravity-accounts-bridge.ts
  • src/preload/index.ts
  • src/renderer/src/components/settings/AccountsPane.test.tsx
  • src/renderer/src/components/settings/AccountsPane.tsx
  • src/renderer/src/components/settings/AntigravityAccountsSection.test.tsx
  • src/renderer/src/components/settings/AntigravityAccountsSection.tsx
  • src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx
  • src/renderer/src/components/settings/accounts-pane-provider-setting-sections.tsx
  • src/renderer/src/components/settings/accounts-search.test.ts
  • src/renderer/src/components/settings/accounts-search.ts
  • src/renderer/src/components/settings/appearance-status-bar-search.ts
  • src/renderer/src/components/status-bar/StatusBarVisibilityMenu.tsx
  • src/renderer/src/components/status-bar/status-bar-agent-gating.test.ts
  • src/renderer/src/components/status-bar/status-bar-agent-gating.ts
  • src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts
  • src/renderer/src/components/status-bar/status-bar-provider-visibility.ts
  • src/renderer/src/components/status-bar/tooltip.tsx
  • src/renderer/src/components/status-bar/usage-provider-settings-target.test.ts
  • src/renderer/src/components/status-bar/usage-provider-settings-target.ts
  • src/renderer/src/components/status-bar/use-status-bar-controller.ts
  • src/renderer/src/web/preload-api/web-agent-accounts-api.ts
  • src/renderer/src/web/web-preload-api.ts
  • src/shared/default-global-settings.ts
  • src/shared/global-settings-types.ts
  • src/shared/rate-limit-types.ts
  • src/shared/telemetry-property-schemas.ts
💤 Files with no reviewable changes (7)
  • src/main/rate-limits/service/service-polling.ts
  • src/renderer/src/components/settings/appearance-status-bar-search.ts
  • src/renderer/src/components/status-bar/status-bar-agent-gating.ts
  • src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx
  • src/main/rate-limits/antigravity-usage-mirror.test.ts
  • src/main/rate-limits/antigravity-usage-mirror.ts
  • src/renderer/src/components/status-bar/status-bar-agent-gating.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment on lines +60 to +69
break
}
} catch {
// ignore parse error
}
}
}

// Also check OpenCode auth.json fallback
if (!hasCreds && !email) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fall through to the next usable OAuth source. When authJson.google exists, fetchAntigravityRateLimits uses fetchViaAuthJson, which returns on refresh or 401 failure and never calls readAntigravityCredentials; valid .gemini/oauth_creds.json or .antigravity/oauth_creds.json credentials remain unused. getAntigravityAccountStatus also breaks after the first local credential file and then skips OpenCode fallback, so a stale first source reports tokenFresh: false despite a later fresh source. Continue after stale or authentication-failing sources, stop when a source is fresh or succeeds, and return a terminal error only after all supported sources fail.

if (err && typeof err === 'object' && 'code' in err && err.code === 'ENOENT') {
continue
}
throw err

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Continue credential discovery after malformed optional files.

A malformed earlier JSON file throws instead of allowing the next credential source. fetchAntigravityRateLimits reads OpenCode auth.json before OAuth credential files, so malformed OpenCode JSON returns an error even when a valid Gemini or Antigravity credential exists.

  • src/main/rate-limits/antigravity-oauth-sources.ts#L50-L50: Continue on SyntaxError so another OpenCode location or OAuth credential source can be checked. Keep filesystem permission errors observable.
  • src/main/rate-limits/antigravity-oauth-sources.ts#L84-L84: Continue on SyntaxError so a malformed primary Gemini credential does not block a valid Antigravity fallback credential.

Add regression tests with malformed first candidates and valid later candidates.

📍 Affects 1 file
  • src/main/rate-limits/antigravity-oauth-sources.ts#L50-L50 (this comment)
  • src/main/rate-limits/antigravity-oauth-sources.ts#L84-L84

const targetPath = creds.credsFilePath || PRIMARY_OAUTH_CREDS_PATH
const tmpPath = `${targetPath}.${process.pid}.tmp`
const { credsFilePath: _ignored, ...dataToSave } = creds
await writeFile(tmpPath, JSON.stringify(dataToSave, null, 2), 'utf-8')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

set -eu
file=$(fd -t f '^antigravity-oauth-sources\.ts$' . | head -n 1)
printf '%s\n' "$file"
sed -n '1,130p' "$file"

Repository: stablyai/orca

Length of output: 4237


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 23126


Sensitive Data Exposure

Reachability: Internal
Exploitability: Moderate
CWE: CWE-732 — Incorrect Permission Assignment for Critical Resource

Restrict permissions on the credential temporary file.

dataToSave contains OAuth credentials. Create the temporary file with mode 0o600, an exclusive flag, and a unique name before renaming it.

Proposed fix
+import { randomUUID } from 'node:crypto'
 import { readFile, writeFile, rename } from 'node:fs/promises'
...
-  const tmpPath = `${targetPath}.${process.pid}.tmp`
+  const tmpPath = `${targetPath}.${process.pid}.${randomUUID()}.tmp`
   const { credsFilePath: _ignored, ...dataToSave } = creds
-  await writeFile(tmpPath, JSON.stringify(dataToSave, null, 2), 'utf-8')
+  await writeFile(tmpPath, JSON.stringify(dataToSave, null, 2), {
+    encoding: 'utf-8',
+    mode: 0o600,
+    flag: 'wx'
+  })
📝 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.

Suggested change
await writeFile(tmpPath, JSON.stringify(dataToSave, null, 2), 'utf-8')
await writeFile(tmpPath, JSON.stringify(dataToSave, null, 2), {
encoding: 'utf-8',
mode: 0o600,
flag: 'wx'
})

)
return {
provider: 'antigravity',
session: deriveSessionSummary(buckets),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not publish a one-hour bucket as the five-hour session window.

buildRateLimitBucket sets windowMinutes to 60, and deriveSessionSummary preserves it when assigning session. The renderer maps 60 to 1h, while the ProviderRateLimits.session contract requires 300 (5h). Select only a five-hour bucket for session, or set it to null when none exists.

allowPtyFallback: this.shouldAllowCodexPtyFallback(),
signal
})),
fetchAntigravityRateLimits(antigravityCliOAuthEnabled),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -e
file="src/main/rate-limits/service/service-full-cycle-preparation.ts"
sed -n '120,195p' "$file"
printf '\n-- fetchAntigravityRateLimits definitions and callers --\n'
rg -n -C 8 "fetchAntigravityRateLimits|abortActiveFetchCycle|allSettled" src

Repository: stablyai/orca

Length of output: 50370


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 28231


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '-- exact Antigravity references --'
rg -n -C 12 "fetchAntigravityRateLimits" --glob '*.ts' --glob '*.tsx' .
printf '%s\n' '-- preparation imports and cycle control --'
sed -n '1,125p' src/main/rate-limits/service/service-full-cycle-preparation.ts
printf '%s\n' '-- service cycle lifecycle references --'
rg -n -C 10 "abortActiveFetchCycle|runFetchAllCycle|fetchAll\(" src/main/rate-limits --glob '*.ts' --glob '*.tsx' | head -300

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '-- Antigravity fetcher --'
sed -n '1,235p' src/main/rate-limits/antigravity-usage-fetcher.ts
printf '%s\n' '-- service cycle implementation --'
rg -n -C 18 "abortActiveFetchCycle|runFetchAllCycle|activeFetch|fetchAll\(" src/main/rate-limits/service.ts src/main/rate-limits/service/*.ts

Repository: stablyai/orca

Length of output: 46498


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '-- fetcher completion --'
sed -n '225,330p' src/main/rate-limits/antigravity-usage-fetcher.ts
printf '%s\n' '-- OAuth sources used by Antigravity --'
rg -n -C 14 "export (async )?function (loadProjectId|readAuthJson|readAntigravityCredentials|saveAntigravityCredentials|tryRefreshTokenFromBundle)|net\.fetch|fetch\(" src/main/rate-limits/antigravity-oauth-sources.ts src/main/rate-limits/gemini-cli-oauth-extractor.ts
printf '%s\n' '-- exact fetch-cycle application --'
sed -n '1,145p' src/main/rate-limits/service/service-full-cycle-application.ts

Repository: stablyai/orca

Length of output: 19022


Pass the cycle abort signal to Antigravity.

prepareFetchAllCycle calls fetchAntigravityRateLimits without the active signal. When RateLimitService.stop() aborts the cycle, the Antigravity path can continue through credential loading, project lookup, and quota fetching. Promise.allSettled then waits for those operations to complete or reach their independent timeouts.

Thread the signal through the fetcher and its cancellable credential and network operations. Add a regression test for a pending Antigravity request that settles when the service stops.

Proposed call-site change
-      fetchAntigravityRateLimits(antigravityCliOAuthEnabled),
+      fetchAntigravityRateLimits(antigravityCliOAuthEnabled, { signal }),

const loadStatus = useCallback(async (): Promise<void> => {
try {
if (window.api.antigravityAccounts?.getStatus) {
const next = await window.api.antigravityAccounts.getStatus()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Make the Antigravity account-status request scope-aware.

When localAccountRuntime.runtime is wsl or isRemoteAccountScope is true, AccountsPane still renders AntigravityAccountsSection. Its window.api.antigravityAccounts.getStatus() call reaches src/main/ipc/antigravity-accounts.ts, which calls getAntigravityAccountStatus() with no target; that function reads credential files under the desktop main process homedir(). The card can therefore display the desktop email and token freshness for the selected WSL or remote scope. Pass the selected runtime or remote target through the IPC contract, or hide the status card when the target cannot be resolved, and add WSL and remote-scope regression coverage.

Render "Resets in Xh Ym" labels that tick live for Antigravity model
buckets and the 5-hour session window, reusing the shared
boundary-scheduled countdown clock and formatter from the status bar.
Falls back to the static resetDescription when resetsAt is unknown.

Harden quota fetching in the main process:
- fall back to global fetch when Electron net is unavailable
- swallow loadCodeAssist failures and fall back to projects.json, then
  the CLI's own default-cli-project default (mirrors the agy binary)
- omit the optional pluginType from the ANTIGRAVITY client metadata

Localize the percentage badges and sync the en.json / runtime-required
catalogs with every AntigravityAccountsSection key so the localization
gates pass.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new live reset countdown renders Resets Resets in 2h 4m: formatResetCountdown already includes the Resets in prefix, and the section wraps its output in the 'Resets {{when}}' template. The added test misses it because its regex is unanchored.

Reviewed changes

Incremental delta against the prior pullfrog review (deeb3c9), which retired the reset-label thread.

  • Live reset countdown — model buckets and the 5-hour session window now derive Resets in … from resetsAt via useResetCountdownClock + formatResetCountdown, keeping the static resetDescription as fallback.
  • Quota-fetch hardening — falls back to global fetch when net is unavailable, swallows loadCodeAssist failures, falls back to projects.json then default-cli-project, and drops the optional pluginType from the ANTIGRAVITY metadata.
  • Localization — % used badges are localized and the en.json / runtime-required catalogs were regenerated.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

<div className="grid grid-cols-1 sm:grid-cols-2 gap-2">
{buckets.map((bucket) => {
const resetCountdown = bucket.resetsAt
? formatResetCountdown(bucket.resetsAt - now)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatResetCountdown already returns a string beginning with Resets (Resets in 2h 4m / Resets now), so wrapping its output in the 'Resets {{when}}' template renders Resets Resets in 2h 4m for every bucket that has resetsAt. The 5-hour session branch at lines 288-291 has the same double prefix.

Technical details
# Live reset countdown double-prefixes "Resets"

## Affected sites
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:246-249` — bucket `resetWhen` = `formatResetCountdown(resetsAt - now)`.
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:259-263` — wraps `resetWhen` in `translate('...resetsAt', 'Resets {{when}}')`.
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:288-291`, `303-307` — same for the session window.
- `src/shared/rate-limit-reset-format.ts:29-32` — `formatResetCountdown` already returns `Resets in …` / `Resets now`.
- `src/renderer/src/components/settings/AntigravityAccountsSection.test.tsx:201` — unanchored `/Resets in 2h [45]m/` matches the doubled string, so the test cannot catch it.

## Required outcome
The label reads `Resets in 2h 4m`, not `Resets Resets in 2h 4m`.

## Suggested approach
Render the formatted countdown directly (as `status-bar/tooltip.tsx:234` does) and keep the `Resets {{when}}` template only for the static `resetDescription` fallback; or use a template without the leading `Resets` for the countdown branch. Tighten the test to assert the full string (anchor the regex) so a regression fails.

clientId: string,
clientSecret: string
): Promise<RefreshTokenResult> {
const fetchFn = net?.fetch ?? fetch

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

net?.fetch ?? fetch aliases the function before calling it, which hides the global-fetch fallback from global-fetch-call-site-audit.test.ts (its pattern only matches literal fetch( / globalThis.fetch). Every other rate-limit fetcher calls net.fetch directly and net is always defined in Electron main, so this fallback looks unreachable in production while opening a body-leak path on !res.ok.

Technical details
# `?? fetch` introduces an unaudited global-fetch path in main

## Affected sites
- `src/main/rate-limits/antigravity-oauth-sources.ts:109,139` and `src/main/rate-limits/antigravity-usage-fetcher.ts:50` — `net?.fetch ?? fetch`.
- `src/main/global-fetch-call-site-audit.test.ts:57` — the guard matches only `fetch(` / `globalThis.fetch`, so the alias is invisible to it.
- `refreshAccessToken` (`antigravity-oauth-sources.ts:122`) and `fetchQuota` (`antigravity-usage-fetcher.ts:57`) return on `!res.ok` without consuming or cancelling the response body.

## Required outcome
Main-process HTTP stays on Electron `net.fetch`, which the repo's global-fetch audit enforces because a bare undici `fetch` with a leaked body can crash the whole main process (orca#8695).

## Open questions for the human
Is there a runtime where `net`/`net.fetch` is genuinely absent for these modules? If not, drop the `?? fetch` fallback to match the other fetchers (the tests already mock `net.fetch`). If the fallback is required, consume/cancel the body on every path and add these files to `AUDITED_GLOBAL_FETCH_LINES`.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: a797914d-b897-40a6-bd2c-b031cb53e9e9

📥 Commits

Reviewing files that changed from the base of the PR and between deeb3c9 and 941f084.

📒 Files selected for processing (6)
  • src/main/rate-limits/antigravity-oauth-sources.ts
  • src/main/rate-limits/antigravity-usage-fetcher.ts
  • src/renderer/src/components/settings/AntigravityAccountsSection.test.tsx
  • src/renderer/src/components/settings/AntigravityAccountsSection.tsx
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Comment on lines +259 to +263
{translate(
'auto.components.settings.AntigravityAccountsSection.resetsAt',
'Resets {{when}}',
{ when: resetWhen }
)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Render the reset prefix once.

formatResetCountdown returns Resets in ... or Resets now. Both the bucket and session paths pass this value to the translated Resets {{when}} template, which renders Resets Resets in 2h. Render resetCountdown and sessionCountdown directly. Keep the template for the bare resetDescription fallback.

Comment on lines +1063 to +1070
"96f3649526": "Use Gemini CLI credentials (experimental)",
"973741a871": "Configure Gemini provider settings.",
"b10cb4f696": "adding",
"b11078a9c2": "wsl",
"b43e761fe5": "MiniMax cookie update failed.",
"b8c2905c2b": "Could not load Codex accounts.",
"c2aee76420": "Extracts OAuth credentials from your local Gemini CLI installation to authenticate with Google for {{value0}}. This uses credentials issued to the Gemini CLI app, not Orca. May break if Google updates the CLI. Use at your own risk.",
"d676c41fc6": "Extracts OAuth credentials from your local Gemini CLI installation to authenticate with Google. This uses credentials issued to the Gemini CLI app, not Orca. May break if Google updates the CLI. Use at your own risk.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 3 \
  '0c64dc2a64|0c7f915b01|96f3649526|973741a871|c2aee76420|d676c41fc6|b7c2cee442|bada4a3218|d819755b02|e8e1ff3887|geminiToggleDescription' \
  src

rg -n -C 3 'Gemini Usage|Use Gemini CLI credentials|antigravityCliOAuthEnabled|AntigravityAccountsSection' \
  src/renderer/src

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- account search/status definitions and consumers ---'
rg -n -C 5 \
  'getAccounts(Search|AntigravitySearch)Entries|geminiToggleDescription|Gemini Usage|d819755b02|c1df0d67ec|9660c5b2f1|antigravityUsageDetails|AntigravityAccountsSection' \
  src/renderer/src/components src/renderer/src/i18n/en-runtime-required.json

printf '%s\n' '--- direct Gemini provider and account references outside locale catalogs ---'
rg -n -C 3 \
  'gemini|Gemini|geminiToggleDescription|geminiUsage|Gemini Usage' \
  src/renderer/src \
  -g '!i18n/locales/**' \
  -g '!i18n/en-runtime-required.json' \
  -g '!**/*.test.*' \
  | head -n 240

Repository: stablyai/orca

Length of output: 50369


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 27008


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- accounts-search.ts ---'
cat -n src/renderer/src/components/settings/accounts-search.ts | sed -n '1,245p'

printf '%s\n' '--- Gemini/Antigravity references outside i18n and tests ---'
rg -n -C 4 -i \
  'gemini|antigravity|statusBar.*(usage|toggle)|usage.*statusBar' \
  src/renderer/src \
  -g '!**/i18n/**' \
  -g '!**/*.test.*' \
  | head -n 320

printf '%s\n' '--- status-bar translation call sites ---'
rg -n -C 5 \
  'geminiToggleDescription|Gemini Usage|Open Gemini usage details|Show Gemini token|antigravity.*(Usage|usage|status)|Usage details' \
  src/renderer/src/components src/renderer/src/store src/renderer/src/hooks \
  -g '!**/*.test.*'

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- legacy catalog identifiers outside localization files ---'
rg -n \
  '0c64dc2a64|0c7f915b01|96f3649526|973741a871|c2aee76420|d676c41fc6|b7c2cee442|bada4a3218|d819755b02|e8e1ff3887|2804a920ad|5bfb874d05|9660c5b2f1|c1df0d67ec|d2375976eb|geminiToggleDescription' \
  src/renderer/src \
  -g '!**/i18n/**' \
  -g '!**/*.test.*' || true

printf '%s\n' '--- active status-bar menu and controller paths ---'
cat -n src/renderer/src/components/status-bar/StatusBarVisibilityMenu.tsx | sed -n '1,130p'
cat -n src/renderer/src/components/status-bar/use-status-bar-controller.ts | sed -n '95,170p'

printf '%s\n' '--- settings search registration ---'
cat -n src/renderer/src/hooks/settings-navigation-capability-sections.ts | sed -n '45,68p'

Repository: stablyai/orca

Length of output: 11481


Remove Gemini from the active Accounts description.

src/renderer/src/hooks/settings-navigation-capability-sections.ts:57 still lists Gemini, but the active account registry uses Antigravity instead. Replace Gemini with Antigravity and update the generated catalog entry. The Gemini identifiers in en-runtime-required.json have no active renderer consumers.

…Models, UA

The quota pipeline was dead end-to-end: the legacy ~/.gemini/oauth_creds.json
token is minted by the retired Gemini CLI client and cloudcode-pa rejects it
with 403 SUBSCRIPTION_REQUIRED ("UNSUPPORTED_CLIENT"), and the old
retrieveUserQuota endpoint 403s even for fresh tokens. Verified against the
real service:

- cloudcode-pa requires an Antigravity User-Agent on every v1internal call
  (the contract agy and sub2api follow); plain node/electron UAs get 403.
- The agy CLI stores its live token in the OS keychain (macOS: generic
  password service "gemini" / account "antigravity", go-keyring-base64).
  Read it as the primary credential source - it works today. Keychain
  tokens are client-bound to agy, so they are never refreshed by Orca
  (surfacing a "run agy" hint instead); agy refreshes them itself.
- Quota now comes from v1internal:fetchAvailableModels (per-model
  quotaInfo with remainingFraction/resetTime) instead of retrieveUserQuota.
  Models lacking numeric remainingFraction are unlimited-tier and skipped;
  buckets keep distinct models (no dedupe collapse) and the session window
  prefers timed buckets.
- loadCodeAssist sends the ANTIGRAVITY metadata with UA (returns the real
  cloudaicompanion project); fallback chain gains agy's
  default_project_id.txt cache before the default-cli-project constant.
Add Orca-managed Google accounts alongside the CLI-credential sources:

- managed-oauth.ts: full browser sign-in flow against Google with the
  public Gemini-CLI client (the same one sub2api/Antigravity-Manager
  ship): PKCE + loopback callback server + consent, code exchange,
  userinfo email, and loadCodeAssist project resolution.
- token-store.ts: safeStorage-encrypted vault in userData for refresh
  tokens (settings persist only account metadata; no plaintext tokens).
- Settings schema: antigravityManagedAccounts (id/email/projectId).
- IPC: antigravityAccounts:addAccount / removeAccount / getManagedUsage
  with preload + web-client bridges.
- Fetcher precedence: managed accounts first (per-account refresh +
  fetch, aggregated to the most-constrained window for the status bar),
  then the agy keychain token, then the legacy files. The accounts pane
  lists each account with live used% and reset countdown.
- AntigravityAccountsSection splits the managed-accounts list into its
  own component (file line budget) with add/remove UI.

All gates pass: typecheck, oxlint (incl. child_process boundary via
runProcess), localization extraction/coverage/runtime catalog, and 1.7k+
unit tests across rate-limits + settings.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

The new multi-account sign-in flow cannot exchange its authorization code on a machine with the Gemini CLI installed — the authorize URL and the token request use different OAuth clients. Two more issues in the same range: the account list never refreshes after add/remove, and concurrent token refreshes can drop a persisted token.

Reviewed changes

Incremental delta against the prior pullfrog review (941f0842), covering the quota-endpoint switch and the multi-account OAuth work.

  • Quota client — replaced retrieveUserQuota with v1internal:fetchAvailableModels, mapping per-model quotaInfo into buckets in a new antigravity-quota-client.ts and sending the required Antigravity User-Agent.
  • Credential sources — added the macOS agy keychain token and set precedence Orca-managed accounts → keychain → OpenCode auth.json → legacy oauth_creds.json.
  • Multi-account OAuth — added browser sign-in (PKCE + loopback) in managed-oauth.ts, a safeStorage vault in token-store.ts, per-account refresh in antigravity-managed-account-fetch.ts, and addAccount/removeAccount/getManagedUsage IPC with preload and web bridges.
  • Managed-account UI — new AntigravityManagedAccountsList with per-account quota, and aggregation of all managed accounts into the status-bar snapshot.
  • Settings — added antigravityManagedAccounts and the AntigravityManagedAccount type.

ℹ️ The new managed-account modules have no focused tests

managed-oauth.ts and token-store.ts ship untested; the only spec that imports managed-oauth mocks refreshAntigravityAccountToken and node:fs/promises. A test asserting the authorize/exchange client ids agree would have caught the first finding, and a concurrent-write test would have caught the vault race.

Technical details
# No focused coverage for the new OAuth/vault modules

## Affected sites
- `src/main/antigravity-accounts/managed-oauth.ts` — no spec.
- `src/main/antigravity-accounts/token-store.ts` — no spec.
- `src/main/rate-limits/antigravity-managed-usage.test.ts:37` — mocks `refreshAntigravityAccountToken`; fs is mocked at `:41-45`.

## Required outcome
The client-id consistency between authorize and exchange, and concurrent vault writes, are exercised by tests.

ℹ️ Nitpicks

  • src/main/antigravity-accounts/managed-oauth.ts:54 — callbackHtml interpolates the Google error query param (fed from :165) into HTML unescaped; encode it before rendering.
  • src/main/rate-limits/antigravity-quota-client.ts:74 and src/main/antigravity-accounts/managed-oauth.ts:87,121,252 repeat net?.fetch ?? fetch; the global-fetch audit matches only literal fetch( / globalThis.fetch, so the alias stays invisible (also flagged in the open thread at antigravity-oauth-sources.ts:175).

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

}
redirectUri = `http://localhost:${address.port}/callback`
const authUrl = new URL(AUTH_URL)
authUrl.searchParams.set('client_id', OAUTH_CLIENT_ID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The authorize URL hardcodes OAUTH_CLIENT_ID (:11), but the code exchange and refresh use resolveClientCredentials(), which prefers the locally extracted Gemini CLI client. Google rejects an authorization code redeemed by a different client_id (invalid_grant), so sign-in fails on any machine with the Gemini CLI installed — the flow only works when extraction fails.

Technical details
# Authorize and token requests use different OAuth clients

## Affected sites
- `src/main/antigravity-accounts/managed-oauth.ts:217` — authorize URL `client_id` = hardcoded `OAUTH_CLIENT_ID` (`1071006060591-…`).
- `src/main/antigravity-accounts/managed-oauth.ts:88,93` — `exchangeCode` sends `resolveClientCredentials().clientId`.
- `src/main/antigravity-accounts/managed-oauth.ts:253,258` — refresh has the same preference.
- `src/main/antigravity-accounts/managed-oauth.ts:74-80` — returns the extracted Gemini CLI client (`681255809395-…`) when present, constant only as fallback.
- `src/main/rate-limits/gemini-cli-oauth-extractor.ts:70-77` — extracts the CLI's `OAUTH_CLIENT_ID`.

## Required outcome
Authorize, exchange, and refresh all use the same client identity.

## Suggested approach
Resolve the client once and pass the same `clientId` into the authorize URL, or drop the extractor for this flow and use the constant pair throughout. Also correct the `PUBLIC_FALLBACK_CLIENT_SECRET` comment (`:12-16`): that secret belongs to the Antigravity client, not gemini-cli (`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`).

setAddAccountError(result.error ?? 'Sign-in failed')
}
await loadStatus()
await refreshRateLimits()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

managedAccounts comes from the settings prop (:81), but neither handler refetches settings after the IPC call — they only call loadStatus/refreshRateLimits/loadManagedUsage. Since the main handler updates settings without notifyListeners, the renderer keeps the old list: a newly added account never appears, and a removed row stays with a working trash button.

Technical details
# Managed-account list is stale after add/remove

## Affected sites
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:81` — `managedAccounts` read from `settings`.
- `src/renderer/src/components/settings/AntigravityAccountsSection.tsx:108-137` — add/remove handlers omit `fetchSettings()`.
- `src/main/ipc/antigravity-accounts.ts:53,70-72` — main `updateSettings` without `notifyListeners`, so no renderer push.
- `src/renderer/src/components/settings/accounts-pane-account-actions.ts:62,161` — Claude/Codex runners refetch after mutations.

## Required outcome
The list reflects the mutation without a pane remount.

## Suggested approach
Call the store's `fetchSettings()` after `addAccount`/`removeAccount` succeeds, matching the existing account-action runners.

Comment on lines +92 to +94
const vault = await readVault()
vault.entries[accountId] = encodeTokens(tokens)
await writeVault(vault)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write() is an unguarded read-modify-write of the whole vault, and every caller refreshes accounts with Promise.all — so two accounts whose tokens expire together each read the same snapshot and the last rename wins, dropping the other's refreshed token. Because writeVault reuses one per-process temp path (:74), overlapping writes also share an inode.

Technical details
# Lost update in the Antigravity token vault

## Affected sites
- `src/main/antigravity-accounts/token-store.ts:86-95` — `write()` does `readVault` → mutate → `writeVault` with no serialization.
- `src/main/antigravity-accounts/token-store.ts:71-77` — temp path `${target}.${process.pid}.tmp` is shared across concurrent writes.
- `src/main/rate-limits/antigravity-managed-account-fetch.ts:84-86` — `Promise.all` over accounts, each refreshing + persisting.
- `src/main/ipc/antigravity-accounts.ts:84-89` — same fan-out, bypassing the service fetch queue.

## Required outcome
Concurrent refreshes never lose a persisted refresh/access token.

## Suggested approach
Serialize read-modify-write inside `createAntigravityTokenStore` (promise chain/mutex) and make the temp name unique per write (`randomUUID()`), so overlapping writers cannot share an inode. Note: if Google rotated a refresh token, the lost write leaves the account stuck on an invalidated token.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8196ddd5-8d9a-4515-a151-8b92124931b9

📥 Commits

Reviewing files that changed from the base of the PR and between 941f084 and d5340dd.

📒 Files selected for processing (21)
  • src/main/antigravity-accounts/managed-oauth.ts
  • src/main/antigravity-accounts/token-store.ts
  • src/main/ipc/antigravity-accounts.ts
  • src/main/ipc/register-core-handlers/register-core-handlers.ts
  • src/main/rate-limits/antigravity-managed-account-fetch.ts
  • src/main/rate-limits/antigravity-managed-usage.test.ts
  • src/main/rate-limits/antigravity-oauth-sources.ts
  • src/main/rate-limits/antigravity-quota-client.ts
  • src/main/rate-limits/antigravity-usage-fetcher.test.ts
  • src/main/rate-limits/antigravity-usage-fetcher.ts
  • src/main/startup/main-process-account-services.ts
  • src/preload/api/agent-account-api.ts
  • src/preload/api/antigravity-accounts-bridge.ts
  • src/renderer/src/components/settings/AntigravityAccountsSection.test.tsx
  • src/renderer/src/components/settings/AntigravityAccountsSection.tsx
  • src/renderer/src/components/settings/AntigravityManagedAccountsList.tsx
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/web/preload-api/web-agent-accounts-api.ts
  • src/shared/default-global-settings.ts
  • src/shared/global-settings-types.ts
  • src/shared/managed-account-types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/src/i18n/locales/en.json

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

400,
callbackHtml(
'Authorization failed',
`Google returned: ${error}. You can close this tab.`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file="src/main/antigravity-accounts/managed-oauth.ts"
printf '%s\n' '--- relevant definitions and callback flow ---'
sed -n '1,190p' "$file"

Repository: stablyai/orca

Length of output: 7320


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 8015


XSS

Reachability: External
Exploitability: Difficult
CWE: CWE-79 — Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')

Escape the OAuth error before rendering it as HTML.

The error query parameter is interpolated into callbackHtml and returned as an HTML response. Encode it before interpolation, or render it through text-only DOM content.

Comment on lines +228 to +231
void shell.openExternal(authUrl.toString()).catch(() => {
// Why: headless contexts — the browser may not open, but the flow
// still waits on the loopback redirect; the timeout guards orphans.
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Reject the flow when the browser cannot open.

If shell.openExternal rejects, the user cannot continue because the authorization URL is not exposed elsewhere. The handler still waits for the five-minute timeout.

Call rejectFlow from this rejection handler.

Proposed fix
-        void shell.openExternal(authUrl.toString()).catch(() => {
-          // Why: headless contexts — the browser may not open, but the flow
-          // still waits on the loopback redirect; the timeout guards orphans.
-        })
+        void shell.openExternal(authUrl.toString()).catch((error) => {
+          rejectFlow(
+            error instanceof Error
+              ? error
+              : new Error('Failed to open the authorization page')
+          )
+        })
📝 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.

Suggested change
void shell.openExternal(authUrl.toString()).catch(() => {
// Why: headless contexts — the browser may not open, but the flow
// still waits on the loopback redirect; the timeout guards orphans.
})
void shell.openExternal(authUrl.toString()).catch((error) => {
rejectFlow(
error instanceof Error
? error
: new Error('Failed to open the authorization page')
)
})

Comment on lines +71 to +77
async function writeVault(vault: VaultFile): Promise<void> {
const target = vaultFilePath()
await mkdir(path.dirname(target), { recursive: true })
const tmp = `${target}.${process.pid}.tmp`
await writeFile(tmp, JSON.stringify(vault, null, 2), 'utf-8')
await rename(tmp, target)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Serialize the complete vault read-modify-write

fetchManagedAccountsAggregate and antigravityAccounts:getManagedUsage refresh managed accounts through Promise.all. Each refresh can read the same vault snapshot, update one entry, and write through the fixed temporary path. Concurrent writes can lose another account’s rotated token or cause a rename failure.

Protect the complete readVault → update → writeVault sequence for write and remove with a process-wide mutex shared by all AntigravityTokenStore instances. Locking only writeVault, or using a unique temporary path alone, would not prevent stale snapshots and lost updates.

Comment on lines +38 to +53
await tokenStore.write(grant.accountId, {
refreshToken: grant.refreshToken,
accessToken: grant.accessToken,
expiryDate: grant.expiryDate
})
const now = Date.now()
const existing = deps.getSettings().antigravityManagedAccounts ?? []
const account: AntigravityManagedAccount = {
id: grant.accountId,
email: grant.email,
projectId: grant.projectId,
createdAt: now,
updatedAt: now,
lastAuthenticatedAt: now
}
deps.updateSettings({ antigravityManagedAccounts: [...existing, account] })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Keep token and account metadata mutations consistent.

The handlers update two persistence systems without rollback. If updateSettings fails after Line 38, the vault retains an orphan refresh token. If updateSettings fails after Line 68, settings retain an account whose credentials were deleted.

Add compensation or a startup reconciliation step for both mutation orders.

Also applies to: 68-72

// tokens that cloudcode-pa now rejects with 403.
const keychainCreds = await readAntigravityKeychainCredentials()
if (keychainCreds) {
return await fetchViaOauthCreds(keychainCreds, antigravityCliOAuthEnabled)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle keychain credentials in the 401 retry path.

Line 168 sends keychain credentials into fetchViaOauthCreds. If the quota request returns 401 before expiry_date, the retry at Lines 116-120 uses Gemini CLI client credentials even though Lines 72-78 identify that refresh flow as incompatible.

Check currentCreds.source before the 401 refresh. Return the existing “run agy” error for keychain credentials. Add a regression test for a non-expired keychain token that receives 401.

@nwparker

Copy link
Copy Markdown
Contributor

Review verdict: Do not merge as-is. The PR is dirty and combines account-provider replacement, OAuth/token storage, and quota behavior in a broad pre-stack design. The maintained migration now separates native account work (#21784 → #21797 → #21798) from CLI quota work (#21654 → #21678); review those scoped stacks instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants