promote: test → main (delete dead sandbox helpers)#513
Merged
sweetmantech merged 3 commits intomainfrom May 4, 2026
Merged
Conversation
) * refactor(sandbox): callers use open-agents abstraction (Phase 2.2) Replaces direct @vercel/sandbox SDK calls with the open-agents sandbox abstraction layer (inlined in Phase 2.1) for sandbox lifecycle (create + reconnect). HTTP response shapes preserved exactly. Per the agreed Option B (hybrid): only the lifecycle creator helpers get refactored. installClaudeCode / runClaudeCode / getSandboxStatus stay on the SDK directly because the abstraction does not cover their needs (sudo, stdout/stderr streaming, simple status reads). Those two install/run files are also dead orphans (defined but never called) and will be removed entirely after the full migration. Production refactor: createSandbox.ts Sandbox.create(...) -> VercelSandbox.create(...) Input: VercelSandboxConfig (was SDK params) Snapshot trigger: restoreSnapshotId field (was source: { type: "snapshot", ... }) Returns VercelSandbox (was SDK Sandbox) createSandboxWithFallback.ts cascade — passes restoreSnapshotId to createSandbox createSandboxFromSnapshot.ts type cascade only (Sandbox -> VercelSandbox) getActiveSandbox.ts Sandbox.get({name}) -> VercelSandbox.connect(name, {}) Status check: sandbox.status -> sandbox.sdkStatus getOrCreateSandbox.ts no code change — type cascades automatically processCreateSandbox.ts reads sandbox.sdkStatus instead of sandbox.status defensive nullish on createdAt Abstraction extension: vercel/sandbox/VercelSandbox.ts adds two readonly getters following the existing host/environmentDetails/expiresAt pattern: get sdkStatus(): string — raw SDK session status (running/pending/ stopped/failed/aborted/snapshotting), distinct from the abstraction's normalized status getter get createdAt(): Date | undefined — SDK session.createdAt These give api callers what they need to construct the existing HTTP response shape without breaking the abstraction's interface. Tests updated: createSandbox.test.ts mocks VercelSandbox.create instead of Sandbox.create; mock object uses sdkStatus instead of status createSandboxWithFallback.test.ts asserts restoreSnapshotId pass-through getActiveSandbox.test.ts mocks VercelSandbox.connect; sdkStatus on mock objects processCreateSandbox.test.ts mockSandbox uses sdkStatus Verification: - pnpm lint:check: clean - pnpm test: 2391/2391 pass - HTTP response shape unchanged: same fields, same enum values for sandboxStatus (sourced from the SDK now via sdkStatus, was directly via SDK Sandbox.status before — identical strings either way) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address PR #509 review feedback Three real issues from CodeRabbit + cubic: 1. createdAt staleness (CodeRabbit minor) The new `createdAt` getter on VercelSandbox skipped the `refreshStateFromCurrentSession()` step that `sdkStatus` uses, so readers right after a reconnect could see stale session metadata. Add the refresh. 2. Fabricated createdAt (cubic P2) Both createSandbox.ts and processCreateSandbox.ts had a `?? new Date().toISOString()` fallback that fabricated creation metadata when sandbox.createdAt was missing. The SDK guarantees createdAt is populated for any reachable instance, so the fallback was both wrong (fabricates data) and unnecessary. Tighten the getter to return `Date` (not `Date | undefined`) and throw with an explicit "SDK contract violation" message if the field is missing — fail-fast surfaces a real contract bug instead of silently lying. Drop the `?? new Date()` fallbacks at both call sites. 3. Misleading snapshot-restore branching (CodeRabbit major) createSandbox.ts had two paths — a "snapshot" branch that omitted DEFAULT_VCPUS/DEFAULT_RUNTIME (intent: let snapshot dictate), and a "fresh" branch that applied defaults. But VercelSandbox.create internally defaults vcpus=4 and runtime="node22" regardless, so the omission was a no-op — the abstraction always forwarded those to the SDK. Drop the misleading branching. Document the actual behavior at the top of createSandbox: "VercelSandbox.create applies its own defaults regardless of source — those apply to the runtime resources of the new sandbox even when restoring from a snapshot." Updated the snapshot-restore test to assert the actual call shape (vcpus + runtime + timeout + restoreSnapshotId) instead of just the original SDK-style truncated args. Verification: - pnpm lint:check: clean - pnpm test: 2391/2391 pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(sandbox): delete dead Claude Code helpers (Phase 2.3)
installClaudeCode and runClaudeCode were defined but never imported
anywhere in api production code — confirmed by grep on main:
$ grep -rn "installClaudeCode\b\|runClaudeCode\b" lib/ app/
lib/sandbox/installClaudeCode.ts:9: export async function installClaudeCode(...)
lib/sandbox/runClaudeCode.ts:10: export async function runClaudeCode(...)
Both files were skipped during the Phase 2.2 abstraction refactor
(per the agreed Option B — they used SDK features the abstraction
doesn't expose: sudo, stdout/stderr streaming, batched writes). With
the broader migration moving to Vercel Workflow + open-agents' agent
package for sandbox bootstrap, these orphans have no path to being
called again.
Removed:
lib/sandbox/installClaudeCode.ts (32 lines)
lib/sandbox/runClaudeCode.ts (29 lines)
lib/sandbox/__tests__/installClaudeCode.test.ts (4 tests)
lib/sandbox/__tests__/runClaudeCode.test.ts (6 tests)
Verification:
- pnpm lint:check: clean
- pnpm test: 2381/2381 pass (was 2391 — net -10 tests from the
two deleted test files)
Note: getOrCreateSandbox.ts also has zero importers per the audit
and is similarly dead, but is intentionally NOT deleted in this PR
since it was not explicitly flagged as orphan in the migration plan.
Worth a separate follow-up decision.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(sandbox): also delete getOrCreateSandbox + getActiveSandbox (YAGNI)
Cascade audit found two more truly-dead helpers per YAGNI:
getOrCreateSandbox.ts 0 importers (self-only references)
getActiveSandbox.ts only called by getOrCreateSandbox — orphan
once that goes
Removed:
lib/sandbox/getOrCreateSandbox.ts (39 lines)
lib/sandbox/getActiveSandbox.ts (33 lines)
lib/sandbox/__tests__/getOrCreateSandbox.test.ts (3 tests)
lib/sandbox/__tests__/getActiveSandbox.test.ts (4 tests)
Live consumers of related helpers preserved:
- createSandboxFromSnapshot still used by processCreateSandbox
- selectAccountSandboxes still used by aggregateAccountSandboxStats,
buildGetSandboxesParams, getSandboxesHandler, validateGetSandboxesRequest
Verification:
- pnpm lint:check: clean
- pnpm test: 2374/2374 pass (was 2381 — net -7 from the two deleted
test files; -3 from getOrCreateSandbox.test.ts + -4 from
getActiveSandbox.test.ts)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
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.
|
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
📝 WalkthroughWalkthroughFour sandbox helper functions related to Claude code execution are removed: ChangesSandbox Helper Functions Removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
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.
Promotes #512 (Phase 2.3 — delete dead sandbox helpers per YAGNI) from `test` to `main`.
Contents
What's removed
Four orphan helpers + their tests, all confirmed by grep to have zero importers in api production code:
Risk
Zero. Pure deletion of unreferenced code. Build success + 2374/2374 tests prove no silent dependencies. cubic approved 5/5; CodeRabbit was rate-limited so didn't post a follow-up but had nothing to flag in its prior pass.
Summary by cubic
Remove dead sandbox helpers and their tests to reduce maintenance surface. No runtime changes; these helpers had zero importers.
installClaudeCode,runClaudeCode,getOrCreateSandbox,getActiveSandboxand their tests.Written for commit e1ac582. Summary will update on new commits.
Summary by CodeRabbit