feat(runner): deliver gateway tools to Claude on Daytona via an in-sandbox stdio MCP shim#5244
feat(runner): deliver gateway tools to Claude on Daytona via an in-sandbox stdio MCP shim#5244mmabrouk wants to merge 5 commits into
Conversation
…ndbox stdio MCP shim Slice 1 of in-sandbox-tool-mcp (docs/design/agent-workflows/projects/in-sandbox-tool-mcp/). Today a Claude run on Daytona with any custom tool is refused up front (REMOTE_TOOLS_UNSUPPORTED_MESSAGE): execution via the file relay is solved, but nothing inside the sandbox advertises the tools to an MCP-client harness. This revives PR #4873 onto today's paths, consuming the relay client PR #5243 extracted. - tools/tool-mcp-stdio.ts: a dependency-free stdio MCP shim (9.6 kB bundled) the harness's ACP adapter spawns inside the sandbox. NDJSON JSON-RPC; tools/list serves the run's public specs from a specs FILE; tools/call writes a relay request through the shared relay client (atomic publication, per-tool timeoutMs) and maps failures to isError results. Fail-loud exit 1 on missing env/specs. Env contract in tools/tool-mcp-env.ts: AGENTA_AGENT_TOOLS_RELAY_DIR + AGENTA_AGENT_TOOLS_PUBLIC_SPECS_FILE (a file, not an unbounded env value copied through four exec layers). - engines/sandbox_agent/tool-mcp-assets.ts: bundle + specs upload, always-write (closes bundle-version skew in reused sandboxes), THROWS on missing bundle/failed upload on the path that requires the shim. Bundle override SANDBOX_AGENT_RELAY_MCP_BUNDLE. - engines/sandbox_agent/mcp.ts: dedicated internal-entry constructor building the typeless ACP stdio shape named agenta-tools (claude_settings.py couples permission rules to it); McpServerStdio moved here out of mcp-bridge.ts; user-declared servers may not claim the reserved name; toAcpMcpServers still cannot emit stdio; user-stdio refusal unrelaxed. - run-plan.ts: the remote-tools gate narrows - executable tools on Daytona pass (new toolMcpDir ephemeral sibling of the relay dir), client tools refuse loud with their own message, non-Daytona remote providers stay fail-closed. - engines/sandbox_agent/relay-guard.ts: the relay execution guard now guards EVERY harness (review finding): allow/deny identical everywhere; ask consumes the Pi grant ledger on Pi and passes on MCP harnesses whose own dialog gates the call, with the forged-ask residual documented. Previously Claude relay records executed unguarded. - The slice-0 restart spike locked the A2 transport: the pinned Claude ACP adapter (claude-agent-acp 0.22.2 in snapshot agenta-sandbox-pi) respawns the stdio MCP subprocess on the session/load path after a VM stop/restart (spike-restart.md in the workspace). Tests: 25+ new (shim handler + real startToolRelay round trip incl. crash-after-write, concurrency, stdout purity; assets fail-loud; three-layer session MCP pin; gate matrix; non-Pi guard semantics). Suite 1054 passed / 4 known pre-existing failures; typecheck clean for the slice; both bundles build with the forbidden-server-symbols gate. Claude-Session: https://claude.ai/code/session_01Fr4A5zjs5rsufRaWgWiUNC
…internal MCP name The engine half of the shim slice, split from the previous commit after a cross-lane hunk-dependency snag (a formatting pass had touched the parallel Daytona-secret lane's committed regions; reverted, insertions re-anchored): - sandbox_agent.ts: upload the shim bundle + specs file on the Daytona non-Pi executable-tools path (fail-loud helper, deps-injectable) and pass the assets into buildSessionMcpServers; the relay execution guard is now built for EVERY harness via buildRelayExecutionGuard (allow/deny identical everywhere; ask consumes the Pi grant ledger on Pi and passes on MCP harnesses whose own dialog gates the call). - mcp.ts: materialization-time reserved-name defense (assertNoReservedUserMcpName at the top of buildSessionMcpServers). - run-plan.ts: declaration-time refusal of a user MCP server named agenta-tools, before any resource is created, plus its gate test. Claude-Session: https://claude.ai/code/session_01Fr4A5zjs5rsufRaWgWiUNC
The three-layer test used the credentials wire field owned by the parallel Daytona-secret work, so this lane's tip failed typecheck standalone. The layering pin is about WHICH servers are delivered; auth delivery has its own tests. Also swap the 'no Bearer' assertion for 'no user MCP url leaks into the internal entry'. Claude-Session: https://claude.ai/code/session_01Fr4A5zjs5rsufRaWgWiUNC
…CP shim The internal gateway-tool channel now has two transports (local loopback HTTP; the in-sandbox stdio shim on Daytona): update the runner-to-MCP-server interface page (shim env contract, SANDBOX_AGENT_RELAY_MCP_BUNDLE, reserved server name, narrowed remote gates, the every-harness relay execution guard and its stated ask residual), the interface index rows, the harness-adapters and mcp-models coupling notes, the permission-responder guard note, the tools/ground-truth/claude-code narrative pages, and the operator env-var inventory in running-the-agent. Claude-Session: https://claude.ai/code/session_01Fr4A5zjs5rsufRaWgWiUNC
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Self-review finder round (verification pass skipped per budget; treat as candidates for the Codex review):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
services/runner/src/engines/sandbox_agent/mcp.ts (1)
118-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the response-watch env name
AGENTA_AGENT_TOOLS_RELAY_RESPONSE_WATCH_ENABLEDis duplicated inmcp.ts,pi-assets.ts, andrelay-client.ts. Add a shared constant intool-mcp-env.tsand reuse it on both sides so the kill switch can’t drift silently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9291b848-7c61-404f-9d04-40a63d0c6607
📒 Files selected for processing (25)
docs/design/agent-workflows/documentation/adapters/claude-code.mddocs/design/agent-workflows/documentation/ground-truth.mddocs/design/agent-workflows/documentation/running-the-agent.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/cross-service/README.mddocs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/in-service/mcp-models-and-resolution.mddocs/design/agent-workflows/interfaces/in-service/permission-responder.mdservices/runner/scripts/build-extension.mjsservices/runner/src/engines/sandbox_agent.tsservices/runner/src/engines/sandbox_agent/mcp.tsservices/runner/src/engines/sandbox_agent/relay-guard.tsservices/runner/src/engines/sandbox_agent/run-plan.tsservices/runner/src/engines/sandbox_agent/tool-mcp-assets.tsservices/runner/src/tools/mcp-bridge.tsservices/runner/src/tools/tool-mcp-env.tsservices/runner/src/tools/tool-mcp-stdio.tsservices/runner/tests/unit/sandbox-agent-orchestration.test.tsservices/runner/tests/unit/sandbox-agent-run-plan.test.tsservices/runner/tests/unit/session-mcp-layering.test.tsservices/runner/tests/unit/tool-mcp-assets.test.tsservices/runner/tests/unit/tool-mcp-stdio.test.tsservices/runner/tests/unit/tool-relay-guard.test.ts
| Separately from the dialog gates, the relay re-checks every execute record with a runner-side | ||
| execution guard (`buildRelayExecutionGuard` in | ||
| `services/runner/src/engines/sandbox_agent/relay-guard.ts`), on EVERY harness: the relay dir is | ||
| sandbox-writable, so a forged request file proves nothing about any dialog having run. | ||
| `allow` passes and `deny` refuses identically everywhere. `ask` splits by harness: on Pi the | ||
| guard consumes a dialog-recorded execution grant, so a forged or replayed record for an `ask` | ||
| tool fails closed; on an MCP harness (Claude) the guard passes `ask`, because the harness's own | ||
| dialog gates the call before it reaches the shim and the runner holds no grant for it. The | ||
| stated residual: on the MCP path a forged relay file can still trigger an ask-tool without a | ||
| dialog; reflecting the harness approval into the grant ledger is a documented follow-up. The | ||
| hard deny boundary holds on every harness. | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not ship the forged-ask execution bypass as a documented residual.
Because the relay directory is sandbox-writable and the non-Pi guard accepts ask without consuming an approval grant, a forged relay file can execute an ask-protected tool without a Claude dialog. Bind relay records to authenticated harness approvals or make the runner require a verifiable grant before execution.
| return { | ||
| jsonrpc: "2.0", | ||
| id, | ||
| error: { code: -32601, message: `method not found: ${method}` }, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the surrounding handler.
git ls-files services/runner/src/tools/tool-mcp-stdio.ts
echo '--- outline ---'
ast-grep outline services/runner/src/tools/tool-mcp-stdio.ts --view expanded || true
echo '--- targeted excerpt ---'
sed -n '1,340p' services/runner/src/tools/tool-mcp-stdio.ts | cat -n | sed -n '180,280p'Repository: Agenta-AI/agenta
Length of output: 4939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- repo ping references ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' '\bping\b|tools/list|initialize|jsonrpc' .
echo '--- package references for MCP / Claude SDK ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'mcp|claude|anthropic|acp' package.json pnpm-lock.yaml services web . || trueRepository: Agenta-AI/agenta
Length of output: 50373
🌐 Web query:
Model Context Protocol ping request empty result error code -32601 specification
💡 Result:
The Model Context Protocol (MCP) specifies that a ping request is a standard JSON-RPC 2.0 request with no parameters, used to verify connection health [1][2]. There is no "empty result error code" for a ping request. According to the protocol specification, a successful ping request MUST be answered by the receiver with an empty result object [2][3]. The correct response format is a JSON-RPC success response containing an empty JSON object as the result [2]: { "jsonrpc": "2.0", "id": "your-request-id", "result": {} } If a receiver fails to respond to a ping, it is typically treated as a connection timeout or failure, not as a specific error code [2]. MCP uses standard JSON-RPC 2.0 error codes (such as -32600 to -32603) for protocol-level failures and reserved ranges (-32020 to -32099) for defined specification errors, but these are not applicable to the successful completion of a ping [4][5][6]. If you encounter an error when sending a ping, it would be a standard JSON-RPC error response indicating a transport or protocol issue, rather than a specific "empty result error code" [4][7].
Citations:
- 1: https://modelcontextprotocol.io/specification/2025-03-26/basic/utilities/ping
- 2: https://modelcontextprotocol.io/specification/2024-11-05/basic/utilities/ping
- 3: https://github.com/modelcontextprotocol/csharp-sdk/blob/v1.4.0/src/ModelContextProtocol.Core/Protocol/PingResult.cs
- 4: https://modelcontextprotocol.io/specification/draft/basic
- 5: https://mcp.mintlify.app/specification/draft/basic/index
- 6: https://mcp.mintlify.app/specification/draft/basic
- 7: https://modelcontextprotocol.io/specification/2025-11-25/basic
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '162,260p' services/runner/tests/unit/tool-mcp-stdio.test.ts | cat -nRepository: Agenta-AI/agenta
Length of output: 4079
Handle ping as a successful MCP response
ping currently falls through to -32601, but MCP expects an empty success result ({}). Add an explicit branch before the unknown-method fallback so liveness probes don’t treat the server as unhealthy.
Suggested handling
+ if (method === "ping") {
+ return { jsonrpc: "2.0", id, result: {} };
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| jsonrpc: "2.0", | |
| id, | |
| error: { code: -32601, message: `method not found: ${method}` }, | |
| }; | |
| if (method === "ping") { | |
| return { jsonrpc: "2.0", id, result: {} }; | |
| } | |
| return { | |
| jsonrpc: "2.0", | |
| id, | |
| error: { code: -32601, message: `method not found: ${method}` }, | |
| }; |
|
Reviewed with Codex (high effort). Delivery works and the hard The mechanical items are fixed in #5248: correct MCP One thing is yours to decide: the non-Pi |
dosubot
Bot
added
size:XXL
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
I think we should accept the ask situation as is. In any case, in the future we will move from this relay mechanism to a real connection path between the sandbox and the runner, probably through a virtual network.
mmabrouk
added
lgtm
Addresses the mechanical review findings on #5243, #5244, and #5245 that carry no design decision and need no live verification. Landed as one commit on top of the stack because GitButler cannot retro-assign a new delta onto a lower applied lane; each fix is mapped to its PR below. Relay (#5243): - relay.ts: bound every relay-file removal with a timeout so a hung daemon deletion can no longer wedge pickup, stale-sweep, or shutdown; a timed-out removal is treated as failed and the loop continues. - relay-watch.ts: tie a safety-poll miss to the active watch generation so a later zero-exit of the same generation cannot erase it (the demotion counter now accumulates); validate the whole watch-window string instead of parseInt accepting partial garbage. Shim (#5244): - tool-mcp-stdio.ts: answer MCP 'ping' with an empty result; return -32700 on unparseable input and -32600 on an invalid request; document the at-least-once relay delivery contract. Protocol-version negotiation is left unchanged (that needs a live check). - tool-mcp-assets.ts: cache the shim bundle by path and write bundle + specs concurrently, removing avoidable serial Daytona round trips. - restored the direct client-only Daytona refusal test; pinned the Pi vs non-Pi ask guard wiring; updated the build:extension docs. Continuation (#5245): - tool-mcp-http.ts: authenticate before the method check so an unauthenticated GET/DELETE gets 401, not 405. - added auth tests: unauthorized GET/DELETE, wrong scheme, empty and whitespace tokens, and that neither the callback executor nor the client relay runs on an unauthorized call. Deferred (need a design decision or a live check, not in this commit): the non-Pi ask authorization model (#5244), the bearer-in-argv delivery (#5245), MCP protocol-version negotiation (#5244). Claude-Session: https://claude.ai/code/session_0127AM79khCdvD2b8BG2joZL
mmabrouk
changed the base branch from
feat-event-driven-tool-relay
to
big-agents
1 participant
Context
A Claude run on Daytona that carries any custom tool is refused up front. Tool execution over the file relay works there, but nothing inside the sandbox tells the harness which tools exist, so the fail-loud gate (#5047) blocks the whole combination. The harness we want people to use for real agents cannot use platform tools in the sandbox we want to be the default.
This PR delivers gateway and callback tools to Claude on Daytona through a small in-sandbox stdio MCP shim. The shim advertises the run's tools and turns each call into a relay request that the runner executes with its own credentials, so the sandbox never sees a secret. Design:
docs/design/agent-workflows/projects/in-sandbox-tool-mcp/.The one real risk, proven first
Warm reuse was the unproven part. After a sandbox parks to stopped and the engine reconnects, does the Claude ACP adapter respawn a dead stdio MCP subprocess on
session/load? It does. Proven live on Daytona (spike-restart.md): the reload returned the same session, respawned the shim (pid 145 to 147) with a freshinitializeandtools/list. No cold fallback path needed.Changes
tool-mcp-stdio.ts): newline-delimited JSON-RPC over stdio.tools/listserves the run's public specs from an uploaded file (schemas are unbounded, so a file, not an env value).tools/callwrites a relay request and maps failures to tool errors. It exits loud on missing input.tool-mcp-assets.ts): writes the shim bundle and specs into an ephemeral in-VM directory next to the relay dir, and fails loud rather than delivering a half-set.mcp.ts): a dedicated builder makes the stdio entry namedagenta-tools. A user-declared MCP server may not claim that reserved name; it is refused when declared and again when the entry is built.run-plan.ts): executable tools on Daytona now pass. Client tools on a remote sandbox still refuse loud (an explicit v1 cut), and non-Daytona remote providers stay fail-closed.relay-guard.ts): the guard now runs for every harness, not just Pi.denyis enforced identically everywhere, so a forged relay file cannot run a deny-policy tool. One residual is documented and called out for your decision below.No
/runwire change. Pi and local-Claude delivery are unchanged.Open decision for you: the
askresidualThe review flagged this as exploitable, and I agree it needs your ruling. On a non-Pi harness the guard lets an
ask-policy tool through, because Claude's own dialog is meant to gate it. But the relay directory is sandbox-writable, so a prompt-injected Claude could forge a request for anask-protected tool and the runner would run it with no approval event. The harddenyboundary is not affected; this is only theaskpath.Two options: fail closed for non-Pi
ask(safe, butask-protected gateway tools become unavailable on Claude until a real grant exists), or keep the residual (functional, but accept the bypass risk). The full fix is a one-use grant recorded from the approval event, which is a separate project.Acceptance (qa-slice2.md, live on the dev stack and Daytona EU)
agenta-toolsentry. The model turn then failed on Anthropic key credit, so the tool call itself did not runagenta-toolsin user configSandbox hygiene verified twice: zero sandboxes left on the account.
Merge gate: do not merge until the credit-blocked cells run a real tool end to end on Claude and Daytona (cold, warm reuse, restart). What they need is Anthropic credit on the box's key, not Daytona (the Daytona sandboxes were created fine).
status.mdcarries the same gate.What to QA
Prerequisites: the dev stack with Daytona creds, a runner on this branch with both bundles rebuilt (
pnpm run build:extension; the dev sidecar needs the freshdist/copied in before restart), and an Anthropic key with credit in the request.cd services/runner && pnpm test && pnpm run typecheck && pnpm run build:extension: suite green, both bundles built./runwithharness: claude,sandbox: daytona, and one callback tool pointing at an echo server that returns an unguessable token (exact request inqa-slice2.md).agenta-toolsis refused; a run with no tools uploads no shim.Notes
pingand JSON-RPC error handling in the shim, faster asset upload, and two restored test cases.plan.md): client tools through the shim, Codex on Daytona, snapshot bake, Pi as an MCP client, and theask-grant work above.https://claude.ai/code/session_01Fr4A5zjs5rsufRaWgWiUNC