feat: check sandbox snapshot on login, trigger setup if missing#8
Conversation
Remove client-side just-bash/browser dependency for command execution. All terminal commands now go through /api/exec which creates and reuses a Vercel Sandbox VM, eliminating split logic between local and remote command execution. - Add /api/exec endpoint that creates/reuses sandbox sessions - Refactor input-handler to accept generic exec function - Refactor agent-command to standalone handler (no defineCommand) - Update Terminal.tsx to route commands: static (local) → agent (API) → sandbox (API) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous approach read _agent-data from disk via __dirname, which doesn't work in serverless since each route is bundled independently. Now fetches files via the existing /api/fs endpoint. Added top-level try/catch so errors return useful messages instead of raw 500s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DRY: Move readSourceFiles and createSandbox into app/api/_lib/ so both /api/agent and /api/exec reuse the same sandbox creation logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds useSetupSandbox hook that runs once after Privy authentication. Checks GET /api/sandboxes for existing snapshots, and if none exist, fires POST /api/sandboxes/setup in the background to provision a GitHub repo + snapshot before the user starts using the terminal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces a sandbox execution infrastructure by adding new helper modules for sandbox creation and file reading, establishing a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client<br/>(Terminal)
participant Server as /api/exec
participant FSApi as /api/fs
participant Sandbox as Sandbox<br/>Environment
Client->>Server: POST /api/exec<br/>(command, sandboxId?)
activate Server
alt Reusing existing sandbox
Server->>Sandbox: Get existing sandbox
alt Sandbox unavailable
Server->>FSApi: GET source files
FSApi-->>Server: File contents
Server->>Sandbox: Create & seed<br/>new sandbox
end
else Creating new sandbox
Server->>FSApi: GET source files
FSApi-->>Server: File contents
Server->>Sandbox: Create & seed<br/>new sandbox
end
Server->>Sandbox: Execute command<br/>(bash -c)
Sandbox-->>Server: stdout, stderr,<br/>exitCode
deactivate Server
Server-->>Client: {stdout, stderr,<br/>exitCode, sandboxId}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/terminal-parts/agent-command.ts (1)
334-341:⚠️ Potential issue | 🟡 Minor
thinkingTimeoutis not cleared on the error path.If the
catchblock on Line 334 is reached, thesetTimeoutfromresetThinkingTimer()(Line 137) may still be pending. It will fire after the handler returns, writing "Thinking..." to the terminal at an unexpected time.🐛 Proposed fix — clear timer in catch
} catch (error) { + clearThinking(false); agentMessages.pop(); return {
🤖 Fix all issues with AI agents
In `@app/api/_lib/createSandbox.ts`:
- Around line 6-14: The createSandbox function can leak a created Sandbox when
Sandbox.create() succeeds but sandbox.writeFiles(files) throws; update
createSandbox to ensure the sandbox is cleaned up on error by wrapping the
writeFiles call in a try/catch (or try/finally) and calling the sandbox cleanup
method (e.g., sandbox.stop() or sandbox.dispose()) if writeFiles throws, then
rethrow the original error; only return the sandbox instance after successful
writeFiles. Ensure you reference Sandbox.create(), sandbox.writeFiles(...), and
the sandbox cleanup method (sandbox.stop()/sandbox.dispose()) when implementing
the fix.
In `@app/api/agent/route.ts`:
- Line 52: Remove the unconditional console.log that prints user-submitted
content (console.log("Prompt:", lastUserMessage?.parts?.[0]?.text)); either
delete it or gate it behind a debug/config flag (e.g., process.env.DEBUG) or use
a logger at debug level (e.g., processLogger.debug) so prompts are not written
to production logs; locate the console.log call and update it to only run when
debugging is enabled, referencing the console.log call and lastUserMessage
variable to find the exact spot.
- Around line 54-55: readSourceFiles is doing synchronous disk I/O on every POST
which blocks the event loop; change the code so AGENT_DATA_DIR is read once and
cached at module load instead of per-request: move the call to
readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD) out of the request handler in
route.ts into top-level initialization, store the result in a cachedFiles
constant (or lazy-initialize it on first access), and then pass cachedFiles into
createSandbox(...) inside the handler (or reuse a cached sandbox if
appropriate), ensuring readSourceFiles, AGENT_DATA_DIR, SANDBOX_CWD and
createSandbox are the referenced symbols to modify.
In `@app/api/exec/route.ts`:
- Around line 67-81: The code creates new sandboxes via createAndSeedSandbox()
when sandboxId is missing or not found but never stops them, causing resource
leaks; update the exec route to (1) ensure any sandbox you create is stopped
after the command completes (e.g., call sandbox.stop() or a provided
Sandbox.stop(activeSandboxId) in the finally/cleanup path), (2) return the
activeSandboxId in the response so clients can reuse an existing sandbox, and
(3) add or trigger a TTL/cleanup mechanism (e.g., enqueue the sandboxId for
background cleanup or set an expiration) for sandboxes created here to avoid
long-lived resources. Ensure changes reference the Sandbox instance,
activeSandboxId and createAndSeedSandbox() locations so the cleanup runs
regardless of success/failure.
- Around line 83-98: The sandbox.runCommand calls can hang on long commands;
update sandbox creation in createSandbox.ts to include a lifetime timeout (e.g.,
Sandbox.create({ timeout: 30000 })) and add per-command cancellation by creating
an AbortController around sandbox.runCommand (use a timeout to call
controller.abort() and pass signal: controller.signal to sandbox.runCommand) so
runCommand is aborted if it exceeds the intended duration; modify the code paths
that call sandbox.runCommand (the runCommand invocation in route.ts) to use this
AbortController and ensure any controller cleanup is handled.
In `@app/hooks/useSetupSandbox.ts`:
- Around line 32-35: The un-awaited fetch in useSetupSandbox.ts (the POST to
`${RECOUP_API_URL}/api/sandboxes/setup`) can reject outside the surrounding
try/catch and cause unhandled promise rejections; fix it by either awaiting the
fetch inside the try block (so rejections are caught) or explicitly handling
errors on the returned promise (e.g., append .catch(() => {}) to swallow/handle
failures) depending on whether you want the call truly fire-and-forget or to
surface errors; update the fetch invocation in useSetupSandbox.ts accordingly so
the promise rejection is handled.
🧹 Nitpick comments (5)
app/api/_lib/readSourceFiles.ts (1)
16-27: Consider guarding against symlink cycles and expanding the exclusion list.The recursive traversal has no depth limit and doesn't check for symbolic links. A circular symlink would cause infinite recursion and a stack overflow. Additionally, directories like
.next,dist, orbuildcould bloat the returned file list unnecessarily.A
lstatSynccheck and/or a max-depth guard would make this safer. Expanding the exclusion set (or making it configurable) would avoid reading build artifacts.app/components/terminal-parts/input-handler.ts (1)
12-18:ExecResulttype is duplicated across modules.The identical
ExecResulttype is defined locally in bothinput-handler.ts(Line 12) andagent-command.ts(Line 4). Consider extracting it into a shared types file (e.g.,terminal-parts/types.ts) and importing it in both places.app/hooks/useSetupSandbox.ts (1)
15-40: No cleanup on unmount — in-flight requests continue after the component is removed.If the component unmounts while the async IIFE is executing (e.g., during navigation), the fetch calls and their callbacks continue running against a stale closure. Consider using an
AbortControllersignal passed to the fetch calls and aborting in the effect cleanup.app/api/exec/route.ts (1)
6-19: Self-fetch to/api/fsadds unnecessary latency and fragility.
fetchSourceFilesmakes an HTTP call back to the same deployment to read files. This adds a network round-trip, risks timeout/failure during cold starts, and means the/api/fsroute must be publicly accessible. Consider importing the file-reading logic directly (similar to howapp/api/agent/route.tsusesreadSourceFilessynchronously) instead of going through HTTP.app/components/Terminal.tsx (1)
62-63:sandboxIdis lost if the effect re-runs due togetAccessTokenidentity change.
sandboxIdlives as aletinside the effect closure. IfgetAccessTokenis not referentially stable across renders (common with Privy), the effect re-runs, the terminal is disposed and recreated, and the sandbox session is lost — causing a new sandbox to be provisioned on each re-render cycle.Consider storing
getAccessTokenin a ref so the effect doesn't depend on its identity, or storesandboxIdin a ref that survives effect re-runs.Option: stabilize getAccessToken via ref
+ const getAccessTokenRef = useRef(getAccessToken); + useEffect(() => { + getAccessTokenRef.current = getAccessToken; + }, [getAccessToken]); + useEffect(() => { const container = terminalRef.current; if (!container) return; ... - const token = await getAccessToken(); + const token = await getAccessTokenRef.current(); ... - }, [getAccessToken]); + }, []);Also applies to: 176-176
| export async function createSandbox( | ||
| files: Array<{ path: string; content: Buffer }> | ||
| ): Promise<Sandbox> { | ||
| const sandbox = await Sandbox.create(); | ||
| if (files.length > 0) { | ||
| await sandbox.writeFiles(files); | ||
| } | ||
| return sandbox; | ||
| } |
There was a problem hiding this comment.
Sandbox leaks if writeFiles throws.
If Sandbox.create() succeeds but sandbox.writeFiles(files) throws, the created sandbox is never stopped/cleaned up by the caller (since createSandbox rejects before returning the instance).
🛡️ Proposed fix
export async function createSandbox(
files: Array<{ path: string; content: Buffer }>
): Promise<Sandbox> {
const sandbox = await Sandbox.create();
- if (files.length > 0) {
- await sandbox.writeFiles(files);
+ try {
+ if (files.length > 0) {
+ await sandbox.writeFiles(files);
+ }
+ } catch (err) {
+ await sandbox.stop().catch(() => {});
+ throw err;
}
return sandbox;
}🤖 Prompt for AI Agents
In `@app/api/_lib/createSandbox.ts` around lines 6 - 14, The createSandbox
function can leak a created Sandbox when Sandbox.create() succeeds but
sandbox.writeFiles(files) throws; update createSandbox to ensure the sandbox is
cleaned up on error by wrapping the writeFiles call in a try/catch (or
try/finally) and calling the sandbox cleanup method (e.g., sandbox.stop() or
sandbox.dispose()) if writeFiles throws, then rethrow the original error; only
return the sandbox instance after successful writeFiles. Ensure you reference
Sandbox.create(), sandbox.writeFiles(...), and the sandbox cleanup method
(sandbox.stop()/sandbox.dispose()) when implementing the fix.
| @@ -80,14 +51,10 @@ export async function POST(req: Request) { | |||
| .pop(); | |||
| console.log("Prompt:", lastUserMessage?.parts?.[0]?.text); | |||
There was a problem hiding this comment.
User prompt is logged to the console.
console.log("Prompt:", lastUserMessage?.parts?.[0]?.text) logs user-submitted content to server logs. Depending on usage, prompts may contain sensitive or personally identifiable information. Consider removing this or gating it behind a debug flag.
🤖 Prompt for AI Agents
In `@app/api/agent/route.ts` at line 52, Remove the unconditional console.log that
prints user-submitted content (console.log("Prompt:",
lastUserMessage?.parts?.[0]?.text)); either delete it or gate it behind a
debug/config flag (e.g., process.env.DEBUG) or use a logger at debug level
(e.g., processLogger.debug) so prompts are not written to production logs;
locate the console.log call and update it to only run when debugging is enabled,
referencing the console.log call and lastUserMessage variable to find the exact
spot.
| const files = readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD); | ||
| const sandbox = await createSandbox(files); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Synchronous filesystem traversal runs on every request, blocking the event loop.
readSourceFiles uses readdirSync/readFileSync and is invoked on every POST. For a non-trivial source tree this blocks the Node.js event loop, increasing latency for all concurrent requests. Since AGENT_DATA_DIR is static, consider reading the files once at module load time and caching the result:
♻️ Suggested caching approach
const AGENT_DATA_DIR = join(__dirname, "./_agent-data");
const SANDBOX_CWD = "/vercel/sandbox";
+
+// Read once at module load — AGENT_DATA_DIR is static
+const AGENT_FILES = readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD);
// ... inside POST handler:
- const files = readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD);
- const sandbox = await createSandbox(files);
+ const sandbox = await createSandbox(AGENT_FILES);📝 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.
| const files = readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD); | |
| const sandbox = await createSandbox(files); | |
| const sandbox = await createSandbox(AGENT_FILES); |
🤖 Prompt for AI Agents
In `@app/api/agent/route.ts` around lines 54 - 55, readSourceFiles is doing
synchronous disk I/O on every POST which blocks the event loop; change the code
so AGENT_DATA_DIR is read once and cached at module load instead of per-request:
move the call to readSourceFiles(AGENT_DATA_DIR, SANDBOX_CWD) out of the request
handler in route.ts into top-level initialization, store the result in a
cachedFiles constant (or lazy-initialize it on first access), and then pass
cachedFiles into createSandbox(...) inside the handler (or reuse a cached
sandbox if appropriate), ensuring readSourceFiles, AGENT_DATA_DIR, SANDBOX_CWD
and createSandbox are the referenced symbols to modify.
| let sandbox: Sandbox; | ||
| let activeSandboxId: string; | ||
|
|
||
| if (sandboxId) { | ||
| try { | ||
| sandbox = await Sandbox.get({ sandboxId }); | ||
| activeSandboxId = sandboxId; | ||
| } catch { | ||
| sandbox = await createAndSeedSandbox(); | ||
| activeSandboxId = sandbox.sandboxId; | ||
| } | ||
| } else { | ||
| sandbox = await createAndSeedSandbox(); | ||
| activeSandboxId = sandbox.sandboxId; | ||
| } |
There was a problem hiding this comment.
Sandboxes are created but never stopped — potential resource leak.
When sandboxId is absent (or the existing one can't be found), createAndSeedSandbox() provisions a new sandbox. However, the sandbox is never explicitly stopped after the command completes. Unlike the agent route (which stops the sandbox after the stream ends), this endpoint leaves every sandbox running until the platform garbage-collects it. Under sustained traffic this could exhaust your sandbox quota.
Consider stopping the sandbox after the command finishes, or at minimum returning the sandboxId so the client can reuse it (which it does), and adding a TTL/cleanup mechanism.
🤖 Prompt for AI Agents
In `@app/api/exec/route.ts` around lines 67 - 81, The code creates new sandboxes
via createAndSeedSandbox() when sandboxId is missing or not found but never
stops them, causing resource leaks; update the exec route to (1) ensure any
sandbox you create is stopped after the command completes (e.g., call
sandbox.stop() or a provided Sandbox.stop(activeSandboxId) in the
finally/cleanup path), (2) return the activeSandboxId in the response so clients
can reuse an existing sandbox, and (3) add or trigger a TTL/cleanup mechanism
(e.g., enqueue the sandboxId for background cleanup or set an expiration) for
sandboxes created here to avoid long-lived resources. Ensure changes reference
the Sandbox instance, activeSandboxId and createAndSeedSandbox() locations so
the cleanup runs regardless of success/failure.
| try { | ||
| const result = await sandbox.runCommand({ | ||
| cmd: "bash", | ||
| args: ["-c", command], | ||
| cwd: SANDBOX_CWD, | ||
| }); | ||
|
|
||
| const stdout = await result.stdout(); | ||
| const stderr = await result.stderr(); | ||
|
|
||
| return Response.json({ | ||
| stdout, | ||
| stderr, | ||
| exitCode: result.exitCode, | ||
| sandboxId: activeSandboxId, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@vercel/sandbox runCommand timeout option documentation
💡 Result:
@vercel/sandbox does not expose a timeout option on sandbox.runCommand(). The runCommand() options include things like cwd, env, sudo, detached, stdout/stderr, and an AbortSignal (signal) to cancel the command, but no per-command timeout field. [1]
If you’re trying to avoid long-running work, you have two supported approaches:
- Set the sandbox lifetime timeout when creating the sandbox:
Sandbox.create({ timeout: <ms> })(default is 5 minutes; plan maximums apply). [2][3] - Extend the sandbox lifetime while it’s running:
sandbox.extendTimeout(<ms>). [2][3]
Source(s):
sandbox.runCommand()API/options (notimeoutparameter): [1]- Sandbox lifetime
timeoutis configured onSandbox.create()/extendTimeout(): [2][3]
🏁 Script executed:
# Find and examine the route.ts file mentioned in the review
find . -name "route.ts" -path "*api/exec*" | head -20Repository: recoupable/bash
Length of output: 82
🏁 Script executed:
# Search for package.json to check the `@vercel/sandbox` version
find . -name "package.json" -type f | head -5Repository: recoupable/bash
Length of output: 381
🏁 Script executed:
# Look for any timeout-related code in the codebase
rg "timeout" -i --type ts --type js | head -30Repository: recoupable/bash
Length of output: 1837
🏁 Script executed:
cat -n ./app/api/exec/route.tsRepository: recoupable/bash
Length of output: 4130
🏁 Script executed:
# Search for Sandbox.create and timeout configuration in the codebase
rg "Sandbox.create" -A 5 -B 2Repository: recoupable/bash
Length of output: 1018
🏁 Script executed:
# Look for AbortSignal usage in the same file
rg "AbortSignal|AbortController" -A 3 -B 2Repository: recoupable/bash
Length of output: 41
Add timeout protection to prevent long-running commands from blocking the serverless function.
sandbox.runCommand() lacks timeout protection. A user could submit a command like sleep 9999 or an infinite loop, consuming the entire serverless function execution time. While @vercel/sandbox does not expose a per-command timeout option, you can mitigate this in two ways:
- Set sandbox lifetime timeout when creating the sandbox in
createSandbox.ts(line 29):Sandbox.create({ timeout: 30000 })(adjust milliseconds as needed). - Use AbortSignal to cancel long-running commands in the
runCommandcall (lines 84–88): implement an AbortController with a timeout and passsignal: controller.signalto runCommand.
Either approach prevents indefinite blocking. The first is simpler; the second offers per-command granularity.
🤖 Prompt for AI Agents
In `@app/api/exec/route.ts` around lines 83 - 98, The sandbox.runCommand calls can
hang on long commands; update sandbox creation in createSandbox.ts to include a
lifetime timeout (e.g., Sandbox.create({ timeout: 30000 })) and add per-command
cancellation by creating an AbortController around sandbox.runCommand (use a
timeout to call controller.abort() and pass signal: controller.signal to
sandbox.runCommand) so runCommand is aborted if it exceeds the intended
duration; modify the code paths that call sandbox.runCommand (the runCommand
invocation in route.ts) to use this AbortController and ensure any controller
cleanup is handled.
| fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, { | ||
| method: "POST", | ||
| headers, | ||
| }); |
There was a problem hiding this comment.
Unawaited fetch escapes the try/catch, causing unhandled promise rejections.
The POST to /api/sandboxes/setup is fire-and-forget (no await). If this fetch rejects (e.g., network error), the rejection is not caught by the surrounding try/catch, which defeats the "silent background provisioning" intent and will trigger an unhandledrejection event in the browser.
Either await it or explicitly .catch(() => {}) the promise:
🐛 Proposed fix
- fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, {
+ await fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, {
method: "POST",
headers,
});Or if truly fire-and-forget:
fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, {
method: "POST",
headers,
- });
+ }).catch(() => {});📝 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.
| fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, { | |
| method: "POST", | |
| headers, | |
| }); | |
| await fetch(`${RECOUP_API_URL}/api/sandboxes/setup`, { | |
| method: "POST", | |
| headers, | |
| }); |
🤖 Prompt for AI Agents
In `@app/hooks/useSetupSandbox.ts` around lines 32 - 35, The un-awaited fetch in
useSetupSandbox.ts (the POST to `${RECOUP_API_URL}/api/sandboxes/setup`) can
reject outside the surrounding try/catch and cause unhandled promise rejections;
fix it by either awaiting the fetch inside the try block (so rejections are
caught) or explicitly handling errors on the returned promise (e.g., append
.catch(() => {}) to swallow/handle failures) depending on whether you want the
call truly fire-and-forget or to surface errors; update the fetch invocation in
useSetupSandbox.ts accordingly so the promise rejection is handled.
Moves createSandbox.ts and readSourceFiles.ts out of the API-specific _lib directory into app/lib so they are accessible project-wide. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Replaced by clean PR from a branch based on main |
1 participant
Summary
useSetupSandboxhook that runs once after Privy authenticationGET /api/sandboxesfor existing snapshots; if none exist, firesPOST /api/sandboxes/setupin the backgroundTest plan
POST /api/sandboxes/setupis called🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor