From 65927e2de652f22bc91838237052fe73060ae5fd Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 7 May 2026 08:24:18 -0500 Subject: [PATCH 1/4] feat(sandbox): port POST /api/sandbox + GET /api/sandbox/status from open-agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the two session-scoped sandbox endpoints required to drive the chat "loading sandbox..." UX on session entry — matching the contract documented in recoupable/docs#192 (now merged on main). POST /api/sandbox provisions or resumes a Sandbox via the abstraction inlined in #507. When sessionId is supplied, the deterministic sandboxName ensures resume idempotency and the resolved sandbox state is persisted onto the session row (sandbox_state, lifecycle_state = "active", lifecycle_version bumped, sandbox_expires_at, last_activity_at) so subsequent GET /api/sandbox/status calls report the sandbox as active. GET /api/sandbox/status is DB-only — reads the session row, computes status as "active" when sandbox_state is set and not expired (10s buffer to match open-agents), otherwise "no_sandbox". hasSnapshot is true when snapshot_url is set. Mirrors the lifecycle envelope shape from open-agents so the frontend cutover is byte-identical. Files follow existing api conventions: - Route shells in app/api/sandbox/ delegate to handlers in lib/sandbox/ - Auth via validateAuthContext (Privy Bearer or x-api-key) - Validation via Zod (validateCreateSandboxBody) - Supabase ops in lib/supabase/sessions/ (one fn per file) - Error envelope { status: "error", error } matches sessions PRs TDD red → green: - 7 new test files added covering validator, helper, Supabase wrapper, both handlers, and the two route shells - 30 new tests, all passing (was 2461, now 2491) - pnpm lint:check clean Out of scope (deferred to follow-up PRs): - Org-snapshot lookup / kickBuildOrgSnapshotWorkflow (cold-start opt) - Skill installation (installSessionGlobalSkills) - Lifecycle workflow kick (no workflow infra in api yet) - DELETE /api/sandbox + PUT /api/sandbox/snapshot (no UI callers identified during the open-agents grep audit) - /api/sandbox/{extend,activity,reconnect,snapshot} sub-routes — to follow once these two land and the chat UX is validated Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/sandbox/__tests__/route.test.ts | 19 +++ app/api/sandbox/route.ts | 29 ++++ .../sandbox/status/__tests__/route.test.ts | 21 +++ app/api/sandbox/status/route.ts | 29 ++++ .../__tests__/createSandboxHandler.test.ts | 156 ++++++++++++++++++ .../__tests__/getSandboxStatusHandler.test.ts | 138 ++++++++++++++++ .../__tests__/getSessionSandboxName.test.ts | 13 ++ .../validateCreateSandboxBody.test.ts | 103 ++++++++++++ lib/sandbox/createSandboxHandler.ts | 127 ++++++++++++++ lib/sandbox/getSandboxStatusHandler.ts | 80 +++++++++ lib/sandbox/getSessionSandboxName.ts | 14 ++ lib/sandbox/validateCreateSandboxBody.ts | 54 ++++++ .../updateSessionSandboxState.test.ts | 82 +++++++++ .../sessions/updateSessionSandboxState.ts | 46 ++++++ 14 files changed, 911 insertions(+) create mode 100644 app/api/sandbox/__tests__/route.test.ts create mode 100644 app/api/sandbox/route.ts create mode 100644 app/api/sandbox/status/__tests__/route.test.ts create mode 100644 app/api/sandbox/status/route.ts create mode 100644 lib/sandbox/__tests__/createSandboxHandler.test.ts create mode 100644 lib/sandbox/__tests__/getSandboxStatusHandler.test.ts create mode 100644 lib/sandbox/__tests__/getSessionSandboxName.test.ts create mode 100644 lib/sandbox/__tests__/validateCreateSandboxBody.test.ts create mode 100644 lib/sandbox/createSandboxHandler.ts create mode 100644 lib/sandbox/getSandboxStatusHandler.ts create mode 100644 lib/sandbox/getSessionSandboxName.ts create mode 100644 lib/sandbox/validateCreateSandboxBody.ts create mode 100644 lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts create mode 100644 lib/supabase/sessions/updateSessionSandboxState.ts diff --git a/app/api/sandbox/__tests__/route.test.ts b/app/api/sandbox/__tests__/route.test.ts new file mode 100644 index 000000000..e8b7235ab --- /dev/null +++ b/app/api/sandbox/__tests__/route.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect, vi } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { POST } from "@/app/api/sandbox/route"; +import { createSandboxHandler } from "@/lib/sandbox/createSandboxHandler"; + +vi.mock("@/lib/sandbox/createSandboxHandler", () => ({ + createSandboxHandler: vi.fn(async () => NextResponse.json({ ok: true }, { status: 200 })), +})); + +describe("POST /api/sandbox route shell", () => { + it("delegates to createSandboxHandler", async () => { + const req = new NextRequest("http://localhost/api/sandbox", { method: "POST" }); + const res = await POST(req); + + expect(createSandboxHandler).toHaveBeenCalledWith(req); + expect(res.status).toBe(200); + }); +}); diff --git a/app/api/sandbox/route.ts b/app/api/sandbox/route.ts new file mode 100644 index 000000000..01fb45e0c --- /dev/null +++ b/app/api/sandbox/route.ts @@ -0,0 +1,29 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { createSandboxHandler } from "@/lib/sandbox/createSandboxHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 204, + headers: getCorsHeaders(), + }); +} + +/** + * `POST /api/sandbox` — provision (or resume) a Sandbox bound to a session. + * + * @param request - The incoming request. + * @returns A NextResponse with `{ createdAt, timeout, currentBranch, mode, timing }` on 200, or an error. + */ +export async function POST(request: NextRequest) { + return createSandboxHandler(request); +} + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; diff --git a/app/api/sandbox/status/__tests__/route.test.ts b/app/api/sandbox/status/__tests__/route.test.ts new file mode 100644 index 000000000..ca9956776 --- /dev/null +++ b/app/api/sandbox/status/__tests__/route.test.ts @@ -0,0 +1,21 @@ +import { describe, it, expect, vi } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { GET } from "@/app/api/sandbox/status/route"; +import { getSandboxStatusHandler } from "@/lib/sandbox/getSandboxStatusHandler"; + +vi.mock("@/lib/sandbox/getSandboxStatusHandler", () => ({ + getSandboxStatusHandler: vi.fn(async () => NextResponse.json({ ok: true }, { status: 200 })), +})); + +describe("GET /api/sandbox/status route shell", () => { + it("delegates to getSandboxStatusHandler", async () => { + const req = new NextRequest("http://localhost/api/sandbox/status?sessionId=s", { + method: "GET", + }); + const res = await GET(req); + + expect(getSandboxStatusHandler).toHaveBeenCalledWith(req); + expect(res.status).toBe(200); + }); +}); diff --git a/app/api/sandbox/status/route.ts b/app/api/sandbox/status/route.ts new file mode 100644 index 000000000..4a062b663 --- /dev/null +++ b/app/api/sandbox/status/route.ts @@ -0,0 +1,29 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { getSandboxStatusHandler } from "@/lib/sandbox/getSandboxStatusHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 204, + headers: getCorsHeaders(), + }); +} + +/** + * `GET /api/sandbox/status?sessionId=...` — current lifecycle/runtime state for the sandbox bound to a session. + * + * @param request - The incoming request. + * @returns A NextResponse with `{ status, hasSnapshot, lifecycleVersion, lifecycle }` on 200, or an error. + */ +export async function GET(request: NextRequest) { + return getSandboxStatusHandler(request); +} + +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; +export const revalidate = 0; diff --git a/lib/sandbox/__tests__/createSandboxHandler.test.ts b/lib/sandbox/__tests__/createSandboxHandler.test.ts new file mode 100644 index 000000000..8cebb3f15 --- /dev/null +++ b/lib/sandbox/__tests__/createSandboxHandler.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { createSandboxHandler } from "@/lib/sandbox/createSandboxHandler"; +import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBody"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { connectSandbox } from "@/lib/sandbox/factory"; +import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/sandbox/validateCreateSandboxBody", () => ({ + validateCreateSandboxBody: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); +vi.mock("@/lib/sandbox/factory", () => ({ + connectSandbox: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/updateSessionSandboxState", () => ({ + updateSessionSandboxState: vi.fn(), +})); + +const ACCOUNT_ID = "acc-1"; + +function makeReq(): NextRequest { + return new NextRequest("http://localhost/api/sandbox", { method: "POST" }); +} + +function fakeSandbox(overrides: Partial> = {}) { + return { + timeout: 1_800_000, + expiresAt: Date.parse("2030-01-01T00:00:00.000Z"), + getState: () => ({ type: "vercel", sandboxName: "session-sess-1" }), + ...overrides, + }; +} + +describe("createSandboxHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(validateCreateSandboxBody).mockResolvedValue({ + body: { + repoUrl: "https://github.com/o/r", + sessionId: "sess-1", + branch: "main", + }, + auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, + }); + vi.mocked(selectSessions).mockResolvedValue([{ id: "sess-1", account_id: ACCOUNT_ID } as any]); + vi.mocked(connectSandbox).mockResolvedValue( + fakeSandbox() as unknown as Awaited>, + ); + vi.mocked(updateSessionSandboxState).mockResolvedValue({} as any); + }); + + it("short-circuits with the validator's response on validation failure", async () => { + const fail = NextResponse.json({ status: "error", error: "bad" }, { status: 400 }); + vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce(fail); + + const res = await createSandboxHandler(makeReq()); + + expect(res).toBe(fail); + expect(connectSandbox).not.toHaveBeenCalled(); + }); + + it("returns 404 when sessionId is provided but the session does not exist", async () => { + vi.mocked(selectSessions).mockResolvedValueOnce([]); + + const res = await createSandboxHandler(makeReq()); + + expect(res.status).toBe(404); + expect(connectSandbox).not.toHaveBeenCalled(); + }); + + it("returns 403 when the session is not owned by the authenticated account", async () => { + vi.mocked(selectSessions).mockResolvedValueOnce([ + { id: "sess-1", account_id: "someone-else" } as any, + ]); + + const res = await createSandboxHandler(makeReq()); + + expect(res.status).toBe(403); + expect(connectSandbox).not.toHaveBeenCalled(); + }); + + it("returns 502 when the sandbox provider throws", async () => { + vi.mocked(connectSandbox).mockRejectedValueOnce(new Error("vercel down")); + + const res = await createSandboxHandler(makeReq()); + + expect(res.status).toBe(502); + }); + + it("returns 200 with the documented response shape on success", async () => { + const res = await createSandboxHandler(makeReq()); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toMatchObject({ + timeout: 1_800_000, + currentBranch: "main", + mode: "vercel", + }); + expect(typeof body.createdAt).toBe("number"); + expect(typeof body.timing.readyMs).toBe("number"); + }); + + it("persists sandbox state to the session row when sessionId is provided", async () => { + await createSandboxHandler(makeReq()); + + expect(updateSessionSandboxState).toHaveBeenCalledWith( + expect.objectContaining({ + id: "sess-1", + sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, + }), + ); + }); + + it("skips the session-row write when no sessionId is provided", async () => { + vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce({ + body: { repoUrl: "https://github.com/o/r", branch: "main" }, + auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, + }); + + const res = await createSandboxHandler(makeReq()); + + expect(res.status).toBe(200); + expect(updateSessionSandboxState).not.toHaveBeenCalled(); + expect(selectSessions).not.toHaveBeenCalled(); + }); + + it("uses isNewBranch=true to flip source.branch into source.newBranch", async () => { + vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce({ + body: { + repoUrl: "https://github.com/o/r", + sessionId: "sess-1", + branch: "feat/x", + isNewBranch: true, + }, + auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, + }); + + await createSandboxHandler(makeReq()); + + const arg = vi.mocked(connectSandbox).mock.calls[0]?.[0]; + expect(arg).toBeDefined(); + if (!arg || !("state" in arg)) throw new Error("expected new-API config shape"); + + const source = (arg.state as any).source; + expect(source.newBranch).toBe("feat/x"); + expect(source.branch).toBeUndefined(); + }); +}); diff --git a/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts b/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts new file mode 100644 index 000000000..ce0730cfe --- /dev/null +++ b/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts @@ -0,0 +1,138 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { getSandboxStatusHandler } from "@/lib/sandbox/getSandboxStatusHandler"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); +vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ + selectSessions: vi.fn(), +})); + +const ACCOUNT_ID = "acc-1"; +const FAR_FUTURE = "2099-01-01T00:00:00.000Z"; +const FAR_PAST = "2000-01-01T00:00:00.000Z"; + +function makeReq(query = "?sessionId=sess-1"): NextRequest { + return new NextRequest(`http://localhost/api/sandbox/status${query}`, { + method: "GET", + }); +} + +const baseRow = { + id: "sess-1", + account_id: ACCOUNT_ID, + sandbox_state: null as unknown, + lifecycle_state: null as string | null, + lifecycle_version: 0, + sandbox_expires_at: null as string | null, + hibernate_after: null as string | null, + last_activity_at: null as string | null, + snapshot_url: null as string | null, +}; + +describe("getSandboxStatusHandler", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: ACCOUNT_ID, + orgId: null, + authToken: "k", + }); + }); + + it("returns the auth response unchanged when auth fails", async () => { + const fail = NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValueOnce(fail); + + const res = await getSandboxStatusHandler(makeReq()); + + expect(res).toBe(fail); + }); + + it("returns 400 when sessionId is missing from the query string", async () => { + const res = await getSandboxStatusHandler(makeReq("")); + + expect(res.status).toBe(400); + }); + + it("returns 404 when no session exists with the given id", async () => { + vi.mocked(selectSessions).mockResolvedValue([]); + + const res = await getSandboxStatusHandler(makeReq()); + + expect(res.status).toBe(404); + }); + + it("returns 403 when the session is not owned by the authenticated account", async () => { + vi.mocked(selectSessions).mockResolvedValue([ + { ...baseRow, account_id: "someone-else" } as any, + ]); + + const res = await getSandboxStatusHandler(makeReq()); + + expect(res.status).toBe(403); + }); + + it("returns status='active' when sandbox_state is set and not expired", async () => { + vi.mocked(selectSessions).mockResolvedValue([ + { + ...baseRow, + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + lifecycle_state: "active", + lifecycle_version: 3, + sandbox_expires_at: FAR_FUTURE, + } as any, + ]); + + const res = await getSandboxStatusHandler(makeReq()); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.status).toBe("active"); + expect(body.lifecycleVersion).toBe(3); + expect(body.lifecycle.state).toBe("active"); + expect(typeof body.lifecycle.serverTime).toBe("number"); + expect(body.lifecycle.sandboxExpiresAt).toBe(Date.parse(FAR_FUTURE)); + }); + + it("returns status='no_sandbox' when sandbox_state is null", async () => { + vi.mocked(selectSessions).mockResolvedValue([{ ...baseRow } as any]); + + const res = await getSandboxStatusHandler(makeReq()); + + const body = await res.json(); + expect(body.status).toBe("no_sandbox"); + expect(body.hasSnapshot).toBe(false); + }); + + it("returns status='no_sandbox' when sandbox is expired", async () => { + vi.mocked(selectSessions).mockResolvedValue([ + { + ...baseRow, + sandbox_state: { type: "vercel" }, + sandbox_expires_at: FAR_PAST, + } as any, + ]); + + const res = await getSandboxStatusHandler(makeReq()); + + const body = await res.json(); + expect(body.status).toBe("no_sandbox"); + }); + + it("returns hasSnapshot=true when snapshot_url is set", async () => { + vi.mocked(selectSessions).mockResolvedValue([{ ...baseRow, snapshot_url: "snap://x" } as any]); + + const res = await getSandboxStatusHandler(makeReq()); + + const body = await res.json(); + expect(body.hasSnapshot).toBe(true); + }); +}); diff --git a/lib/sandbox/__tests__/getSessionSandboxName.test.ts b/lib/sandbox/__tests__/getSessionSandboxName.test.ts new file mode 100644 index 000000000..99a642edb --- /dev/null +++ b/lib/sandbox/__tests__/getSessionSandboxName.test.ts @@ -0,0 +1,13 @@ +import { describe, it, expect } from "vitest"; +import { getSessionSandboxName } from "@/lib/sandbox/getSessionSandboxName"; + +describe("getSessionSandboxName", () => { + it("returns a deterministic name prefixed with 'session-'", () => { + expect(getSessionSandboxName("abc123")).toBe("session-abc123"); + }); + + it("returns the same value for the same input", () => { + const input = "uuid-style-id"; + expect(getSessionSandboxName(input)).toBe(getSessionSandboxName(input)); + }); +}); diff --git a/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts new file mode 100644 index 000000000..563b4ddd2 --- /dev/null +++ b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts @@ -0,0 +1,103 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBody"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), +})); +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: vi.fn(), +})); + +const ACCOUNT_ID = "acc-1"; + +function makeReq(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/sandbox", { + method: "POST", + headers: { "Content-Type": "application/json", "x-api-key": "k" }, + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} + +describe("validateCreateSandboxBody", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: ACCOUNT_ID, + orgId: null, + authToken: "k", + }); + }); + + it("returns the auth response unchanged when auth fails", async () => { + const failure = NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValueOnce(failure); + + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "x" })); + + expect(result).toBe(failure); + }); + + it("returns 400 when repoUrl is missing", async () => { + const result = await validateCreateSandboxBody(makeReq({})); + + expect(result).toBeInstanceOf(NextResponse); + const res = result as NextResponse; + expect(res.status).toBe(400); + const json = await res.json(); + expect(json.status).toBe("error"); + expect(json.missing_fields).toEqual(["repoUrl"]); + }); + + it("returns 400 when repoUrl is empty", async () => { + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "" })); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + + it("returns 400 when JSON body is malformed", async () => { + const result = await validateCreateSandboxBody(makeReq("not-json")); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + + it("returns the validated body + auth on a minimal happy path", async () => { + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "https://github.com/o/r" })); + + expect(result).not.toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) return; + expect(result.body.repoUrl).toBe("https://github.com/o/r"); + expect(result.body.sessionId).toBeUndefined(); + expect(result.body.branch).toBeUndefined(); + expect(result.body.isNewBranch).toBeUndefined(); + expect(result.auth.accountId).toBe(ACCOUNT_ID); + }); + + it("accepts a full request with sessionId, branch, isNewBranch", async () => { + const result = await validateCreateSandboxBody( + makeReq({ + repoUrl: "https://github.com/o/r", + sessionId: "sess-1", + branch: "feat/x", + isNewBranch: true, + }), + ); + + expect(result).not.toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) return; + expect(result.body.sessionId).toBe("sess-1"); + expect(result.body.branch).toBe("feat/x"); + expect(result.body.isNewBranch).toBe(true); + }); + + it("returns 400 when isNewBranch is the wrong type", async () => { + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "x", isNewBranch: "yes" })); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); +}); diff --git a/lib/sandbox/createSandboxHandler.ts b/lib/sandbox/createSandboxHandler.ts new file mode 100644 index 000000000..d081a82f7 --- /dev/null +++ b/lib/sandbox/createSandboxHandler.ts @@ -0,0 +1,127 @@ +import ms from "ms"; +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBody"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import { connectSandbox } from "@/lib/sandbox/factory"; +import { getSessionSandboxName } from "@/lib/sandbox/getSessionSandboxName"; +import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; +import type { Json } from "@/types/database.types"; + +const DEFAULT_TIMEOUT_MS = ms("30m"); +const DEFAULT_PORTS = [3000]; + +interface SourceConfig { + repo: string; + branch?: string; + newBranch?: string; +} + +function buildSource({ + repoUrl, + branch, + isNewBranch, +}: { + repoUrl: string; + branch?: string; + isNewBranch?: boolean; +}): SourceConfig { + if (isNewBranch && branch) { + return { repo: repoUrl, newBranch: branch }; + } + return branch ? { repo: repoUrl, branch } : { repo: repoUrl }; +} + +/** + * Handles `POST /api/sandbox`. Provisions a Sandbox bound to the given + * session (or a one-shot sandbox when no `sessionId` is supplied). When + * a session is bound, the resolved `sandboxState`, lifecycle, and + * expiry are written back to the `sessions` row so subsequent reads via + * `GET /api/sandbox/status` can report the sandbox as active. + */ +export async function createSandboxHandler(request: NextRequest): Promise { + const validated = await validateCreateSandboxBody(request); + if (validated instanceof NextResponse) { + return validated; + } + const { body, auth } = validated; + + const sessionId = body.sessionId; + + let currentLifecycleVersion = 0; + if (sessionId) { + const rows = await selectSessions({ id: sessionId }); + const row = rows[0]; + + if (!row) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (row.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + currentLifecycleVersion = row.lifecycle_version; + } + + const sandboxName = sessionId ? getSessionSandboxName(sessionId) : undefined; + const branch = body.branch ?? "main"; + const startTime = Date.now(); + + let sandbox; + try { + sandbox = await connectSandbox({ + state: { + type: "vercel", + ...(sandboxName ? { sandboxName } : {}), + source: buildSource({ + repoUrl: body.repoUrl, + branch: body.branch, + isNewBranch: body.isNewBranch, + }), + }, + options: { + timeout: DEFAULT_TIMEOUT_MS, + ports: DEFAULT_PORTS, + persistent: !!sandboxName, + resume: !!sandboxName, + createIfMissing: !!sandboxName, + }, + }); + } catch (error) { + console.error("[createSandboxHandler] connectSandbox failed:", error); + return NextResponse.json( + { status: "error", error: "Failed to provision sandbox" }, + { status: 502, headers: getCorsHeaders() }, + ); + } + + if (sessionId && sandbox.getState) { + const nextState = sandbox.getState() as Json; + const expiresAt = + typeof sandbox.expiresAt === "number" ? new Date(sandbox.expiresAt).toISOString() : null; + await updateSessionSandboxState({ + id: sessionId, + sandboxState: nextState, + sandboxExpiresAt: expiresAt, + lifecycleVersion: currentLifecycleVersion + 1, + }); + } + + return NextResponse.json( + { + createdAt: Date.now(), + timeout: sandbox.timeout ?? DEFAULT_TIMEOUT_MS, + currentBranch: branch, + mode: "vercel", + timing: { readyMs: Date.now() - startTime }, + }, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sandbox/getSandboxStatusHandler.ts b/lib/sandbox/getSandboxStatusHandler.ts new file mode 100644 index 000000000..a6cf19584 --- /dev/null +++ b/lib/sandbox/getSandboxStatusHandler.ts @@ -0,0 +1,80 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; +import type { Tables } from "@/types/database.types"; + +const SANDBOX_EXPIRES_BUFFER_MS = 10_000; + +function isoToEpochMs(value: string | null): number | null { + if (!value) return null; + const ms = Date.parse(value); + return Number.isFinite(ms) ? ms : null; +} + +function buildLifecycle(row: Tables<"sessions">) { + return { + serverTime: Date.now(), + state: row.lifecycle_state, + lastActivityAt: isoToEpochMs(row.last_activity_at), + hibernateAfter: isoToEpochMs(row.hibernate_after), + sandboxExpiresAt: isoToEpochMs(row.sandbox_expires_at), + }; +} + +function isSandboxActive(row: Tables<"sessions">): boolean { + if (!row.sandbox_state) return false; + const expiresAt = isoToEpochMs(row.sandbox_expires_at); + if (expiresAt === null) return true; + return Date.now() < expiresAt - SANDBOX_EXPIRES_BUFFER_MS; +} + +/** + * Handles `GET /api/sandbox/status`. Returns the current lifecycle and + * runtime state for the sandbox bound to a session — DB-only read, no + * upstream probe. Status is `"active"` when the session row carries a + * non-expired `sandbox_state`, otherwise `"no_sandbox"`. `hasSnapshot` + * is true when the row records a saved snapshot the UI can offer to + * resume. + */ +export async function getSandboxStatusHandler(request: NextRequest): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const sessionId = request.nextUrl.searchParams.get("sessionId"); + if (!sessionId) { + return NextResponse.json( + { status: "error", error: "Missing sessionId" }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + const rows = await selectSessions({ id: sessionId }); + const row = rows[0]; + + if (!row) { + return NextResponse.json( + { status: "error", error: "Session not found" }, + { status: 404, headers: getCorsHeaders() }, + ); + } + + if (row.account_id !== auth.accountId) { + return NextResponse.json( + { status: "error", error: "Forbidden" }, + { status: 403, headers: getCorsHeaders() }, + ); + } + + return NextResponse.json( + { + status: isSandboxActive(row) ? "active" : "no_sandbox", + hasSnapshot: !!row.snapshot_url, + lifecycleVersion: row.lifecycle_version, + lifecycle: buildLifecycle(row), + }, + { status: 200, headers: getCorsHeaders() }, + ); +} diff --git a/lib/sandbox/getSessionSandboxName.ts b/lib/sandbox/getSessionSandboxName.ts new file mode 100644 index 000000000..5c1f24ca0 --- /dev/null +++ b/lib/sandbox/getSessionSandboxName.ts @@ -0,0 +1,14 @@ +/** + * Deterministic Vercel Sandbox name for a given session id. + * + * The sandbox provider keys persistent sandboxes by name; deriving the + * name from the session id makes resume idempotent — calling the create + * endpoint twice for the same session will reconnect to the existing + * sandbox instead of provisioning a duplicate. + * + * @param sessionId - The owning session id. + * @returns The persistent sandbox name (e.g. `session-abc123`). + */ +export function getSessionSandboxName(sessionId: string): string { + return `session-${sessionId}`; +} diff --git a/lib/sandbox/validateCreateSandboxBody.ts b/lib/sandbox/validateCreateSandboxBody.ts new file mode 100644 index 000000000..03ccf60bc --- /dev/null +++ b/lib/sandbox/validateCreateSandboxBody.ts @@ -0,0 +1,54 @@ +import { NextRequest, NextResponse } from "next/server"; +import { z } from "zod"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { safeParseJson } from "@/lib/networking/safeParseJson"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import type { AuthContext } from "@/lib/auth/validateAuthContext"; + +export const createSandboxBodySchema = z.object({ + repoUrl: z.string({ message: "repoUrl is required" }).min(1, "repoUrl cannot be empty"), + sessionId: z.string().optional(), + branch: z.string().optional(), + isNewBranch: z.boolean().optional(), +}); + +export type CreateSandboxBody = z.infer; + +export interface ValidatedCreateSandboxRequest { + body: CreateSandboxBody; + auth: AuthContext; +} + +/** + * Validates a `POST /api/sandbox` request: authenticates the caller, + * tolerates malformed JSON (treated as an empty body), then enforces + * the Zod schema. Returns either the first 4xx response or the + * validated `{ body, auth }`. + */ +export async function validateCreateSandboxBody( + request: NextRequest, +): Promise { + const auth = await validateAuthContext(request); + if (auth instanceof NextResponse) { + return auth; + } + + const rawBody = await safeParseJson(request); + const result = createSandboxBodySchema.safeParse(rawBody); + if (!result.success) { + const firstError = result.error.issues[0]; + return NextResponse.json( + { + status: "error", + missing_fields: firstError.path, + error: firstError.message, + }, + { + status: 400, + headers: getCorsHeaders(), + }, + ); + } + + return { body: result.data, auth }; +} diff --git a/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts b/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts new file mode 100644 index 000000000..0f086ed5f --- /dev/null +++ b/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; + +const updateChain = vi.fn(); +const eqChain = vi.fn(); +const selectChain = vi.fn(); +const singleChain = vi.fn(); + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { + from: vi.fn(() => ({ + update: updateChain, + })), + }, +})); + +beforeEach(() => { + vi.clearAllMocks(); + updateChain.mockReturnValue({ eq: eqChain }); + eqChain.mockReturnValue({ select: selectChain }); + selectChain.mockReturnValue({ single: singleChain }); +}); + +describe("updateSessionSandboxState", () => { + const baseRow = { + id: "sess-1", + account_id: "acc-1", + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + lifecycle_state: "active", + lifecycle_version: 2, + sandbox_expires_at: "2030-01-01T00:00:00.000Z", + last_activity_at: "2030-01-01T00:00:00.000Z", + }; + + it("returns the updated row on success", async () => { + singleChain.mockResolvedValue({ data: baseRow, error: null }); + + const result = await updateSessionSandboxState({ + id: "sess-1", + sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, + sandboxExpiresAt: "2030-01-01T00:00:00.000Z", + lifecycleVersion: 2, + }); + + expect(result).toEqual(baseRow); + expect(updateChain).toHaveBeenCalledOnce(); + expect(eqChain).toHaveBeenCalledWith("id", "sess-1"); + }); + + it("returns null when supabase reports an error", async () => { + singleChain.mockResolvedValue({ data: null, error: { message: "db down" } }); + + const result = await updateSessionSandboxState({ + id: "sess-x", + sandboxState: null, + sandboxExpiresAt: null, + lifecycleVersion: 1, + }); + + expect(result).toBeNull(); + }); + + it("writes the supplied lifecycle fields onto the row", async () => { + singleChain.mockResolvedValue({ data: baseRow, error: null }); + + await updateSessionSandboxState({ + id: "sess-1", + sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, + sandboxExpiresAt: "2030-01-01T00:00:00.000Z", + lifecycleVersion: 5, + }); + + const updatePayload = updateChain.mock.calls[0]?.[0]; + expect(updatePayload).toMatchObject({ + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + lifecycle_state: "active", + lifecycle_version: 5, + sandbox_expires_at: "2030-01-01T00:00:00.000Z", + }); + expect(updatePayload.last_activity_at).toBeTypeOf("string"); + }); +}); diff --git a/lib/supabase/sessions/updateSessionSandboxState.ts b/lib/supabase/sessions/updateSessionSandboxState.ts new file mode 100644 index 000000000..12def75b4 --- /dev/null +++ b/lib/supabase/sessions/updateSessionSandboxState.ts @@ -0,0 +1,46 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Json, Tables } from "@/types/database.types"; + +interface UpdateSessionSandboxStateInput { + /** Owning session id. */ + id: string; + /** Persistable sandbox state from `sandbox.getState()`, or null to clear. */ + sandboxState: Json | null; + /** Sandbox expiry as ISO string, or null when not yet known. */ + sandboxExpiresAt: string | null; + /** Bumped lifecycle version (optimistic concurrency token). */ + lifecycleVersion: number; +} + +/** + * Writes sandbox runtime metadata onto a `sessions` row. Called when a + * sandbox has just been provisioned (or resumed) for a session — sets + * `lifecycle_state="active"` and refreshes `last_activity_at`. Returns + * the updated row, or null on Supabase error. + */ +export async function updateSessionSandboxState({ + id, + sandboxState, + sandboxExpiresAt, + lifecycleVersion, +}: UpdateSessionSandboxStateInput): Promise | null> { + const { data, error } = await supabase + .from("sessions") + .update({ + sandbox_state: sandboxState, + lifecycle_state: "active", + lifecycle_version: lifecycleVersion, + sandbox_expires_at: sandboxExpiresAt, + last_activity_at: new Date().toISOString(), + }) + .eq("id", id) + .select() + .single(); + + if (error) { + console.error("[updateSessionSandboxState] error:", error); + return null; + } + + return data; +} From f77e5c393d097fbce8f879aa2483ac4180195d30 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 7 May 2026 10:40:53 -0500 Subject: [PATCH 2/4] fix(sandbox): treat type-stub sandbox_state as no_sandbox in /status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Smoke test against the preview deployment caught a regression that defeated the entire loading-state UX this PR exists to enable: GET /api/sandbox/status reported `"active"` immediately after POST /api/sessions, before any sandbox had been provisioned. Root cause: POST /api/sessions (PR #515) inserts `sandbox_state` as the type stub `{ type: "vercel" }`. The previous `isSandboxActive` check `if (!row.sandbox_state) return false` saw a truthy object and fell through; with `sandbox_expires_at = null` (no expiry yet), the function returned true. Fix: introduce `hasRuntimeSandboxState(state)` that distinguishes the type stub from real runtime metadata by requiring a non-empty `sandboxName` (set by `getSessionSandboxName(sessionId)` in POST /api/sandbox and preserved by the abstraction's `getState()`). Mirrors open-agents' equivalent helper. TDD red → green: - Regression test pinned to the exact production scenario (sandbox_state = {type:"vercel"}, sandbox_expires_at = null, lifecycle_state = "provisioning") asserting status=no_sandbox - Companion test asserting status=active once sandboxName is set - 6 unit tests for the new helper covering null/undefined, scalars, type stub, populated state, and empty-string sandboxName edge case - Confirmed RED before implementing, GREEN after - Suite: 2491 → 2499 (+8 new tests), pnpm lint:check clean Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/getSandboxStatusHandler.test.ts | 38 +++++++++++++++++++ .../__tests__/hasRuntimeSandboxState.test.ts | 30 +++++++++++++++ lib/sandbox/getSandboxStatusHandler.ts | 7 +++- lib/sandbox/hasRuntimeSandboxState.ts | 24 ++++++++++++ 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 lib/sandbox/__tests__/hasRuntimeSandboxState.test.ts create mode 100644 lib/sandbox/hasRuntimeSandboxState.ts diff --git a/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts b/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts index ce0730cfe..2bf66ea01 100644 --- a/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts +++ b/lib/sandbox/__tests__/getSandboxStatusHandler.test.ts @@ -135,4 +135,42 @@ describe("getSandboxStatusHandler", () => { const body = await res.json(); expect(body.hasSnapshot).toBe(true); }); + + // Regression: see PR #522 smoke-test comment. POST /api/sessions writes + // sandbox_state: { type: "vercel" } as a type stub on insert. Before the + // fix, isSandboxActive treated any truthy sandbox_state + null + // sandbox_expires_at as active, so the chat loading UX would flip to + // "ready" the moment the session was created — before any sandbox + // existed. Status must report no_sandbox until real runtime metadata + // (sandboxName) is written by POST /api/sandbox. + it("returns status='no_sandbox' for the freshly-created-session type stub (no sandboxName, no expiry)", async () => { + vi.mocked(selectSessions).mockResolvedValue([ + { + ...baseRow, + sandbox_state: { type: "vercel" }, + sandbox_expires_at: null, + lifecycle_state: "provisioning", + } as any, + ]); + + const res = await getSandboxStatusHandler(makeReq()); + + const body = await res.json(); + expect(body.status).toBe("no_sandbox"); + }); + + it("returns status='active' once sandboxName is set on the state, even without explicit expiry", async () => { + vi.mocked(selectSessions).mockResolvedValue([ + { + ...baseRow, + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + sandbox_expires_at: null, + } as any, + ]); + + const res = await getSandboxStatusHandler(makeReq()); + + const body = await res.json(); + expect(body.status).toBe("active"); + }); }); diff --git a/lib/sandbox/__tests__/hasRuntimeSandboxState.test.ts b/lib/sandbox/__tests__/hasRuntimeSandboxState.test.ts new file mode 100644 index 000000000..6b4f18966 --- /dev/null +++ b/lib/sandbox/__tests__/hasRuntimeSandboxState.test.ts @@ -0,0 +1,30 @@ +import { describe, it, expect } from "vitest"; +import { hasRuntimeSandboxState } from "@/lib/sandbox/hasRuntimeSandboxState"; + +describe("hasRuntimeSandboxState", () => { + it("returns false for null", () => { + expect(hasRuntimeSandboxState(null)).toBe(false); + }); + + it("returns false for undefined", () => { + expect(hasRuntimeSandboxState(undefined)).toBe(false); + }); + + it("returns false for non-object scalars", () => { + expect(hasRuntimeSandboxState("vercel")).toBe(false); + expect(hasRuntimeSandboxState(42)).toBe(false); + expect(hasRuntimeSandboxState(true)).toBe(false); + }); + + it("returns false for the type-only stub written at session creation", () => { + expect(hasRuntimeSandboxState({ type: "vercel" })).toBe(false); + }); + + it("returns true when sandboxName is set", () => { + expect(hasRuntimeSandboxState({ type: "vercel", sandboxName: "session-x" })).toBe(true); + }); + + it("returns false when sandboxName is the empty string", () => { + expect(hasRuntimeSandboxState({ type: "vercel", sandboxName: "" })).toBe(false); + }); +}); diff --git a/lib/sandbox/getSandboxStatusHandler.ts b/lib/sandbox/getSandboxStatusHandler.ts index a6cf19584..b1c826fc4 100644 --- a/lib/sandbox/getSandboxStatusHandler.ts +++ b/lib/sandbox/getSandboxStatusHandler.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { hasRuntimeSandboxState } from "@/lib/sandbox/hasRuntimeSandboxState"; import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import type { Tables } from "@/types/database.types"; @@ -23,7 +24,11 @@ function buildLifecycle(row: Tables<"sessions">) { } function isSandboxActive(row: Tables<"sessions">): boolean { - if (!row.sandbox_state) return false; + // Reject the type-only stub written by POST /api/sessions — only real + // runtime metadata (a non-empty sandboxName) counts as an active sandbox. + // Without this guard, every freshly-created session reports "active" + // before any sandbox has actually been provisioned. + if (!hasRuntimeSandboxState(row.sandbox_state)) return false; const expiresAt = isoToEpochMs(row.sandbox_expires_at); if (expiresAt === null) return true; return Date.now() < expiresAt - SANDBOX_EXPIRES_BUFFER_MS; diff --git a/lib/sandbox/hasRuntimeSandboxState.ts b/lib/sandbox/hasRuntimeSandboxState.ts new file mode 100644 index 000000000..5dc7e4171 --- /dev/null +++ b/lib/sandbox/hasRuntimeSandboxState.ts @@ -0,0 +1,24 @@ +/** + * Returns true when `sandbox_state` carries actual runtime metadata + * (i.e. a sandbox has been provisioned and bound to the session) rather + * than the type-only stub written at session creation. + * + * `POST /api/sessions` (api PR #515) inserts `sandbox_state` as + * `{ type: "vercel" }` — a type discriminator with no runtime data. + * Callers must NOT treat this stub as evidence of a live sandbox; doing + * so causes `GET /api/sandbox/status` to report `"active"` immediately + * after session creation, which defeats the chat loading-state UX. + * + * Runtime presence is currently keyed off a non-empty `sandboxName` — + * `POST /api/sandbox` writes this via `getSessionSandboxName(sessionId)` + * and the abstraction's `connectSandbox(...).getState()` preserves it. + * + * @param state - The persisted `sandbox_state` JSON column value. + * @returns true when the state has real runtime metadata; false for + * null/undefined, scalars, the empty type stub, or empty sandboxName. + */ +export function hasRuntimeSandboxState(state: unknown): boolean { + if (!state || typeof state !== "object") return false; + const candidate = state as { sandboxName?: unknown }; + return typeof candidate.sandboxName === "string" && candidate.sandboxName.length > 0; +} From e0a2b97e1a517035cb26ed48006dbc401d1d8ffc Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 7 May 2026 10:57:49 -0500 Subject: [PATCH 3/4] refactor(sandbox): SRP/KISS extractions + Tier 1 correctness fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on PR #522 and the "missing from open-agents" audit: User-flagged review comments: - SRP: extract `buildSource` to lib/sandbox/buildSource.ts - YAGNI: drop `isNewBranch` from POST /api/sandbox — chat never sets it (note: docs PR #192 still documents it; will open follow-up docs PR to drop from sandbox.json) - SRP: extract `isoToEpochMs` to lib/sandbox/isoToEpochMs.ts - SRP: extract `buildLifecycle` to lib/sandbox/buildLifecycle.ts - SRP: extract `isSandboxActive` to lib/sandbox/isSandboxActive.ts - KISS: rename lib/supabase/sessions/updateSessionSandboxState.ts -> updateSession.ts, generalize signature to (id, TablesUpdate<"sessions">) Tier 1 correctness gaps from the open-agents comparison: 1. GitHub URL validation via parseGitHubRepoUrl in validateCreateSandboxBody — bad URLs now return a clean 400 instead of falling through to a confusing 502 from the sandbox provider 2. Service GitHub token plumbed into connectSandbox options via new lib/github/getServiceGithubToken.ts — private repos can now clone 3. snapshot_url + snapshot_created_at cleared on fresh provision so GET /api/sandbox/status no longer surfaces stale snapshot URLs from prior runs TDD red -> green: - 5 new unit-test files for the extracted helpers (buildSource, isoToEpochMs, buildLifecycle, isSandboxActive, getServiceGithubToken) - updateSession.test.ts replaces updateSessionSandboxState.test.ts - Updated validator + handler tests for the contract changes (drop isNewBranch, add bad-URL 400 cases, assert githubToken plumbing, assert snapshot_url/snapshot_created_at: null in update payload) - Confirmed RED before each implementation - Suite: 2499 -> 2516 (+17 net new tests), pnpm lint:check clean Files net delta: -241 / +70 lines (extractions + handler shrinks) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/getServiceGithubToken.test.ts | 32 ++++++++ lib/github/getServiceGithubToken.ts | 12 +++ lib/sandbox/__tests__/buildLifecycle.test.ts | 42 ++++++++++ lib/sandbox/__tests__/buildSource.test.ts | 17 ++++ .../__tests__/createSandboxHandler.test.ts | 55 ++++++------- lib/sandbox/__tests__/isSandboxActive.test.ts | 50 +++++++++++ lib/sandbox/__tests__/isoToEpochMs.test.ts | 20 +++++ .../validateCreateSandboxBody.test.ts | 26 +++--- lib/sandbox/buildLifecycle.ts | 19 +++++ lib/sandbox/buildSource.ts | 20 +++++ lib/sandbox/createSandboxHandler.ts | 50 ++++------- lib/sandbox/getSandboxStatusHandler.ts | 39 ++------- lib/sandbox/isSandboxActive.ts | 21 +++++ lib/sandbox/isoToEpochMs.ts | 12 +++ lib/sandbox/validateCreateSandboxBody.ts | 13 ++- .../sessions/__tests__/updateSession.test.ts | 63 ++++++++++++++ .../updateSessionSandboxState.test.ts | 82 ------------------- lib/supabase/sessions/updateSession.ts | 29 +++++++ .../sessions/updateSessionSandboxState.ts | 46 ----------- 19 files changed, 407 insertions(+), 241 deletions(-) create mode 100644 lib/github/__tests__/getServiceGithubToken.test.ts create mode 100644 lib/github/getServiceGithubToken.ts create mode 100644 lib/sandbox/__tests__/buildLifecycle.test.ts create mode 100644 lib/sandbox/__tests__/buildSource.test.ts create mode 100644 lib/sandbox/__tests__/isSandboxActive.test.ts create mode 100644 lib/sandbox/__tests__/isoToEpochMs.test.ts create mode 100644 lib/sandbox/buildLifecycle.ts create mode 100644 lib/sandbox/buildSource.ts create mode 100644 lib/sandbox/isSandboxActive.ts create mode 100644 lib/sandbox/isoToEpochMs.ts create mode 100644 lib/supabase/sessions/__tests__/updateSession.test.ts delete mode 100644 lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts create mode 100644 lib/supabase/sessions/updateSession.ts delete mode 100644 lib/supabase/sessions/updateSessionSandboxState.ts diff --git a/lib/github/__tests__/getServiceGithubToken.test.ts b/lib/github/__tests__/getServiceGithubToken.test.ts new file mode 100644 index 000000000..5fb28265c --- /dev/null +++ b/lib/github/__tests__/getServiceGithubToken.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; + +const ORIGINAL = process.env.GITHUB_TOKEN; + +beforeEach(() => { + delete process.env.GITHUB_TOKEN; +}); + +afterEach(() => { + if (ORIGINAL === undefined) { + delete process.env.GITHUB_TOKEN; + } else { + process.env.GITHUB_TOKEN = ORIGINAL; + } +}); + +describe("getServiceGithubToken", () => { + it("returns undefined when GITHUB_TOKEN is unset", () => { + expect(getServiceGithubToken()).toBeUndefined(); + }); + + it("returns undefined when GITHUB_TOKEN is the empty string", () => { + process.env.GITHUB_TOKEN = ""; + expect(getServiceGithubToken()).toBeUndefined(); + }); + + it("returns the token when set", () => { + process.env.GITHUB_TOKEN = "ghs_secret"; + expect(getServiceGithubToken()).toBe("ghs_secret"); + }); +}); diff --git a/lib/github/getServiceGithubToken.ts b/lib/github/getServiceGithubToken.ts new file mode 100644 index 000000000..b3af7da06 --- /dev/null +++ b/lib/github/getServiceGithubToken.ts @@ -0,0 +1,12 @@ +/** + * Returns the service-account GitHub token used for cloning private + * repositories into sandboxes. Returns undefined when the env var is + * unset or empty so callers can fall back to public-repo behavior + * without crashing. + * + * @returns The token string, or undefined. + */ +export function getServiceGithubToken(): string | undefined { + const token = process.env.GITHUB_TOKEN; + return token && token.length > 0 ? token : undefined; +} diff --git a/lib/sandbox/__tests__/buildLifecycle.test.ts b/lib/sandbox/__tests__/buildLifecycle.test.ts new file mode 100644 index 000000000..63a8014ee --- /dev/null +++ b/lib/sandbox/__tests__/buildLifecycle.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from "vitest"; +import { buildLifecycle } from "@/lib/sandbox/buildLifecycle"; + +const ISO = "2030-01-01T00:00:00.000Z"; +const EPOCH = Date.parse(ISO); + +describe("buildLifecycle", () => { + it("converts every ISO timestamp on the row to epoch ms and sets serverTime", () => { + const row = { + lifecycle_state: "active", + last_activity_at: ISO, + hibernate_after: ISO, + sandbox_expires_at: ISO, + } as any; + + const result = buildLifecycle(row); + + expect(result).toEqual({ + serverTime: expect.any(Number), + state: "active", + lastActivityAt: EPOCH, + hibernateAfter: EPOCH, + sandboxExpiresAt: EPOCH, + }); + }); + + it("preserves null fields and a null lifecycle_state as-is", () => { + const row = { + lifecycle_state: null, + last_activity_at: null, + hibernate_after: null, + sandbox_expires_at: null, + } as any; + + const result = buildLifecycle(row); + + expect(result.state).toBeNull(); + expect(result.lastActivityAt).toBeNull(); + expect(result.hibernateAfter).toBeNull(); + expect(result.sandboxExpiresAt).toBeNull(); + }); +}); diff --git a/lib/sandbox/__tests__/buildSource.test.ts b/lib/sandbox/__tests__/buildSource.test.ts new file mode 100644 index 000000000..09c8fc931 --- /dev/null +++ b/lib/sandbox/__tests__/buildSource.test.ts @@ -0,0 +1,17 @@ +import { describe, it, expect } from "vitest"; +import { buildSource } from "@/lib/sandbox/buildSource"; + +describe("buildSource", () => { + it("returns repo + branch when branch is provided", () => { + expect(buildSource({ repoUrl: "https://github.com/o/r", branch: "main" })).toEqual({ + repo: "https://github.com/o/r", + branch: "main", + }); + }); + + it("omits branch when not provided", () => { + expect(buildSource({ repoUrl: "https://github.com/o/r" })).toEqual({ + repo: "https://github.com/o/r", + }); + }); +}); diff --git a/lib/sandbox/__tests__/createSandboxHandler.test.ts b/lib/sandbox/__tests__/createSandboxHandler.test.ts index 8cebb3f15..345fe6353 100644 --- a/lib/sandbox/__tests__/createSandboxHandler.test.ts +++ b/lib/sandbox/__tests__/createSandboxHandler.test.ts @@ -5,7 +5,7 @@ import { createSandboxHandler } from "@/lib/sandbox/createSandboxHandler"; import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBody"; import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import { connectSandbox } from "@/lib/sandbox/factory"; -import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; +import { updateSession } from "@/lib/supabase/sessions/updateSession"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }), @@ -19,8 +19,11 @@ vi.mock("@/lib/supabase/sessions/selectSessions", () => ({ vi.mock("@/lib/sandbox/factory", () => ({ connectSandbox: vi.fn(), })); -vi.mock("@/lib/supabase/sessions/updateSessionSandboxState", () => ({ - updateSessionSandboxState: vi.fn(), +vi.mock("@/lib/supabase/sessions/updateSession", () => ({ + updateSession: vi.fn(), +})); +vi.mock("@/lib/github/getServiceGithubToken", () => ({ + getServiceGithubToken: vi.fn(() => "ghs_test_token"), })); const ACCOUNT_ID = "acc-1"; @@ -53,7 +56,7 @@ describe("createSandboxHandler", () => { vi.mocked(connectSandbox).mockResolvedValue( fakeSandbox() as unknown as Awaited>, ); - vi.mocked(updateSessionSandboxState).mockResolvedValue({} as any); + vi.mocked(updateSession).mockResolvedValue({} as any); }); it("short-circuits with the validator's response on validation failure", async () => { @@ -108,17 +111,29 @@ describe("createSandboxHandler", () => { expect(typeof body.timing.readyMs).toBe("number"); }); - it("persists sandbox state to the session row when sessionId is provided", async () => { + it("persists sandbox state and clears stale snapshot fields on the session row", async () => { await createSandboxHandler(makeReq()); - expect(updateSessionSandboxState).toHaveBeenCalledWith( + expect(updateSession).toHaveBeenCalledWith( + "sess-1", expect.objectContaining({ - id: "sess-1", - sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + lifecycle_state: "active", + snapshot_url: null, + snapshot_created_at: null, }), ); }); + it("plumbs the service github token into connectSandbox options", async () => { + await createSandboxHandler(makeReq()); + + const arg = vi.mocked(connectSandbox).mock.calls[0]?.[0]; + expect(arg).toBeDefined(); + if (!arg || !("options" in arg)) throw new Error("expected new-API config shape"); + expect(arg.options?.githubToken).toBe("ghs_test_token"); + }); + it("skips the session-row write when no sessionId is provided", async () => { vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce({ body: { repoUrl: "https://github.com/o/r", branch: "main" }, @@ -128,29 +143,7 @@ describe("createSandboxHandler", () => { const res = await createSandboxHandler(makeReq()); expect(res.status).toBe(200); - expect(updateSessionSandboxState).not.toHaveBeenCalled(); + expect(updateSession).not.toHaveBeenCalled(); expect(selectSessions).not.toHaveBeenCalled(); }); - - it("uses isNewBranch=true to flip source.branch into source.newBranch", async () => { - vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce({ - body: { - repoUrl: "https://github.com/o/r", - sessionId: "sess-1", - branch: "feat/x", - isNewBranch: true, - }, - auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, - }); - - await createSandboxHandler(makeReq()); - - const arg = vi.mocked(connectSandbox).mock.calls[0]?.[0]; - expect(arg).toBeDefined(); - if (!arg || !("state" in arg)) throw new Error("expected new-API config shape"); - - const source = (arg.state as any).source; - expect(source.newBranch).toBe("feat/x"); - expect(source.branch).toBeUndefined(); - }); }); diff --git a/lib/sandbox/__tests__/isSandboxActive.test.ts b/lib/sandbox/__tests__/isSandboxActive.test.ts new file mode 100644 index 000000000..c8f08a1f3 --- /dev/null +++ b/lib/sandbox/__tests__/isSandboxActive.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { isSandboxActive } from "@/lib/sandbox/isSandboxActive"; + +const FAR_FUTURE = "2099-01-01T00:00:00.000Z"; +const FAR_PAST = "2000-01-01T00:00:00.000Z"; + +const baseRow = { + sandbox_state: null as unknown, + sandbox_expires_at: null as string | null, +}; + +describe("isSandboxActive", () => { + it("returns false when sandbox_state has no runtime metadata", () => { + expect(isSandboxActive({ ...baseRow, sandbox_state: { type: "vercel" } } as any)).toBe(false); + }); + + it("returns false when sandbox_state is null", () => { + expect(isSandboxActive({ ...baseRow } as any)).toBe(false); + }); + + it("returns true with a runtime sandboxName and a far-future expiry", () => { + expect( + isSandboxActive({ + ...baseRow, + sandbox_state: { type: "vercel", sandboxName: "session-x" }, + sandbox_expires_at: FAR_FUTURE, + } as any), + ).toBe(true); + }); + + it("returns false when expiry is in the past", () => { + expect( + isSandboxActive({ + ...baseRow, + sandbox_state: { type: "vercel", sandboxName: "session-x" }, + sandbox_expires_at: FAR_PAST, + } as any), + ).toBe(false); + }); + + it("returns true when sandboxName is set but expiry is null (no expiry to compare against)", () => { + expect( + isSandboxActive({ + ...baseRow, + sandbox_state: { type: "vercel", sandboxName: "session-x" }, + sandbox_expires_at: null, + } as any), + ).toBe(true); + }); +}); diff --git a/lib/sandbox/__tests__/isoToEpochMs.test.ts b/lib/sandbox/__tests__/isoToEpochMs.test.ts new file mode 100644 index 000000000..a8d448ca7 --- /dev/null +++ b/lib/sandbox/__tests__/isoToEpochMs.test.ts @@ -0,0 +1,20 @@ +import { describe, it, expect } from "vitest"; +import { isoToEpochMs } from "@/lib/sandbox/isoToEpochMs"; + +describe("isoToEpochMs", () => { + it("returns null for null input", () => { + expect(isoToEpochMs(null)).toBeNull(); + }); + + it("returns null for an empty string", () => { + expect(isoToEpochMs("")).toBeNull(); + }); + + it("returns null for an unparseable string", () => { + expect(isoToEpochMs("not-a-date")).toBeNull(); + }); + + it("converts a valid ISO string to epoch milliseconds", () => { + expect(isoToEpochMs("2030-01-01T00:00:00.000Z")).toBe(Date.parse("2030-01-01T00:00:00.000Z")); + }); +}); diff --git a/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts index 563b4ddd2..9672f6214 100644 --- a/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts +++ b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts @@ -65,6 +65,20 @@ describe("validateCreateSandboxBody", () => { expect((result as NextResponse).status).toBe(400); }); + it("returns 400 when repoUrl is not a valid GitHub repository URL", async () => { + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "https://gitlab.com/o/r" })); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + + it("returns 400 when repoUrl is not a URL at all", async () => { + const result = await validateCreateSandboxBody(makeReq({ repoUrl: "x" })); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + it("returns the validated body + auth on a minimal happy path", async () => { const result = await validateCreateSandboxBody(makeReq({ repoUrl: "https://github.com/o/r" })); @@ -73,17 +87,15 @@ describe("validateCreateSandboxBody", () => { expect(result.body.repoUrl).toBe("https://github.com/o/r"); expect(result.body.sessionId).toBeUndefined(); expect(result.body.branch).toBeUndefined(); - expect(result.body.isNewBranch).toBeUndefined(); expect(result.auth.accountId).toBe(ACCOUNT_ID); }); - it("accepts a full request with sessionId, branch, isNewBranch", async () => { + it("accepts a full request with sessionId and branch", async () => { const result = await validateCreateSandboxBody( makeReq({ repoUrl: "https://github.com/o/r", sessionId: "sess-1", branch: "feat/x", - isNewBranch: true, }), ); @@ -91,13 +103,5 @@ describe("validateCreateSandboxBody", () => { if (result instanceof NextResponse) return; expect(result.body.sessionId).toBe("sess-1"); expect(result.body.branch).toBe("feat/x"); - expect(result.body.isNewBranch).toBe(true); - }); - - it("returns 400 when isNewBranch is the wrong type", async () => { - const result = await validateCreateSandboxBody(makeReq({ repoUrl: "x", isNewBranch: "yes" })); - - expect(result).toBeInstanceOf(NextResponse); - expect((result as NextResponse).status).toBe(400); }); }); diff --git a/lib/sandbox/buildLifecycle.ts b/lib/sandbox/buildLifecycle.ts new file mode 100644 index 000000000..4f2337354 --- /dev/null +++ b/lib/sandbox/buildLifecycle.ts @@ -0,0 +1,19 @@ +import { isoToEpochMs } from "@/lib/sandbox/isoToEpochMs"; +import type { Tables } from "@/types/database.types"; + +/** + * Projects the lifecycle-relevant columns of a `sessions` row into the + * docs-spec lifecycle envelope used by GET /api/sandbox/status. + * + * @param row - The `sessions` row. + * @returns The lifecycle envelope: serverTime, state, and three epoch-ms timestamps. + */ +export function buildLifecycle(row: Tables<"sessions">) { + return { + serverTime: Date.now(), + state: row.lifecycle_state, + lastActivityAt: isoToEpochMs(row.last_activity_at), + hibernateAfter: isoToEpochMs(row.hibernate_after), + sandboxExpiresAt: isoToEpochMs(row.sandbox_expires_at), + }; +} diff --git a/lib/sandbox/buildSource.ts b/lib/sandbox/buildSource.ts new file mode 100644 index 000000000..44e266aec --- /dev/null +++ b/lib/sandbox/buildSource.ts @@ -0,0 +1,20 @@ +export interface SandboxSource { + repo: string; + branch?: string; +} + +/** + * Builds the `source` shape that `connectSandbox` consumes. + * + * @param input - The repo URL and optional branch to check out. + * @returns A `{repo, branch?}` source descriptor. + */ +export function buildSource({ + repoUrl, + branch, +}: { + repoUrl: string; + branch?: string; +}): SandboxSource { + return branch ? { repo: repoUrl, branch } : { repo: repoUrl }; +} diff --git a/lib/sandbox/createSandboxHandler.ts b/lib/sandbox/createSandboxHandler.ts index d081a82f7..b51f60895 100644 --- a/lib/sandbox/createSandboxHandler.ts +++ b/lib/sandbox/createSandboxHandler.ts @@ -5,39 +5,23 @@ import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBo import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import { connectSandbox } from "@/lib/sandbox/factory"; import { getSessionSandboxName } from "@/lib/sandbox/getSessionSandboxName"; -import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; +import { buildSource } from "@/lib/sandbox/buildSource"; +import { updateSession } from "@/lib/supabase/sessions/updateSession"; +import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; import type { Json } from "@/types/database.types"; const DEFAULT_TIMEOUT_MS = ms("30m"); const DEFAULT_PORTS = [3000]; -interface SourceConfig { - repo: string; - branch?: string; - newBranch?: string; -} - -function buildSource({ - repoUrl, - branch, - isNewBranch, -}: { - repoUrl: string; - branch?: string; - isNewBranch?: boolean; -}): SourceConfig { - if (isNewBranch && branch) { - return { repo: repoUrl, newBranch: branch }; - } - return branch ? { repo: repoUrl, branch } : { repo: repoUrl }; -} - /** * Handles `POST /api/sandbox`. Provisions a Sandbox bound to the given * session (or a one-shot sandbox when no `sessionId` is supplied). When * a session is bound, the resolved `sandboxState`, lifecycle, and * expiry are written back to the `sessions` row so subsequent reads via - * `GET /api/sandbox/status` can report the sandbox as active. + * `GET /api/sandbox/status` can report the sandbox as active. Stale + * `snapshot_url` / `snapshot_created_at` are cleared on a fresh + * provision so the UI does not surface a snapshot that no longer + * matches the current sandbox. */ export async function createSandboxHandler(request: NextRequest): Promise { const validated = await validateCreateSandboxBody(request); @@ -80,15 +64,12 @@ export async function createSandboxHandler(request: NextRequest): Promise) { - return { - serverTime: Date.now(), - state: row.lifecycle_state, - lastActivityAt: isoToEpochMs(row.last_activity_at), - hibernateAfter: isoToEpochMs(row.hibernate_after), - sandboxExpiresAt: isoToEpochMs(row.sandbox_expires_at), - }; -} - -function isSandboxActive(row: Tables<"sessions">): boolean { - // Reject the type-only stub written by POST /api/sessions — only real - // runtime metadata (a non-empty sandboxName) counts as an active sandbox. - // Without this guard, every freshly-created session reports "active" - // before any sandbox has actually been provisioned. - if (!hasRuntimeSandboxState(row.sandbox_state)) return false; - const expiresAt = isoToEpochMs(row.sandbox_expires_at); - if (expiresAt === null) return true; - return Date.now() < expiresAt - SANDBOX_EXPIRES_BUFFER_MS; -} /** * Handles `GET /api/sandbox/status`. Returns the current lifecycle and * runtime state for the sandbox bound to a session — DB-only read, no * upstream probe. Status is `"active"` when the session row carries a - * non-expired `sandbox_state`, otherwise `"no_sandbox"`. `hasSnapshot` - * is true when the row records a saved snapshot the UI can offer to - * resume. + * non-expired `sandbox_state` (with real runtime metadata), otherwise + * `"no_sandbox"`. `hasSnapshot` is true when the row records a saved + * snapshot the UI can offer to resume. */ export async function getSandboxStatusHandler(request: NextRequest): Promise { const auth = await validateAuthContext(request); diff --git a/lib/sandbox/isSandboxActive.ts b/lib/sandbox/isSandboxActive.ts new file mode 100644 index 000000000..a97c72d87 --- /dev/null +++ b/lib/sandbox/isSandboxActive.ts @@ -0,0 +1,21 @@ +import { hasRuntimeSandboxState } from "@/lib/sandbox/hasRuntimeSandboxState"; +import { isoToEpochMs } from "@/lib/sandbox/isoToEpochMs"; +import type { Tables } from "@/types/database.types"; + +const SANDBOX_EXPIRES_BUFFER_MS = 10_000; + +/** + * Decides whether the sandbox bound to a session row should be reported + * as `"active"` by GET /api/sandbox/status. Active iff the row carries + * real runtime metadata (not the type-only stub from session creation) + * AND the recorded expiry is at least 10s in the future. + * + * @param row - The `sessions` row. + * @returns true when the sandbox is alive and unexpired. + */ +export function isSandboxActive(row: Tables<"sessions">): boolean { + if (!hasRuntimeSandboxState(row.sandbox_state)) return false; + const expiresAt = isoToEpochMs(row.sandbox_expires_at); + if (expiresAt === null) return true; + return Date.now() < expiresAt - SANDBOX_EXPIRES_BUFFER_MS; +} diff --git a/lib/sandbox/isoToEpochMs.ts b/lib/sandbox/isoToEpochMs.ts new file mode 100644 index 000000000..7558f58d3 --- /dev/null +++ b/lib/sandbox/isoToEpochMs.ts @@ -0,0 +1,12 @@ +/** + * Converts an ISO-8601 timestamp string into epoch milliseconds. + * Returns null for null / empty / unparseable input. + * + * @param value - The ISO timestamp from Supabase. + * @returns Epoch milliseconds, or null when the value cannot be parsed. + */ +export function isoToEpochMs(value: string | null): number | null { + if (!value) return null; + const ms = Date.parse(value); + return Number.isFinite(ms) ? ms : null; +} diff --git a/lib/sandbox/validateCreateSandboxBody.ts b/lib/sandbox/validateCreateSandboxBody.ts index 03ccf60bc..cb58519b0 100644 --- a/lib/sandbox/validateCreateSandboxBody.ts +++ b/lib/sandbox/validateCreateSandboxBody.ts @@ -4,12 +4,17 @@ import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { safeParseJson } from "@/lib/networking/safeParseJson"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import type { AuthContext } from "@/lib/auth/validateAuthContext"; +import { parseGitHubRepoUrl } from "@/lib/github/parseGitHubRepoUrl"; export const createSandboxBodySchema = z.object({ - repoUrl: z.string({ message: "repoUrl is required" }).min(1, "repoUrl cannot be empty"), + repoUrl: z + .string({ message: "repoUrl is required" }) + .min(1, "repoUrl cannot be empty") + .refine(value => parseGitHubRepoUrl(value) !== null, { + message: "repoUrl must be a valid GitHub repository URL", + }), sessionId: z.string().optional(), branch: z.string().optional(), - isNewBranch: z.boolean().optional(), }); export type CreateSandboxBody = z.infer; @@ -22,8 +27,8 @@ export interface ValidatedCreateSandboxRequest { /** * Validates a `POST /api/sandbox` request: authenticates the caller, * tolerates malformed JSON (treated as an empty body), then enforces - * the Zod schema. Returns either the first 4xx response or the - * validated `{ body, auth }`. + * the Zod schema (including a strict GitHub URL check). Returns either + * the first 4xx response or the validated `{ body, auth }`. */ export async function validateCreateSandboxBody( request: NextRequest, diff --git a/lib/supabase/sessions/__tests__/updateSession.test.ts b/lib/supabase/sessions/__tests__/updateSession.test.ts new file mode 100644 index 000000000..c1c34df12 --- /dev/null +++ b/lib/supabase/sessions/__tests__/updateSession.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { updateSession } from "@/lib/supabase/sessions/updateSession"; + +const updateChain = vi.fn(); +const eqChain = vi.fn(); +const selectChain = vi.fn(); +const singleChain = vi.fn(); + +vi.mock("@/lib/supabase/serverClient", () => ({ + default: { + from: vi.fn(() => ({ update: updateChain })), + }, +})); + +beforeEach(() => { + vi.clearAllMocks(); + updateChain.mockReturnValue({ eq: eqChain }); + eqChain.mockReturnValue({ select: selectChain }); + selectChain.mockReturnValue({ single: singleChain }); +}); + +describe("updateSession", () => { + it("returns the updated row on success", async () => { + const row = { id: "sess-1", title: "renamed" }; + singleChain.mockResolvedValue({ data: row, error: null }); + + const result = await updateSession("sess-1", { title: "renamed" }); + + expect(result).toEqual(row); + expect(updateChain).toHaveBeenCalledWith({ title: "renamed" }); + expect(eqChain).toHaveBeenCalledWith("id", "sess-1"); + }); + + it("returns null when supabase reports an error", async () => { + singleChain.mockResolvedValue({ data: null, error: { message: "down" } }); + + const result = await updateSession("sess-x", { title: "x" }); + + expect(result).toBeNull(); + }); + + it("forwards the entire updates object to the .update() call", async () => { + singleChain.mockResolvedValue({ data: {}, error: null }); + + await updateSession("sess-1", { + sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, + lifecycle_state: "active", + lifecycle_version: 5, + sandbox_expires_at: "2030-01-01T00:00:00.000Z", + last_activity_at: "2030-01-01T00:00:00.000Z", + snapshot_url: null, + snapshot_created_at: null, + }); + + const payload = updateChain.mock.calls[0]?.[0]; + expect(payload).toMatchObject({ + lifecycle_state: "active", + lifecycle_version: 5, + snapshot_url: null, + snapshot_created_at: null, + }); + }); +}); diff --git a/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts b/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts deleted file mode 100644 index 0f086ed5f..000000000 --- a/lib/supabase/sessions/__tests__/updateSessionSandboxState.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { updateSessionSandboxState } from "@/lib/supabase/sessions/updateSessionSandboxState"; - -const updateChain = vi.fn(); -const eqChain = vi.fn(); -const selectChain = vi.fn(); -const singleChain = vi.fn(); - -vi.mock("@/lib/supabase/serverClient", () => ({ - default: { - from: vi.fn(() => ({ - update: updateChain, - })), - }, -})); - -beforeEach(() => { - vi.clearAllMocks(); - updateChain.mockReturnValue({ eq: eqChain }); - eqChain.mockReturnValue({ select: selectChain }); - selectChain.mockReturnValue({ single: singleChain }); -}); - -describe("updateSessionSandboxState", () => { - const baseRow = { - id: "sess-1", - account_id: "acc-1", - sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, - lifecycle_state: "active", - lifecycle_version: 2, - sandbox_expires_at: "2030-01-01T00:00:00.000Z", - last_activity_at: "2030-01-01T00:00:00.000Z", - }; - - it("returns the updated row on success", async () => { - singleChain.mockResolvedValue({ data: baseRow, error: null }); - - const result = await updateSessionSandboxState({ - id: "sess-1", - sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, - sandboxExpiresAt: "2030-01-01T00:00:00.000Z", - lifecycleVersion: 2, - }); - - expect(result).toEqual(baseRow); - expect(updateChain).toHaveBeenCalledOnce(); - expect(eqChain).toHaveBeenCalledWith("id", "sess-1"); - }); - - it("returns null when supabase reports an error", async () => { - singleChain.mockResolvedValue({ data: null, error: { message: "db down" } }); - - const result = await updateSessionSandboxState({ - id: "sess-x", - sandboxState: null, - sandboxExpiresAt: null, - lifecycleVersion: 1, - }); - - expect(result).toBeNull(); - }); - - it("writes the supplied lifecycle fields onto the row", async () => { - singleChain.mockResolvedValue({ data: baseRow, error: null }); - - await updateSessionSandboxState({ - id: "sess-1", - sandboxState: { type: "vercel", sandboxName: "session-sess-1" }, - sandboxExpiresAt: "2030-01-01T00:00:00.000Z", - lifecycleVersion: 5, - }); - - const updatePayload = updateChain.mock.calls[0]?.[0]; - expect(updatePayload).toMatchObject({ - sandbox_state: { type: "vercel", sandboxName: "session-sess-1" }, - lifecycle_state: "active", - lifecycle_version: 5, - sandbox_expires_at: "2030-01-01T00:00:00.000Z", - }); - expect(updatePayload.last_activity_at).toBeTypeOf("string"); - }); -}); diff --git a/lib/supabase/sessions/updateSession.ts b/lib/supabase/sessions/updateSession.ts new file mode 100644 index 000000000..f582cb7e5 --- /dev/null +++ b/lib/supabase/sessions/updateSession.ts @@ -0,0 +1,29 @@ +import supabase from "@/lib/supabase/serverClient"; +import type { Tables, TablesUpdate } from "@/types/database.types"; + +/** + * Updates a `sessions` row by id with any subset of mutable columns. + * Returns the updated row, or null on Supabase error. + * + * @param id - The session id to update. + * @param updates - Partial column updates (any TablesUpdate<"sessions"> shape). + * @returns The updated row, or null on error. + */ +export async function updateSession( + id: string, + updates: TablesUpdate<"sessions">, +): Promise | null> { + const { data, error } = await supabase + .from("sessions") + .update(updates) + .eq("id", id) + .select() + .single(); + + if (error) { + console.error("[updateSession] error:", error); + return null; + } + + return data; +} diff --git a/lib/supabase/sessions/updateSessionSandboxState.ts b/lib/supabase/sessions/updateSessionSandboxState.ts deleted file mode 100644 index 12def75b4..000000000 --- a/lib/supabase/sessions/updateSessionSandboxState.ts +++ /dev/null @@ -1,46 +0,0 @@ -import supabase from "@/lib/supabase/serverClient"; -import type { Json, Tables } from "@/types/database.types"; - -interface UpdateSessionSandboxStateInput { - /** Owning session id. */ - id: string; - /** Persistable sandbox state from `sandbox.getState()`, or null to clear. */ - sandboxState: Json | null; - /** Sandbox expiry as ISO string, or null when not yet known. */ - sandboxExpiresAt: string | null; - /** Bumped lifecycle version (optimistic concurrency token). */ - lifecycleVersion: number; -} - -/** - * Writes sandbox runtime metadata onto a `sessions` row. Called when a - * sandbox has just been provisioned (or resumed) for a session — sets - * `lifecycle_state="active"` and refreshes `last_activity_at`. Returns - * the updated row, or null on Supabase error. - */ -export async function updateSessionSandboxState({ - id, - sandboxState, - sandboxExpiresAt, - lifecycleVersion, -}: UpdateSessionSandboxStateInput): Promise | null> { - const { data, error } = await supabase - .from("sessions") - .update({ - sandbox_state: sandboxState, - lifecycle_state: "active", - lifecycle_version: lifecycleVersion, - sandbox_expires_at: sandboxExpiresAt, - last_activity_at: new Date().toISOString(), - }) - .eq("id", id) - .select() - .single(); - - if (error) { - console.error("[updateSessionSandboxState] error:", error); - return null; - } - - return data; -} From 5b1cca4518d3d598020e6cbf82dc0917f5ddc9f3 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 7 May 2026 11:14:45 -0500 Subject: [PATCH 4/4] refactor(sandbox): drop branch from POST /api/sandbox contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YAGNI/KISS per review feedback — chat always works off the repo's default branch, so the explicit `branch` input adds no value. - Drop `branch` from createSandboxBodySchema - Inline the now-trivial source object in createSandboxHandler (`{ repo: body.repoUrl }`) and delete `lib/sandbox/buildSource.ts` + its test - Read `currentBranch` for the response from the sandbox handle's own `currentBranch` property (whatever the SDK actually checked out), falling back to "main" — no longer derives from a request field that no longer exists Tests: TDD red -> green. - Validator test asserts `branch` is stripped from the body even if a client still sends it - Handler test asserts `currentBranch` in the response comes from `sandbox.currentBranch` (mocked to "release/v2") not from input - Suite stays at 2516 (-1 from buildSource.test deletion +1 new currentBranch test) Pairs with docs PR recoupable/docs#194 (merged) which already removed `branch` and `isNewBranch` from the published OpenAPI spec. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/sandbox/__tests__/buildSource.test.ts | 17 ---------------- .../__tests__/createSandboxHandler.test.ts | 17 ++++++++++++++-- .../validateCreateSandboxBody.test.ts | 18 +++++++++++++---- lib/sandbox/buildSource.ts | 20 ------------------- lib/sandbox/createSandboxHandler.ts | 14 +++++++------ lib/sandbox/validateCreateSandboxBody.ts | 1 - 6 files changed, 37 insertions(+), 50 deletions(-) delete mode 100644 lib/sandbox/__tests__/buildSource.test.ts delete mode 100644 lib/sandbox/buildSource.ts diff --git a/lib/sandbox/__tests__/buildSource.test.ts b/lib/sandbox/__tests__/buildSource.test.ts deleted file mode 100644 index 09c8fc931..000000000 --- a/lib/sandbox/__tests__/buildSource.test.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { buildSource } from "@/lib/sandbox/buildSource"; - -describe("buildSource", () => { - it("returns repo + branch when branch is provided", () => { - expect(buildSource({ repoUrl: "https://github.com/o/r", branch: "main" })).toEqual({ - repo: "https://github.com/o/r", - branch: "main", - }); - }); - - it("omits branch when not provided", () => { - expect(buildSource({ repoUrl: "https://github.com/o/r" })).toEqual({ - repo: "https://github.com/o/r", - }); - }); -}); diff --git a/lib/sandbox/__tests__/createSandboxHandler.test.ts b/lib/sandbox/__tests__/createSandboxHandler.test.ts index 345fe6353..b79c2b987 100644 --- a/lib/sandbox/__tests__/createSandboxHandler.test.ts +++ b/lib/sandbox/__tests__/createSandboxHandler.test.ts @@ -36,6 +36,7 @@ function fakeSandbox(overrides: Partial> = {}) { return { timeout: 1_800_000, expiresAt: Date.parse("2030-01-01T00:00:00.000Z"), + currentBranch: "main", getState: () => ({ type: "vercel", sandboxName: "session-sess-1" }), ...overrides, }; @@ -48,7 +49,6 @@ describe("createSandboxHandler", () => { body: { repoUrl: "https://github.com/o/r", sessionId: "sess-1", - branch: "main", }, auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, }); @@ -111,6 +111,19 @@ describe("createSandboxHandler", () => { expect(typeof body.timing.readyMs).toBe("number"); }); + it("reports currentBranch from the sandbox handle (not request input)", async () => { + vi.mocked(connectSandbox).mockResolvedValueOnce( + fakeSandbox({ currentBranch: "release/v2" }) as unknown as Awaited< + ReturnType + >, + ); + + const res = await createSandboxHandler(makeReq()); + + const body = await res.json(); + expect(body.currentBranch).toBe("release/v2"); + }); + it("persists sandbox state and clears stale snapshot fields on the session row", async () => { await createSandboxHandler(makeReq()); @@ -136,7 +149,7 @@ describe("createSandboxHandler", () => { it("skips the session-row write when no sessionId is provided", async () => { vi.mocked(validateCreateSandboxBody).mockResolvedValueOnce({ - body: { repoUrl: "https://github.com/o/r", branch: "main" }, + body: { repoUrl: "https://github.com/o/r" }, auth: { accountId: ACCOUNT_ID, orgId: null, authToken: "k" }, }); diff --git a/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts index 9672f6214..e3894f2ed 100644 --- a/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts +++ b/lib/sandbox/__tests__/validateCreateSandboxBody.test.ts @@ -86,22 +86,32 @@ describe("validateCreateSandboxBody", () => { if (result instanceof NextResponse) return; expect(result.body.repoUrl).toBe("https://github.com/o/r"); expect(result.body.sessionId).toBeUndefined(); - expect(result.body.branch).toBeUndefined(); expect(result.auth.accountId).toBe(ACCOUNT_ID); }); - it("accepts a full request with sessionId and branch", async () => { + it("accepts a request with sessionId", async () => { const result = await validateCreateSandboxBody( makeReq({ repoUrl: "https://github.com/o/r", sessionId: "sess-1", - branch: "feat/x", }), ); expect(result).not.toBeInstanceOf(NextResponse); if (result instanceof NextResponse) return; expect(result.body.sessionId).toBe("sess-1"); - expect(result.body.branch).toBe("feat/x"); + }); + + it("strips an unknown branch input from the validated body", async () => { + const result = await validateCreateSandboxBody( + makeReq({ + repoUrl: "https://github.com/o/r", + branch: "feat/x", + }), + ); + + expect(result).not.toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) return; + expect((result.body as Record).branch).toBeUndefined(); }); }); diff --git a/lib/sandbox/buildSource.ts b/lib/sandbox/buildSource.ts deleted file mode 100644 index 44e266aec..000000000 --- a/lib/sandbox/buildSource.ts +++ /dev/null @@ -1,20 +0,0 @@ -export interface SandboxSource { - repo: string; - branch?: string; -} - -/** - * Builds the `source` shape that `connectSandbox` consumes. - * - * @param input - The repo URL and optional branch to check out. - * @returns A `{repo, branch?}` source descriptor. - */ -export function buildSource({ - repoUrl, - branch, -}: { - repoUrl: string; - branch?: string; -}): SandboxSource { - return branch ? { repo: repoUrl, branch } : { repo: repoUrl }; -} diff --git a/lib/sandbox/createSandboxHandler.ts b/lib/sandbox/createSandboxHandler.ts index b51f60895..eb3f9be21 100644 --- a/lib/sandbox/createSandboxHandler.ts +++ b/lib/sandbox/createSandboxHandler.ts @@ -5,18 +5,21 @@ import { validateCreateSandboxBody } from "@/lib/sandbox/validateCreateSandboxBo import { selectSessions } from "@/lib/supabase/sessions/selectSessions"; import { connectSandbox } from "@/lib/sandbox/factory"; import { getSessionSandboxName } from "@/lib/sandbox/getSessionSandboxName"; -import { buildSource } from "@/lib/sandbox/buildSource"; import { updateSession } from "@/lib/supabase/sessions/updateSession"; import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken"; import type { Json } from "@/types/database.types"; const DEFAULT_TIMEOUT_MS = ms("30m"); const DEFAULT_PORTS = [3000]; +const DEFAULT_BRANCH = "main"; /** * Handles `POST /api/sandbox`. Provisions a Sandbox bound to the given - * session (or a one-shot sandbox when no `sessionId` is supplied). When - * a session is bound, the resolved `sandboxState`, lifecycle, and + * session (or a one-shot sandbox when no `sessionId` is supplied) using + * the repo's default branch — there is no input branch override; the + * chat UX always works against whatever the repo treats as default. + * + * When a session is bound, the resolved `sandbox_state`, lifecycle, and * expiry are written back to the `sessions` row so subsequent reads via * `GET /api/sandbox/status` can report the sandbox as active. Stale * `snapshot_url` / `snapshot_created_at` are cleared on a fresh @@ -55,7 +58,6 @@ export async function createSandboxHandler(request: NextRequest): Promise;