Skip to content

feat(sandbox-adapter): add sandbox providers for agentOS, E2B, and Daytona - #5799

Open
eersnington wants to merge 3 commits into
mainfrom
feat/sandbox-adapter
Open

eersnington wants to merge 3 commits into
mainfrom
feat/sandbox-adapter

Conversation

@eersnington

@eersnington eersnington commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Adds @rivet-dev/sandbox-adapter, where an agent's file and shell tools run.

import { e2bProvider } from "@rivet-dev/sandbox-adapter/e2b";

const agent = pi({ model: "anthropic/claude-opus-5-5", sandbox: e2bProvider() });
  • SandboxProvider creates, connects, suspends, and destroys one sandbox per actor. Sandbox is exec plus file operations.
  • Providers for agentOS, E2B, and Daytona. Each SDK is an optional peer dependency.
  • The agentOS provider runs the sandbox in the Services agentOS actor (pool services), so the app registers nothing. The VM sleeps when idle and is kept when the agent is destroyed.
  • Any other provider implements SandboxProvider.

This is part 1 of 3 in a stack:

@railway-app

railway-app Bot commented Sep 25, 2026

Copy link
Copy Markdown

This PR was not deployed automatically as @eersnington does not have access to the Railway project.

In order to get automatic PR deploys, please add @eersnington to your workspace on Railway.

@claude

claude Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review: @rivet-dev/sandbox-adapter

The interface is small and the providers are easy to follow. Most findings concern contract consistency across providers. I read the code only; I did not build or run anything.

Bugs and contract gaps

  • Daytona ignores signal. The interface says aborting rejects exec with Error("aborted"), but daytonaSandbox.exec never checks the signal, so an aborted call resolves normally later. Race the call against the signal and throw, or document the guarantee as provider-dependent.
  • E2B: an already-aborted signal is missed. The abort listener never fires if the signal was aborted before exec starts. Check signal.aborted before spawning.
  • E2B and Daytona drop output on timeout. The timeout path returns empty stdout/stderr, losing output already streamed through onData. runRemoteProcess keeps partial output, so providers are inconsistent.
  • Daytona folds stderr into stdout. Document this on SandboxExecResult.
  • runRemoteProcess does not kill the process if poll throws. A transient read error rejects exec and leaves the process running. Wrap the loop and kill on error.
  • Unbounded output buffering. stdout/stderr accumulate with no cap. Consider a byte limit on the retained result while still streaming onData.
  • Empty catch {} in killQuietly. Add a comment explaining why the error is ignored, and fix the formatting.

Design and API

  • agentOS destroy is omitted. The VM actor and files are never reclaimed when the agent is destroyed, so storage grows per destroyed agent. Consider an opt-in destroy or a follow-up.
  • agentOS cwd. posix.normalize("/workspace/") keeps the trailing slash.
  • Default agentOS exec needs sh, but the default config installs no software. Fail early with a clear error or default to the needed package.
  • Shadowing. process destructured from handle.v1 shadows the Node global. Rename it.
  • writeFile is text only while readFile returns bytes.
  • c.client() as AgentOSClient is an unchecked cast. Validate and throw an actionable error, per the Fail-By-Default guideline.
  • The 100ms poll loop is the pattern CLAUDE.md discourages. The rationale comment is fair; please track a follow-up for an awaitable wait action on the agentOS actor.

Tests
There are no tests. At minimum: runRemoteProcess with a hand-written fake RemoteProcess (ordering, sequence dedupe, timeout, abort, poll error); an integration test for the agentOS provider; and E2B/Daytona tests gated on API-key env vars.

Housekeeping

  • The package version is hardcoded to 2.3.20. Confirm it matches release tooling.
  • pnpm-lock.yaml has about 1.3k added lines. Confirm there are no unrelated bumps.
  • Consider a README or docs page, and check .claude/reference/docs-sync.md for new-package requirements.

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 4 medium-severity findings

Reviewed commit 0dbd4fe.

Comment thread shared/typescript/sandbox-adapter/src/daytona.ts Outdated
Comment on lines +61 to +62
const kill = () => void handle.kill();
options.signal?.addEventListener("abort", kill, { once: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Handle signals that abort before the listener is attached

AbortSignal.addEventListener does not replay an abort that already happened. If the signal is pre-aborted, or aborts while commands.run is awaiting startup, this listener never kills the handle; exec waits for the command to finish and only then throws. Check signal.aborted before launching and again immediately after acquiring the handle (killing it in the latter case) before installing the listener.

Comment thread shared/typescript/sandbox-adapter/src/agentos.ts Outdated
Comment thread shared/typescript/sandbox-adapter/src/remote-process.ts
@eersnington
eersnington force-pushed the feat/sandbox-adapter branch 2 times, most recently from e89e085 to c791b5c Compare September 26, 2026 00:04

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant