feat: load sandbox from Recoup API snapshot#4
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThese changes refactor sandbox initialization to use snapshot-backed creation with Bearer token authentication. A new Changes
Sequence DiagramsequenceDiagram
participant Route as Agent Route
participant Snapshot as getSnapshotId
participant API as Recoup API
participant Sandbox as Sandbox Creation
participant FileOps as File Operations
Route->>Snapshot: getSnapshotId(bearerToken)
Snapshot->>API: GET /api/sandboxes (Bearer auth)
API-->>Snapshot: snapshot_id or error
Snapshot-->>Route: snapshotId | null
alt snapshotId exists
Route->>Sandbox: Sandbox.create({snapshot})
Sandbox-->>Route: Sandbox (success)
else no snapshotId or creation fails
Route->>Sandbox: Sandbox.create()
Sandbox-->>Route: Fresh Sandbox
Route->>FileOps: readSourceFiles(agentDataDir)
FileOps-->>Route: files[]
Route->>Sandbox: sandbox.writeFiles(files)
Sandbox-->>Route: Sandbox (populated)
end
Route-->>Route: Continue with agent creation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 |
Use snapshot-based sandbox creation when available, eliminating the file upload step. Falls back to fresh sandbox + file upload when no snapshot exists or on any failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e37bcc7 to
8291967
Compare
Move snapshot resolution, sandbox creation, and file upload fallback logic out of the route handler into a dedicated module (SRP). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/sandbox/createNewSandbox.ts`:
- Around line 48-55: The Sandbox created by Sandbox.create() is leaked if
sandbox.writeFiles(files) throws; wrap the writeFiles call in a try/catch, and
in the catch call a cleanup method on the sandbox (e.g., sandbox.destroy() or
sandbox.shutdown()/sandbox.teardown() if that is the actual API) then rethrow
the original error so callers still observe the failure; ensure the sandbox
variable is declared before the try so cleanup can always run if writeFiles
fails.
🧹 Nitpick comments (1)
lib/sandbox/createNewSandbox.ts (1)
8-30: Synchronous file I/O on the request path blocks the event loop.
readdirSync/readFileSyncare called inside anasyncfunction that runs per-request. IfagentDataDiris large or slow (e.g., NFS), every concurrent request stalls behind this work. Consider switching tofs/promises(readdir/readFile) to keep the event loop free.♻️ Async alternative sketch
-import { readdirSync, readFileSync } from "fs"; +import { readdir, readFile } from "fs/promises"; import { join, relative } from "path"; -function readSourceFiles( +async function readSourceFiles( dir: string, baseDir?: string, -): Array<{ path: string; content: Buffer }> { +): Promise<Array<{ path: string; content: Buffer }>> { const base = baseDir ?? dir; const files: Array<{ path: string; content: Buffer }> = []; - for (const entry of readdirSync(dir, { withFileTypes: true })) { + for (const entry of await readdir(dir, { withFileTypes: true })) { const fullPath = join(dir, entry.name); if (entry.isDirectory()) { if (entry.name === "node_modules" || entry.name === ".git") continue; - files.push(...readSourceFiles(fullPath, base)); + files.push(...(await readSourceFiles(fullPath, base))); } else { const relPath = relative(base, fullPath); files.push({ path: join(SANDBOX_CWD, relPath), - content: readFileSync(fullPath), + content: await readFile(fullPath), }); } } return files; }
| const sandbox = await Sandbox.create(); | ||
|
|
||
| const files = readSourceFiles(agentDataDir); | ||
| if (files.length > 0) { | ||
| await sandbox.writeFiles(files); | ||
| } | ||
|
|
||
| return sandbox; |
There was a problem hiding this comment.
Sandbox leaks if writeFiles throws.
If sandbox.writeFiles(files) on Line 52 rejects, the function propagates the error without returning the sandbox, so the caller has no handle to shut it down. The sandbox resource is orphaned.
🐛 Proposed fix — clean up on failure
const sandbox = await Sandbox.create();
- const files = readSourceFiles(agentDataDir);
- if (files.length > 0) {
- await sandbox.writeFiles(files);
+ try {
+ const files = readSourceFiles(agentDataDir);
+ if (files.length > 0) {
+ await sandbox.writeFiles(files);
+ }
+ } catch (err) {
+ await sandbox.close?.().catch(() => {});
+ throw err;
}
return sandbox;🤖 Prompt for AI Agents
In `@lib/sandbox/createNewSandbox.ts` around lines 48 - 55, The Sandbox created by
Sandbox.create() is leaked if sandbox.writeFiles(files) throws; wrap the
writeFiles call in a try/catch, and in the catch call a cleanup method on the
sandbox (e.g., sandbox.destroy() or sandbox.shutdown()/sandbox.teardown() if
that is the actual API) then rethrow the original error so callers still observe
the failure; ensure the sandbox variable is declared before the try so cleanup
can always run if writeFiles fails.
Instead of fetching a snapshot_id and creating sandboxes directly, call POST /api/sandboxes on the Recoup API which handles snapshot resolution internally, then connect via Sandbox.get(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch between prod and test Recoup API based on NEXT_PUBLIC_VERCEL_ENV, matching the pattern in Recoup-Chat. Preview deployments now correctly hit test-recoup-api.vercel.app instead of production. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Log elapsed time for each step: POST /api/sandboxes, Sandbox.get, createBashTool, and fallback paths. Will help diagnose the 20s gap between sandbox creation and first bash execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tool discovery runs `ls /usr/bin ...` in the sandbox which takes ~34s due to sandbox warm-up after snapshot restore. Provide a static toolPrompt to skip discovery entirely — the available tools on a node22 sandbox are always the same. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/api/agent/route.ts`:
- Around line 55-57: The call to createNewSandbox(bearerToken, AGENT_DATA_DIR)
is outside the try/catch so any throw (including from Sandbox.create() inside
createNewSandbox) results in an unhandled 500; wrap the createNewSandbox
invocation in a try/catch inside the route handler, catch the error, log it
(including the error object and relevant context like
bearerToken/AGENT_DATA_DIR), and return a structured JSON error response (e.g.,
using NextResponse.json with an { error: "message", details: ... } payload and
status 500) so callers get a proper error body instead of a raw 500.
In `@lib/recoup-api/createSandbox.ts`:
- Around line 1-4: The current RECOUP_API_URL constant ignores the documented
env override; change the RECOUP_API_URL initialization so it first honors
process.env.RECOUP_API_URL (use it if defined) and only if absent fall back to
the existing production/test selection based on
process.env.NEXT_PUBLIC_VERCEL_ENV (the IS_PROD logic). Update the const
RECOUP_API_URL in createSandbox.ts (and keep IS_PROD as-is) so callers get the
env-provided value when present and the hardcoded URLs only as a fallback.
🧹 Nitpick comments (1)
lib/recoup-api/createSandbox.ts (1)
24-26: Consider logging the error before swallowing it.The silent
catchdiscards all error context. When diagnosing production issues, knowing why the API call failed (network timeout, 5xx, DNS, etc.) is valuable. A singleconsole.warnpreserves the fallback behavior while aiding debugging.Suggested fix
- } catch { + } catch (err) { + console.warn("[createSandbox] Failed, falling back:", err); return null; }
app/api/agent/route.ts
Outdated
| const t0 = Date.now(); | ||
| const sandbox = await createNewSandbox(bearerToken, AGENT_DATA_DIR); | ||
| console.log(`[timing] createNewSandbox: ${Date.now() - t0}ms`); |
There was a problem hiding this comment.
createNewSandbox is outside the try/catch — an unhandled throw yields a raw 500.
If both the API path and Sandbox.create() fallback inside createNewSandbox fail, the exception propagates with no structured error response. Consider wrapping this in a try/catch that returns a proper JSON error.
Suggested fix
const t0 = Date.now();
- const sandbox = await createNewSandbox(bearerToken, AGENT_DATA_DIR);
- console.log(`[timing] createNewSandbox: ${Date.now() - t0}ms`);
+ let sandbox;
+ try {
+ sandbox = await createNewSandbox(bearerToken, AGENT_DATA_DIR);
+ console.log(`[timing] createNewSandbox: ${Date.now() - t0}ms`);
+ } catch (err) {
+ console.error("Failed to create sandbox:", err);
+ return Response.json(
+ { error: "Failed to create sandbox" },
+ { status: 503 },
+ );
+ }📝 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 t0 = Date.now(); | |
| const sandbox = await createNewSandbox(bearerToken, AGENT_DATA_DIR); | |
| console.log(`[timing] createNewSandbox: ${Date.now() - t0}ms`); | |
| const t0 = Date.now(); | |
| let sandbox; | |
| try { | |
| sandbox = await createNewSandbox(bearerToken, AGENT_DATA_DIR); | |
| console.log(`[timing] createNewSandbox: ${Date.now() - t0}ms`); | |
| } catch (err) { | |
| console.error("Failed to create sandbox:", err); | |
| return Response.json( | |
| { error: "Failed to create sandbox" }, | |
| { status: 503 }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@app/api/agent/route.ts` around lines 55 - 57, The call to
createNewSandbox(bearerToken, AGENT_DATA_DIR) is outside the try/catch so any
throw (including from Sandbox.create() inside createNewSandbox) results in an
unhandled 500; wrap the createNewSandbox invocation in a try/catch inside the
route handler, catch the error, log it (including the error object and relevant
context like bearerToken/AGENT_DATA_DIR), and return a structured JSON error
response (e.g., using NextResponse.json with an { error: "message", details: ...
} payload and status 500) so callers get a proper error body instead of a raw
500.
| const IS_PROD = process.env.NEXT_PUBLIC_VERCEL_ENV === "production"; | ||
| const RECOUP_API_URL = IS_PROD | ||
| ? "https://recoup-api.vercel.app" | ||
| : "https://test-recoup-api.vercel.app"; |
There was a problem hiding this comment.
Hardcoded URLs don't match the documented RECOUP_API_URL env-var override.
The PR description states RECOUP_API_URL is an optional, configurable env var, but the implementation hardcodes two URLs with no override mechanism. Consider honoring an env var for flexibility:
Suggested fix
-const IS_PROD = process.env.NEXT_PUBLIC_VERCEL_ENV === "production";
-const RECOUP_API_URL = IS_PROD
- ? "https://recoup-api.vercel.app"
- : "https://test-recoup-api.vercel.app";
+const IS_PROD = process.env.NEXT_PUBLIC_VERCEL_ENV === "production";
+const RECOUP_API_URL =
+ process.env.RECOUP_API_URL ??
+ (IS_PROD
+ ? "https://recoup-api.vercel.app"
+ : "https://test-recoup-api.vercel.app");📝 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 IS_PROD = process.env.NEXT_PUBLIC_VERCEL_ENV === "production"; | |
| const RECOUP_API_URL = IS_PROD | |
| ? "https://recoup-api.vercel.app" | |
| : "https://test-recoup-api.vercel.app"; | |
| const IS_PROD = process.env.NEXT_PUBLIC_VERCEL_ENV === "production"; | |
| const RECOUP_API_URL = | |
| process.env.RECOUP_API_URL ?? | |
| (IS_PROD | |
| ? "https://recoup-api.vercel.app" | |
| : "https://test-recoup-api.vercel.app"); |
🤖 Prompt for AI Agents
In `@lib/recoup-api/createSandbox.ts` around lines 1 - 4, The current
RECOUP_API_URL constant ignores the documented env override; change the
RECOUP_API_URL initialization so it first honors process.env.RECOUP_API_URL (use
it if defined) and only if absent fall back to the existing production/test
selection based on process.env.NEXT_PUBLIC_VERCEL_ENV (the IS_PROD logic).
Update the const RECOUP_API_URL in createSandbox.ts (and keep IS_PROD as-is) so
callers get the env-provided value when present and the hardcoded URLs only as a
fallback.
The snapshot contains the user's Recoup Sandbox content, not the just-bash/bash-tool source files. Write agent data files to the sandbox in all cases so the agent can explore the source code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only upload source files in the fallback path (fresh sandbox). Snapshot sandboxes should have the needed files pre-baked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Run a no-op command (true) right after Sandbox.get() to absorb the snapshot cold-start latency during setup rather than during the agent's first tool call. This eliminates the long pause users see after the first bash command appears on screen. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use GET /api/sandboxes to fetch the snapshot_id, then create the
sandbox locally with Sandbox.create({ source: { type: "snapshot" } })
instead of POST + Sandbox.get(). This tests whether the 33s cold
start was caused by cross-project Sandbox.get() vs local creation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clean up all [timing] console.log statements and the warm-up command. Remove unused lib/recoup-api/createSandbox.ts (replaced by getSnapshotId). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
- /new page uses /api/agent/new which creates a fresh Sandbox.create() + file upload (no snapshot) for A/B comparison against snapshot path - Terminal and agent-command accept configurable agentEndpoint prop Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
3 similar comments
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
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>
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>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
snapshot_idfrom Recoup API (GET /api/sandboxes) by forwarding the user's Bearer tokenChanges
lib/recoup-api/getSnapshotId.ts— fetches snapshot ID with 5s timeout and null fallbackapp/api/agent/route.ts— snapshot-based sandbox creation with fallback to current behaviorEnvironment variable
RECOUP_API_URL(optional, defaults tohttps://recoup-api.vercel.app) — add to Vercel deployment if neededTest plan
RECOUP_API_URLgracefully falls back to fresh sandbox🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements