feat(sandbox): add Daytona as a manual-flip failover for E2B#5860
feat(sandbox): add Daytona as a manual-flip failover for E2B#5860TheodoreSpeaks wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Call sites switch imports only — function execute, doc compile/extract/render, Pi cloud backends, copilot payloads, and tests now use Operational/testing additions: provider conformance tests (same scenarios on both providers), Reviewed by Cursor Bugbot for commit 7b693c1. Bugbot is set up for automated code reviews on this repo. Configure here. |
github-actions
Bot
added
the
requires-mothership-merge
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds Daytona as a manually selected alternative to E2B. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "fix(sandbox): enforce timeout on Daytona..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
E2B was a hard single point of failure: lib/execution/e2b.ts had no retry
and no fallback, so a failed Sandbox.create() killed Python function blocks,
JS-with-imports, shell, doc generation and the Pi cloud agent outright.
Extract a SandboxRunner boundary (lib/execution/remote-sandbox) with an E2B
runner and a Daytona runner, selected once per execution by the
sandbox-provider-daytona AppConfig flag. Everything above the provider
boundary — marker parsing, mount materialization, file export, corruption
handling — is unchanged.
Selection resolves before create() and never mid-execution, since user code
has side effects. Each sandbox kind fails closed when its snapshot id is unset.
Notes on the Daytona adapter:
- language binds at create(), not per call: Daytona applies it as a sandbox
label and silently runs JS through Python if passed to codeRun
- Python routes via CodeInterpreter for its {name,value,traceback} error shape,
which matches E2B's and keeps formatE2BError's line offsets correct
- timeouts convert ms to seconds
- the streaming path delivers env via the filesystem API, as
SessionExecuteRequest has no env field and secrets must not reach a command line
Drops the dead E2BExecutionResult.images field (populated, never consumed).
Replaces the boolean sandbox-provider-daytona feature flag with a
SANDBOX_PROVIDER env var naming the provider ('e2b' default, or 'daytona').
A boolean doesn't scale to a third adapter; a keyed registry does.
- PROVIDERS is a Record<SandboxProviderId, SandboxProvider>, so adding an
adapter is one entry plus one id-union member — an unhandled provider is a
compile error, not a runtime surprise
- resolveProvider() reads env synchronously and throws on an unknown value
(fail fast) instead of an async feature-flag lookup
- drops the sandbox-provider-daytona flag and SANDBOX_PROVIDER_DAYTONA fallback
Verified end-to-end: a Python function block through the running app routes to
Daytona (Creating Daytona sandbox, kind: code) with SANDBOX_PROVIDER=daytona.
0048bbc to
d4327d3
Compare
Addresses the review round on #5860. - Availability was gated on isE2bEnabled / isE2BDocEnabled, so a Daytona-only deployment (E2B_ENABLED unset) had its Python/shell/JS-with-imports and doc paths rejected before the provider-neutral sandbox call could run. Replace both with provider-aware flags (isRemoteSandboxEnabled / isDocSandboxEnabled) derived from the selected SANDBOX_PROVIDER's own credentials + image. E2B behavior is unchanged (the E2B branch mirrors the old definitions exactly). - Make the function-block gate error messages provider-neutral. - Daytona's streaming runCommand (Pi) returned empty stdout/stderr and delivered output only via callbacks, so the Pi cloud flow — which parses markers from stdout and formats errors from stderr — saw nothing. Accumulate the streamed chunks and return them while still forwarding to the callbacks. Renames the env-flag exports (and the @sim/testing mock) to match. Adds a conformance test that the streamed Pi output lands in stdout/stderr.
|
@cursor review |
Addresses the round-2 review on #5860. env-flags lowercased SANDBOX_PROVIDER for the availability gate, but resolveProvider looked up the raw value in a lowercase-keyed map — so 'Daytona' passed the gate then threw Unknown SANDBOX_PROVIDER at create. resolveProvider now normalizes casing identically.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bf6991c. Configure here.
check:utils flagged the inline `error instanceof Error ? error.message : ...` pattern in the two new scripts. Use getErrorMessage from @sim/utils/errors, matching the repo convention the check enforces.
Daytona merges both streams into stdout and returns an empty stderr, but the shell-error, base64-export, and URL-mount error builders read only result.stderr — so Daytona failures surfaced a generic 'Process exited with code N' / 'base64 failed' / 'curl exited N' instead of the real command output that the API and agents rely on. Fall back to stdout before the generic message (provider-agnostic: E2B still populates stderr). Strengthens the shell-error conformance test to assert the real output surfaces.
|
@cursor review |
… copy
- Daytona's streaming path (Pi) started the command with runAsync:true and then
awaited getSessionCommandLogs with no bound, so a hung command never timed out
the way E2B's commands.run({ timeoutMs }) does. Race the log stream against the
timeout; on expiry return exit 124 with the accumulated output, and the finally's
deleteSession terminates the still-running command.
- Two user-facing strings still named E2B after the provider-neutral rename (the
isolated-vm sandboxPath remediation and the disabled-xlsx message). Made both
provider-neutral.
Adds a streaming-timeout conformance test.
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7b693c1. Configure here.
| stdout, | ||
| stderr: stderr || `Command timed out after ${options.timeoutMs}ms`, | ||
| exitCode: 124, | ||
| } |
There was a problem hiding this comment.
Orphaned stream promise on timeout
Medium Severity
When the streaming timeout wins Promise.race, the pending getSessionCommandLogs promise is abandoned. deleteSession in finally then tears down the session, which commonly rejects that orphaned promise with no handler, risking an unhandledRejection during Pi timeouts.
Reviewed by Cursor Bugbot for commit 7b693c1. Configure here.
| const finished = await this.sandbox.process.getSessionCommand(sessionId, commandId) | ||
| return { stdout, stderr, exitCode: finished.exitCode ?? 0 } | ||
| } catch (error) { | ||
| return { stdout, stderr, exitCode: 1 } |
There was a problem hiding this comment.
Streaming errors drop failure detail
Medium Severity
The streaming catch returns accumulated stdout/stderr and exit code 1 but never records the thrown error. Failures before any chunks (env file write, executeSessionCommand, missing cmdId) surface as empty output, so Pi clone/agent errors become opaque unknown error messages.
Reviewed by Cursor Bugbot for commit 7b693c1. Configure here.
1 participant
Summary
lib/execution/e2b.tshad no retry and no fallback, so one failedSandbox.create()killed Python function blocks, JS-with-imports, shell, doc generation, and the Pi cloud agentSandboxRunnerboundary (lib/execution/remote-sandbox) with an E2B runner and a Daytona runner, selected by thesandbox-provider-daytonaAppConfig flag — flip it during an E2B outage, no redeployscripts/verify-sandbox-parity.ts— 8 live executions against whichever provider the flag selects, as the pre-flip confidence checkE2BExecutionResult.imagesfield (populated, never consumed)Companion: simstudioai/mothership#369 (builds the shell + doc images on Daytona)
Notes
Selection resolves once before
create()and never mid-execution, since user code has side effects. Every sandbox kind fails closed when its snapshot id is unset.Three Daytona quirks the adapter absorbs:
languagebinds atcreate(), not per call — Daytona applies it as a sandbox label and silently runs JS through Python if passed tocodeRunCodeInterpreterfor its{name,value,traceback}error shape, which matches E2B's and keepsformatE2BError's line offsets correctSessionExecuteRequesthas noenvfield and secrets must not reach a command lineKnown caveat: Daytona cold-starts a sandbox in ~45s when it lands on a runner without the image cached (warm is ~800ms vs E2B's ~200ms). It costs wall-clock, not correctness —
timeoutMsgoes torunCode, notcreate, and plan sync budgets are 300s/3000s.Type of Change
Testing
verify-sandbox-parity.tsagainst both providers with live sandboxes: E2B 8/8, Daytona 8/8 (marker round-trip, data-science imports, structured errors, outbound network, JS-under-node, shell envs, file mount + text export, xlsx compile + base64 binary export)bun run lintandbun run check:api-validation:strictpass;tsc --noEmitcleancloud-review-tools.test.tsspawning Python that needs a realrgbinary, absent locallyChecklist