Repository navigation
feat: add @e2b/acp for running ACP agents in a sandbox - #2025
max-sudolabs wants to merge 21 commits into
Conversation
🦋 Changeset detectedLatest commit: 336eea0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
TASTE.md review of @e2b/acp, covering the new packages/acp-js package and the startStreaming / retainOutput changes in packages/js-sdk.
Rules checked: T-1/T-2 parity, T-3 golden rule, T-4 factories, T-9 verbs, T-14 enums over booleans, T-19/T-20 data types, T-22/T-23 option and result naming, T-24 overloads, T-26/T-27/T-30 handles, callbacks and stop(), T-44–T-47 timeouts and defaults, T-54 entry point, T-57–T-63 errors, T-69/T-70/T-72 docs.
Violations: 5 inline, plus the following, which isn't tied to a single line:
- T-1 (parity):
@e2b/acpis JS-only. The PR has no Python package and no change underpackages/python-sdkfor either the ACP entry point or the new streaming mode for background commands. If the Python port is planned as a follow-up (the branch isint-155-acp-js), please link the tracking issue in the description so the new names and options can be ported 1:1.
Already compliant: the required arguments are positional with a trailing named AcpStartOpts, which extends the shared command options (T-3, T-10, T-22). The constructors are @hidden/@internal (T-26). onStderr is awaited, so a slow consumer applies backpressure (T-27). The handle uses stop() rather than close() (T-30). DEFAULT_STOP_TIMEOUT_MS is a named constant documented with @default (T-47). Validating maxMessageBytes before anything starts throws InvalidArgumentError (T-59). The value and type exports in index.ts are kept separate (T-54).
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
New package for running Agent Client Protocol agents in E2B sandboxes. This first part has the newline-delimited JSON framing with a validated per-message size limit, and the rules that keep the agent's tools in the sandbox: initialize (also inside a batch) turns off the client fs and terminal capabilities, and agent fs/* and terminal/* requests are answered with method-not-found.
connectAcpAgent(sandbox, { command, client }) starts the agent over a
streaming command and returns an AcpAgent with the ACP
ClientSideConnection, close(), wait() and kill().
Writes, transport replies and EOF share one queue. close() bounds the
whole shutdown from the call, kills an agent whose output stream failed
without a known exit, and stops delivering later output. Closing the
writable is a graceful EOF that still reports a failed exit and a
failure to close stdin.
A small Python agent in a sandbox exercises the capability rewrite, rejected fs/terminal calls, ordering, UTF-8 split across bytes, the parse-error reply, the size limit, EOF, kill on close, failed exits, a failing stderr callback and cancellation, without any model or key.
Run the @e2b/acp suite when the package or the JS SDK changes, and include it in the release preflight, tests and publish.
kill() now releases agent output that waits for the client, so the command handle can see the exit and wait() resolves. Cleanup after an oversized first message waits for the transport instead of doing nothing when the output arrives before startStreaming() returns.
The line splitter kept the unfinished line as one string and split it again on every chunk, which made a long message quadratic to read. It now keeps the chunks and their byte count and joins them once.
…) to stop() connectAcpAgent(sandbox, command, client, opts?) matches the Python signature and keeps only optional settings in the options object. stop() ends the agent process that connectAcpAgent started, its default timeout is the exported DEFAULT_STOP_TIMEOUT_MS, and AcpStartOpts forwards an AbortSignal to the start request.
A failed output stream, for example after the start signal is aborted or onStderr throws, left the agent running with no working connection. The transport now kills it; the stream still fails with the original error.
…ze error at maxMessageBytes Also export DEFAULT_MAX_MESSAGE_BYTES next to DEFAULT_STOP_TIMEOUT_MS, and describe the JS SDK streaming hook as internal in the changeset.
e2b-dev#2044 added retainOutput to commands.run and commands.connect, so the internal Commands.startStreaming this branch carried is gone. The transport starts the agent with run({ background: true, retainOutput: false }) and the changeset no longer bumps the JS SDK. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
df373e1 to
9f142b0
Compare
There was a problem hiding this comment.
Thanks for the split, @max-sudolabs. I built both PRs and ran the full suites against prod: @e2b/acp 33/33, e2b-acp 34/34, plus lint, typecheck and build. Moving onto the public retainOutput from #2044 is the right call.
I answered the naming questions in their threads and left the rest inline, following TASTE.md in e2b-dev/sdk-harness. The one blocker is the release.yml argument. On my side, I'll set up trusted publishing for @e2b/acp on npm and e2b-acp on PyPI. Whichever PR lands second needs a quick rebase, because both touch the same lines in sdk_tests.yml and pnpm-lock.yaml.
Replace connectAcpAgent() with the static AcpAgent.start(), rename AcpStartOpts/AcpStopOpts to AcpAgentStartOpts/AcpAgentStopOpts and export the StderrHandler type.
…ntSideConnection AcpAgent.start() takes a client() app from @agentclientprotocol/sdk and connects it to the agent's stream, so agent.connection is a ClientConnection. ClientSideConnection is deprecated in @agentclientprotocol/sdk 1.8.0.
buildSession().start() queues every session update until it is read, so with updates handled in onNotification the queue only grows. Also say that closing the connection stops the agent in the background.
OndrejDrapalik
left a comment
There was a problem hiding this comment.
Thanks Max, all review points are addressed. Verified 336eea0 locally against prod: typecheck, lint and build pass, tests 35/35. The ClientApp move is the right call, and JS with no separate factory is fine since client() builds handlers per connection.
|
Moved to #2060 with the same branch and commits, so CI can run with repo secrets (GitHub does not pass them to PRs from forks, so the live tests here failed with an empty |
Summary
Adds
@e2b/acp(packages/acp-js) for running Agent Client Protocol agents (goose, OpenCode, Claude Code through its ACP adapter) inside an E2B sandbox and driving them with the official TypeScript ACP client,@agentclientprotocol/sdk(peer dependency).AcpAgent.start(sandbox, command, client, opts?)starts the agent with stdin open, speaks newline-delimited JSON-RPC over its stdin/stdout, connects yourclient()app from the ACP SDK to it, and returns anAcpAgentwith theClientConnectionpluspid,wait(),stop()andkill(). The client filesystem and terminal capabilities are turned off ininitialize, and agent requests forfs/*/terminal/*are rejected in the transport, so the agent edits files and runs commands with its own tools inside the sandbox. Network access and MCP servers passed to the agent are not restricted.The agent is started with
commands.run(cmd, { background: true, retainOutput: false })from #2044 (SDK-407), so a long-running agent does not grow the handle's buffers. No core changes. SDK-408 tracks dropping retained output for background commands in the next major.Split out of #1992 together with the Python package (#2026). The two PRs are independent and can merge in either order.
Usage
Review guide
Worth reading (~686 lines):
packages/acp-js/src/transport.tspackages/acp-js/src/utils.tsinitializerewrite,fs/*/terminal/*rejectionpackages/acp-js/src/index.tsAcpAgentandAcpAgent.startSkim:
.github/workflows/acp_js_tests.ymland the ACP JS entries insdk_tests.yml/release.yml(mirror code-interpreter),package.json, README,.changeset/acp-js-package.md.For automated reviewers
Behaviour the code is meant to guarantee, and the test that fails if it breaks. Offline tests use a fake command handle and need no API key; live tests start a scripted Python agent in a sandbox.
initializeforces them off (also inside a batch), agentfs/*/terminal/*requests get-32601, notifications are droppedrestrictClientCapabilities,filterAgentMessageutils.test.ts; end to end intransport.test.tsenqueueWritestop(timeoutMs): EOF and exit are bounded bytimeoutMsfrom the call; then kill unless an exit status is known; rejects if the kill failsAcpTransport.closekill()releases output waiting on a full read queue, so the handle sees the exit andwait()resolvesAcpTransport.killstop(), messages already queued stay readable and later ones are dropped, including later lines of a chunk that was waiting on backpressureclosingchecks inonStdoutanddeliverCountQueuingStrategy+resumeReadingLineSplittersignal, throwingonStderr) kills the agent and keeps the original erroronConnecthandler does not leak the started process; an oversized message that arrives beforerun()returns still stops the agentAcpAgent.start,closeTransportClientConnectionstops the agentcancel/ writableabort→closeTransportOut of scope: restricting network or MCP, batch semantics beyond the boundary check, reconnecting to a running agent.
Testing
@e2b/acp: 35 tests (unit, offline, live), typecheck and lint on this branch rebased on currentmain(includes feat(sdk): add retainOutput option to command handles #2044).AcpAgent.startandclient(): goose withgpt-5.6-lunaran the README example end to end (file written, exit 0).claude-agent-acp0.90.0) wrote and ran a file in the sandbox. The client received nofs/*orterminal/*calls, and the agent process was gone after it was stopped.Release notes for maintainers
@e2b/acpdepends one2bthroughworkspace:^, so it requires the core version released with it; it needsretainOutputfrom feat(sdk): add retainOutput option to command handles #2044, which is not released yet.@e2b/acpneeds trusted publishing configured before the first release.pkg_artifacts.ymlpreview builds (an ACP preview needs the matching preview core) and staging jobs insdk_tests.yml. Happy to add either if you want them.