Skip to content

fix(antigravity): preserve grouped OAuth quota summaries - #4084

Closed
steipete wants to merge 3 commits into
mainfrom
triage/20260921-antigravity-2
Closed

steipete wants to merge 3 commits into
mainfrom
triage/20260921-antigravity-2

Conversation

@steipete

@steipete steipete commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

OAuth skipped Antigravity's grouped quota-summary endpoint and its strategy treated a snapshot without per-model rows as identity-only. Request measured grouped quotas first, reuse the local/CLI parser, and preserve grouped snapshots through the OAuth strategy. Explicit bucket cadence now takes precedence over legacy names, including weekly-only Starter allowances.

The selected OAuth account supplies the email and plan. The optional summary request has a two-second timeout cap; unsupported, unmeasured, and legacy model-bucket responses retain the existing model fallback. HTTP 401 and cancellation propagate. Production code decreases by 12 lines through shared request construction, shared validation, and removal of a handwritten memberwise initializer.

This is a partial repair of the reported cases. CLI print reports still contain no account identity, and explicit OAuth retains its existing source authority. No live Antigravity account was configured for verification.

Verification

  • swift build --jobs 2 --product CodexBarCore: passed.
  • Synthetic linked-core reproduction: baseline failed three quota assertions; rebuilt core passed all four checks, including account email preservation.
  • The committed regression suite passed 4 tests / 12 parameter cases. The full selected set then passed 316 tests in 20 suites, including menu models, quota history, CLI fallback, account isolation, and ProviderArchitectureGatekeeperTests.
  • Local swift test --jobs 2 --filter ... stalled while rebuilding the app target. The successful run used the same committed test sources with Swift Testing, Swift 6 strict concurrency, SwiftPM's rebuilt core library and unchanged testable app/CLI/widget objects. Command: SWIFT_TESTING=1 PACKAGE_RESOURCE_BUNDLE_PATH="$PWD/.build/out/Products/Debug" .build/out/Products/Debug/AntigravityLanePackageTests --filter "$(cat /tmp/antigravity-2-selected-test-filter.txt)", after sourcing Scripts/test_environment.sh and removing inherited Antigravity credentials and screenshot-output settings.
  • TMPDIR="$PWD/.build/verification-tmp" make check: exit 0. SwiftFormat: 0/2672 files require formatting, 6 files skipped. SwiftLint: Found 0 violations, 0 serious in 2671 files. Earlier attempts hit process-cleanup fixture timing failures; the final 102-test helper run passed with one skip.
  • Independent Codex Autoreview at P2: scoped-clean for implementation, documentation, and the gatekeeper correction.

The first CI run failed on two obsolete gatekeeper anchors after duplicate family classification was removed. Commit 883b6f549bb5 removes only those stale entries; the remaining classifier anchors and all gatekeeper tests pass locally. The CI retry passed on exact head 883b6f549bb5c037e1e3784880e0b7d0ac51e2c7.

Refs #2427
Refs #3789
Refs #3790

Request measured quota summaries before falling back to model quotas and
reuse the local/CLI parser, preserving the selected OAuth account identity.
Keep grouped snapshots through the OAuth strategy and honor explicit bucket
cadence, including weekly-only Starter allowances.

Deduplicate quota requests and snapshot validation without growing production
code. Add synthetic parity, fallback, authentication, and cancellation tests.

Refs #2427, #3789, #3790.
@clawsweeper

clawsweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 28, 2026
@clawsweeper

clawsweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 28, 2026, 1:05 AM ET / 05:05 UTC (Revision 4).

ClawSweeper review

What this changes

The branch tries grouped Antigravity quotas for the selected OAuth account, shares the local and CLI summary parser, honors explicit bucket cadence, and updates tests and documentation.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 3 items remain

This PR remains useful, but a summary-only HTTP 401 still blocks model quotas that the selected OAuth token can fetch. Grouped OAuth responses have also been tested only with synthetic data; current main and v0.68.0 do not provide the proposed behavior.

Priority: P2
Reviewed head: 883b6f549bb5c037e1e3784880e0b7d0ac51e2c7

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Focused tests and reduced production code provide useful signal, but the reproducible 401 regression and unobserved grouped OAuth response limit readiness.
Proof confidence 🦐 gold shrimp (3/6) Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Its new production OAuth summary path has synthetic transport coverage, but no observed after-fix grouped response from a real account; no stored-data contract changes.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Its new production OAuth summary path has synthetic transport coverage, but no observed after-fix grouped response from a real account; no stored-data contract changes.
Evidence reviewed 8 items Introduced 401 stop: The new summary request rethrows notLoggedIn before the existing model-quota request.
HTTP response mapping: The shared transport maps any 401 to notLoggedIn; the established model request follows the new summary block and uses the same token.
Regression fixture: The new test supplies a 401 only for the summary endpoint and a successful model response, yet expects notLoggedIn instead of the model quota.
Findings 1 actionable finding [P1] Preserve model quotas after a summary-only 401
Security None None.

How this fits together

CodexBar fetches Antigravity usage from the selected Google OAuth account or local Antigravity sources. It converts returned quotas into usage windows for the menu and history views.

flowchart LR
A[Selected OAuth account] --> B[OAuth quota request]
B --> C{Measured groups returned?}
C -->|Yes| D[Shared summary parser]
C -->|No| E[Model quota request]
D --> F[Account usage windows]
E --> F
F --> G[Menu and history]
Loading

Before merge

  • Preserve model quotas after a summary-only 401 (P1) - The new optional summary request can return 401 while fetchAvailableModels still accepts the selected token. Rethrowing notLoggedIn here prevents the established model request from running; the added test supplies that successful model response but expects failure. Try the model route first and retain the auth error if it also rejects the token.
  • Resolve merge risk (P1) - Current documentation says observed OAuth summary responses are model-bucket shaped; no successful same-account grouped OAuth response establishes that the new grouped branch is reached in a real account.
  • Complete next step (P2) - Repair the summary-only 401 fallback and its regression test, then establish a redacted same-account grouped OAuth response or qualify the documentation before merge.

Findings

  • [P1] Preserve model quotas after a summary-only 401 — Sources/CodexBarCore/Providers/Antigravity/AntigravityRemoteUsageFetcher.swift:149-154
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code and test lines production -12, tests +112 The branch reduces production code while adding focused synthetic coverage for the new request and parser path.

Merge-risk options

Maintainer options:

  1. Repair same-token fallback (recommended)
    Let a summary-only 401 reach the existing model request, preserve the auth error if that request also rejects the token, and verify grouped behavior with redacted same-account evidence.
  2. Hold the grouped claim
    Keep the PR pending if a successful grouped OAuth response cannot be established for the account and endpoint described.

Technical review

Best possible solution:

Keep explicit OAuth account-scoped, try the established model endpoint when only the optional summary endpoint returns 401, and verify a measured grouped response before describing grouped OAuth quotas as observed behavior.

Do we have a high-confidence way to reproduce the issue?

Yes for the proposed patch: its injected transport supplies a summary-only 401 and a successful model response, while source shows the fetch throws before requesting models. No live account run was performed.

Is this the best way to solve the issue?

No. The shared parser and selected-account identity are appropriate, but the optional request must preserve the existing same-token model route and its grouped-response claim needs observed support.

Full review comments:

  • [P1] Preserve model quotas after a summary-only 401 — Sources/CodexBarCore/Providers/Antigravity/AntigravityRemoteUsageFetcher.swift:149-154
    The new optional summary request can return 401 while fetchAvailableModels still accepts the selected token. Rethrowing notLoggedIn here prevents the established model request from running; the added test supplies that successful model response but expects failure. Try the model route first and retain the auth error if it also rejects the token.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 579f68406855.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: The change addresses limited Antigravity quota visibility, while the introduced 401 regression requires repair before merge.
  • merge-risk: 🚨 compatibility: An existing setup with a working model-quota endpoint can lose its usage display when only the new summary endpoint returns 401.
  • merge-risk: 🚨 auth-provider: The added OAuth request can turn a same-token endpoint-specific rejection into an account authentication failure.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Its new production OAuth summary path has synthetic transport coverage, but no observed after-fix grouped response from a real account; no stored-data contract changes.

Evidence

Acceptance criteria:

  • [P1] swift test --filter AntigravityQuotaSourceParityTests.
  • [P1] make test.
  • [P1] make check.

What I checked:

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • abnormal749: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • sobczi: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Preserve the existing model-quota result after a summary-only 401 and keep an auth error when the model endpoint also rejects the token.
  • Capture a redacted successful same-account grouped OAuth response, or qualify the documentation's grouped-response claim.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-28T02:24:48.575Z sha ed595e9 :: blocked before merge. :: [P1] Preserve working OAuth model quotas after a summary 401
  • reviewed 2026-09-28T03:51:30.539Z sha 883b6f5 :: blocked before merge. :: [P1] Preserve model quotas after a summary-only 401
  • reviewed 2026-09-28T04:00:42.732Z sha 883b6f5 :: blocked before merge. :: [P1] Preserve model quotas after a summary-only 401

steipete added a commit that referenced this pull request Sep 28, 2026
…4084)

Antigravity: repair grouped model-family quotas on the OAuth path to match the agy CLI grouping, and parse weekly-only Starter (free-tier) quota fixtures with explicit cadence through the shared parser. Refs #2427 #3789.
@steipete

Copy link
Copy Markdown
Owner Author

Landed on main as 62acfd8 via merge train #4096 (one green CI run for the whole train).

@steipete steipete closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant