feat(sandbox): port POST /api/sandbox + GET /api/sandbox/status from open-agents#522
Merged
sweetmantech merged 4 commits intotestfrom May 7, 2026
Merged
feat(sandbox): port POST /api/sandbox + GET /api/sandbox/status from open-agents#522sweetmantech merged 4 commits intotestfrom
sweetmantech merged 4 commits intotestfrom
Conversation
…open-agents 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) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 14 files
Confidence score: 3/5
- There is a concrete user-impact risk:
lib/supabase/sessions/updateSessionSandboxState.tscan returnnullon DB failure, which may let sandbox creation appear successful even when session state is not persisted (severity 7/10, high confidence). lib/sandbox/createSandboxHandler.tscurrently does not check theupdateSessionSandboxStateresult, so a failed write can still return 200 and later cause/api/sandbox/statusto reportno_sandboxunexpectedly (severity 6/10).- The remaining findings are maintainability/style-only (100-line rule) in
lib/sandbox/createSandboxHandler.tsand test files, which are lower severity and not direct runtime blockers. - Pay close attention to
lib/supabase/sessions/updateSessionSandboxState.ts,lib/sandbox/createSandboxHandler.ts- ensure DB write failures are handled and surfaced so success responses match persisted session state.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/sandbox/__tests__/validateCreateSandboxBody.test.ts">
<violation number="1" location="lib/sandbox/__tests__/validateCreateSandboxBody.test.ts:21">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
This new test file exceeds the repository’s 100-line limit required by Rule 3.</violation>
</file>
<file name="lib/sandbox/__tests__/createSandboxHandler.test.ts">
<violation number="1" location="lib/sandbox/__tests__/createSandboxHandler.test.ts:1">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
New test file exceeds the 100-line file-size limit required by the maintainability rule.</violation>
</file>
<file name="lib/sandbox/createSandboxHandler.ts">
<violation number="1" location="lib/sandbox/createSandboxHandler.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
File exceeds the 100-line maximum required by Rule 3.</violation>
<violation number="2" location="lib/sandbox/createSandboxHandler.ts:109">
P2: The return value of `updateSessionSandboxState` is not checked. If the DB write fails (returns `null`), the client gets a 200 but the session row isn't marked active—causing `GET /api/sandbox/status` to report `no_sandbox` while the sandbox is actually running. Consider checking the return value and either retrying or returning an error to the client.</violation>
</file>
<file name="lib/sandbox/__tests__/getSandboxStatusHandler.test.ts">
<violation number="1" location="lib/sandbox/__tests__/getSandboxStatusHandler.test.ts:1">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Test file exceeds the 100-line maintainability limit.</violation>
</file>
<file name="lib/supabase/sessions/updateSessionSandboxState.ts">
<violation number="1" location="lib/supabase/sessions/updateSessionSandboxState.ts:42">
P1: Returning `null` on DB failure allows sandbox creation to report success even when state persistence fails.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Chat Client (Browser)
participant Route as Next.js Route (/api/sandbox)
participant StatusRoute as Next.js Route (/api/sandbox/status)
participant Auth as Auth Validator (validateAuthContext)
participant CreateBody as Body Validator (validateCreateSandboxBody)
participant CreateHandler as createSandboxHandler
participant StatusHandler as getSandboxStatusHandler
participant SessionDB as Supabase (Sessions Table)
participant SandboxProvider as Vercel Sandbox Provider
Note over UI,SandboxProvider: POST /api/sandbox – Provision/Resume Sandbox
UI->>Route: POST /api/sandbox { repoUrl, sessionId?, branch?, isNewBranch? }
Note over UI,Route: Headers: Authorization (Privy Bearer) or x-api-key
Route->>CreateBody: validateCreateSandboxBody(request)
CreateBody->>Auth: validateAuthContext(request)
alt Auth fails (401)
Auth-->>CreateBody: NextResponse 401
CreateBody-->>Route: Forward 401 response
Route-->>UI: 401 Unauthorized
else Auth success
Auth-->>CreateBody: { accountId, orgId, authToken }
CreateBody->>CreateBody: safeParseJson + Zod schema check
alt Validation fails (400)
CreateBody-->>Route: NextResponse 400
Route-->>UI: 400 Bad Request (missing_fields)
else Valid body
CreateBody-->>Route: { body: CreateSandboxBody, auth: AuthContext }
Route->>CreateHandler: createSandboxHandler(request) [with validated data]
CreateHandler->>SessionDB: selectSessions({ id: sessionId }) [only if sessionId provided]
alt Session not found
SessionDB-->>CreateHandler: []
CreateHandler-->>Route: 404 Session not found
Route-->>UI: 404
else Wrong owner
SessionDB-->>CreateHandler: [{ account_id: "other-acc" }]
CreateHandler-->>Route: 403 Forbidden
Route-->>UI: 403
else Session owned by caller
SessionDB-->>CreateHandler: [{ lifecycle_version, ... }]
CreateHandler->>CreateHandler: getSessionSandboxName(sessionId) => "session-{id}"
CreateHandler->>CreateHandler: buildSource({ repoUrl, branch, isNewBranch })
Note over CreateHandler: isNewBranch=true moves branch to source.newBranch
CreateHandler->>SandboxProvider: connectSandbox({ state: { type: "vercel", sandboxName, source }, options: { timeout, ports, persistent, resume, createIfMissing } })
alt Provider error
SandboxProvider-->>CreateHandler: throws
CreateHandler-->>Route: 502 Failed to provision sandbox
Route-->>UI: 502
else Provider success
SandboxProvider-->>CreateHandler: sandbox object { timeout, expiresAt, getState }
alt sessionId provided
CreateHandler->>SessionDB: updateSessionSandboxState({ id, sandboxState, sandboxExpiresAt, lifecycleVersion: ++ })
Note over CreateHandler,SessionDB: Persists: sandbox_state, lifecycle_state="active", lifecycle_version, sandbox_expires_at, last_activity_at
SessionDB-->>CreateHandler: Updated row
else No sessionId (one-shot)
Note over CreateHandler: Skip DB write
end
CreateHandler-->>Route: 200 { createdAt, timeout, currentBranch, mode: "vercel", timing: { readyMs } }
Route-->>UI: 200 Success
end
end
end
end
Note over UI,SandboxProvider: GET /api/sandbox/status – Poll Sandbox Status (DB only)
UI->>StatusRoute: GET /api/sandbox/status?sessionId=xxx
StatusRoute->>StatusHandler: getSandboxStatusHandler(request)
StatusHandler->>Auth: validateAuthContext(request)
alt Auth fails (401)
Auth-->>StatusHandler: NextResponse 401
StatusHandler-->>StatusRoute: Forward 401
StatusRoute-->>UI: 401
else Auth success
Auth-->>StatusHandler: { accountId, ... }
alt Missing sessionId param
StatusHandler-->>StatusRoute: 400 Missing sessionId
StatusRoute-->>UI: 400
else
StatusHandler->>SessionDB: selectSessions({ id: sessionId })
alt Session not found
SessionDB-->>StatusHandler: []
StatusHandler-->>StatusRoute: 404
StatusRoute-->>UI: 404
else Wrong owner
SessionDB-->>StatusHandler: [{ account_id: "other-acc" }]
StatusHandler-->>StatusRoute: 403
StatusRoute-->>UI: 403
else Owned session
SessionDB-->>StatusHandler: [session row]
StatusHandler->>StatusHandler: isSandboxActive(row) ?
Note over StatusHandler: Active when sandbox_state set AND sandbox_expires_at is >= 10s in future
alt Active
StatusHandler-->>StatusRoute: 200 { status: "active", hasSnapshot, lifecycleVersion, lifecycle: { serverTime, state, lastActivityAt, hibernateAfter, sandboxExpiresAt } }
else Not active (null state or expired)
StatusHandler-->>StatusRoute: 200 { status: "no_sandbox", ... }
end
StatusRoute-->>UI: 200
end
end
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const nextState = sandbox.getState() as Json; | ||
| const expiresAt = | ||
| typeof sandbox.expiresAt === "number" ? new Date(sandbox.expiresAt).toISOString() : null; | ||
| await updateSessionSandboxState({ |
There was a problem hiding this comment.
P2: The return value of updateSessionSandboxState is not checked. If the DB write fails (returns null), the client gets a 200 but the session row isn't marked active—causing GET /api/sandbox/status to report no_sandbox while the sandbox is actually running. Consider checking the return value and either retrying or returning an error to the client.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/createSandboxHandler.ts, line 109:
<comment>The return value of `updateSessionSandboxState` is not checked. If the DB write fails (returns `null`), the client gets a 200 but the session row isn't marked active—causing `GET /api/sandbox/status` to report `no_sandbox` while the sandbox is actually running. Consider checking the return value and either retrying or returning an error to the client.</comment>
<file context>
@@ -0,0 +1,127 @@
+ const nextState = sandbox.getState() as Json;
+ const expiresAt =
+ typeof sandbox.expiresAt === "number" ? new Date(sandbox.expiresAt).toISOString() : null;
+ await updateSessionSandboxState({
+ id: sessionId,
+ sandboxState: nextState,
</file context>
| @@ -0,0 +1,103 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
This new test file exceeds the repository’s 100-line limit required by Rule 3.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/__tests__/validateCreateSandboxBody.test.ts, line 21:
<comment>This new test file exceeds the repository’s 100-line limit required by Rule 3.</comment>
<file context>
@@ -0,0 +1,103 @@
+ method: "POST",
+ headers: { "Content-Type": "application/json", "x-api-key": "k" },
+ body: typeof body === "string" ? body : JSON.stringify(body),
+ });
+}
+
</file context>
| @@ -0,0 +1,156 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
New test file exceeds the 100-line file-size limit required by the maintainability rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/__tests__/createSandboxHandler.test.ts, line 1:
<comment>New test file exceeds the 100-line file-size limit required by the maintainability rule.</comment>
<file context>
@@ -0,0 +1,156 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+
</file context>
| @@ -0,0 +1,138 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
Test file exceeds the 100-line maintainability limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/__tests__/getSandboxStatusHandler.test.ts, line 1:
<comment>Test file exceeds the 100-line maintainability limit.</comment>
<file context>
@@ -0,0 +1,138 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+
</file context>
sweetmantech
commented
May 7, 2026
Contributor
Author
There was a problem hiding this comment.
KISS
- actual: lib/supabase/sessions/updateSessionSandboxState.ts
- required: lib/supabase/sessions/updateSession.ts
Contributor
Author
Smoke test results — preview deploymentRan end-to-end against ✅ All negative paths pass
✅ Happy path provisions correctly
{
"createdAt": 1778167819889,
"timeout": 1800000,
"currentBranch": "main",
"mode": "vercel",
"timing": { "readyMs": 1121 }
}
{
"status": "active",
"hasSnapshot": false,
"lifecycleVersion": 1,
"lifecycle": {
"serverTime": 1778167820412,
"state": "active",
"lastActivityAt": 1778167819855,
"hibernateAfter": null,
"sandboxExpiresAt": 1778169619854
}
}🐛 Bug found — needs a fix before merge
Repro:
Root cause: Impact: the chat loading UX would flip out of "loading sandbox..." immediately on session entry, before any sandbox actually exists. That's exactly the bug this endpoint is meant to prevent. Fix: distinguish "runtime state" (has |
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Contributor
Author
|
Pushed Your review comments
I also extracted Tier 1 correctness fixes (from the open-agents comparison)
One thing left to flagThe YAGNI removal of Open cubic-bot comments not actioned
|
sweetmantech
commented
May 7, 2026
| */ | ||
| export function buildSource({ | ||
| repoUrl, | ||
| branch, |
Contributor
Author
There was a problem hiding this comment.
YAGNI - When is branch ever defined?
Suggested change
| branch, |
| message: "repoUrl must be a valid GitHub repository URL", | ||
| }), | ||
| sessionId: z.string().optional(), | ||
| branch: z.string().optional(), |
Contributor
Author
There was a problem hiding this comment.
KISS - Let's remove this branch name. We'll always be working off of the default branch.
There was a problem hiding this comment.
1 issue found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/supabase/sessions/__tests__/updateSession.test.ts">
<violation number="1" location="lib/supabase/sessions/__tests__/updateSession.test.ts:4">
P2: Move the mock state into `vi.hoisted`; the hoisted `vi.mock` factory cannot safely close over these module-scope `vi.fn()` variables.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Comment on lines
+4
to
+7
| const updateChain = vi.fn(); | ||
| const eqChain = vi.fn(); | ||
| const selectChain = vi.fn(); | ||
| const singleChain = vi.fn(); |
There was a problem hiding this comment.
P2: Move the mock state into vi.hoisted; the hoisted vi.mock factory cannot safely close over these module-scope vi.fn() variables.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/sessions/__tests__/updateSession.test.ts, line 4:
<comment>Move the mock state into `vi.hoisted`; the hoisted `vi.mock` factory cannot safely close over these module-scope `vi.fn()` variables.</comment>
<file context>
@@ -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();
</file context>
Suggested change
| const updateChain = vi.fn(); | |
| const eqChain = vi.fn(); | |
| const selectChain = vi.fn(); | |
| const singleChain = vi.fn(); | |
| const { updateChain, eqChain, selectChain, singleChain } = vi.hoisted(() => ({ | |
| updateChain: vi.fn(), | |
| eqChain: vi.fn(), | |
| selectChain: vi.fn(), | |
| singleChain: vi.fn(), | |
| })); |
Tip: Review your code locally with the cubic CLI to iterate faster.
3 tasks
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) <noreply@anthropic.com>
Contributor
Author
|
Pushed
TDD red → green:
Net: -50 / +37 lines. |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/sandbox/createSandboxHandler.ts">
<violation number="1" location="lib/sandbox/createSandboxHandler.ts:107">
P2: `currentBranch` can be reported incorrectly as `"main"` for repos whose default branch is not main.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
| { | ||
| createdAt: Date.now(), | ||
| timeout: sandbox.timeout ?? DEFAULT_TIMEOUT_MS, | ||
| currentBranch: sandbox.currentBranch ?? DEFAULT_BRANCH, |
There was a problem hiding this comment.
P2: currentBranch can be reported incorrectly as "main" for repos whose default branch is not main.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/sandbox/createSandboxHandler.ts, line 107:
<comment>`currentBranch` can be reported incorrectly as `"main"` for repos whose default branch is not main.</comment>
<file context>
@@ -102,7 +104,7 @@ export async function createSandboxHandler(request: NextRequest): Promise<NextRe
createdAt: Date.now(),
timeout: sandbox.timeout ?? DEFAULT_TIMEOUT_MS,
- currentBranch: branch,
+ currentBranch: sandbox.currentBranch ?? DEFAULT_BRANCH,
mode: "vercel",
timing: { readyMs: Date.now() - startTime },
</file context>
Tip: Review your code locally with the cubic CLI to iterate faster.
Contributor
Author
Smoke test results — re-run after
|
| Test | Expected | Got |
|---|---|---|
POST /api/sandbox no auth |
401 | 401 {"status":"error","error":"Exactly one of x-api-key or Authorization must be provided"} |
POST /api/sandbox auth + {} |
400 missing repoUrl | 400 {"status":"error","missing_fields":["repoUrl"],"error":"repoUrl is required"} |
GET /api/sandbox/status no sessionId |
400 | 400 {"status":"error","error":"Missing sessionId"} |
GET /api/sandbox/status?sessionId=00000000-… |
404 | 404 {"status":"error","error":"Session not found"} |
✅ NEW: Tier 1 GitHub URL validation
POST /api/sandbox with {"repoUrl":"https://gitlab.com/o/r"}:
HTTP 400
{"status":"error","missing_fields":["repoUrl"],"error":"repoUrl must be a valid GitHub repository URL"}Clean 400 with a real explanation instead of the previous "fall through to confusing 502 from the provider."
✅ FIX VERIFIED: status='no_sandbox' before provision
The bug we caught last round (status reported "active" immediately after session creation, defeating the loading UX) is fixed. New session → status:
HTTP 200
{
"status": "no_sandbox",
"hasSnapshot": false,
"lifecycleVersion": 0,
"lifecycle": { "state": "provisioning", "sandboxExpiresAt": null, ... }
}After POST /api/sandbox provisions, status correctly flips to "active" with bumped lifecycleVersion: 1 and a real sandbox_expires_at (~30min ahead).
✅ Branch input dropped from contract
POST /api/sandbox with {"repoUrl": "...", "sessionId": "...", "branch": "feat/should-be-ignored"}:
HTTP 200
{ "createdAt": ..., "timeout": 1800000, "currentBranch": "main", "mode": "vercel", "timing": { "readyMs": 1285 } }Rogue branch input silently stripped by Zod; sandbox provisions on the default branch as expected. Response currentBranch reads from the sandbox handle ("main") rather than echoing input.
✅ Happy path
POST /api/sandbox with {"repoUrl":"https://github.com/recoupable/api","sessionId":"..."} (no branch field):
HTTP 200
{ "createdAt": ..., "timeout": 1800000, "currentBranch": "main", "mode": "vercel", "timing": { "readyMs": 1307 } }PR is fully exercised end-to-end and behaves to spec.
sweetmantech
added a commit
that referenced
this pull request
May 7, 2026
…open-agents (#522) (#524) * feat(sandbox): port POST /api/sandbox + GET /api/sandbox/status from open-agents 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 * fix(sandbox): treat type-stub sandbox_state as no_sandbox in /status 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 * refactor(sandbox): SRP/KISS extractions + Tier 1 correctness fixes 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) * refactor(sandbox): drop branch from POST /api/sandbox contract 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) <noreply@anthropic.com>
6 tasks
sweetmantech
added a commit
that referenced
this pull request
May 7, 2026
* feat(sandbox): port GET /api/sandbox/reconnect from open-agents Implements the live runtime probe endpoint required for the chat loading-UX cutover on session re-entry / tab refocus. Matches the contract documented in recoupable/docs#195 (now merged on main). Unlike GET /api/sandbox/status (DB-only read), /reconnect actually runs `sandbox.exec("pwd")` inside the runtime so the UI can distinguish a truly-alive sandbox from a DB row whose sandbox no longer exists. Behavior: - 200 status="no_sandbox" when sandbox_state lacks runtime metadata (delegates to hasRuntimeSandboxState — the same gate /status uses) - 200 status="connected" with sandbox.expiresAt when the probe succeeds - 200 status="expired" when the probe throws — also clears sandbox_state on the session row and sets lifecycle_state to "hibernated" so subsequent /status reads agree with the probe - hasSnapshot derived from snapshot_url across all three outcomes - 4xx for auth (401), missing sessionId (400), forbidden (403), not-found (404) — same envelope as /status Files follow the existing api conventions established by PR #522: - app/api/sandbox/reconnect/route.ts: thin GET delegation + OPTIONS - lib/sandbox/getSandboxReconnectHandler.ts: handler logic - Reuses validateAuthContext, selectSessions, hasRuntimeSandboxState, buildLifecycle, connectSandbox, updateSession - Error envelope { status, error } matches sessions PRs TDD red -> green: - Handler tests cover auth fail, missing sessionId, 404, 403, no_sandbox, hasSnapshot derivation, connected (with expiresAt), expired (with state-clear assertion), and lifecycle envelope shape - Thin route shell test asserts delegation - Suite: 2516 -> 2526 (+10 net new tests), pnpm lint:check clean Out of scope (deferred per the gap analysis): - Transient vs unavailable error distinction. open-agents preserves runtime state on transient errors (network blip != dead sandbox). v1 treats every probe failure as expired, which is safer for the loading UX (user can retry). Worth porting once we have a real signal that this is happening in production. - Stale-state lifecycle workflow kick (open-agents' /status has it too — needs workflow infra in api). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(sandbox): extract noSandboxResponse to its own file (SRP) Per review feedback on PR #525 — pulls the inline `noSandboxResponse` helper out of `getSandboxReconnectHandler.ts` into its own file so it can be reused by future endpoints (e.g., `/snapshot` resume) and so the handler file stops carrying response-shape construction logic. The narrowed `ReconnectBody` type in the handler now only covers the two outcomes the handler actually constructs locally (`connected` / `expired`); the `no_sandbox` shape lives with its builder. TDD red -> green: 3 unit tests for the extracted helper covering 200 status, hasSnapshot derivation, and lifecycle envelope projection. Suite: 2526 -> 2529. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(sandbox): extract validateSandboxReconnectRequest + fix preview build Two changes bundled: 1) SRP per review feedback (sweetmantech) — extract the auth + sessionId-from-query + session lookup + ownership check pre-flight from `getSandboxReconnectHandler` into its own `validateSandboxReconnectRequest.ts`. Mirrors the `validateCreateSandboxBody` pattern: returns either a 4xx NextResponse describing the first failure, or `{ row, auth }` for the handler to consume. 2) Type fix for `next build` — `connectSandbox(row.sandbox_state as SandboxState)` failed to compile against `Json` (union includes primitives + arrays); cast through `unknown` first. The `hasRuntimeSandboxState` gate above ensures the runtime shape is safe at the call site, so the double cast is justified — comment added explaining why. The vitest pass alone wasn't enough to catch the type error — `next build` runs a separate `tsc` step that the test runner skips. Caught by the Vercel preview build failing on the previous commit. TDD red -> green: - 5 unit tests for the new validator covering auth fail (passes through), missing sessionId (400), session not found (404), ownership mismatch (403), and happy-path return shape ({row, auth}) - Existing handler tests pass unchanged — the module-level mocks for validateAuthContext / selectSessions still intercept the calls now that they're behind the new validator - Suite: 2529 -> 2534, pnpm lint:check clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the two session-scoped sandbox endpoints needed to drive the chat "loading sandbox…" UX on session entry. Matches the contract documented in recoupable/docs#192 (merged on main as commit `2e9099a`).
This is the next route-by-route port from open-agents into api after #514 (GET sessions) and #515 (POST sessions). Reuses the sandbox abstraction inlined in #507.
Why these two endpoints first
POST `/api/sessions` (already shipped in #515) only creates the session + initial chat row — it does not spawn a sandbox. To drive a UX where a new session lands on a page showing "loading sandbox…" until the sandbox is ready, chat needs:
Without these, the chat UI cannot render the loading flow.
Architecture
Following api conventions per CLAUDE.md:
Behavior
POST `/api/sandbox`
GET `/api/sandbox/status?sessionId=…`
TDD
Red → green throughout. 30 new tests, all passing (suite: 2461 → 2491):
Coverage: success paths, all 4xx/5xx response codes, auth short-circuit, ownership 403, missing-session 404, validator edge cases (malformed JSON, wrong types, missing required fields), `isNewBranch` flipping `source.branch → source.newBranch`, status-active vs no_sandbox vs expired, hasSnapshot derivation.
Verification
Out of scope (deferred)
Test plan
What follows this PR
🤖 Generated with Claude Code
Summary by cubic
Ports session-scoped sandbox endpoints to power the chat “loading sandbox…” flow. Adds strict GitHub repo URL validation, private-repo cloning via a service token, drops
branchfrom the POST contract, and fixes a status bug where fresh sessions reported ready.New Features
POST /api/sandbox: provisions or resumes; validates{ repoUrl, sessionId? }; enforces ownership whensessionIdis set; uses deterministicsession-{id}for idempotent resume; persistssandbox_state,lifecycle_state="active",lifecycle_version++,sandbox_expires_at,last_activity_at, and clearssnapshot_url/snapshot_created_at; supports private repos viagetServiceGithubToken; returns{ createdAt, timeout, currentBranch, mode: "vercel", timing: { readyMs } }(readscurrentBranchfrom the sandbox handle).GET /api/sandbox/status?sessionId=…: DB-only; returnsstatus: "active"only whensandbox_statehas runtime metadata (non-emptysandboxName) and is not expired (10s buffer), else"no_sandbox"; includeshasSnapshot,lifecycleVersion, andlifecycle { serverTime, state, lastActivityAt, hibernateAfter, sandboxExpiresAt }; enforces ownership and 404s when missing.Bug Fixes
"active"from the type-only stub{ type: "vercel" }written at session creation; introducedhasRuntimeSandboxStateand tightenedisSandboxActiveto require a non-emptysandboxName.Written for commit 5b1cca4. Summary will update on new commits.