feat(web-search): live xai executor with opt-in x_search (#2188 L7) - #2242
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 (29)
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: 0f2d670c0e
ℹ️ 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".
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head 0f2d670. The Codex findings are confirmed in the current code and are merge blockers:
- runXaiWebSearch does not attach the existing cancelBodyOnAbort guard after fetchWithResetRetry resolves, leaving the abort-before-reader race uncontained.
- parseXaiResponsesSSE breaks on the byte bound but only releases the reader lock; it must cancel the upstream body so repeated oversized streams cannot retain transport resources.
- the management PUT mutates config.webSearchSidecar fields before xSearch validation. An invalid xSearch therefore returns 400 after changing live process state.
- supplied malformed xSearch fields are silently omitted by lift/type checks. Invalid handle arrays, dates, or enabled values must be rejected rather than broadening the requested search while returning 200.
- the public docs and the type comment still describe xai as inert even though this PR makes the executor live and adds user-visible xSearch configuration.
Please stage and validate the complete webSearch candidate before committing it, add malformed-input/no-partial-mutation regressions, mirror the established abort/body-cancellation pattern, cover the oversized-stream cancellation path, and update the English source docs plus non-contradictory translations. This stacked PR is also downstream of #2238, which currently has unresolved requested changes, so it cannot advance independently.
Split the opt-in DTO into a write boolean vs read tri-state; record the concrete #2238 (3) and #2242 (5) review blockers in doc 120; recast doc 150 as the final aggregate gate with the full GUI/i18n/docs chain; replace temporal API-key rows with exact wire+tier assertions; state the explicit wp9->wp8->wp11->wp10 execution sequence.
리뷰 · 우선순위 62 / 80#2188 L7임. 베이스가 지금 실행기 본체는 방향 맞음. 저장된 Grok OAuth만. 근데
2.28 블로커 아님. 기본 Grok 구멍은 #2240 쪽 (#2217 첫 턴, #2237 둘째 턴). 이 PR은 사이드카 백엔드임. 패스스루 Responses가 아님. 레지스트리 해결방안: L1-L6 이 댓글은 grok-bot이 작성했습니다 |
|
All five blockers addressed in b2c2054: (1) runXaiWebSearch attaches cancelBodyOnAbort right after fetchWithResetRetry resolves, mirroring the OpenAI/Anthropic executors; (2) parseXaiResponsesSSE cancels the upstream body at the byte bound instead of only releasing the reader lock, with an oversized-stream regression; (3) the management PUT stages and validates the complete webSearch candidate before committing — invalid xSearch no longer mutates live state (no-partial-mutation regression); (4) malformed xSearch handle arrays/dates/enabled values are rejected with 400 rather than silently broadened (malformed-input regressions); (5) English docs + type comment no longer call xai inert, translations (fr/ja/ko/ru/tr/zh-cn/zh-tw) updated consistently. Fresh local: 47/0 across four suites + 12/0, tsc clean, docs build 393 pages. Ready for re-review. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Re-reviewed exact head b2c2054b59d84490cab801dfd8af75e3e9c491a0. The five prior code blockers are fixed: the xAI body receives the abort guard immediately after fetch, overflow cancels the reader, the full management update is staged before commit, malformed handle/date input is rejected instead of broadened, and the xAI/xSearch documentation is synchronized across locales.
Independent local verification on this head: 52 focused tests passed; bun run typecheck and bun run privacy:scan passed; the docs site built all 393 pages.
I am not approving this SHA yet because GitHub currently reports the stacked PR as CONFLICTING / DIRTY: its parent #2238 advanced after this head. Please rebase onto the current codex/sidecar-backend-union head, resolve the stack conflict, and rerun exact-head CI. The previous finding threads can be closed as fixed, but approval belongs on the rebased SHA.
d9a1fbc to
7d015ba
Compare
runXaiWebSearch POSTs the pinned api.x.ai Responses endpoint with the stored Grok OAuth credential, hosted web_search (+ opt-in x_search with doc-validated handle/date limits), and reduces the SSE stream to text + deduped sources from url_citation annotations unioned with web_search_call action.sources. custom_tool_call items and skeleton action are tolerated per live captures. planWebSearch's xai arm goes live fail-closed; the loop dispatches it without touching forward headers; the registry activates the backend on OAuth presence; PUT validates the xSearch block before persisting.
…, round-trip xSearch Origin pinning by prefix admitted https://api.x.ai.evil — parse and compare url.origin, falling back to the canonical endpoint (Critical). A missing xaiSidecar on the xai loop arm now yields an error outcome instead of falling through to the forward-header OpenAI executor (High). GET and the PUT response now carry the persisted xSearch block so a dashboard reload cannot lose visible configuration (High). Tests: lookalike-origin bearer containment, xSearch 400/round-trip/clear.
Attach abort-driven body cancellation immediately after the xAI fetch, cancel oversized SSE readers upstream, and cover transport release. Stage and validate complete web-search management candidates before commit; reject malformed xSearch types without partial mutation or broadened searches. Document the live Grok OAuth/x_search backend across canonical docs, locales, type comments, and structure sources. Evidence: 47 focused tests pass; bun x tsc --noEmit; docs Astro build 393 pages.
…er rebase Cross-layer reconciliation: #2209 introduced the (backend, model) pair contract typed as openai|anthropic; #2238 widened the configured union to five backends. The options module now re-exports WebSearchBackendId and passes config into isActive, so inert arms simply never produce rows. tsc clean; 48/0 across the three web-search suites.
3f574c3 to
a03f51a
Compare
Split the opt-in DTO into a write boolean vs read tri-state; record the concrete #2238 (3) and #2242 (5) review blockers in doc 120; recast doc 150 as the final aggregate gate with the full GUI/i18n/docs chain; replace temporal API-key rows with exact wire+tier assertions; state the explicit wp9->wp8->wp11->wp10 execution sequence.
…#2245) * feat(web-search): exa executor and the non-LLM search lane (#2188 L9) runExaWebSearch POSTs api.exa.ai/search with the operator key and maps ranked results to a digest the routed model synthesizes from. The key never rides the SidecarPlan — core.ts reads it from config at unpack time — and the executor scrubs the literal key from every error string (pattern-based redaction cannot know an arbitrary operator key; canary-tested). Plan, loop, and registry arms fail closed without the key. docs-site gains the explicit-only backend table. * fix(web-search): scrub the exa key before truncating error bodies Reviewer blocker (L9 round 2): error(t.slice(0,200)) truncated before the literal-key scrub, so a key straddling the 200-char boundary left an unscrubbable prefix in the returned tool error. Scrub first, then slice. Adds truncation-boundary and fetch-rejection canaries; 9/9 focused tests, tsc and privacy:scan green. * docs(devlog): integration merge-train roadmap 100-150 (chat default, global order, opt-in switch) Amends the 260820 unit with the audited (3-round sol-medium, round-3 PASS) roadmap: 100 chat-default regression as an atomic #2227+tier-policy unit with a 5-row regression matrix and the E2E reasoning-streaming proof; 110 global cross-train merge order and 21-PR triage matrix (#2072 deferred, #2217 RESHAPE); 120 sidecar L1-L9 merge execution with the fresh blocker inventory; 130 atomic xai Responses opt-in switch (single provider id, auth-mode-scoped sections, virtual PATCH field); 140 release prep; 150 blocking lidge final gate. DeepSeek explicitly out of scope per user decision. * docs(devlog): fold C-gate blockers into roadmap 100-150 Split the opt-in DTO into a write boolean vs read tri-state; record the concrete #2238 (3) and #2242 (5) review blockers in doc 120; recast doc 150 as the final aggregate gate with the full GUI/i18n/docs chain; replace temporal API-key rows with exact wire+tier assertions; state the explicit wp9->wp8->wp11->wp10 execution sequence. * docs(devlog): doc 100 API-key opt-in row preserves current tier forwarding C-gate round 2: current dev forwards caller service_tier verbatim on the API-key + explicit openai-responses route (fastPolicyForModel proof). The tier drop is an OAuth-route policy only; the API-key row now states preserve-current semantics, consistent with doc 130. * docs(devlog): wp9 execution record — all six chain blockers resolved and pushed
2 participants
Summary
Layer 7 of the #2188 follow-up chain (parent: #2238). The xAI web-search executor goes live:
src/web-search/xai-executor.ts:runXaiWebSearchPOSTs the EXACT-origin-pinnedapi.x.aiResponses endpoint (parsedurl.origincomparison — a prefix check admitted lookalike hosts, review-caught Critical) with the stored Grok OAuth credential, hostedweb_search+ opt-inx_search(doc-validated: ≤20 handles, allow XOR exclude, ISO dates),include: web_search_call.action.sources, andreasoning.effort(probe-verified).redirect: "manual", never-throws, 401/403 entitlement-distinct errors, byte-bounded SSE reducer.url_citationannotations ∪web_search_call.action.sources, deduped;custom_tool_callitems (live x_search reality) and skeletonactiontolerated per the devlog 003 captures.planWebSearch's xai arm goes live fail-closed (no credential → no plan; invalid persisted xSearch → no plan);SidecarPlan.xaiSidecar→ core.ts handoff → loop arm that FAILS CLOSED when the sidecar is absent (review-caught High: the fallthrough would have reached the forward-header executor). No Codex pool outcome recording on this arm.xSearchconfig block: PUT-validated (400 on doc-limit violations), GET/PUT round-tripped (review-caught High), null clears.Live E2E through the real executor: web_search → 130 chars + 12 sources; x_search with
allowed_x_handles: ["xai"]→ 492 chars.Design doc:
devlog/_plan/260820_sidecar_selection_unification/070_layer7_xai_executor.md(rev 2) + 003 research.Verification
Checklist