feat: extract sandbox runtimes and orchestration (Phase B/B′)#3
Conversation
…debase Moves the provider-agnostic sandbox layer into this package as sanitized content. History is deliberately NOT ported: every pre-sanitization revision would otherwise be permanently public. Contents: - port.ts SandboxRuntime port + SandboxRuntimeCapabilities - types.ts bootstrap plane (WorkflowRuntime/RuntimeCapabilities) - daytona/ DaytonaRuntime + tests - e2b/ E2BSandboxRuntime - local/ LocalSandboxRuntime - orchestrator.ts SandboxOrchestrator - mount-script.ts relayfile-mount shell builders Two capability types coexist under distinct names on purpose: SandboxRuntimeCapabilities (outer orchestration) and RuntimeCapabilities (in-sandbox bootstrap). They describe different planes and merging them would be a category error. No baked-in configuration: sandbox template, home directory, and mount state directory are required arguments. No credential resolution and no infrastructure bindings cross into this package — callers inject an API key or auth token. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
khaliqgant
added
the
no-agent-relay-review
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a provider-agnostic sandbox runtime and orchestration package supporting Daytona, E2B, and Local runtimes, along with a relayfile-mount daemon integration. The review feedback highlights several critical areas for improvement: resolving potential unhandled promise rejections from Promise.race timeout helpers, escaping user messages in shell scripts to prevent syntax or injection issues, respecting user-configured timeouts during detached sandbox creation instead of capping them at 15 seconds, appending random suffixes to session IDs to prevent concurrent execution collisions, and parallelizing file uploads using Promise.all to improve performance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async function awaitLookupOperation<T>( | ||
| operation: Promise<T>, | ||
| deadline: LookupDeadline, | ||
| description: string, | ||
| ): Promise<T> { | ||
| const remainingMs = deadline.endsAt - Date.now(); | ||
| if (remainingMs <= 0) { | ||
| throw new Error(`Daytona sandbox lookup exceeded ${deadline.timeoutMs}ms while ${description}`); | ||
| } | ||
| let timer: ReturnType<typeof setTimeout> | undefined; |
There was a problem hiding this comment.
Using Promise.race for timeouts can lead to unhandled promise rejections if the raced operation promise rejects after the timeout has already fired. To prevent this, attach a silent .catch() handler to the operation promise.
async function awaitLookupOperation<T>(
operation: Promise<T>,
deadline: LookupDeadline,
description: string,
): Promise<T> {
const remainingMs = deadline.endsAt - Date.now();
if (remainingMs <= 0) {
throw new Error(`Daytona sandbox lookup exceeded ${deadline.timeoutMs}ms while ${description}`);
}
operation.catch(() => {});
let timer: ReturnType<typeof setTimeout> | undefined;| async function awaitLocalSandboxLookupOperation<T>( | ||
| operation: Promise<T>, | ||
| deadline: LocalSandboxLookupDeadline, | ||
| description: string, | ||
| ): Promise<T> { | ||
| const remainingMs = deadline.endsAt - Date.now(); | ||
| if (remainingMs <= 0) { | ||
| throw new Error( | ||
| `Local sandbox lookup exceeded ${deadline.timeoutMs}ms while ${description}`, | ||
| ); | ||
| } | ||
| let timer: ReturnType<typeof setTimeout> | undefined; |
There was a problem hiding this comment.
Using Promise.race for timeouts can lead to unhandled promise rejections if the raced operation promise rejects after the timeout has already fired. To prevent this, attach a silent .catch() handler to the operation promise.
async function awaitLocalSandboxLookupOperation<T>(
operation: Promise<T>,
deadline: LocalSandboxLookupDeadline,
description: string,
): Promise<T> {
const remainingMs = deadline.endsAt - Date.now();
if (remainingMs <= 0) {
throw new Error(
`Local sandbox lookup exceeded ${deadline.timeoutMs}ms while ${description}`,
);
}
operation.catch(() => {});
let timer: ReturnType<typeof setTimeout> | undefined;| function cleanupStatusShell(message: string | undefined): string { | ||
| if (!message) return ""; | ||
| return ` printf '{"message":"${message}","flushExitCode":%s,"killAttempted":%s,"killExitCode":%s,"pendingWriteback":%s,"hasPendingWriteback":%s,"outboxNeedsAttention":%s,"commandDraftWrittenThisRun":%s,"commandDraftsUndeliverable":%s}\\n' "$relayfile_mount_status" "$relayfile_mount_kill_attempted" "$relayfile_mount_kill_status" "$relayfile_mount_pending_writeback" "$relayfile_mount_has_pending_writeback" "$relayfile_mount_outbox_needs_attention" "$relayfile_mount_command_draft" "\${relayfile_mount_command_drafts_undeliverable:-null}" >&2`; | ||
| } |
There was a problem hiding this comment.
The message parameter is directly interpolated into a single-quoted printf shell command without escaping. If the message contains a single quote (e.g., worker's cleanup) or double quotes, it will break the shell command syntax or the JSON structure, causing the entire cleanup trap to fail. Escape single and double quotes to ensure safety.
function cleanupStatusShell(message: string | undefined): string {
if (!message) return "";
const safeMessage = message.replaceAll('"', '\\"').replaceAll("'", "'\\''");
return ` printf '{"message":"${safeMessage}","flushExitCode":%s,"killAttempted":%s,"killExitCode":%s,"pendingWriteback":%s,"hasPendingWriteback":%s,"outboxNeedsAttention":%s,"commandDraftWrittenThisRun":%s,"commandDraftsUndeliverable":%s}\\n' "$relayfile_mount_status" "$relayfile_mount_kill_attempted" "$relayfile_mount_kill_status" "$relayfile_mount_pending_writeback" "$relayfile_mount_has_pending_writeback" "$relayfile_mount_outbox_needs_attention" "$relayfile_mount_command_draft" "\\${relayfile_mount_command_drafts_undeliverable:-null}" >&2`;
}| target: client.target, | ||
| }, | ||
| undefined, | ||
| createOptions ? { timeout: Math.min(createOptions.timeout, 15) * 1000 } : undefined, |
There was a problem hiding this comment.
Capping the detached sandbox creation timeout at 15 seconds (Math.min(createOptions.timeout, 15)) is extremely short and will cause detached launches to fail prematurely if the cloud provider takes longer than 15 seconds to provision the VM. The user-configured timeout should be respected.
| createOptions ? { timeout: Math.min(createOptions.timeout, 15) * 1000 } : undefined, | |
| createOptions ? { timeout: createOptions.timeout * 1000 } : undefined, |
| throw new Error('Daytona session execution is not available on this sandbox'); | ||
| } | ||
|
|
||
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}`; |
There was a problem hiding this comment.
Generating the default sessionId using only Date.now() can lead to collisions if multiple scripts are started concurrently on the same sandbox handle within the same millisecond. This will cause them to write to and read from the same log/exit files, leading to severe log corruption. Append a small random suffix to guarantee uniqueness.
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}`; | |
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; |
| suppressInputEcho?: boolean; | ||
| }): Promise<AsyncRunStartResult> { | ||
| const sandbox = await this.requireSandbox(handle); | ||
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}`; |
There was a problem hiding this comment.
Generating the default sessionId using only Date.now() can lead to collisions if multiple scripts are started concurrently on the same sandbox handle within the same millisecond. This will cause them to write to and read from the same log/exit files, leading to severe log corruption. Append a small random suffix to guarantee uniqueness.
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}`; | |
| const sessionId = options.sessionId ?? `run-${handle.id}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; |
| for (const file of options.files) { | ||
| await this.uploadFile(handle, file.source, file.destination); | ||
| } |
There was a problem hiding this comment.
Uploading files sequentially in a for...of loop can be slow for bundles with multiple files. Using Promise.all to upload them concurrently will significantly improve efficiency.
| for (const file of options.files) { | |
| await this.uploadFile(handle, file.source, file.destination); | |
| } | |
| await Promise.all( | |
| options.files.map((file) => this.uploadFile(handle, file.source, file.destination)) | |
| ); |
| for (const file of options.files) { | ||
| const data = typeof file.source === "string" ? file.source : toArrayBuffer(file.source); | ||
| await sandbox.files.write(file.destination, data); | ||
| } |
There was a problem hiding this comment.
Uploading files sequentially in a for...of loop can be slow for bundles with multiple files. Using Promise.all to upload them concurrently will significantly improve efficiency.
await Promise.all(
options.files.map(async (file) => {
const data = typeof file.source === "string" ? file.source : toArrayBuffer(file.source);
await sandbox.files.write(file.destination, data);
})
);
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aba5fcdc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sandbox = await statics.create(this.template, { | ||
| apiKey: this.apiKey, | ||
| ...(Object.keys(metadata).length > 0 ? { metadata } : {}), | ||
| ...(hasEntries(options.env) ? { envs: options.env } : {}), | ||
| timeoutMs, |
There was a problem hiding this comment.
When launching an E2B sandbox, the v2 SDK interprets Sandbox.create's timeoutMs as the sandbox's automatic lifetime, not the HTTP creation deadline. Consequently, the default createTimeoutMs kills foreground/bootstrap sandboxes after 120 seconds, while createTimeoutSeconds unexpectedly changes their lifetime; only startScript later extends it. Use requestTimeoutMs for the creation budget and configure the sandbox lifetime separately.
Useful? React with 👍 / 👎.
| const wrapped = | ||
| `mkdir -p ${shellSingleQuote(dir)}; ` + | ||
| `{ ${options.command}\n} > ${shellSingleQuote(outPath)} 2>&1; ` + | ||
| `echo $? > ${shellSingleQuote(exitPath)}`; |
There was a problem hiding this comment.
When options.command contains a shell-level exit or exec, the brace group executes it in the wrapper shell, preventing the following echo $? from creating the exit sentinel. getScriptStatus then reports exitCode: null indefinitely even though the command has ended, stranding the async poll until an outer deadline or sandbox expiry. Execute the user command in a nested shell or otherwise guarantee sentinel creation.
Useful? React with 👍 / 👎.
| result?: string; | ||
| }; | ||
| return { | ||
| output: body.output ?? body.stdout ?? body.stderr ?? body.result ?? "", |
There was a problem hiding this comment.
When the local service returns separate stdout and stderr fields without output, this nullish chain keeps only stdout, dropping all stderr; a combined result is likewise ignored whenever stdout exists. The runtime contract requires output to be merged, and consumers such as SandboxOrchestrator capture only that field, so diagnostics and failure details disappear. Combine both streams or prefer an explicitly combined result.
Useful? React with 👍 / 👎.
1 participant
Moves the provider-agnostic sandbox layer into this package. Packaging change only — no delivery semantics are altered.
What landed
src/port.tsSandboxRuntimeport +SandboxRuntimeCapabilitiessrc/types.tsWorkflowRuntime,RuntimeCapabilities,RuntimeHandle)src/daytona/runtime.tsDaytonaRuntime(+ 1108 lines of tests)src/e2b/runtime.tsE2BSandboxRuntimesrc/local/runtime.tsLocalSandboxRuntimesrc/orchestrator.tsSandboxOrchestratorsrc/mount-script.tsrelayfile-mountshell buildersGate: build clean · typecheck clean · 35 tests, 34 pass, 0 fail, 1 skipped (the skip is a live-provider smoke test that needs real credentials).
Two capability types, deliberately not merged
SandboxRuntimeCapabilities(outer orchestration: async exec, reattach, warm lease, lifecycle) andRuntimeCapabilities(in-sandbox bootstrap: pty, snapshots, isolation, streaming logs) describe different planes. They previously lived in separate packages and never shared a namespace; consolidating here made the distinction load-bearing.The naive neutralization would have collapsed both to
RuntimeCapabilitiesand silently merged two unrelated concepts. They keep distinct names, andsrc/index.tsdocuments why — so the split is enforced by the type system rather than by memory.Configuration is injected, never baked in
Values that are environment- or account-specific are now required arguments, not defaults:
E2BSandboxRuntimeOptions.template— templates are account-scopedDaytonaRuntimeOptions.defaultHomeDir— image-specificRelayfileMountShellOptions.stateDir— image-specificA default here is worse than no default: it silently works for one consumer and misleads everyone else.
No credential resolution crosses this boundary
The package resolves zero credentials and holds zero infrastructure bindings. Callers inject an API key or auth token. Two separate credential-resolution paths were found during extraction and both stayed behind with the deployment that owns them.
Notes for reviewers
daytona/carries tests here. The orchestrator, mount-script, E2B, and local runtimes have suites that did not move with them, so roughly a third of the package is directly covered. Porting those suites is tracked as a prerequisite before any publish, not as a follow-up.noUncheckedIndexedAccess, which surfaced a regex capture group that could parse asNaN. Fixed here by guarding the group; filed separately against the original so the two stay aligned.Summary by cubic
Extracted the provider-agnostic sandbox runtimes and orchestration into
@agent-relay/sandbox. Packaging-only change; behavior and delivery semantics are unchanged.E2BSandboxRuntimeOptions.templateDaytonaRuntimeOptions.defaultHomeDirRelayfileMountShellOptions.stateDir@daytonaio/sdk@>=0.180.0 <0.181.0ande2b@^2.Written for commit 4aba5fc. Summary will update on new commits.