Repository navigation
fix(rate-limits): a failed shared-quota read is an Antigravity error, not an absent reading - #17200
brennanb2025 wants to merge 3 commits into
Conversation
… not an absent reading Antigravity has no endpoint of its own: the Gemini read of shared Google Code Assist quota is Antigravity's read, which is why a successful one is mirrored verbatim. #15876 settled a *failed* one as `unavailable` to stop the row saying "Refresh failed" for a request Orca never sent. That fixed the wording, but `unavailable` is a retention verdict, not a label — `applyStalePolicy` discards on `unavailable` and retains on `error` — so one failure wiped the Antigravity row while sparing Gemini's, from the same read. Settle a failed quota read as `error` so both rows keep the last good reading. A Gemini `unavailable` (opt-in off) still maps to `unavailable`: nothing was read, which is genuinely absent. #15876's real protections are untouched and still pinned — the row never quotes Gemini's raw error and never blames a sign-in that exists, now including while it retains a reading. The two tests that pinned the old status are updated in place, not deleted.
Greptile SummaryThe PR distinguishes malformed Gemini credentials and failed shared-quota reads from genuinely absent credentials, allowing Gemini and Antigravity to retain their last successful quota readings consistently.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/main/rate-limits/antigravity-usage-mirror.ts | Reclassifies failed shared-quota reads as Antigravity errors while keeping absent-sign-in handling unavailable and preserving provider-specific wording. |
| src/main/rate-limits/gemini-oauth-sources.ts | Throws for a present but structurally invalid OAuth credential file while continuing to treat a missing file as absent. |
| src/main/rate-limits/service.ts | Integrates the revised Antigravity mirror semantics into the existing shared refresh and stale-retention flow. |
| src/main/rate-limits/service-antigravity-usage.test.ts | Verifies symmetric stale-reading retention and prevents Gemini’s raw failure text from leaking into Antigravity output. |
| src/main/rate-limits/gemini-usage-fetcher.fallback.test.ts | Distinguishes malformed-present credentials from genuinely missing credentials in fallback behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Read Gemini credentials] --> B{Credentials present and valid?}
B -->|Missing| C[Gemini unavailable]
B -->|Malformed| D[Gemini error]
B -->|Valid| E[Read shared Code Assist quota]
E -->|Success| F[Gemini and Antigravity OK]
E -->|Failure| D
D --> G[Antigravity error with provider-specific reason]
D --> H[Retain recent Gemini snapshot]
G --> I[Retain recent Antigravity snapshot]
C --> J[Gemini and Antigravity unavailable; discard stale readings]
Reviews (3): Last reviewed commit: "fix(rate-limits): a malformed Gemini oau..." | Re-trigger Greptile
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
Merge Risk: ⚪ Minimal · up to Failed shared-quota reads now retain cached data while surfacing an error with provider-specific messaging preserved; the localized change has no actionable merge-blocking risk remaining after normal checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides detailed context, rationale, testing evidence, linked issue references, and scope. However, it does not follow the repository template structure and omits or leaves unclear required items such as an explicit ELI5, What Changed, Why, Linked Issue using Resolution Update the description to include the required template headings and information. Add an explicit linked issue using
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
…lable usage This lane now retains the last reading, so the tooltip prints the reason above a live meter; the old wording contradicted the numbers beside it. Also corrects the service comment that still stated the reversed rule.
…d, not absent readGeminiCredentials() returned null both when the file was missing and when it was present but did not match the credential contract. The caller turns null into status 'unavailable' / "credentials not found", so a corrupt or wrong-typed oauth_creds.json was reported to the user as signed out — the failed read became an absent one before the shared-quota rule could see it, and absent takes the discard path. Throw on the present-but-invalid case so it reaches the caller's error path, and keep ENOENT returning null so an actually missing file still reads as absent. Both sides of that boundary are now pinned.
|
Migration coordination: #21654 replaces the Gemini-derived Antigravity mirror with the native, version-guarded The underlying retention requirement remains valid for Antigravity's own last successful reading. I checked the current takeover stack and reran all six ELI5: retain the last numbers from Antigravity when its refresh temporarily fails; do not borrow Gemini's numbers. The native migration preserves that distinction. This is service-level evidence using mocked readings. Actual installed agy 1.2.7 has returned empty quota groups in our live verification, so populated OAuth readings and live stale retention are not yet proven. No merge or closure is requested on that basis. |
This branch has not been deployed
2 participants
Adjudication: the unreadable-response rule vs. the reverted Antigravity judgement
A stated rule (
unreadable-usage-response.ts, on #17193) and a documented prior decision (#15876)disagreed about the Antigravity usage mirror. This PR settles it. Outcome: the prior decision was
made under an assumption that no longer holds. It is changed deliberately; its tests are updated
in place, not deleted; and the reasoning is written into the file the next person meets.
The previous author's reasoning (#15876,
636f428c25)Before #15876 the mirror was
{...gemini, provider: 'antigravity'}. On a Gemini failure theAntigravity row said "Refresh failed" and quoted Gemini's raw error (
Gemini project ID not found) — for a request Orca never sent, since Antigravity's token lives in the OS keyring and Orcahas no Antigravity endpoint. #15876 gave the row two Antigravity-owned reason strings and settled it
unavailable, pinned byservice-antigravity-usage.test.ts— "never leaves a cached Antigravitysnapshot in the error retry lane / Why: stale-retention would otherwise show Gemini numbers as
'Refresh failed' Antigravity usage."
That reasoning is about wording and blame, and it is sound. It is preserved here in full.
The assumption that no longer holds
unavailablewas picked as a label choice. It is a retention verdict:applyStalePolicydiscards on
unavailableand retains onerror. Driven ona1f198be0d, one Gemini failure:The same read backs both rows. #15876's own second test blesses a successful Gemini read as
Antigravity's reading — so that read cannot be Antigravity's when it succeeds and nobody's when it
fails. That is exactly the shape the rule names: a failed read reclassified as an absent one.
The asymmetry, and why it now has a stated reason
It no longer exists on a failed read — both rows retain, both settle
error. It is retained,with a reason, for the other Gemini verdict:
unavailable(opt-in off) still maps tounavailableon both, because nothing was read. Driven post-fix:
On the second harm — the hidden status surface
Driven, and it does not apply to Antigravity, uniquely:
isProviderConfiguredreturns false forunavailable, butgetVisibleUsageProviderre-renders the row from the settings signal(
antigravityUsageConfigured && geminiCliOAuthEnabled), which is what makes the row exist at all.The row survived either way. Only the destroyed-snapshot half was ever real here — reported rather
than overstated.
"Refresh failed" comes back — deliberately
The label is status-derived. With a retained reading it reads "Refresh failed — showing cached
data" over the Antigravity reason line, which names the shared quota as the thing that failed, so
no Antigravity-specific request is implied. A bespoke provider label (the
usage-error-copy.tsfailureKindmechanism, which already existed in Aug 2026) was considered and declined: itwould suppress the more useful "showing cached data" suffix. Stated in the file header.
Verification
Failing-first, 1-to-1: the two new assertions were red on unmodified
mainbefore the fix; the twoprior-author status assertions went red only after it, and were then updated in place with their
new reasoning. Ablations run serially, each mutation verified applied and reverted clean, over
src/main/rate-limits+src/renderer/src/components/status-bar(89 files / 741 tests, green):statusback to'unavailable')true(all non-ok →error)gemini.errorinto the Antigravity reasonBoth halves are separately load-bearing, and #15876's message protection is still strongly pinned.
Read sites grepped, including the chip:
status-bar-provider-visibility.ts,usage-error-copy.ts,usage-provider-settings-target.ts(Antigravity's CTA points ataccounts-gemini, consistent),StatusBar.tsx. No new fetch churn:deriveAntigravityRateLimitsonly emitserrorwhen Gemini iserror, and Gemini already forces the same non-individually-refreshablefetchAll()lane.Gates:
pnpm tcall projects exit 0 ·check:code-quality:changedexit 0 (native + type-aware--deny-warningsexit 0(oxlint proven non-vacuous with a control violation) ·
git diff --checkclean · formatted withnpx oxfmt --write <paths>. No suppressions, nomax-linesdisable.Scope: nothing here touches #17193. The conclusion is that Antigravity now conforms to the
rule, so the rule file needs no exception — but #17193's note that the rule "contradicts the
reverted antigravity judgement" is now resolved and its owner may want to drop it.