From 375673ab386888adcab098acc98e052ffbd612db Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 4 May 2026 06:36:00 -0500 Subject: [PATCH 1/2] refactor(sandbox): callers use open-agents abstraction (Phase 2.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces direct @vercel/sandbox SDK calls with the open-agents sandbox abstraction layer (inlined in Phase 2.1) for sandbox lifecycle (create + reconnect). HTTP response shapes preserved exactly. Per the agreed Option B (hybrid): only the lifecycle creator helpers get refactored. installClaudeCode / runClaudeCode / getSandboxStatus stay on the SDK directly because the abstraction does not cover their needs (sudo, stdout/stderr streaming, simple status reads). Those two install/run files are also dead orphans (defined but never called) and will be removed entirely after the full migration. Production refactor: createSandbox.ts Sandbox.create(...) -> VercelSandbox.create(...) Input: VercelSandboxConfig (was SDK params) Snapshot trigger: restoreSnapshotId field (was source: { type: "snapshot", ... }) Returns VercelSandbox (was SDK Sandbox) createSandboxWithFallback.ts cascade — passes restoreSnapshotId to createSandbox createSandboxFromSnapshot.ts type cascade only (Sandbox -> VercelSandbox) getActiveSandbox.ts Sandbox.get({name}) -> VercelSandbox.connect(name, {}) Status check: sandbox.status -> sandbox.sdkStatus getOrCreateSandbox.ts no code change — type cascades automatically processCreateSandbox.ts reads sandbox.sdkStatus instead of sandbox.status defensive nullish on createdAt Abstraction extension: vercel/sandbox/VercelSandbox.ts adds two readonly getters following the existing host/environmentDetails/expiresAt pattern: get sdkStatus(): string — raw SDK session status (running/pending/ stopped/failed/aborted/snapshotting), distinct from the abstraction's normalized status getter get createdAt(): Date | undefined — SDK session.createdAt These give api callers what they need to construct the existing HTTP response shape without breaking the abstraction's interface. Tests updated: createSandbox.test.ts mocks VercelSandbox.create instead of Sandbox.create; mock object uses sdkStatus instead of status createSandboxWithFallback.test.ts asserts restoreSnapshotId pass-through getActiveSandbox.test.ts mocks VercelSandbox.connect; sdkStatus on mock objects processCreateSandbox.test.ts mockSandbox uses sdkStatus Verification: - pnpm lint:check: clean - pnpm test: 2391/2391 pass - HTTP response shape unchanged: same fields, same enum values for sandboxStatus (sourced from the SDK now via sdkStatus, was directly via SDK Sandbox.status before — identical strings either way) Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/sandbox/__tests__/createSandbox.test.ts | 38 +++++++------ .../createSandboxWithFallback.test.ts | 2 +- .../__tests__/getActiveSandbox.test.ts | 25 ++++----- .../__tests__/processCreateSandbox.test.ts | 6 +- lib/sandbox/createSandbox.ts | 55 ++++++++++--------- lib/sandbox/createSandboxFromSnapshot.ts | 4 +- lib/sandbox/createSandboxWithFallback.ts | 2 +- lib/sandbox/getActiveSandbox.ts | 11 ++-- lib/sandbox/processCreateSandbox.ts | 4 +- lib/sandbox/vercel/sandbox/VercelSandbox.ts | 20 +++++++ 10 files changed, 95 insertions(+), 72 deletions(-) diff --git a/lib/sandbox/__tests__/createSandbox.test.ts b/lib/sandbox/__tests__/createSandbox.test.ts index b80dbc44b..f7835fb66 100644 --- a/lib/sandbox/__tests__/createSandbox.test.ts +++ b/lib/sandbox/__tests__/createSandbox.test.ts @@ -1,17 +1,17 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { createSandbox } from "../createSandbox"; -import { Sandbox } from "@vercel/sandbox"; +import { VercelSandbox } from "../vercel"; const mockSandbox = { name: "sbx_test123", - status: "running", + sdkStatus: "running", timeout: 1800000, createdAt: new Date("2024-01-01T00:00:00Z"), }; -vi.mock("@vercel/sandbox", () => ({ - Sandbox: { +vi.mock("../vercel", () => ({ + VercelSandbox: { create: vi.fn(() => Promise.resolve(mockSandbox)), }, })); @@ -28,21 +28,21 @@ describe("createSandbox", () => { vi.clearAllMocks(); }); - it("creates sandbox with default configuration when no params provided", async () => { + it("creates sandbox with default configuration when no config provided", async () => { await createSandbox(); - expect(Sandbox.create).toHaveBeenCalledWith({ - resources: { vcpus: 4 }, + expect(VercelSandbox.create).toHaveBeenCalledWith({ + vcpus: 4, timeout: 1800000, runtime: "node22", }); }); - it("creates sandbox from snapshot when source is provided", async () => { - await createSandbox({ source: { type: "snapshot", snapshotId: "snap_abc123" } }); + it("restores from snapshot when restoreSnapshotId is provided", async () => { + await createSandbox({ restoreSnapshotId: "snap_abc123" }); - expect(Sandbox.create).toHaveBeenCalledWith({ - source: { type: "snapshot", snapshotId: "snap_abc123" }, + expect(VercelSandbox.create).toHaveBeenCalledWith({ + restoreSnapshotId: "snap_abc123", timeout: 1800000, }); }); @@ -50,18 +50,18 @@ describe("createSandbox", () => { it("allows overriding default timeout", async () => { await createSandbox({ timeout: 300000 }); - expect(Sandbox.create).toHaveBeenCalledWith({ - resources: { vcpus: 4 }, + expect(VercelSandbox.create).toHaveBeenCalledWith({ + vcpus: 4, timeout: 300000, runtime: "node22", }); }); - it("allows overriding default resources", async () => { - await createSandbox({ resources: { vcpus: 2 } }); + it("allows overriding default vcpus", async () => { + await createSandbox({ vcpus: 2 }); - expect(Sandbox.create).toHaveBeenCalledWith({ - resources: { vcpus: 2 }, + expect(VercelSandbox.create).toHaveBeenCalledWith({ + vcpus: 2, timeout: 1800000, runtime: "node22", }); @@ -84,7 +84,9 @@ describe("createSandbox", () => { ...mockSandbox, stop: vi.fn(), }; - vi.mocked(Sandbox.create).mockResolvedValue(mockSandboxWithStop as unknown as Sandbox); + vi.mocked(VercelSandbox.create).mockResolvedValue( + mockSandboxWithStop as unknown as VercelSandbox, + ); await createSandbox(); diff --git a/lib/sandbox/__tests__/createSandboxWithFallback.test.ts b/lib/sandbox/__tests__/createSandboxWithFallback.test.ts index 3af6d1fed..badc56b51 100644 --- a/lib/sandbox/__tests__/createSandboxWithFallback.test.ts +++ b/lib/sandbox/__tests__/createSandboxWithFallback.test.ts @@ -27,7 +27,7 @@ describe("createSandboxWithFallback", () => { const result = await createSandboxWithFallback("snap_abc"); expect(mockCreateSandbox).toHaveBeenCalledWith({ - source: { type: "snapshot", snapshotId: "snap_abc" }, + restoreSnapshotId: "snap_abc", }); expect(result).toEqual({ ...mockCreateResult, fromSnapshot: true }); }); diff --git a/lib/sandbox/__tests__/getActiveSandbox.test.ts b/lib/sandbox/__tests__/getActiveSandbox.test.ts index 0f6ca2e6f..db36ca821 100644 --- a/lib/sandbox/__tests__/getActiveSandbox.test.ts +++ b/lib/sandbox/__tests__/getActiveSandbox.test.ts @@ -1,13 +1,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { Sandbox } from "@vercel/sandbox"; +import { VercelSandbox } from "@/lib/sandbox/vercel"; import { getActiveSandbox } from "../getActiveSandbox"; const mockSelectAccountSandboxes = vi.fn(); -vi.mock("@vercel/sandbox", () => ({ - Sandbox: { - get: vi.fn(), +vi.mock("@/lib/sandbox/vercel", () => ({ + VercelSandbox: { + connect: vi.fn(), }, })); @@ -25,17 +25,16 @@ describe("getActiveSandbox", () => { const mockSandbox = { name: "sbx_123", - status: "running", - runCommand: vi.fn(), + sdkStatus: "running", }; - vi.mocked(Sandbox.get).mockResolvedValue(mockSandbox as unknown as Sandbox); + vi.mocked(VercelSandbox.connect).mockResolvedValue(mockSandbox as unknown as VercelSandbox); const result = await getActiveSandbox("acc_1"); expect(mockSelectAccountSandboxes).toHaveBeenCalledWith({ accountIds: ["acc_1"], }); - expect(Sandbox.get).toHaveBeenCalledWith({ name: "sbx_123" }); + expect(VercelSandbox.connect).toHaveBeenCalledWith("sbx_123", {}); expect(result).toBe(mockSandbox); }); @@ -45,7 +44,7 @@ describe("getActiveSandbox", () => { const result = await getActiveSandbox("acc_1"); expect(result).toBeNull(); - expect(Sandbox.get).not.toHaveBeenCalled(); + expect(VercelSandbox.connect).not.toHaveBeenCalled(); }); it("returns null when sandbox is not running", async () => { @@ -55,21 +54,21 @@ describe("getActiveSandbox", () => { const mockSandbox = { name: "sbx_stopped", - status: "stopped", + sdkStatus: "stopped", }; - vi.mocked(Sandbox.get).mockResolvedValue(mockSandbox as unknown as Sandbox); + vi.mocked(VercelSandbox.connect).mockResolvedValue(mockSandbox as unknown as VercelSandbox); const result = await getActiveSandbox("acc_1"); expect(result).toBeNull(); }); - it("returns null when Sandbox.get throws", async () => { + it("returns null when VercelSandbox.connect throws", async () => { mockSelectAccountSandboxes.mockResolvedValue([ { sandbox_id: "sbx_expired", account_id: "acc_1" }, ]); - vi.mocked(Sandbox.get).mockRejectedValue(new Error("Sandbox not found")); + vi.mocked(VercelSandbox.connect).mockRejectedValue(new Error("Sandbox not found")); const result = await getActiveSandbox("acc_1"); diff --git a/lib/sandbox/__tests__/processCreateSandbox.test.ts b/lib/sandbox/__tests__/processCreateSandbox.test.ts index 160cf287c..789b0a998 100644 --- a/lib/sandbox/__tests__/processCreateSandbox.test.ts +++ b/lib/sandbox/__tests__/processCreateSandbox.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import type { Sandbox } from "@vercel/sandbox"; +import type { VercelSandbox } from "@/lib/sandbox/vercel"; import { processCreateSandbox } from "../processCreateSandbox"; import { createSandboxFromSnapshot } from "@/lib/sandbox/createSandboxFromSnapshot"; @@ -15,10 +15,10 @@ vi.mock("@/lib/trigger/triggerPromptSandbox", () => ({ const mockSandbox = { name: "sbx_123", - status: "running", + sdkStatus: "running", timeout: 600000, createdAt: new Date("2024-01-01T00:00:00.000Z"), -} as unknown as Sandbox; +} as unknown as VercelSandbox; describe("processCreateSandbox", () => { beforeEach(() => { diff --git a/lib/sandbox/createSandbox.ts b/lib/sandbox/createSandbox.ts index d6c732128..868aafdd4 100644 --- a/lib/sandbox/createSandbox.ts +++ b/lib/sandbox/createSandbox.ts @@ -1,53 +1,54 @@ import ms from "ms"; -import { Sandbox } from "@vercel/sandbox"; +import { VercelSandbox, type VercelSandboxConfig } from "@/lib/sandbox/vercel"; export interface SandboxCreatedResponse { - sandboxId: Sandbox["name"]; - sandboxStatus: Sandbox["status"]; - timeout: Sandbox["timeout"]; + sandboxId: VercelSandbox["name"]; + sandboxStatus: string; + timeout: VercelSandbox["timeout"]; createdAt: string; } export interface SandboxCreateResult { - sandbox: Sandbox; + sandbox: VercelSandbox; response: SandboxCreatedResponse; } -/** Extract CreateSandboxParams from Sandbox.create method signature */ -export type CreateSandboxParams = NonNullable[0]>; +/** Parameters for the api-side createSandbox helper. Wraps the abstraction's + * VercelSandboxConfig so callers do not need to import it directly. */ +export type CreateSandboxParams = VercelSandboxConfig; const DEFAULT_TIMEOUT = ms("30m"); const DEFAULT_VCPUS = 4; -const DEFAULT_RUNTIME = "node22"; +const DEFAULT_RUNTIME = "node22" as const; /** - * Creates a Vercel Sandbox and returns its info. + * Creates a Vercel Sandbox via the open-agents abstraction and returns + * its info. The sandbox is left running so subsequent prompts can run + * against it. * - * The sandbox is left running so that prompts can be executed via the prompt_sandbox tool. - * Accepts the same parameters as Sandbox.create from @vercel/sandbox. + * Restores from a snapshot when `restoreSnapshotId` is provided — + * snapshots already encode resources/runtime so we skip our defaults + * in that case. * - * @param params - Sandbox creation parameters (source, timeout, resources, runtime, ports) - * @returns The sandbox creation response + * @param config - VercelSandboxConfig (timeout, vcpus, runtime, + * restoreSnapshotId, source, ports, env, etc.) + * @returns The sandbox creation result (instance + response shape) * @throws Error if sandbox creation fails */ export async function createSandbox( - params: CreateSandboxParams = {}, + config: CreateSandboxParams = {}, ): Promise { - const hasSnapshotSource = - params.source && "type" in params.source && params.source.type === "snapshot"; - - // Pass params directly to SDK - it handles all the type variants - const sandbox = await Sandbox.create( - hasSnapshotSource + const sandbox = await VercelSandbox.create( + config.restoreSnapshotId ? { - ...params, - timeout: params.timeout ?? DEFAULT_TIMEOUT, + ...config, + timeout: config.timeout ?? DEFAULT_TIMEOUT, } : { - resources: { vcpus: DEFAULT_VCPUS }, - timeout: params.timeout ?? DEFAULT_TIMEOUT, + vcpus: DEFAULT_VCPUS, + timeout: config.timeout ?? DEFAULT_TIMEOUT, runtime: DEFAULT_RUNTIME, - ...params, + ...config, }, ); @@ -55,9 +56,9 @@ export async function createSandbox( sandbox, response: { sandboxId: sandbox.name, - sandboxStatus: sandbox.status, + sandboxStatus: sandbox.sdkStatus, timeout: sandbox.timeout, - createdAt: sandbox.createdAt.toISOString(), + createdAt: sandbox.createdAt?.toISOString() ?? new Date().toISOString(), }, }; } diff --git a/lib/sandbox/createSandboxFromSnapshot.ts b/lib/sandbox/createSandboxFromSnapshot.ts index 2dcfb7ef4..b1132a4b6 100644 --- a/lib/sandbox/createSandboxFromSnapshot.ts +++ b/lib/sandbox/createSandboxFromSnapshot.ts @@ -1,10 +1,10 @@ -import type { Sandbox } from "@vercel/sandbox"; +import type { VercelSandbox } from "@/lib/sandbox/vercel"; import { createSandboxWithFallback } from "@/lib/sandbox/createSandboxWithFallback"; import { getValidSnapshotId } from "@/lib/sandbox/getValidSnapshotId"; import { insertAccountSandbox } from "@/lib/supabase/account_sandboxes/insertAccountSandbox"; export interface CreateSandboxFromSnapshotResult { - sandbox: Sandbox; + sandbox: VercelSandbox; fromSnapshot: boolean; } diff --git a/lib/sandbox/createSandboxWithFallback.ts b/lib/sandbox/createSandboxWithFallback.ts index 93014ac43..d728485c5 100644 --- a/lib/sandbox/createSandboxWithFallback.ts +++ b/lib/sandbox/createSandboxWithFallback.ts @@ -14,7 +14,7 @@ export async function createSandboxWithFallback( ): Promise { if (snapshotId) { try { - const result = await createSandbox({ source: { type: "snapshot", snapshotId } }); + const result = await createSandbox({ restoreSnapshotId: snapshotId }); return { ...result, fromSnapshot: true }; } catch (error) { console.error("Snapshot sandbox creation failed, falling back to fresh sandbox:", error); diff --git a/lib/sandbox/getActiveSandbox.ts b/lib/sandbox/getActiveSandbox.ts index f47dec4f6..c0ba2796f 100644 --- a/lib/sandbox/getActiveSandbox.ts +++ b/lib/sandbox/getActiveSandbox.ts @@ -1,13 +1,14 @@ -import { Sandbox } from "@vercel/sandbox"; +import { VercelSandbox } from "@/lib/sandbox/vercel"; import { selectAccountSandboxes } from "@/lib/supabase/account_sandboxes/selectAccountSandboxes"; /** * Finds the most recent sandbox for an account and returns it if still running. + * Reconnects via the open-agents sandbox abstraction. * * @param accountId - The account ID to find an active sandbox for - * @returns The running Sandbox instance, or null if none found + * @returns The running VercelSandbox instance, or null if none found */ -export async function getActiveSandbox(accountId: string): Promise { +export async function getActiveSandbox(accountId: string): Promise { const sandboxes = await selectAccountSandboxes({ accountIds: [accountId], }); @@ -19,9 +20,9 @@ export async function getActiveSandbox(accountId: string): Promise Date: Mon, 4 May 2026 08:32:48 -0500 Subject: [PATCH 2/2] fix: address PR #509 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real issues from CodeRabbit + cubic: 1. createdAt staleness (CodeRabbit minor) The new `createdAt` getter on VercelSandbox skipped the `refreshStateFromCurrentSession()` step that `sdkStatus` uses, so readers right after a reconnect could see stale session metadata. Add the refresh. 2. Fabricated createdAt (cubic P2) Both createSandbox.ts and processCreateSandbox.ts had a `?? new Date().toISOString()` fallback that fabricated creation metadata when sandbox.createdAt was missing. The SDK guarantees createdAt is populated for any reachable instance, so the fallback was both wrong (fabricates data) and unnecessary. Tighten the getter to return `Date` (not `Date | undefined`) and throw with an explicit "SDK contract violation" message if the field is missing — fail-fast surfaces a real contract bug instead of silently lying. Drop the `?? new Date()` fallbacks at both call sites. 3. Misleading snapshot-restore branching (CodeRabbit major) createSandbox.ts had two paths — a "snapshot" branch that omitted DEFAULT_VCPUS/DEFAULT_RUNTIME (intent: let snapshot dictate), and a "fresh" branch that applied defaults. But VercelSandbox.create internally defaults vcpus=4 and runtime="node22" regardless, so the omission was a no-op — the abstraction always forwarded those to the SDK. Drop the misleading branching. Document the actual behavior at the top of createSandbox: "VercelSandbox.create applies its own defaults regardless of source — those apply to the runtime resources of the new sandbox even when restoring from a snapshot." Updated the snapshot-restore test to assert the actual call shape (vcpus + runtime + timeout + restoreSnapshotId) instead of just the original SDK-style truncated args. Verification: - pnpm lint:check: clean - pnpm test: 2391/2391 pass Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/sandbox/__tests__/createSandbox.test.ts | 4 ++- lib/sandbox/createSandbox.ts | 29 +++++++++------------ lib/sandbox/processCreateSandbox.ts | 2 +- lib/sandbox/vercel/sandbox/VercelSandbox.ts | 13 ++++++--- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/sandbox/__tests__/createSandbox.test.ts b/lib/sandbox/__tests__/createSandbox.test.ts index f7835fb66..83feae2b7 100644 --- a/lib/sandbox/__tests__/createSandbox.test.ts +++ b/lib/sandbox/__tests__/createSandbox.test.ts @@ -42,8 +42,10 @@ describe("createSandbox", () => { await createSandbox({ restoreSnapshotId: "snap_abc123" }); expect(VercelSandbox.create).toHaveBeenCalledWith({ - restoreSnapshotId: "snap_abc123", + vcpus: 4, + runtime: "node22", timeout: 1800000, + restoreSnapshotId: "snap_abc123", }); }); diff --git a/lib/sandbox/createSandbox.ts b/lib/sandbox/createSandbox.ts index 868aafdd4..cc813cf98 100644 --- a/lib/sandbox/createSandbox.ts +++ b/lib/sandbox/createSandbox.ts @@ -26,9 +26,11 @@ const DEFAULT_RUNTIME = "node22" as const; * its info. The sandbox is left running so subsequent prompts can run * against it. * - * Restores from a snapshot when `restoreSnapshotId` is provided — - * snapshots already encode resources/runtime so we skip our defaults - * in that case. + * Note: VercelSandbox.create applies its own defaults for vcpus and + * runtime (vcpus=4, runtime="node22") regardless of source — those + * apply to the runtime resources of the new sandbox even when restoring + * from a snapshot. We pass our preferred defaults explicitly so api's + * intent is documented at the call site. * * @param config - VercelSandboxConfig (timeout, vcpus, runtime, * restoreSnapshotId, source, ports, env, etc.) @@ -38,19 +40,12 @@ const DEFAULT_RUNTIME = "node22" as const; export async function createSandbox( config: CreateSandboxParams = {}, ): Promise { - const sandbox = await VercelSandbox.create( - config.restoreSnapshotId - ? { - ...config, - timeout: config.timeout ?? DEFAULT_TIMEOUT, - } - : { - vcpus: DEFAULT_VCPUS, - timeout: config.timeout ?? DEFAULT_TIMEOUT, - runtime: DEFAULT_RUNTIME, - ...config, - }, - ); + const sandbox = await VercelSandbox.create({ + vcpus: DEFAULT_VCPUS, + runtime: DEFAULT_RUNTIME, + timeout: DEFAULT_TIMEOUT, + ...config, + }); return { sandbox, @@ -58,7 +53,7 @@ export async function createSandbox( sandboxId: sandbox.name, sandboxStatus: sandbox.sdkStatus, timeout: sandbox.timeout, - createdAt: sandbox.createdAt?.toISOString() ?? new Date().toISOString(), + createdAt: sandbox.createdAt.toISOString(), }, }; } diff --git a/lib/sandbox/processCreateSandbox.ts b/lib/sandbox/processCreateSandbox.ts index 40e411943..e3cd2a71c 100644 --- a/lib/sandbox/processCreateSandbox.ts +++ b/lib/sandbox/processCreateSandbox.ts @@ -26,7 +26,7 @@ export async function processCreateSandbox( sandboxId: sandbox.name, sandboxStatus: sandbox.sdkStatus, timeout: sandbox.timeout, - createdAt: sandbox.createdAt?.toISOString() ?? new Date().toISOString(), + createdAt: sandbox.createdAt.toISOString(), }; // Trigger the prompt execution task if a prompt was provided diff --git a/lib/sandbox/vercel/sandbox/VercelSandbox.ts b/lib/sandbox/vercel/sandbox/VercelSandbox.ts index 0674f0813..61db65676 100644 --- a/lib/sandbox/vercel/sandbox/VercelSandbox.ts +++ b/lib/sandbox/vercel/sandbox/VercelSandbox.ts @@ -974,11 +974,16 @@ ${hostLine}${portLines}${runtimeEnvLine}`; /** * Timestamp when the underlying SDK sandbox was created. Sourced from - * the SDK session's createdAt field. Returns `undefined` if the SDK has - * not yet populated it (rare — should be set immediately after create). + * the SDK session's createdAt field. The SDK populates this on create + * and connect, so it is always defined for any reachable instance — + * we throw rather than fabricate a fallback if the contract is broken. */ - get createdAt(): Date | undefined { - return this.session.createdAt ?? undefined; + get createdAt(): Date { + this.refreshStateFromCurrentSession(); + if (!this.session.createdAt) { + throw new Error("VercelSandbox session is missing createdAt — SDK contract violation"); + } + return this.session.createdAt; } /**