docs(sandbox): drop branch + isNewBranch from POST /api/sandbox#194
Merged
sweetmantech merged 1 commit intomainfrom May 7, 2026
Merged
docs(sandbox): drop branch + isNewBranch from POST /api/sandbox#194sweetmantech merged 1 commit intomainfrom
sweetmantech merged 1 commit intomainfrom
Conversation
YAGNI/KISS — the chat UX always works off the repo's default branch. The api implementation is dropping these inputs in the matching api PR (#522), so the published reference needs to drop them too to keep the contract honest. Removed: - `branch` (request property) - `isNewBranch` (request property) Kept: - `currentBranch` (response field) — informational; reports which branch the sandbox actually checked out Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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}
sweetmantech
added a commit
to recoupable/api
that referenced
this pull request
May 7, 2026
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>
Merged
7 tasks
sweetmantech
added a commit
to recoupable/api
that referenced
this pull request
May 7, 2026
…open-agents (#522) * 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 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sweetmantech
added a commit
to recoupable/api
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>
4 tasks
sidneyswift
added a commit
that referenced
this pull request
May 7, 2026
Pulls in three commits from main: - Sandbox API: GET /api/sandbox/reconnect added (PR #195) — wired into our Sessions group alongside sandbox/create + sandbox/status - Sandbox API: POST /api/sandbox request schema dropped branch + isNewBranch (PR #194) — auto-merged - Music-video workflow: substantive content updates (PR #193) — async pipeline section, sandbox file fetch path, resolution. Took main's version verbatim then re-applied this PR's prose sweep (no em-dashes, no 'recipe' synonym for 'workflow').
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
Removes the
branchandisNewBranchrequest properties from thePOST /api/sandboxOpenAPI spec so the published reference matches the contract the api implementation is shipping in recoupable/api#522.Why
YAGNI/KISS per review feedback on the api PR:
The chat UX always works off the repo's default branch — there's no caller that passes a custom branch or wants to create a new branch through this endpoint.
What changed
branch(request)\"main\"isNewBranch(request)falsecurrentBranch(response)Net diff: -10 lines.
Test plan
Coordination
This PR pairs with the matching api change in #522. Either order is fine — neither breaks the other since the api Zod schema treats unknown input fields as ignorable, and the docs spec only describes accepted fields.
🤖 Generated with Claude Code
Summary by cubic
Updated the
POST /api/sandboxOpenAPI reference to remove thebranchandisNewBranchrequest fields so the docs match the API. The request body now only includesrepoUrlandsessionId; the response still returnscurrentBranch.Written for commit 04a2607. Summary will update on new commits.