Vercel Sandbox Integration#640
Conversation
- Add @vercel/sandbox dependency and types - Implement `sandbox` tool with support for JS/TS and live previews - Create UI components for streaming logs and iframe previews - Update researcher agent with code execution capabilities - Handle tool result reconstruction in message history Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Warning Review limit reached
More reviews will be available in 33 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughAdds code execution capability via Vercel's microVM sandbox. The feature includes validation schemas, a sandbox tool that executes JavaScript/TypeScript code in isolation with dependency management, agent routing that guides when to use the tool, UI components for displaying logs and live preview, and integration into the action layer that streams results to the frontend. ChangesSandbox Code Execution Integration
Sequence DiagramsequenceDiagram
participant Agent
participant SandboxTool
participant Vercel as Vercel Sandbox
participant UIStream
Agent->>SandboxTool: execute(code, language, filename, dependencies)
SandboxTool->>Vercel: write file to sandbox
SandboxTool->>Vercel: npm install (if needed)
SandboxTool->>Vercel: spawn tsx/node command
Vercel->>SandboxTool: stdout/stderr streams
SandboxTool->>UIStream: append logs
alt server-like code
SandboxTool->>Vercel: get domain for port 3000
SandboxTool->>UIStream: update preview URL
else script
SandboxTool->>Vercel: wait for exit
SandboxTool->>Vercel: stop sandbox
end
SandboxTool->>Agent: return sandbox_result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Review Summary by QodoIntegrate Vercel Sandbox for code execution and live previews
WalkthroughsDescription• Integrates Vercel Sandbox for isolated JavaScript/TypeScript code execution • Adds real-time log streaming and live preview capabilities for web servers • Creates UI components for terminal-like log viewer and iframe previews • Updates researcher agent to support code execution and data transformation • Adds sandbox tool configuration and environment variables Diagramflowchart LR
A["Researcher Agent"] -->|uses sandbox tool| B["Sandbox Tool"]
B -->|executes code| C["Vercel Sandbox MicroVM"]
C -->|streams logs| D["SandboxSection Component"]
D -->|displays| E["SandboxLogs"]
C -->|detects server| F["SandboxPreview Component"]
F -->|renders| G["Live Preview iframe"]
File Changes1. lib/schema/sandbox.ts
|
Code Review by Qodo
1. Npm install injection
|
| if (dependencies && dependencies.length > 0) { | ||
| const installCommand = await sandbox.runCommand(`npm install ${dependencies.join(' ')}`) | ||
| for await (const log of installCommand.logs()) { | ||
| const logEntry = { type: log.stream as 'stdout' | 'stderr', content: log.data } | ||
| logs.push(logEntry) | ||
| streamableLogs.update([...logs]) | ||
| } |
There was a problem hiding this comment.
1. Npm install injection 🐞 Bug ⛨ Security
sandboxTool builds an npm install ... command by string-concatenating dependencies, allowing command injection in the microVM (extra commands beyond npm). This breaks the tool’s security boundary and can be used to run arbitrary unintended commands during the install step.
Agent Prompt
### Issue description
`sandboxTool` constructs `npm install` as a single shell string using `dependencies.join(' ')`, which enables command injection if any dependency string includes shell metacharacters.
### Issue Context
The tool parameter schema allows arbitrary strings in `dependencies`, and the tool executes them inside the sandbox via `runCommand`.
### Fix Focus Areas
- Replace string command with structured args (no shell parsing).
- Validate/whitelist dependency names (e.g., npm package name grammar) and reject anything else.
### Fix Focus Areas (code locations)
- lib/agents/tools/sandbox.tsx[37-44]
- lib/schema/sandbox.ts[8-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const isServer = /http|express|serve|listen|app\.get|app\.post|createServer/.test(code) | ||
| const cmd = language === 'typescript' ? 'tsx' : 'node' | ||
|
|
||
| const command = await sandbox.runCommand({ | ||
| cmd, | ||
| args: [filename], | ||
| detached: isServer | ||
| }) | ||
|
|
||
| let previewUrl: string | undefined | ||
| if (isServer) { | ||
| previewUrl = await sandbox.domain(3000) | ||
| // Update UI with preview URL | ||
| uiStream.update( | ||
| <SandboxSection | ||
| key={nanoid()} | ||
| logs={streamableLogs.value} | ||
| previewUrl={previewUrl} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| const streamLogs = async () => { | ||
| for await (const log of command.logs()) { | ||
| const logEntry = { type: log.stream as 'stdout' | 'stderr', content: log.data } | ||
| logs.push(logEntry) | ||
| streamableLogs.update([...logs]) | ||
| } | ||
| streamableLogs.done(logs) | ||
| } | ||
|
|
||
| if (isServer) { | ||
| // For servers, stream logs in the background and return immediately | ||
| streamLogs() | ||
| } else { | ||
| // For regular scripts, wait for execution to complete | ||
| await streamLogs() | ||
| const result = await (command as any).wait() | ||
| await sandbox.stop() | ||
| return { | ||
| logs, | ||
| exitCode: result.exitCode, | ||
| type: 'sandbox_result' | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| logs, | ||
| exitCode: 0, | ||
| previewUrl, | ||
| type: 'sandbox_result' | ||
| } |
There was a problem hiding this comment.
2. Sandbox never stopped 🐞 Bug ☼ Reliability
When isServer is true, the tool detaches the process and returns without ever calling sandbox.stop(), leaving microVMs running and consuming resources. The /http|express|.../ heuristic is broad enough to misclassify non-server code, increasing the frequency of this leak.
Agent Prompt
### Issue description
In the server/detached execution path, the sandbox is never stopped, so microVMs can remain alive after the tool returns.
### Issue Context
`sandbox.stop()` is only called in the non-server branch. Server mode is triggered by a regex against the source code, which can produce false positives.
### Fix Focus Areas
- Ensure `sandbox.stop()` is invoked for server runs (e.g., via a `finally` block) or implement a controlled lifecycle/TTL shutdown strategy.
- Avoid heuristic server detection: add an explicit `mode`/`isServer` parameter or detect actual listening port readiness.
- Avoid unhandled background async failures (use `void streamLogs().catch(...)`).
### Fix Focus Areas (code locations)
- lib/agents/tools/sandbox.tsx[46-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const isServer = /http|express|serve|listen|app\.get|app\.post|createServer/.test(code) | ||
| const cmd = language === 'typescript' ? 'tsx' : 'node' | ||
|
|
||
| const command = await sandbox.runCommand({ | ||
| cmd, | ||
| args: [filename], | ||
| detached: isServer | ||
| }) |
There was a problem hiding this comment.
3. Typescript runner missing 🐞 Bug ≡ Correctness
TypeScript execution uses cmd = 'tsx' but the tool never installs tsx, so a TypeScript run will fail unless the caller knows to include tsx in dependencies. This makes the advertised TypeScript support unreliable by default.
Agent Prompt
### Issue description
The tool selects `tsx` as the command for TypeScript, but does not guarantee `tsx` exists in the sandbox environment.
### Issue Context
The tool only runs `npm install` when `dependencies` are provided, and it does not add any implicit dependencies based on the selected `language`.
### Fix Focus Areas
- If `language === 'typescript'`, install `tsx` (and typically `typescript`/`@types/node`) automatically before execution.
- Or run through `npx`/a pinned package runner with explicit args.
### Fix Focus Areas (code locations)
- lib/agents/tools/sandbox.tsx[37-53]
- lib/schema/sandbox.ts[8-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Can you synchronize this branch to the main branch. |
I have merged the latest changes from the main branch into this feature branch. I also resolved a conflict in |
- Merged latest changes from main branch - Resolved bun.lock conflict - Fixed type error in Map3D component center prop - Verified build passes Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/sandbox-preview.tsx`:
- Around line 25-31: The preview controls and iframe lack accessible names; add
explicit accessible labels to the icon-only controls and the iframe: give the
reload Button (onClick={reload}) an aria-label="Reload preview" (or wrap the
icon with a visually-hidden <span> for screen readers), give the external link
anchor or the Button with asChild an aria-label="Open preview in new tab" (or
include hidden text), and add a descriptive title and/or aria-label to the
iframe (e.g., title="Live preview of user sandbox" and aria-label="Sandbox
preview iframe") so assistive tech can identify each control and the frame;
update the elements referenced (the Button with onClick={reload}, the Button
asChild wrapping <a href={url} ...>, and the iframe element) accordingly.
In `@lib/agents/researcher.tsx`:
- Around line 50-55: Update the sandbox tool description (the "sandbox" entry in
lib/agents/researcher.tsx) to explicitly document network-access security
implications: state that user code can make arbitrary HTTP requests
(SSRF/exfiltration risk), potential to reach internal/localhost services or
bypass IP allowlists, and recommend mitigations such as running sandboxes in an
isolated network namespace, applying egress filtering, and clearly defining
allowed/forbidden endpoints; include a brief guidance sentence for operators on
when to enable the feature and where to configure these controls.
In `@lib/agents/tools/sandbox.tsx`:
- Around line 26-30: The Sandbox.create call currently hardcodes ports [3000]
and timeout 60000; update this to accept configurable values instead: add
optional parameters (e.g., ports and timeout) to the function or config schema
that lead to Sandbox.create({ runtime: 'node24', ports: providedPorts || [3000],
timeout: providedTimeout || 60000 }), and ensure callers can pass multiple ports
(array) and a numeric timeout; reference the Sandbox.create invocation and the
sandbox variable so you change the invocation site and propagate the new
optional parameters through the calling code and schema.
- Line 46: The current fragile detection using the isServer regex in sandbox.tsx
can miss frameworks and hit false-positives; add an optional isServer: boolean
to sandboxSchema and prefer that explicit flag in the code that computes
isServer (use the schema value when present), and as a fallback expand detection
patterns to include Fastify/Koa/Hapi/Socket.io/ws and strip comments/strings
before regex testing (or use a simple AST/token scan) so function/method name
references like isServer and the server detection logic in sandbox.tsx reliably
reflect intent.
- Around line 77-97: When isServer is true, avoid leaking and misreporting by:
(1) running streamLogs() with error handling (call streamLogs().catch(err => {
/* log and stop sandbox */ })) so errors aren’t swallowed; (2) ensure
sandbox.stop() is called on any streamLogs error and register the sandbox with a
lifecycle manager (e.g., add to an activeSandboxes registry with a stop() API
and/or TTL) so server-mode sandboxes can be cleaned up; (3) don’t return a live
reference to logs—return a shallow copy (e.g., [...logs]) to avoid race
conditions; and (4) don’t return exitCode: 0 immediately — either wait a short
startup period or return exitCode: undefined (or a "running" state) so callers
know the server may still fail; update the code paths around isServer,
streamLogs, sandbox.stop, logs and previewUrl accordingly.
- Line 13: Change the execute function signature to use the concrete
SandboxParameters type instead of any: import the SandboxParameters type from
the schema and update execute: async (params: SandboxParameters) => { ... }
(then destructure const { code, language, filename, dependencies } = params).
Update any other references to the execute parameter shape to use
SandboxParameters so the function and callers get full type safety.
In `@lib/schema/sandbox.ts`:
- Around line 13-15: The filename schema (the z.string() chain for filename in
lib/schema/sandbox.ts) currently accepts arbitrary strings; update it to
validate that values contain no path separators or navigational components by
adding a refinement or regex check (reject '/' and '\' and any '..' segments) —
e.g., use z.string().refine(...) or z.string().regex(...) to enforce that the
input equals its basename and does not include ".." or path separators, and
provide a clear error message for invalid filenames.
- Around line 16-19: The dependencies schema currently allows any string
(dependencies: z.array(z.string()).optional()), so add validation that each
entry matches npm package naming rules: update the array element from z.string()
to either z.string().regex(/.../) with a regex implementing npm name constraints
or z.string().refine(v => validateNpmPackageName(v), { message: 'invalid npm
package name' }) and include a clear error message; keep the .optional() and
.describe(...) and reference the symbol name "dependencies" in
lib/schema/sandbox.ts when making this change.
- Line 30: PartialSandboxParameters is incorrectly defined using
DeepPartial<typeof sandboxSchema>; DeepPartial expects a TypeScript type, not a
Zod schema. Replace DeepPartial<typeof sandboxSchema> with
DeepPartial<SandboxParameters> (the TS type corresponding to sandboxSchema) and
ensure SandboxParameters is imported or defined in the file so
PartialSandboxParameters becomes DeepPartial<SandboxParameters>.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dac3a1ff-a3d3-4ca1-85e6-64ac57eeff1e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.env.local.exampleapp/actions.tsxcomponents/sandbox-logs.tsxcomponents/sandbox-preview.tsxcomponents/sandbox-section.tsxcomponents/section.tsxlib/agents/researcher.tsxlib/agents/tools/index.tsxlib/agents/tools/sandbox.tsxlib/schema/sandbox.tslib/types/index.tspackage.json
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-06-07T16:19:00.111Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-06-07T16:19:00.111Z
Learning: In the QCX repository, `package.json` must keep all `ai-sdk/*` provider packages at the `^1.x` series (e.g., `ai-sdk/openai@^1.3.22`, `ai-sdk/anthropic@^1.1.6`, `ai-sdk/google@^1.0.27`, `ai-sdk/xai@^1.2.18`, `ai-sdk/amazon-bedrock@^2.2.10`) to remain compatible with the pinned `ai4.3.19` core. Upgrading providers to `^3.x` or `^4.x` causes `AI_UnsupportedModelVersionError` because those versions implement model spec "v2" which AI SDK 4 does not support.
Applied to files:
package.json
📚 Learning: 2026-02-08T16:50:33.791Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-02-08T16:50:33.791Z
Learning: The PR branch implements RAG (Retrieval-Augmented Generation) via `lib/actions/rag.ts` which uses Supabase RPC functions `generate_embedding()` and `hybrid_search()` to retrieve relevant context from past conversations. However, there's a bug on line 433 of `app/actions.tsx` where `currentSystemPrompt` is passed to the researcher instead of `augmentedSystemPrompt`, preventing the retrieved context from being used.
Applied to files:
lib/agents/researcher.tsx
🪛 dotenv-linter (4.0.0)
.env.local.example
[warning] 36-36: [UnorderedKey] The VERCEL_PROJECT_ID key should go before the VERCEL_TEAM_ID key
(UnorderedKey)
🔇 Additional comments (12)
components/sandbox-logs.tsx (1)
16-50: LGTM!components/sandbox-section.tsx (1)
21-49: LGTM!components/section.tsx (1)
11-12: LGTM!Also applies to: 23-23, 54-56
app/actions.tsx (1)
30-30: LGTM!Also applies to: 901-916
.env.local.example (1)
32-37: LGTM!package.json (1)
55-55: Check@vercel/sandboxsecurity advisories and lockfile pin
- GitHub’s NPM ecosystem security-advisory lookup for
@vercel/sandboxreturns no findings.package.jsonuses^2.0.2, which already allows newer2.xversions; the “is 2.0.2 current?” concern only applies to what the lockfile actually resolves—confirm the pinned version inbun.lockand bump if you want the latest.lib/types/index.ts (1)
77-85: LGTM!lib/agents/tools/sandbox.tsx (2)
37-44: Potential npm install timeout for large dependency trees.Large or complex dependency installations (e.g.,
tensorflow,puppeteer) can take significantly longer than the 60-second sandbox timeout, especially on cold starts. This could lead to timeout errors before installation completes.Consider whether the 60-second timeout is sufficient for realistic dependency installations. If this becomes a problem in production, you may need to:
- Increase the timeout for dependency installation scenarios
- Pre-install common dependencies in the sandbox image
- Document limitations on which packages can be installed
83-83: Avoid(command as any).wait()unless@vercel/sandbox@2.0.2’sCommandtype includeswait().The concern is still valid: the cast bypasses type safety. However, the requested local verification can’t be completed here because
node_modules/@vercel/sandbox/dist/index.d.tsis missing in this environment, so the presence/absence of thewait()typing can’t be confirmed from the SDK types.lib/agents/researcher.tsx (1)
77-80: LGTM!lib/agents/tools/index.tsx (2)
43-46: LGTM!
42-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty string environment variables pass the truthiness check.
The condition
process.env.VERCEL_TOKEN && process.env.VERCEL_TEAM_ID && process.env.VERCEL_PROJECT_IDwill be truthy even if these variables are set to empty strings (e.g.,VERCEL_TOKEN=""). This could lead to runtime errors when the sandbox tries to authenticate.🔧 Proposed fix
- if (process.env.VERCEL_TOKEN && process.env.VERCEL_TEAM_ID && process.env.VERCEL_PROJECT_ID) { + if (process.env.VERCEL_TOKEN?.trim() && process.env.VERCEL_TEAM_ID?.trim() && process.env.VERCEL_PROJECT_ID?.trim()) { tools.sandbox = sandboxTool({ uiStream, fullResponse }) }> Likely an incorrect or invalid review comment.
| <Button variant="ghost" size="icon" className="h-7 w-7" onClick={reload}> | ||
| <RefreshCw className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| <Button variant="ghost" size="icon" className="h-7 w-7" asChild> | ||
| <a href={url} target="_blank" rel="noopener noreferrer"> | ||
| <ExternalLink className="h-3.5 w-3.5" /> | ||
| </a> |
There was a problem hiding this comment.
Add accessible labels to preview controls and iframe.
Line 25 and Line 28 render icon-only actions without accessible names, and Line 41 renders an unlabeled iframe. This makes the preview controls/frame hard to use with assistive tech.
Suggested fix
- <Button variant="ghost" size="icon" className="h-7 w-7" onClick={reload}>
+ <Button
+ variant="ghost"
+ size="icon"
+ className="h-7 w-7"
+ onClick={reload}
+ aria-label="Reload sandbox preview"
+ >
<RefreshCw className="h-3.5 w-3.5" />
</Button>
<Button variant="ghost" size="icon" className="h-7 w-7" asChild>
- <a href={url} target="_blank" rel="noopener noreferrer">
+ <a
+ href={url}
+ target="_blank"
+ rel="noopener noreferrer"
+ aria-label="Open sandbox preview in a new tab"
+ >
<ExternalLink className="h-3.5 w-3.5" />
</a>
</Button>
@@
<iframe
key={key}
src={url}
+ title="Sandbox live preview"
className="w-full h-full border-none"
sandbox="allow-scripts allow-same-origin allow-forms"
onLoad={() => setIsLoading(false)}
/>Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/sandbox-preview.tsx` around lines 25 - 31, The preview controls
and iframe lack accessible names; add explicit accessible labels to the
icon-only controls and the iframe: give the reload Button (onClick={reload}) an
aria-label="Reload preview" (or wrap the icon with a visually-hidden <span> for
screen readers), give the external link anchor or the Button with asChild an
aria-label="Open preview in new tab" (or include hidden text), and add a
descriptive title and/or aria-label to the iframe (e.g., title="Live preview of
user sandbox" and aria-label="Sandbox preview iframe") so assistive tech can
identify each control and the frame; update the elements referenced (the Button
with onClick={reload}, the Button asChild wrapping <a href={url} ...>, and the
iframe element) accordingly.
| #### **3. Code Execution and Data Transformation** | ||
| - **Tool**: \`sandbox\` | ||
| - **When to use**: | ||
| Use this to execute JavaScript or TypeScript code snippets, perform complex data transformations, generate dynamic HTML reports, or visualize data. The sandbox provides an isolated environment with network access and can host web servers for live previews. | ||
| - **Features**: Supports installing npm dependencies and provides a public preview URL if a web server (e.g., Express) is detected. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting network access security implications.
The prompt states the sandbox provides "network access," which enables powerful data-fetching use cases but also introduces SSRF (Server-Side Request Forgery) risk. While this is inherent to the feature's design, consider documenting the security implications:
- User code can make arbitrary HTTP requests to internal/external services
- Could be used to scan internal networks or exfiltrate data
- Could bypass IP allowlists or access localhost services
If the sandbox runs in a controlled environment (e.g., isolated network namespace, egress filtering), mention this in documentation. Otherwise, ensure users understand the security boundaries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/researcher.tsx` around lines 50 - 55, Update the sandbox tool
description (the "sandbox" entry in lib/agents/researcher.tsx) to explicitly
document network-access security implications: state that user code can make
arbitrary HTTP requests (SSRF/exfiltration risk), potential to reach
internal/localhost services or bypass IP allowlists, and recommend mitigations
such as running sandboxes in an isolated network namespace, applying egress
filtering, and clearly defining allowed/forbidden endpoints; include a brief
guidance sentence for operators on when to enable the feature and where to
configure these controls.
| return { | ||
| description: 'Execute code in an isolated microVM environment. Use this to run JavaScript or TypeScript code, perform data transformations, or generate dynamic HTML content.', | ||
| parameters: sandboxSchema, | ||
| execute: async ({ code, language, filename, dependencies }: any) => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Replace any with proper TypeScript types.
The execute function parameters should be typed using SandboxParameters from the schema to ensure type safety and better IDE support.
♻️ Proposed fix
+import { SandboxParameters } from '`@/lib/schema/sandbox`'
- execute: async ({ code, language, filename, dependencies }: any) => {
+ execute: async ({ code, language, filename, dependencies }: SandboxParameters) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| execute: async ({ code, language, filename, dependencies }: any) => { | |
| import { SandboxParameters } from '`@/lib/schema/sandbox`' | |
| execute: async ({ code, language, filename, dependencies }: SandboxParameters) => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/tools/sandbox.tsx` at line 13, Change the execute function
signature to use the concrete SandboxParameters type instead of any: import the
SandboxParameters type from the schema and update execute: async (params:
SandboxParameters) => { ... } (then destructure const { code, language,
filename, dependencies } = params). Update any other references to the execute
parameter shape to use SandboxParameters so the function and callers get full
type safety.
| sandbox = await Sandbox.create({ | ||
| runtime: 'node24', | ||
| ports: [3000], | ||
| timeout: 60000, | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Hardcoded port and timeout may not suit all use cases.
The sandbox is configured with a hardcoded port 3000 and timeout of 60000ms (60 seconds). Consider:
- Port: Some applications may need multiple ports or a different port (e.g., websockets on 8080, API on 3001).
- Timeout: CPU-intensive data transformations or large npm installs might exceed 60 seconds.
Consider making these configurable via the schema or exposing them as optional parameters in a future iteration if users request it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/tools/sandbox.tsx` around lines 26 - 30, The Sandbox.create call
currently hardcodes ports [3000] and timeout 60000; update this to accept
configurable values instead: add optional parameters (e.g., ports and timeout)
to the function or config schema that lead to Sandbox.create({ runtime:
'node24', ports: providedPorts || [3000], timeout: providedTimeout || 60000 }),
and ensure callers can pass multiple ports (array) and a numeric timeout;
reference the Sandbox.create invocation and the sandbox variable so you change
the invocation site and propagate the new optional parameters through the
calling code and schema.
| } | ||
| } | ||
|
|
||
| const isServer = /http|express|serve|listen|app\.get|app\.post|createServer/.test(code) |
There was a problem hiding this comment.
Server detection regex is fragile and incomplete.
The regex only detects Express/HTTP patterns but misses other popular frameworks (Fastify, Koa, Hapi, Socket.io, ws, etc.). Additionally, it can false-positive on comments or string literals containing these keywords. Consider a more robust detection strategy or make the server mode explicit via a schema parameter.
💡 Suggested improvements
Option 1: Add more patterns
-const isServer = /http|express|serve|listen|app\.get|app\.post|createServer/.test(code)
+const isServer = /http|express|fastify|koa|hapi|socket\.io|ws|serve|listen|app\.(get|post|put|delete|patch)|createServer|createHTTPServer/.test(code)Option 2: Add explicit parameter to schema (better)
Add an optional isServer: boolean parameter to sandboxSchema to let the agent explicitly specify server mode rather than relying on heuristics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/tools/sandbox.tsx` at line 46, The current fragile detection using
the isServer regex in sandbox.tsx can miss frameworks and hit false-positives;
add an optional isServer: boolean to sandboxSchema and prefer that explicit flag
in the code that computes isServer (use the schema value when present), and as a
fallback expand detection patterns to include Fastify/Koa/Hapi/Socket.io/ws and
strip comments/strings before regex testing (or use a simple AST/token scan) so
function/method name references like isServer and the server detection logic in
sandbox.tsx reliably reflect intent.
| if (isServer) { | ||
| // For servers, stream logs in the background and return immediately | ||
| streamLogs() | ||
| } else { | ||
| // For regular scripts, wait for execution to complete | ||
| await streamLogs() | ||
| const result = await (command as any).wait() | ||
| await sandbox.stop() | ||
| return { | ||
| logs, | ||
| exitCode: result.exitCode, | ||
| type: 'sandbox_result' | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| logs, | ||
| exitCode: 0, | ||
| previewUrl, | ||
| type: 'sandbox_result' | ||
| } |
There was a problem hiding this comment.
Critical: Server mode has multiple resource and correctness issues.
When isServer is true, this code has several problems:
- Resource leak: The sandbox is never stopped for server mode, causing resource exhaustion over time.
- Unhandled errors:
streamLogs()runs in the background without.catch(), so errors during log streaming are silently lost. - Misleading exit code: Returns
exitCode: 0immediately, even if the server crashes during startup. - Race condition: The background
streamLogs()modifies thelogsarray while the function returns a reference to it.
🔧 Recommended fixes
Fix 1: Add error handling to background streaming
if (isServer) {
// For servers, stream logs in the background and return immediately
- streamLogs()
+ streamLogs().catch(err => {
+ console.error('Background log streaming failed:', err)
+ // Consider appending error to UI stream
+ })
} else {Fix 2: Return a copy of logs to avoid race conditions
return {
- logs,
+ logs: [...logs],
exitCode: 0,
previewUrl,
type: 'sandbox_result'
}Fix 3: Consider lifecycle management for servers
Server-mode sandboxes are never stopped, leading to resource leaks. You need a strategy for cleanup:
- Track active sandboxes and provide a shutdown API
- Set a maximum lifetime for server sandboxes
- Stop the sandbox when the user navigates away or closes the chat
- Document that sandbox resources must be manually managed
Fix 4: More accurate exit code
Consider waiting briefly (e.g., 2-3 seconds) for server startup before returning, or use an exitCode: undefined to indicate "still running" rather than falsely claiming success with 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/agents/tools/sandbox.tsx` around lines 77 - 97, When isServer is true,
avoid leaking and misreporting by: (1) running streamLogs() with error handling
(call streamLogs().catch(err => { /* log and stop sandbox */ })) so errors
aren’t swallowed; (2) ensure sandbox.stop() is called on any streamLogs error
and register the sandbox with a lifecycle manager (e.g., add to an
activeSandboxes registry with a stop() API and/or TTL) so server-mode sandboxes
can be cleaned up; (3) don’t return a live reference to logs—return a shallow
copy (e.g., [...logs]) to avoid race conditions; and (4) don’t return exitCode:
0 immediately — either wait a short startup period or return exitCode: undefined
(or a "running" state) so callers know the server may still fail; update the
code paths around isServer, streamLogs, sandbox.stop, logs and previewUrl
accordingly.
| filename: z | ||
| .string() | ||
| .describe('The name of the file to create and execute (e.g., app.js or index.ts)'), |
There was a problem hiding this comment.
Add path traversal validation to filename.
The filename parameter accepts any string without validation. A malicious or crafted input like "../../../etc/passwd" or "../../node_modules/malicious.js" could potentially write files outside the intended sandbox directory. Add validation to ensure the filename contains no path separators or navigational components.
🛡️ Proposed fix
filename: z
.string()
+ .refine((val) => !val.includes('/') && !val.includes('\\') && !val.includes('..'), {
+ message: 'Filename must not contain path separators or parent directory references'
+ })
.describe('The name of the file to create and execute (e.g., app.js or index.ts)')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| filename: z | |
| .string() | |
| .describe('The name of the file to create and execute (e.g., app.js or index.ts)'), | |
| filename: z | |
| .string() | |
| .refine((val) => !val.includes('/') && !val.includes('\\') && !val.includes('..'), { | |
| message: 'Filename must not contain path separators or parent directory references' | |
| }) | |
| .describe('The name of the file to create and execute (e.g., app.js or index.ts)'), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/schema/sandbox.ts` around lines 13 - 15, The filename schema (the
z.string() chain for filename in lib/schema/sandbox.ts) currently accepts
arbitrary strings; update it to validate that values contain no path separators
or navigational components by adding a refinement or regex check (reject '/' and
'\' and any '..' segments) — e.g., use z.string().refine(...) or
z.string().regex(...) to enforce that the input equals its basename and does not
include ".." or path separators, and provide a clear error message for invalid
filenames.
| dependencies: z | ||
| .array(z.string()) | ||
| .optional() | ||
| .describe('An optional list of npm packages to install before execution') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add validation for npm package names in dependencies.
The dependencies array accepts arbitrary strings without validation. While npm will reject obviously malicious package names, adding basic validation (e.g., matching npm's naming rules) provides defense in depth and clearer error messages.
🛡️ Proposed validation
dependencies: z
.array(z.string())
+ .array(z.string().regex(/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/, 'Invalid npm package name'))
.optional()
.describe('An optional list of npm packages to install before execution')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dependencies: z | |
| .array(z.string()) | |
| .optional() | |
| .describe('An optional list of npm packages to install before execution') | |
| dependencies: z | |
| .array(z.string() | |
| .regex(/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/, 'Invalid npm package name') | |
| ) | |
| .optional() | |
| .describe('An optional list of npm packages to install before execution') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/schema/sandbox.ts` around lines 16 - 19, The dependencies schema
currently allows any string (dependencies: z.array(z.string()).optional()), so
add validation that each entry matches npm package naming rules: update the
array element from z.string() to either z.string().regex(/.../) with a regex
implementing npm name constraints or z.string().refine(v =>
validateNpmPackageName(v), { message: 'invalid npm package name' }) and include
a clear error message; keep the .optional() and .describe(...) and reference the
symbol name "dependencies" in lib/schema/sandbox.ts when making this change.
| /** | ||
| * Partial type for the sandbox schema, useful for streaming tool calls. | ||
| */ | ||
| export type PartialSandboxParameters = DeepPartial<typeof sandboxSchema> |
There was a problem hiding this comment.
Incorrect TypeScript usage of DeepPartial.
DeepPartial<typeof sandboxSchema> is incorrect. DeepPartial expects a type, not a Zod schema object. This should be DeepPartial<SandboxParameters>.
🔧 Proposed fix
/**
* Partial type for the sandbox schema, useful for streaming tool calls.
*/
-export type PartialSandboxParameters = DeepPartial<typeof sandboxSchema>
+export type PartialSandboxParameters = DeepPartial<SandboxParameters>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type PartialSandboxParameters = DeepPartial<typeof sandboxSchema> | |
| export type PartialSandboxParameters = DeepPartial<SandboxParameters> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/schema/sandbox.ts` at line 30, PartialSandboxParameters is incorrectly
defined using DeepPartial<typeof sandboxSchema>; DeepPartial expects a
TypeScript type, not a Zod schema. Replace DeepPartial<typeof sandboxSchema>
with DeepPartial<SandboxParameters> (the TS type corresponding to sandboxSchema)
and ensure SandboxParameters is imported or defined in the file so
PartialSandboxParameters becomes DeepPartial<SandboxParameters>.
…xample - Added comments explaining where to find VERCEL_TOKEN, VERCEL_TEAM_ID, and VERCEL_PROJECT_ID - Re-verified build passes Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
This PR integrates Vercel Sandbox into QCX, enabling the AI to execute JavaScript and TypeScript code in isolated microVMs.
Key features:
Infrastructure:
@vercel/sandboxpackage..env.example.app/actions.tsxto handle sandbox tool results in history.PR created automatically by Jules for task 4831652204283404284 started by @ngoiyaeric
Summary by CodeRabbit