feat(web-search): live gemini executor on the Antigravity CCA transport (#2188 L8) - #2243
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes Limit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
|
✅ Deterministic PR hygiene checks passed. |
![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: 6db16f621b
ℹ️ 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".
| if (!active || active.needsReauth === true) return false; | ||
| return !!(active.credential as { projectId?: string } | undefined)?.projectId; | ||
| }, | ||
| eligibleModel: candidate => candidate.provider === "google-antigravity", |
There was a problem hiding this comment.
When Antigravity OAuth is active, this descriptor adds its models to the Dashboard picker, but gui/src/pages/dashboard-shared.ts:350-352 maps every non-Anthropic model to openai, and the picker saves that inferred backend in dashboard-overview-sections.tsx:515-519. Selecting a Gemini row therefore persists backend: "openai" and sends the Gemini model ID to the ChatGPT sidecar instead of invoking this executor; return backend metadata with each option and teach the picker to preserve gemini.
Useful? React with 👍 / 👎.
| ): Promise<SidecarOutcome> { | ||
| let token: string; | ||
| try { | ||
| token = (await getValidAccessTokenSnapshot(providerName)).accessToken; |
There was a problem hiding this comment.
When the active Antigravity token is expired and its refresh request stalls, this await occurs before signalWithTimeout is created, so neither webSearchSidecar.timeoutMs nor a client abort can stop the authentication phase; the whole Responses turn can outlive its advertised hosted-search deadline. Create the linked signal before credential resolution and race or propagate it through the refresh, as the Antigravity image path does, while cleaning it up on early returns.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| // Default Gemini model for the gemini-backed sidecar (CCA grounding probe, devlog 002). | ||
| const DEFAULT_GEMINI_SIDECAR_MODEL = "gemini-3.7-flash"; |
There was a problem hiding this comment.
This introduces a user-selectable backend and default model without updating the user documentation: docs-site/src/content/docs/guides/sidecars.md:26-28 still describes only OpenAI and Anthropic, while structure/04_transports-and-sidecars.md:898-904 explicitly says Gemini is inert. Update the English configuration/sidecar guidance, scoped translations, and the architecture record so operators can configure the required Antigravity OAuth/project flow without following contradictory instructions.
AGENTS.md reference: AGENTS.md:L279-L280
Useful? React with 👍 / 👎.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head 6db16f6. The Gemini direction is valuable, but I found three merge blockers that the current 9/9 focused suite does not cover:
- runGeminiWebSearch obtains an OAuthAccessSnapshot, keeps only accessToken, then separately re-reads the current active account for projectId. If the active account changes while token refresh or request setup is in flight, the request can pair account A bearer with account B project. The snapshot already carries the projectId bound to the exact account and generation; use that same snapshot atomically and add an account-switch regression.
- After fetchWithResetRetry resolves, neither the error-body read nor the success JSON read attaches cancelBodyOnAbort. This reopens the known abort-before-reader race already documented and fixed in the existing OpenAI and Anthropic sidecars. Attach the guard before either branch reads, and prove an abort immediately after headers cancels and settles the body.
- Both res.text and res.json materialize an unbounded upstream body. A CCA response is untrusted network input and this sidecar should retain a strict byte ceiling. Use the shared bounded body/bytes primitive with the linked signal, cancel on overflow, parse only bounded UTF-8 JSON, and add oversized success and error regressions.
The focused tests and typecheck pass locally, but exact-head CI is currently red on macOS, and this stack is based on #2242, which still has unresolved requested changes. Please fix the three boundaries, make the parent clean, rerun exact-head CI, then retarget only after the parent lands.
리뷰 · 우선순위 60 / 80#2188 L8임. 베이스가 실행기 핀이 맞음. 플랜/루프 fail-closed.
2.28 블로커 아님. 프리뷰 배포 플랜 아님. 지금 HEAD 위에 바로 머지하지 말 것. L1→L7 순서. 해결방안: #2242 머지 후 리타깃. config 이 댓글은 grok-bot이 작성했습니다 |
|
All three blockers addressed in 249cc91: (1) runGeminiWebSearch now uses the projectId carried by the SAME OAuthAccessSnapshot atomically — an account switch mid-flight can no longer pair account A bearer with account B project (account-switch regression); (2) cancelBodyOnAbort attaches immediately after fetchWithResetRetry resolves, before either the error-body or success-JSON branch reads (abort-after-headers regression proves the body cancels and settles); (3) both reads go through a bounded 64 KiB UTF-8 JSON path with the linked signal, cancelling on overflow (oversized success + error regressions). All four regressions observed red before the fix. Fresh local: 50/0 across three suites, tsc clean, privacy:scan pass. Parent #2242 blockers were addressed in b2c2054 on its branch; will rerun exact-head CI after the stack settles. Ready for re-review. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head 249cc91a38b742da8e279c885b31dd10490d390f. The three blockers from my previous review are fixed: the token and project now come from one OAuth snapshot, the response body is guarded immediately after headers, and both success/error payloads use the bounded 64 KiB reader. The focused regressions, typecheck, privacy scan, and exact-head CI are green.
Three current-head blockers remain before this stack layer can be approved:
runGeminiWebSearchstill createssignalWithTimeoutonly aftergetValidAccessTokenSnapshotcompletes. A stalled Antigravity refresh therefore ignores both the caller abort andwebSearchSidecar.timeoutMs. The existing CCA image path creates the deadline before auth and races the non-cancellable refresh against it. Apply the same boundary here and add abort/timeout-during-auth regressions.- The server option DTO still drops backend provenance (
WebSearchModelOptionhas nobackend), while the GUI web-search picker still callssidecarBackendForModel, whose fallback is OpenAI for every non-Anthropic row. Selecting agoogle-antigravitycandidate therefore persists{ backend: "openai" }and never reaches this executor. Carry the exact backend in every server option, widen the GUI web-search backend type to includexai | gemini | exa, preserve it through grandfathered/current options, and add a picker/save regression. - The user and architecture docs are now contradictory:
guides/sidecars.mddocuments only OpenAI/Anthropic, andstructure/04_transports-and-sidecars.mdstill says explicit Gemini is inert. Document the live Antigravity OAuth/project requirement, default model, fail-closed behavior, and update directly affected translations.
Please keep this on top of #2242, fix these boundaries, and rerun exact-head CI. The implementation remains a strong stack candidate after that.
3f574c3 to
a03f51a
Compare
…2188 L8) runGeminiWebSearch sends the CCA envelope (registry-pinned endpoint, IDE fingerprint UA, discovered projectId, google_search tool, effort-mapped thinkingLevel) with the stored Antigravity OAuth and maps candidates[0] text + groundingMetadata.groundingChunks to the sidecar outcome. planWebSearch's gemini arm goes live fail-closed on OAuth or projectId absence; the loop arm fails closed without a resolved provider; the registry activates the backend on the same predicate.
…t shape Non-Error throws reached tool results unredacted in both new executors. Adds the reviewer-demanded request-shape test (registry destination despite a malicious baseUrl, manual redirect, bearer + IDE UA, full CCA envelope) and the gemini loop fail-closed regression mirroring xai.
Bind the bearer token and Cloud Code Assist project to one OAuthAccessSnapshot so an active-account switch cannot cross-pair credentials. Guard the response body immediately after headers, then consume success and error payloads through the shared 64 KiB byte bound with linked cancellation and strict UTF-8 JSON parsing. Add regressions for account switching, post-header abort settlement, and oversized success/error cancellation. Evidence: bun test tests/gemini-web-search.test.ts tests/cancel-body-on-abort.test.ts tests/bounded-body.test.ts (50 pass); bun x tsc --noEmit; bun run privacy:scan.
249cc91 to
ef551e8
Compare
2 participants
Summary
Layer 8 of the #2188 follow-up chain (parent: #2242). The Gemini web-search executor goes live on the Antigravity CCA transport:
src/web-search/gemini-executor.ts:runGeminiWebSearchsends the CCA envelope — REGISTRY-pinned destination (a config-level baseUrl is never trusted for OAuth transmission, same rule assrc/server/images.ts), IDE-fingerprint User-Agent (a plain UA gets 404, probe-verified), discoveredprojectId,tools: [{google_search:{}}], and the effort-mappedthinkingConfig.thinkingLevelviaresolveAntigravityEffortWireModel.redirect: "manual", never-throws, all error branches redacted.mapCcaGroundedResponsemapscandidates[0]text +groundingMetadata.groundingChunks[].webto the sidecar outcome, deduped by uri.planWebSearch's gemini arm goes live fail-closed (OAuth or projectId absent → no plan); the loop arm fails closed without a resolved provider (regression-pinned: zero fetches, zero pool recordings); registry activates the backend on the same predicate.google-antigravity-wire+antigravity-modelsonly — nogoogle.tsadapter drag, reviewer-verified).Live E2E through the real executor: grounded answer 433 chars + 2 grounding sources, with automatic OAuth token rotation mid-call.
Design doc:
devlog/_plan/260820_sidecar_selection_unification/080_layer8_gemini_executor.md(rev 2) + 002 LIVE PROBE.Verification
gemini-3.7-flash-tiered+thinkingLevel: "low"), grounding-map fixtures from the live capture, fail-closed matrix, loop-level fail-closed regression.Checklist