-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/add gemini veo #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Added a new video generation module with support for text-to-video and image-to-video functionalities. - Implemented middleware for logging, retry, and timeout handling in video generation processes. - Created a comprehensive README for the video generators, detailing usage, input/output schemas, and features. - Integrated Veo API for video generation, including support for various model capabilities and operation polling. - Updated package.json and bun.lock to include new dependencies and configurations for the video generation tools.
- Added a new video generation module with support for text-to-video and image-to-video functionalities. - Implemented middleware for logging, retry, and timeout handling in video generation processes. - Created a comprehensive README for the video generators, detailing usage, input/output schemas, and features. - Integrated Veo API for video generation, including support for various model capabilities and operation polling. - Updated package.json and bun.lock to include new dependencies and workspace configurations for the video generation module.
WalkthroughAdds a Veo video-generation service and shared video-generators library, creates a Cloudflare Workers MCP server with Veo/Gemini integration and related tooling/config, exports the shared video-generators package, updates workspace manifests, and removes the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant MCP as MCP Server
participant Contract as Contract MCP
participant VeoClient as Veo client
participant Gemini as Gemini API
participant Storage as ObjectStorage
User->>MCP: POST GENERATE_VIDEO (prompt, options)
MCP->>Contract: CONTRACT_AUTHORIZE(clauses)
Contract-->>MCP: { transactionId }
MCP->>VeoClient: start generation request
VeoClient->>Gemini: start operation -> { operationName }
loop poll (interval)
VeoClient->>Gemini: get operation status
Gemini-->>VeoClient: status
end
VeoClient->>Gemini: download video stream
Gemini-->>VeoClient: video bytes
VeoClient->>Storage: saveVideo (presigned PUT)
Storage-->>VeoClient: { readUrl, key }
MCP->>Contract: CONTRACT_SETTLE(transactionId, clauses)
Contract-->>MCP: settled
MCP-->>User: response { videoUrl, operationName }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
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 |
🚀 Preview Deployments Ready!Your changes have been deployed to preview environments: 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (6)
shared/video-generators/storage.ts (2)
32-34: Minor: Extension extraction could be more robust.The extension extraction via
mimeType.split("/")[1]works for common cases but may produce non-standard extensions for some MIME types (e.g.,video/quicktime→quicktime). Consider using a MIME type to extension map for production-grade handling.const MIME_TO_EXT: Record<string, string> = { "video/mp4": "mp4", "video/webm": "webm", "video/quicktime": "mov", // ... other common types }; const extension = MIME_TO_EXT[mimeType] || mimeType.split("/")[1] || "mp4";
46-58: Suggest adding path context to error message.The error message could be more helpful for debugging by including the storage path.
if (!uploadResponse.ok) { throw new Error( - `Failed to upload video: ${uploadResponse.status} ${uploadResponse.statusText}`, + `Failed to upload video to ${path}: ${uploadResponse.status} ${uploadResponse.statusText}`, ); }veo/wrangler.toml (1)
23-38: Consider clarifying currency formatting in contract body.The contract body uses
U$notation (Line 23: "U$6.00 per video generation"), which is unconventional. Typically, USD is denoted as$orUSD. While the clause prices are correctly set as numeric values (6, 7, 4), the body text could be clearer for users.Consider updating Line 23 to use standard currency notation:
-body = "U$6.00 per video generation, U$7.00 per image-to-video, U$4.00 per video extension" +body = "$6.00 per video generation, $7.00 per image-to-video, $4.00 per video extension"veo/server/tools/veo.ts (1)
31-31: Consider making the model configurable.The model is hardcoded to
"veo-3.1-generate-preview". If multiple Veo models are available or if model selection needs to vary, consider making this configurable through input parameters or environment variables.shared/video-generators/middleware.ts (2)
38-40: Preserve original error context when retries are exhausted
Wrapping the final failure in a newErrorloses the original stack trace, which makes debugging production incidents harder. Re-throw the capturedlastErrorinstead of instantiating a new one (and fall back to a generic error only iflastErroris somehow undefined).Apply this diff:
- throw new Error( - `Failed after ${maxRetries} attempts: ${lastError?.message}`, - ); + if (lastError) { + throw lastError; + } + + throw new Error(`Failed after ${maxRetries} attempts`);
70-78: Clear the timeout once the wrapped call resolves
Promise.raceleaves the scheduled timer in place even afterfn()settles, so the timeout still fires later and triggers a no-op rejection. Clearing the timer whenfn()resolves or rejects avoids unnecessary work (and keeps the code friendlier to environments that flag late rejections).Apply this diff:
- const timeoutPromise = new Promise<TOutput>((_, reject) => - setTimeout( - () => reject(new Error(`Timeout after ${timeoutMs}ms`)), - timeoutMs, - ), - ); - - return Promise.race([fn(), timeoutPromise]); + return new Promise<TOutput>((resolve, reject) => { + const timeoutId = setTimeout(() => { + reject(new Error(`Timeout after ${timeoutMs}ms`)); + }, timeoutMs); + + fn() + .then((result) => { + clearTimeout(timeoutId); + resolve(result); + }) + .catch((error) => { + clearTimeout(timeoutId); + reject(error); + }); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
nanobanana/wrangler.toml(0 hunks)package.json(1 hunks)shared/package.json(1 hunks)shared/video-generators/README.md(1 hunks)shared/video-generators/base.ts(1 hunks)shared/video-generators/index.ts(1 hunks)shared/video-generators/middleware.ts(1 hunks)shared/video-generators/storage.ts(1 hunks)veo/.gitignore(1 hunks)veo/package.json(1 hunks)veo/server/constants.ts(1 hunks)veo/server/main.ts(1 hunks)veo/server/tools/index.ts(1 hunks)veo/server/tools/utils/veo.ts(1 hunks)veo/server/tools/veo.ts(1 hunks)veo/shared/deco.gen.ts(1 hunks)veo/tsconfig.json(1 hunks)veo/vite.config.ts(1 hunks)veo/wrangler.toml(1 hunks)
💤 Files with no reviewable changes (1)
- nanobanana/wrangler.toml
🧰 Additional context used
🧬 Code graph analysis (8)
veo/vite.config.ts (1)
shared/deco-vite-plugin.ts (1)
deco(89-146)
veo/server/main.ts (1)
veo/shared/deco.gen.ts (3)
Env(563-644)StateSchema(550-561)Scopes(646-660)
shared/video-generators/base.ts (3)
shared/storage/interface.ts (1)
ObjectStorage(1-43)shared/video-generators/middleware.ts (6)
Contract(97-106)ContractClause(92-95)applyMiddlewares(82-90)withLogging(45-65)withRetry(3-43)withTimeout(67-80)shared/video-generators/storage.ts (1)
saveVideo(18-64)
shared/video-generators/storage.ts (1)
shared/storage/interface.ts (1)
ObjectStorage(1-43)
veo/server/tools/index.ts (1)
veo/server/tools/veo.ts (1)
veoTools(6-76)
veo/server/tools/veo.ts (4)
shared/video-generators/base.ts (1)
createVideoGeneratorTools(135-214)veo/server/main.ts (1)
Env(22-27)shared/storage/adapters/filesystem.ts (1)
adaptFileSystemBindingToObjectStorage(98-102)veo/server/tools/utils/veo.ts (1)
createVeoClient(657-734)
veo/shared/deco.gen.ts (1)
veo/server/main.ts (1)
Env(22-27)
veo/server/tools/utils/veo.ts (2)
veo/server/main.ts (1)
Env(22-27)veo/server/constants.ts (1)
GEMINI_API_BASE_URL(2-3)
🪛 Biome (2.1.2)
veo/shared/deco.gen.ts
[error] 23-23: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 199-199: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 359-359: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 507-507: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (27)
veo/.gitignore (1)
1-1: LGTM!Correctly excludes
.dev.varsfrom version control, which is standard practice for Cloudflare Workers to prevent sensitive environment variables from being committed.shared/video-generators/README.md (1)
1-308: LGTM! Excellent documentation.The README provides comprehensive coverage of the video-generators module with clear examples, schema documentation, and usage patterns. The inclusion of streaming support documentation and complete working examples will help users understand and adopt the library effectively.
shared/video-generators/storage.ts (1)
3-11: LGTM! Well-defined interface with streaming support.The
SaveVideoOptionsinterface properly supports multiple data formats includingReadableStream, which enables memory-efficient video uploads as documented in the README.veo/shared/deco.gen.ts (1)
1-660: Static analysis hints on empty interfaces in generated code.The file contains several empty interfaces flagged by Biome:
- Line 23:
FS_DELETEOutput- Line 108:
FS_READ_METADATAOutput- Line 199:
CONTRACT_GETInput- Line 359:
DECO_CHAT_VIEWS_LISTInput- Line 507:
GET_USERInputSince this is auto-generated from JSON Schema, consider updating the source schemas to use
typealiases instead of empty interfaces if these truly have no properties, or add at least one property if the empty state is unintentional.package.json (1)
26-27: LGTM!Correctly adds the new
veoworkspace to the monorepo configuration.shared/package.json (1)
12-12: LGTM!Correctly exposes the new video-generators module following the established export pattern.
shared/video-generators/index.ts (1)
1-3: LGTM!Clean barrel export that consolidates the public API surface for the video-generators module.
veo/server/tools/index.ts (1)
1-16: LGTM! Well-structured tool aggregation.The module provides both a unified
toolsexport and domain-specific re-exports, offering flexibility for different import patterns. The clear documentation helps maintainers understand the purpose and design.veo/server/constants.ts (1)
9-10: Polling configuration looks reasonable.The 10-second polling interval and 6-minute maximum wait time are appropriate for long-running video generation operations.
veo/vite.config.ts (1)
1-29: LGTM! Well-configured Vite setup for Cloudflare Workers.The configuration properly integrates Cloudflare and Deco plugins, handles environment variables correctly, and sets up appropriate caching. The
globaltoglobalThismapping ensures compatibility across different runtime environments.veo/tsconfig.json (1)
1-42: LGTM! Excellent TypeScript configuration.The configuration enables strict type checking, includes appropriate linting rules, and properly sets up path aliases for the project structure. The inclusion of Cloudflare Workers types ensures proper typing for the runtime environment.
veo/wrangler.toml (1)
4-4: Verify compatibility date is intentional.The compatibility date is set to
2025-06-17, which is in the future relative to the PR creation date (2025-11-11). Ensure this is intentional or if it should be adjusted.veo/server/main.ts (1)
1-72: LGTM! Well-structured server entry point.The main.ts file properly configures the Cloudflare Workers runtime with appropriate OAuth scopes, state schema, and tools integration. The Env type correctly extends the base types, and the fallback to ASSETS.fetch provides a clean routing strategy. The extensive documentation comments are helpful for maintainability.
veo/package.json (2)
13-13: Verify Bun availability in deployment pipeline.The build script uses
bun --bun vite build, which requires Bun to be installed. Ensure your CI/CD pipeline and deployment environment have Bun available, or developers may encounter build failures.Consider adding a note in the project README about the Bun requirement, or provide an alternative npm/node-based build script for environments without Bun.
31-33: Node 22 requirement is very recent.The engine requirement specifies Node >= 22.0.0, which is quite new. Node 20 is the current LTS version. Ensure your deployment environment and developer machines support Node 22.
If Node 22 features are not strictly required, consider lowering the requirement to Node 20 for broader compatibility:
- "node": ">=22.0.0" + "node": ">=20.0.0"veo/server/tools/veo.ts (2)
6-18: LGTM! Tool configuration is well-structured.The veoTools are properly configured with storage adapter, contract bindings, and appropriate metadata. The integration with the shared video-generators framework is clean and follows the established pattern.
51-65: Good error handling for operation completion.The code properly validates operation completion and checks for generated samples before proceeding. The error messages are descriptive and will help with debugging.
shared/video-generators/base.ts (4)
14-31: LGTM! Well-defined types and schemas.The type definitions and Zod schemas provide strong type safety for video generation parameters. The VideoGeneratorStorage type appropriately limits the ObjectStorage surface to only what's needed.
33-111: LGTM! Comprehensive input/output schemas.The schemas provide excellent coverage of video generation parameters with clear descriptions and appropriate constraints. The discriminated union for success/error handling is type-safe and idiomatic.
113-134: LGTM! Well-defined options interface and reasonable constants.The CreateVideoGeneratorOptions interface provides a clean contract for video generator implementations. The retry count (3) and timeout (6 minutes) are appropriate for long-running video generation operations.
197-210: LGTM! Well-structured middleware application.The middleware stack (logging, retry, timeout) is properly applied and provides good operational visibility and resilience. The order of middlewares is appropriate with logging outermost, then retry, then timeout.
shared/video-generators/middleware.ts (3)
45-65: Nice timing instrumentation
The start/end logging with duration captures success and failure paths cleanly.
86-89: Middleware composition looks good
Left-to-right reduction keeps the wrapping order intuitive and handles the empty-array case cleanly.
92-106: Contract typing aligns with the new workflow
The explicit clause + authorize/settle signatures give downstream callers a solid contract surface.veo/server/tools/utils/veo.ts (3)
19-68: Capability map is comprehensive
Thanks for encoding the per-model feature flags and defaults up front—it keeps downstream logic simple and self-documenting.
253-292: API helper handles error surfaces well
Appreciate the structured error unwrap so callers see the Gemini message without losing status context.
628-652: Polling loop is well-behaved
The bounded wait with interval sleep and early exit on error keeps things predictable.
| const doExecute = async () => { | ||
| const contract = options.getContract(env); | ||
|
|
||
| const { transactionId } = await contract.binding.CONTRACT_AUTHORIZE({ | ||
| clauses: [ | ||
| { | ||
| clauseId: contract.clause.clauseId, | ||
| amount: contract.clause.amount, | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await options.execute({ env, input: context }); | ||
|
|
||
| if ("error" in result) { | ||
| return { | ||
| error: true, | ||
| finishReason: result.finishReason, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify contract authorization cleanup on execution failure.
The contract is authorized (Line 150-157) before executing the video generation (Line 159). If execution fails and returns an error (Lines 161-166), the authorization is never settled. This could leave authorized charges in a pending state.
Verify that there is a mechanism to clean up or expire unused authorizations. If not, consider:
- Moving CONTRACT_AUTHORIZE after successful execution
- Adding a try/catch to settle or cancel the authorization on failure
- Implementing an authorization timeout/expiry mechanism
🏁 Script executed:
#!/bin/bash
# Search for CONTRACT_AUTHORIZE cleanup or cancellation logic
rg -n -C5 "CONTRACT_AUTHORIZE|CONTRACT_CANCEL|CONTRACT_EXPIRE" --type=tsLength of output: 9729
is not applicable here as we need file content
🏁 Script executed:
#!/bin/bash
# Get the complete doExecute function including error handling and settlement logic
cat shared/video-generators/base.ts | sed -n '147,214p'Length of output: 1900
🏁 Script executed:
#!/bin/bash
# Also check the image generator for comparison
cat shared/image-generators/base.ts | sed -n '122,185p'Length of output: 1709
🏁 Script executed:
#!/bin/bash
# Search for CONTRACT_SETTLE usage in context
rg -n -B2 -A10 "CONTRACT_SETTLE" --type=ts shared/video-generators/base.ts shared/image-generators/base.tsLength of output: 1590
🏁 Script executed:
#!/bin/bash
# Check if there are try/finally blocks around CONTRACT_AUTHORIZE
rg -n -B5 -A25 "try\s*{" --type=ts shared/video-generators/base.tsLength of output: 77
Contract authorization not settled on execution failure—add try/finally to ensure cleanup.
The contract is authorized (lines 150–157) before execution (line 159). If execution fails and returns early (lines 161–166), CONTRACT_SETTLE is never called. This leaves authorized charges in a pending state. The retry middleware would create a new authorization on each attempt, compounding the problem.
Move the settlement call into a finally block or explicitly settle failed transactions:
const doExecute = async () => {
const contract = options.getContract(env);
const { transactionId } = await contract.binding.CONTRACT_AUTHORIZE({
clauses: [{ clauseId: contract.clause.clauseId, amount: contract.clause.amount }],
});
try {
const result = await options.execute({ env, input: context });
if ("error" in result) {
return { error: true, finishReason: result.finishReason };
}
// ... save and settle on success
} finally {
// Ensure settlement or cancellation happens
}
};Same pattern exists in shared/image-generators/base.ts.
🤖 Prompt for AI Agents
In shared/video-generators/base.ts around lines 147–166 the code AUTHORIZEs a
contract then calls options.execute but returns early on execution errors so
CONTRACT_SETTLE is never invoked; wrap the execute and subsequent
success-settlement logic in a try/finally where the finally always calls
CONTRACT_SETTLE (or an explicit cancellation/settlement API) using the captured
transactionId, and move any early-return checks inside the try so the finally
still runs; apply the same try/finally fix to the analogous flow in
shared/image-generators/base.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is okay. pre-authorized amounts that were not settled should expire in some minutes and return to the main user account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viktormarinho, thank you for clarifying! I understand now—the pre-authorization expiry mechanism handles cleanup automatically, so there's no risk of charges being stuck in a pending state indefinitely. That makes sense for this design.
✏️ Learnings added
Learnt from: viktormarinho
Repo: decocms/mcps PR: 5
File: shared/video-generators/base.ts:147-166
Timestamp: 2025-11-11T14:23:55.020Z
Learning: In the contract authorization system, pre-authorized amounts that are not settled via CONTRACT_SETTLE will automatically expire within a few minutes and return to the user's main account. This means it's acceptable for CONTRACT_AUTHORIZE to be called without a corresponding CONTRACT_SETTLE in error scenarios, as the system handles cleanup automatically.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| export const GEMINI_API_BASE_URL = | ||
| "https://generativelanguage.googleapis.com/v1beta"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify production readiness of beta API endpoint.
The hardcoded API URL points to Google's v1beta endpoint. Ensure this is intentional and that the beta API is suitable for production use.
🌐 Web query:
Is Google Gemini Veo v1beta API stable for production use?
💡 Result:
Short answer: No — v1beta is a beta channel and not recommended as “stable” for production. Google documents state v1 is the stable API channel and v1beta contains features under development, and community reports show intermittent errors, model changes, and deprecations when using v1beta models. [1][2]
Recommendation: use the v1 (stable) API/models for production where possible, or confirm support/SLA with Google (and test for your expected load, failover and model‑alias changes) before deploying v1beta in production. [1][2]
Sources:
- Google Gemini API release notes (v1 = stable, v1beta = beta). [1]
- Community reports of errors, model removals and instability when using v1beta models. [2]
Migrate to stable Google Gemini API endpoint.
The code uses Google's v1beta endpoint, which is not recommended for production. v1beta is a beta channel containing experimental features, and community reports show intermittent errors, model changes, and deprecations. The stable v1 API channel should be used for production instead. Update the endpoint to https://generativelanguage.googleapis.com/v1 and test with your expected load before deploying.
🤖 Prompt for AI Agents
In veo/server/constants.ts around lines 2 to 3, update the Gemini API base URL
from the beta endpoint to the stable v1 endpoint by replacing
"https://generativelanguage.googleapis.com/v1beta" with
"https://generativelanguage.googleapis.com/v1"; ensure any code that depends on
this constant still works (update tests or mocks if they assert the old URL) and
run integration/load tests against the new endpoint before deploying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plz test if we can use the stable endpoint @viniciusventura29 . If not we can keep this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| .union([z.literal(4), z.literal(6), z.literal(8)]) | ||
| .optional() | ||
| .describe("Video duration in seconds"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow all documented durations in the request schema
veo-2.0 models expose 5 s and 7 s durations, but the schema currently rejects them. Any request that honors the model’s advertised options will fail validation. Please widen the union to cover the full set of durations.
Apply this diff:
- duration: z
- .union([z.literal(4), z.literal(6), z.literal(8)])
+ duration: z
+ .union([
+ z.literal(4),
+ z.literal(5),
+ z.literal(6),
+ z.literal(7),
+ z.literal(8),
+ ])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In veo/server/tools/utils/veo.ts around lines 97 to 99, the zod union for
allowed video durations only includes 4, 6, and 8 and therefore rejects
documented 5s and 7s options; update the union to include z.literal(5) and
z.literal(7) so the schema accepts 4, 5, 6, 7, and 8 seconds (keeping
.optional() and .describe unchanged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also check if this is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viktormarinho Yes, this is true! I've verified the claim by examining the code:
The Issue:
- The
VideoGenerationRequestSchemaduration field (lines 97-99) only accepts4,6, or8seconds - However,
MODEL_CAPABILITIESshows thatveo-2.0models support[5, 6, 7, 8]seconds
Evidence from the code:
// veo-2.0 models in MODEL_CAPABILITIES:
"veo-2.0-generate-001": {
durationOptions: [5, 6, 7, 8] as const, // ← includes 5 and 7
...
},
"veo-2.0-generate-exp": {
durationOptions: [5, 6, 7, 8] as const, // ← includes 5 and 7
...
}The Problem:
If a user tries to generate a video using veo-2.0-generate-001 with a duration of 5 or 7 seconds (which are valid for that model), the Zod schema will reject it before the request even reaches the API.
Recommended Fix:
Update the schema to accept all possible durations across all models:
duration: z
.union([
z.literal(4),
z.literal(5), // Add for veo-2.0
z.literal(6),
z.literal(7), // Add for veo-2.0
z.literal(8),
])
.optional()
.describe("Video duration in seconds")This way, the schema accepts all valid durations, and model-specific validation can happen later in the execution logic if needed.
| `[generateVideo] Request body:`, | ||
| JSON.stringify(requestBody, null, 2), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop logging the entire request payload
Serializing the full request body leaks base64-encoded image data and other user-provided content into logs, which is a sizable privacy/compliance risk and bloats telemetry. Please either remove the log or redact the heavy fields before printing.
Apply this diff:
- console.log(
- `[generateVideo] Request body:`,
- JSON.stringify(requestBody, null, 2),
- );
+ console.log(`[generateVideo] Dispatching request for model ${model}`);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In veo/server/tools/utils/veo.ts around lines 413–415, the code currently logs
the entire requestBody via JSON.stringify which leaks base64 images and user
content; remove the full payload log or replace it with a redaction that omits
or masks heavy fields (e.g., image, file, data, base64, or any keys matching
/image|file|base64|blob/i) and instead log only safe metadata (field names,
sizes, types, or a "[REDACTED]" placeholder). Update the logging call to build a
sanitized object (copy allowed fields and set heavy fields to "[REDACTED]" or
include their length) and log that instead of JSON.stringify(requestBody).
- Changed the package description from "Minimal template for MCP server" to "Veo 3 MCP" for better clarity. - Updated the clauseId format in veo.ts from "veo3:generateVideo" to "veo-3:generateVideo" for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
veo/server/tools/veo.ts (1)
44-48: Import and use constants instead of magic numbers.The polling parameters
360000and10000should be imported fromveo/server/constants.tsasOPERATION_MAX_WAIT_MSandOPERATION_POLL_INTERVAL_MSto centralize configuration and avoid maintenance issues.Apply this diff to use the imported constants:
+import { + OPERATION_MAX_WAIT_MS, + OPERATION_POLL_INTERVAL_MS, +} from "../constants"; + export const veoTools = createVideoGeneratorTools<Env>({ // ... existing code ... execute: async ({ env, input }) => { // ... existing code ... const completed = await client.pollOperationUntilComplete( operation.name, - 360000, - 10000, + OPERATION_MAX_WAIT_MS, + OPERATION_POLL_INTERVAL_MS, );
🧹 Nitpick comments (1)
veo/server/tools/veo.ts (1)
29-41: Video generation call is properly configured.All input parameters are correctly passed through to the Veo API. The hardcoded model
"veo-3.1-generate-preview"appears intentional for this tool.If multiple models should be supported in the future, consider adding a model parameter to the input schema or configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
veo/package.json(1 hunks)veo/server/tools/veo.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
veo/server/tools/veo.ts (5)
shared/video-generators/base.ts (1)
createVideoGeneratorTools(135-214)veo/server/main.ts (1)
Env(22-27)veo/shared/deco.gen.ts (1)
Env(563-644)shared/storage/adapters/filesystem.ts (1)
adaptFileSystemBindingToObjectStorage(98-102)veo/server/tools/utils/veo.ts (1)
createVeoClient(657-734)
🔇 Additional comments (10)
veo/package.json (5)
2-6: LGTM!Package metadata is well-structured and consistent with the workspace setup.
13-13: Verify Bun runtime lock in CI/CD environments.The
buildscript usesbun --bunwhich explicitly locks the Bun runtime. This may cause failures in environments without Bun installed or in CI/CD pipelines that use npm/yarn. Ensure this is intentional and that all build environments (local, CI/CD) have Bun available and configured.Consider whether the runtime lock is necessary or if the script should be:
vite build(lets the task runner decide)bun vite build(prefers Bun but doesn't lock it)- Or documented with a Bun installation requirement
15-18: LGTM!Production dependencies are appropriately versioned. Runtime is pinned for stability; zod uses caret for flexibility with security updates.
19-30: Verify availability of scoped packages and unusual version format.Two concerns:
Line 21:
@cloudflare/workers-typesversion^4.20251014.0uses an unusual calver-style format (date + patch). Verify this version exists and is the intended release.Lines 22, 26:
@decocms/mcps-sharedanddeco-cliare scoped packages. Ensure these are available in the project's package registry and correctly published at the specified versions (1.0.0 and ^0.23.3).Minor observation:
vite(line 28) is pinned to7.2.0without a caret, while other devDependencies use caret ranges. This is stricter but acceptable. Verify this constraint doesn't conflict with other transitive dependencies.
31-33: Verify Node.js engine consistency across workspace.The package requires
node >= 22.0.0. Ensure this is consistent with the rootpackage.jsonand other workspace packages to avoid tooling mismatches.veo/server/tools/veo.ts (5)
1-4: LGTM! Imports are well-organized.All necessary dependencies are correctly imported from the appropriate modules.
6-18: Configuration setup is correct.The metadata, storage adapter, and contract binding are properly configured according to the framework pattern.
19-26: Good defensive handling of aspect ratio.The normalization logic correctly handles Veo's constraints by defaulting unsupported aspect ratios to 16:9.
51-65: Error handling is well-structured.The code properly validates operation completion and video generation with specific error reasons for each failure mode.
67-74: Video download and return logic is correct.The code appropriately downloads the first generated video and returns it with proper metadata including mime type and operation name.
- Updated the polling mechanism in veo.ts to use defined constants for maximum wait time and poll interval, improving code readability and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
veo/server/tools/veo.ts (1)
35-35: Consider extracting model identifier to a constant.The hard-coded
"veo-3.1-generate-preview"string could be extracted to a named constant inveo/server/constants.tsfor easier maintenance and discoverability.Apply this diff to extract the constant:
In
veo/server/constants.ts, add:export const VEO_DEFAULT_MODEL = "veo-3.1-generate-preview";Then update the import and usage:
import { OPERATION_MAX_WAIT_MS, OPERATION_POLL_INTERVAL_MS, + VEO_DEFAULT_MODEL, } from "../constants"; const operation = await client.generateVideo( input.prompt, - "veo-3.1-generate-preview", + VEO_DEFAULT_MODEL, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
veo/server/tools/veo.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
veo/server/tools/veo.ts (4)
shared/video-generators/base.ts (1)
createVideoGeneratorTools(135-214)veo/server/main.ts (1)
Env(22-27)shared/storage/adapters/filesystem.ts (1)
adaptFileSystemBindingToObjectStorage(98-102)veo/server/constants.ts (2)
OPERATION_MAX_WAIT_MS(10-10)OPERATION_POLL_INTERVAL_MS(9-9)
🔇 Additional comments (3)
veo/server/tools/veo.ts (3)
5-8: Previous issue resolved: constants now imported correctly.The magic numbers previously flagged in the past review have been addressed. The constants are now properly imported and used on lines 50-51.
26-30: Aspect ratio normalization handled appropriately.The silent conversion to 16:9 for unsupported aspect ratios is a reasonable approach given the API's constraints. The comment clearly documents the limitation.
71-78: The review comment is incorrect—the code is type-safe as written.Based on the schema definitions in
veo/server/tools/utils/veo.ts:
VideoFileSchemadefinesvideoas a required property (line 130 inGeneratedVideoSchema, and line 149 in the response schema)- Each element in
generatedSampleshas a requiredvideoproperty of typeVideoFileSchema- The length check on line 64 (
!generatedSamples || generatedSamples.length === 0) ensuresgeneratedSamples[0]exists- Therefore,
video.videois guaranteed to exist by both TypeScript's type system and Zod's runtime validationvideo.video.uriis required per the schema;video.video.mimeTypeis optional but has a fallbackThe nested property access on lines 72 and 76 will not throw a
TypeError.Likely an incorrect or invalid review comment.
shared/video-generators/README.md
Outdated
| execute: async ({ env, input }) => { | ||
| // Implement video generation logic | ||
| // This function should return the generated video | ||
| const videoBlob = await generateVideoSomehow(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be outdated with the code
| const doExecute = async () => { | ||
| const contract = options.getContract(env); | ||
|
|
||
| const { transactionId } = await contract.binding.CONTRACT_AUTHORIZE({ | ||
| clauses: [ | ||
| { | ||
| clauseId: contract.clause.clauseId, | ||
| amount: contract.clause.amount, | ||
| }, | ||
| ], | ||
| }); | ||
|
|
||
| const result = await options.execute({ env, input: context }); | ||
|
|
||
| if ("error" in result) { | ||
| return { | ||
| error: true, | ||
| finishReason: result.finishReason, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is okay. pre-authorized amounts that were not settled should expire in some minutes and return to the main user account.
| export const GEMINI_API_BASE_URL = | ||
| "https://generativelanguage.googleapis.com/v1beta"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plz test if we can use the stable endpoint @viniciusventura29 . If not we can keep this one
| .union([z.literal(4), z.literal(6), z.literal(8)]) | ||
| .optional() | ||
| .describe("Video duration in seconds"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also check if this is true
… and model capabilities - Enhanced README.md to reflect the new streaming-first design, emphasizing memory efficiency and handling of large videos. - Added detailed sections on supported aspect ratios, durations, and model capabilities, including a comparison table for Veo models. - Clarified the usage of polling for long-running operations and provided examples for better understanding. - Improved overall structure and clarity of the documentation to assist users in utilizing the video generation tools effectively.
Summary by CodeRabbit
New Features
Documentation
Public API
Chores