feat(videos): carry Grok video bridge onto dev2-go (#582) - #660
Conversation
) * feat(videos): add Grok video bridge for non-OpenAI models Extends the image bridge to support asynchronous video generation via xAI Grok Imagine Video. Video generation uses a submit→poll→download pattern (POST /v1/videos/generations → GET /v1/videos/{id}) with heartbeat forwarding to keep the SSE stream alive during the 30-180s generation window. New files: - src/images/xai-video-client.ts: submitVideoJob + pollVideoJob - src/images/fulfill-video.ts: arg parsing, async polling generator, result builder - tests/videos/: 32 tests (xai-video-client, fulfill-video, plan-video) - docs-site/.../video-bridge.md: user guide Modified files: - src/images/loop.ts: unified image+video fulfillment, per-turn call caps - src/images/types.ts: VideoBridgePlan/VideoCallResult shared types - src/images/synthetic-tool.ts: buildVideoTool, VIDEO_GEN_TOOL_NAME - src/images/plan.ts: planVideoBridge (opt-in via videoBridgeEnabled) - src/images/artifacts.ts: downloadVideoToArtifact (200MB cap, SSRF-protected) - src/server/responses/core.ts: video bridge wiring + tool dedup - src/types.ts: videoGeneration flag, video config fields Video bridge is opt-in (videoBridgeEnabled defaults to false). All upstream image-bridge hardening preserved (SSRF, size caps, abort linking, pinned HTTPS). * fix(videos): address Codex P1/P2 + CodeRabbit review findings P1 fixes: - Non-streaming requests no longer 400 when video bridge is enabled (only image bridge requires stream=true; video-only skips bridge for non-stream requests) - Restore globalThis.fetch after video client tests (afterEach cleanup) P2 fixes: - Use clamped vidPlan.timeoutMs instead of raw config value - Honor videoMaxRounds when both image+video bridges active (tighter wins) - guessVideoExtFromMagic throws on unrecognized magic (was defaulting to mp4) - Poll 4xx permanent failures (except 429) fail fast instead of retrying - buildVideoResult uses pathToFileURL for cross-platform markdown links - Initial poll interval restored to 5s (was 200ms) - done-without-videoUrl returns error instead of spinning until timeout - VideoBudget charges every streamed chunk (was only first chunk) - downloadVideoToArtifact: reader cleanup on file-open failure - readBoundedText: cancel reader body on size cap breach - Namespaced video_gen tools preserved in dedup filter - Fix JSON missing closing brace in docs - Clarify API key auth requirement in docs prerequisites * fix(videos): encode requestId, defer paidVideoCalls, guard plan! assertion - Encode requestId in poll URL to prevent path injection (CodeRabbit) - Move paidVideoCalls++ past arg validation so malformed calls don't burn budget - Add early guard for undefined plan in image branch, removing non-null assertions * fix(videos): batch-aware artifact pruning + clarify provider key docs - Move pruneArtifacts from per-download to post-batch so multi-video turns don't delete earlier videos before tool results are injected (Codex P2) - Document that video bridge requires providers.xai with apiKey authMode, not OAuth from 'ocx login xai' (Codex P2) * fix(videos): web search coexistence, unified timeout budget, VideoBudget ceiling Address Wibias CHANGES_REQUESTED review: 1. Web search coexistence (P1): - When wsPlan is active, media bridge is skipped (existing behavior) - Now emits console.warn so the user sees the skip instead of silent loss - Documented priority rule in video-bridge.md 2. Timeout budget (P2): - Start deadline BEFORE submitVideoJob, not after - Poll receives remaining budget (deadline - now), min 5s floor - Submit (60s) + poll now share one videoTimeoutMs deadline 3. VideoBudget aggregate ceiling (optional, addressed): - VideoBudget now has a cap (600 MiB = 3 × single-download max) - chargeVideoBudget() enforces the ceiling per-chunk during streaming - Exceeding the budget throws mid-download (partial file is cleaned up) 4. New tests: done-without-videoUrl, permanent 4xx poll stop, requestId encoding * fix(videos): gate tool_choice on imgPlan, shared deadline signal, buffer magic sniff Address Wibias second CHANGES_REQUESTED review: Required: B. Gate image tool_choice rewriting on imgPlan — video-only turns no longer rewrite image_generation/image_gen aliases to an undeclared tool (core.ts) C. Shared timeout complete — deadline-bound AbortSignal passed into submitVideoJob; if budget expired after submit, poll is skipped with timeout error (no 5s floor) Preferred: A. Buffer ≥12 bytes before magic-byte sniff — accumulate chunks until sniff minimum so short first reads don't crash guessVideoExtFromMagic (artifacts.ts) Optional: D. Docs: scope web-search priority to runnable sidecar / non-runTurn path * fix(videos): pass deadline-bound signal into poll generator Pass linkedDeadline (not raw client signal) into pollVideoWithHeartbeats so in-flight pollVideoJob fetches and sleep() calls abort when the wall-clock budget expires. Abort error messages now distinguish deadline-expiry from client-cancel based on elapsed time vs timeoutMs threshold. * fix(videos): deadline abort classification, pruned-path guard, video aliases, DNS noun Address Wibias fourth CHANGES_REQUESTED review: 1. Deadline abort classification (required #1): - Replace 0.95 wall-clock heuristic with signal.reason.name === 'TimeoutError' - deadlineSignal declared outside try so catch can check deadlineSignal.aborted - Submit catch returns timeout error when deadlineSignal fires (not generic abort) 2. Don't deliver pruned artifact paths (required #2): - After batch prune, scan fulfilled results and rewrite any whose path no longer exists to ok:false with explanatory error 3. Wire video aliases (required #3): - planVideoBridge seeds toolNames with VIDEO_GEN_TOOL_NAME + any request function tools matching isVideoGenName (image-bridge parity) - core.ts filter only strips unnamespaced video_gen aliases (namespaced MCP tools are left alone) 4. Fix test fixtures (required #4): - authMode 'api_key' → 'key' (valid OcxProviderConfig value) 5. DNS error noun (small): - resolvePublicAddresses accepts optional noun param (default 'image') - downloadVideoToArtifact passes 'video' * fix(videos): complete pruned-path guard, skip namespaced aliases, mock sleep in tests Address Wibias fifth CHANGES_REQUESTED review: 1. Finish pruned-path guard: - Filter files[] through existsSync, not just path - If all pruned: ok:false with no markdown/path - If some survive: refresh path/files/count/markdown from survivors 2. Skip namespaced tools when seeding videoPlan.toolNames: - if (t.namespace) continue — matches core.ts filter rule 3. Fix ubuntu CI timeout: - Mock setTimeout in heartbeat test so 5s poll interval resolves instantly - Suite now runs in ~156ms (was ~5s) * fix(videos): stop hanging ubuntu CI with global setTimeout mock cf76fb1 mocked globalThis.setTimeout to skip the 5s poll sleep. That races parallel Bun workers and cancelled ubuntu-latest at the 12m job cap (938f25e was green; cf76fb1 hung). Inject sleep/poll seams instead and drop mock.module so video tests no longer poison each other. --------- Co-authored-by: Your Name <your.email@example.com> Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com> (cherry picked from commit 257c626)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec48c29bd
ℹ️ 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".
| title: Video Bridge | ||
| description: Generate videos with Grok Imagine Video through a non-OpenAI model. |
There was a problem hiding this comment.
This commit explicitly carries the feature onto dev2-go, but its changed paths are limited to docs-site/, ordinary src/ files, and tests; it touches none of go/, bin/native-runtime.mjs, src/lib/runtime-entry.ts, or the Go release-asset tooling. This is outside the accepted scope of the Go transition line, so retarget or reapply it to dev instead.
AGENTS.md reference: AGENTS.md:L84-L89
Useful? React with 👍 / 👎.
| // image_generation/image_gen aliases would add an undeclared tool that strict upstreams reject. | ||
| const tc = parsed.options.toolChoice; | ||
| if (tc && typeof tc === "object" && "allowedTools" in tc && Array.isArray(tc.allowedTools)) { | ||
| if (imgPlan && tc && typeof tc === "object" && "allowedTools" in tc && Array.isArray(tc.allowedTools)) { |
There was a problem hiding this comment.
When a streaming request declares an unnamespaced alias such as generate_video and selects it through tool_choice or allowed_tools, the bridge removes that declaration and injects video_gen, but this imgPlan-gated rewrite leaves the selection pointing at the removed alias on a video-only turn. Strict upstreams then reject the undeclared selected tool, while adapters that filter by allowed tools can omit video_gen entirely; map every vidPlan.toolNames alias to VIDEO_GEN_TOOL_NAME in both tool-choice shapes.
Useful? React with 👍 / 👎.
| > If you onboarded via `ocx login xai` (OAuth), the provider stays in `authMode: "oauth"` | ||
| > and the bridge silently won't activate. Set `XAI_API_KEY` in the environment **or** | ||
| > hard-code the key as shown above. |
There was a problem hiding this comment.
For a provider created by ocx login xai, merely setting XAI_API_KEY does not activate this bridge: resolveXaiImageApiKey returns immediately whenever authMode === "oauth", before resolving the provider key or its environment reference. The documented environment alternative therefore leaves the bridge silently disabled; instruct users to change the provider to key auth and set apiKey to an XAI_API_KEY environment reference.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
| const existingNames = new Set(bridgeTools.map(t => t.name)); | ||
| if (imgPlan && !existingNames.has(IMAGE_GEN_TOOL_NAME)) bridgeTools.push(buildImageTool()); | ||
| if (vidPlan && !existingNames.has(VIDEO_GEN_TOOL_NAME)) bridgeTools.push(buildVideoTool()); |
There was a problem hiding this comment.
When an MCP namespace contains a tool whose leaf name is video_gen or image_gen, the filter intentionally preserves that namespaced tool, but existingNames records only its leaf name and suppresses injection of the corresponding synthetic unnamespaced tool. Since the adapter exposes the MCP tool under a namespaced wire name such as mcp__foo__video_gen, there is no real collision, and the enabled media bridge becomes unavailable; only unnamespaced tools should satisfy this occupancy check.
Useful? React with 👍 / 👎.
| maxRounds: imgPlan && vidPlan | ||
| ? clampImageMaxRounds(Math.min(config.images?.maxRounds ?? 3, config.images?.videoMaxRounds ?? 2)) |
There was a problem hiding this comment.
When both bridges are enabled, taking the minimum makes videoMaxRounds silently override the existing image bridge's maxRounds even on turns that only call image_gen. With the documented defaults, image generation is forced final after two rounds instead of three, and setting the supported videoMaxRounds: 0 disables image fulfillment entirely despite a positive image limit; preserve the image budget independently and enforce the video limit only for video calls.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| const resolved = await resolvePublicAddresses(url, "video"); | ||
| const pinned = pickPinnedAddress(resolved.addresses); | ||
| const resp = await pinnedHttpsGet(url, pinned, signal, { maxBytes: MAX_VIDEO_DOWNLOAD_BYTES }); |
There was a problem hiding this comment.
For a large or fast CDN response, pinnedHttpsGet switches the IncomingMessage into flowing mode with res.on("data") and enqueues every chunk without pausing when the Web stream queue fills, while this consumer awaits each disk write serially. The new 200 MiB cap therefore permits nearly an entire video to accumulate in heap per request despite the streaming-to-disk claim, and concurrent generations can exhaust memory; make the pinned transport pause/resume according to Web-stream backpressure or pipe directly to the file.
Useful? React with 👍 / 👎.
| The Video Bridge lets you use xAI's Grok Imagine Video generation through any non-OpenAI model | ||
| routed by opencodex. When enabled, a synthetic `video_gen` tool is injected into the conversation. | ||
| The model calls it like any function tool; opencodex intercepts the call, submits a video generation | ||
| job to xAI, polls until completion, and downloads the result. |
There was a problem hiding this comment.
The overview says enabling the bridge injects video_gen, but handleResponses deliberately bypasses the video-only bridge for every stream: false request without returning an error. A non-streaming client following this guide therefore receives an ordinary completion with no generation capability and no explanation; state that stream: true is required, or change the runtime to report the unsupported request explicitly.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
| "images": { | ||
| "bridgeEnabled": true, | ||
| "videoBridgeEnabled": true, |
There was a problem hiding this comment.
Copying this configuration enables the separate paid image bridge even though planVideoBridge requires only videoBridgeEnabled. On clients that include the hosted image_generation tool, a user following this video guide can therefore incur unrelated xAI image charges; omit bridgeEnabled: true from the example or explicitly label it as an optional second paid capability.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
| const buf = Buffer.from(data, "base64"); | ||
| if (budget && !chargeVideoBudget(budget, buf.byteLength)) { | ||
| throw new Error("video data URI exceeds per-turn download budget"); | ||
| } | ||
| if (buf.byteLength > MAX_VIDEO_DOWNLOAD_BYTES) throw new Error("video data URI exceeds size cap"); |
There was a problem hiding this comment.
When the xAI result is a data: URL, Buffer.from(data, "base64") allocates the decoded payload before either the 200 MiB per-video cap or the aggregate budget is checked. A malformed or oversized response can consequently allocate hundreds of megabytes or more and terminate the proxy instead of becoming a normal failed tool result; validate the base64 syntax and encoded-length ceiling before decoding.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| --- | ||
| title: Video Bridge | ||
| description: Generate videos with Grok Imagine Video through a non-OpenAI model. |
There was a problem hiding this comment.
A repo-wide search finds no link to guides/video-bridge, and the manually enumerated Starlight Guides navigation does not include it, so users browsing the published docs cannot discover this new page without guessing its URL. Add it to an appropriate navigation surface or link it from a directly related canonical guide such as Image Bridge or Sidecars.
AGENTS.md reference: docs-site/AGENTS.md:L15-L16
Useful? React with 👍 / 👎.
Summary
dev2-go(TS oracle + docs + tests).257c626condev.Go port
Go already has
go/internal/images/(image bridge) but no video counterpart (video_gen/ xAI video client / fulfill-video).Per Transition rule: opening a tracking issue for the Go video port. This PR lands the TS oracle on
dev2-goso the port has a current reference; it does not claim the Go side is done.