From b5b5f0677a053052c151d58b0af028470a82318a Mon Sep 17 00:00:00 2001 From: Mahmoud Mabrouk Date: Thu, 25 Jun 2026 14:49:24 +0200 Subject: [PATCH] docs(agent): plan to restore gateway-tool MCP for Claude --- .../projects/gateway-tool-mcp/README.md | 61 ++++++++ .../projects/gateway-tool-mcp/context.md | 77 ++++++++++ .../projects/gateway-tool-mcp/plan.md | 141 ++++++++++++++++++ .../projects/gateway-tool-mcp/research.md | 137 +++++++++++++++++ .../projects/gateway-tool-mcp/status.md | 86 +++++++++++ 5 files changed, 502 insertions(+) create mode 100644 docs/design/agent-workflows/projects/gateway-tool-mcp/README.md create mode 100644 docs/design/agent-workflows/projects/gateway-tool-mcp/context.md create mode 100644 docs/design/agent-workflows/projects/gateway-tool-mcp/plan.md create mode 100644 docs/design/agent-workflows/projects/gateway-tool-mcp/research.md create mode 100644 docs/design/agent-workflows/projects/gateway-tool-mcp/status.md diff --git a/docs/design/agent-workflows/projects/gateway-tool-mcp/README.md b/docs/design/agent-workflows/projects/gateway-tool-mcp/README.md new file mode 100644 index 0000000000..ce04cdbb48 --- /dev/null +++ b/docs/design/agent-workflows/projects/gateway-tool-mcp/README.md @@ -0,0 +1,61 @@ +# Gateway-tool MCP delivery to Claude + +Planning workspace for restoring the **internal gateway-tool MCP delivery channel** that +PR #4831 removed by accident when it disabled **user-facing stdio MCP** for security. + +## The one-line problem + +PR #4831 ("enforce sidecar trust + disable unenforceable sandbox boundaries") disabled the +sidecar's stdio MCP implementation to close a real security hole (user stdio/NPX MCP servers +launch arbitrary processes on the runner host, outside the sandbox boundary). In doing so it +ALSO killed the only channel that delivers **Agenta gateway/callback tools** to harnesses that +take tools over MCP only (Claude Code). Those two were conflated into one +`MCP_UNSUPPORTED_MESSAGE` gate. They are different layers and must be separable. This project +restores the internal channel while keeping the user-facing one disabled. + +## The layering distinction (the heart of this project) + +| Layer | What it is | Who declares it | Where it runs | Security posture | #4831 decision | This project | +| --- | --- | --- | --- | --- | --- | --- | +| **User MCP capability** | stdio/NPX MCP servers the user declares in their agent config (incl. code-execution servers) | The user, in `mcpServers` (`transport: "stdio"` + `command`) | A child process **on the runner host**, outside the sandbox boundary | Unsafe: arbitrary process, host network, no sandbox confinement | **DISABLED** (correct) | **Keep disabled** | +| **Internal gateway-tool MCP** | An internal delivery channel the runner stands up so Claude can receive Agenta gateway/callback tools (Claude only accepts tools over MCP) | Nobody — it is synthesized by the runner from the run's resolved `customTools` | An internal MCP endpoint the runner controls; tool execution relays back to the runner where it is resolved server-side | Safe: the channel carries only **public tool metadata**; credentials are resolved server-side a layer above (the sandbox/harness never sees a key) | **DISABLED** (collateral damage) | **Restore** | +| **HTTP (remote) MCP** | User-declared remote MCP servers reached over `https` with a secret in a request header | The user, in `mcpServers` (`transport: "http"` + `url`) | No runner-host process; the harness connects to the remote URL | Safe: no host process, secret in header, subject to the sandbox network policy | **ENABLED** by #4834 (a separate, third thing) | Unchanged; compose with it | + +The trap #4831 fell into: it treated "MCP" as one capability. It is three. Disabling the +user-facing **stdio** one must not disable the internal gateway-tool one. + +## Why the internal channel is secure even though the old one ran on the runner host + +The security concern #4831 closed was the user **stdio** server: an author-supplied +`command`/`args` (e.g. `npx some-mcp`) that the runner launches as an arbitrary host process. +The internal gateway-tool bridge is categorically different: + +- It carries **no user command**. It is synthesized by the runner from the run's already- + resolved `customTools`. +- The channel only ever sees **public tool metadata** (`name`, `description`, `inputSchema`) — + never the Composio key, the connection auth, or the callback bearer. Those stay in runner + memory and are applied server-side via `/tools/call` (the relay), exactly as for the Pi path. +- Gateway-tool execution **already** runs runner-side and is governed by **Layer-3 + tool-permission** (`relay.ts` `resolvePermission`), not by the Layer-2 `sandbox_permission` + network boundary. #4831's own README says gateway tools "need NO action — they are NOT part + of `sandbox_permission`." Delivering them to Claude is the same category: a Layer-3 concern, + not a sandbox-boundary one. + +So the secure thing to restore is **delivery of gateway tools to Claude**, decoupled from the +user-stdio-MCP disable. The recommended transport for the restored channel is **HTTP MCP** +(the same safe transport #4834 enabled for users), so the restored channel reuses the proven, +no-host-process delivery and never re-introduces a stdio child. See [`plan.md`](./plan.md). + +## Files + +- [`context.md`](./context.md) — why this exists, goals/non-goals, the user's framing. +- [`research.md`](./research.md) — verified state: how Claude got gateway tools before #4831, + exactly what #4831 disabled, the fail-loud interaction that makes the regression a hard + failure today, and what #4834 (HTTP MCP) gives us to reuse. +- [`plan.md`](./plan.md) — the smallest correct change, the recommended transport, and the + test plan. +- [`status.md`](./status.md) — state, decisions, open questions. **Source of truth.** + +## Status + +DESIGN ONLY — awaiting author review. No code changed. See [`status.md`](./status.md). diff --git a/docs/design/agent-workflows/projects/gateway-tool-mcp/context.md b/docs/design/agent-workflows/projects/gateway-tool-mcp/context.md new file mode 100644 index 0000000000..38cafb4f47 --- /dev/null +++ b/docs/design/agent-workflows/projects/gateway-tool-mcp/context.md @@ -0,0 +1,77 @@ +# Context + +## Why this work exists + +The runner sidecar (`services/agent/`) runs the agent loop for several harnesses. Pi receives +Agenta gateway/callback tools **natively** through the bundled Pi extension +(`extensions/agenta.ts` `registerTools`), so it never needed MCP. Claude Code accepts tools +**only over MCP**. So the runner stood up an internal MCP bridge to hand Claude the same +resolved gateway tools, with every call relayed back to the runner where the credentials live. + +PR #4831 ("enforce sidecar trust + disable unenforceable sandbox boundaries") disabled the +sidecar's stdio MCP path to close a genuine hole: a **user-declared** stdio/NPX MCP server +launches an arbitrary process on the runner host, outside the sandbox boundary, so a +network-blocked sandbox does not confine it (the same runner-host execution bypass that had +code-tool execution removed). That disable was correct **for user MCP servers**. + +But #4831 routed BOTH the user-facing path and the internal gateway-tool path through one +gate (`MCP_UNSUPPORTED_MESSAGE`). Its own README states the collateral damage plainly: + +> non-Pi harnesses (e.g. Claude) take tools only over MCP, so they can no longer receive +> custom tools + +That is the bug. Gateway tools should still reach Claude. The user's framing: + +- **User-facing MCP servers** (stdio/NPX, incl. code-execution): correctly DISABLED. They run + on the runner host, outside the sandbox boundary. Keep them off. +- **Internal gateway-tool delivery to Claude**: Claude only accepts tools over MCP, so the + runner stands up an INTERNAL MCP to hand Claude the Agenta gateway/callback tools. This is + not a user "MCP capability"; it is an internal delivery channel, and it is SECURE because the + gateway tools are resolved server-side (a layer above) — the sandbox never sees a credential. + Restore this. + +The two were conflated; they must be INDEPENDENT layers. + +## What makes this urgent now (not just a silent drop) + +A later change (commit `5170e577de`, "fail loud on missing harness capabilities") added +`assertRequiredCapabilities`. A Claude run carrying tool specs now does NOT silently drop them +— it reaches `buildSessionMcpServers`, which calls `buildToolMcpServers`, which **throws +`MCP_UNSUPPORTED_MESSAGE`**, so the whole run fails with `{ ok: false, error: "MCP servers are +not supported by the sidecar." }`. So today: **Claude + any gateway tool = hard run failure.** +Pi + the same tools works (native delivery). That asymmetry is the user-visible regression. + +## Goals + +1. Restore delivery of Agenta gateway/callback tools to Claude (and any non-Pi, MCP-only + harness), so a Claude run with gateway tools succeeds and the tools are callable. +2. Keep the two layers independent: disabling user stdio MCP must NOT disable the internal + gateway-tool channel, and vice versa. +3. Preserve the server-side credential invariant: the sandbox/harness sees only public tool + metadata; the Composio key / connection auth / callback bearer never leave runner memory. +4. Compose cleanly with #4834 (user HTTP MCP) and the fail-loud capability work — no + double-gating, no regressions to either. + +## Non-goals + +- **Re-enabling user stdio/NPX MCP servers.** Explicitly out of scope. They stay disabled + until their host-process security is solved (a separate future project). +- Changing the gateway/Composio resolution, the `/tools/call` contract, or Layer-3 + tool-permission semantics. Those are correct and untouched. +- Changing how Pi receives tools (native extension; unaffected). +- New vault routes or new secret handling. The internal channel carries no secrets at all. +- mTLS / payload encryption / scoped tokens — those are the deferred sidecar-trust hardening + items, unrelated to this delivery channel. + +## Relationship to neighboring projects + +- **`sidecar-trust-and-sandbox-enforcement/`** (PR #4831) — the source of the disable. This + project narrows that disable to user stdio MCP only and restores the internal channel. The + two not-implemented gates it added (local `network`, any `filesystem`) and the loopback + binding + `/run` token are unrelated and stay. +- **`http-mcp-transport/`** (PR #4834) — enabled user-declared `transport: "http"` MCP. It + proves the runner can deliver MCP servers over HTTP with no runner-host process and a secret + in a header. This project reuses that exact safe transport for the internal channel. +- **`capability-config/`** — built the Layer-2 (`sandbox_permission`) and Layer-3 + (tool-permission) split. This project lives entirely in Layer-3 / tool delivery; it does not + touch Layer-2. diff --git a/docs/design/agent-workflows/projects/gateway-tool-mcp/plan.md b/docs/design/agent-workflows/projects/gateway-tool-mcp/plan.md new file mode 100644 index 0000000000..4828ef3fae --- /dev/null +++ b/docs/design/agent-workflows/projects/gateway-tool-mcp/plan.md @@ -0,0 +1,141 @@ +# Plan + +Goal: restore delivery of Agenta gateway/callback tools to Claude (and any non-Pi, MCP-only +harness) as an **internal** MCP channel, independent of the disabled **user** stdio MCP path, +with credentials staying server-side. Smallest correct change, no wire/SDK/protocol change. + +## Design: three independent MCP layers, made explicit + +`engines/sandbox_agent/mcp.ts` `buildSessionMcpServers` already builds two terms. Keep them +fully independent and name them in code/comments so the conflation cannot recur: + +``` +buildSessionMcpServers (non-Pi, capabilities.mcpTools) = + buildToolMcpServers(toolSpecs, ...) // Layer: INTERNAL gateway-tool channel -> RESTORE + + toAcpMcpServers(userMcpServers) // Layer: USER MCP capability -> unchanged + // - stdio: throws (#4831) KEEP DISABLED + // - http : delivered (#4834) unchanged +``` + +- **User MCP capability = off** for stdio (the `run-plan.ts` `hasStdioMcpServer` gate + the + `toAcpMcpServers` stdio throw stay exactly as #4831 left them). +- **Internal gateway-tool MCP = on** (this project re-populates `buildToolMcpServers`). +- **HTTP MCP = a third thing** (#4834), delivered for user http servers; the internal channel + also USES the http transport (below) but is a separate entry the user never declares. + +## Recommended transport: internal HTTP MCP on loopback (Slice A, option 1 — preferred) + +Restore `buildToolMcpServers` so it returns an **HTTP** MCP server entry (`McpServerHttp` from +#4834), not a stdio one. The runner serves a tiny in-process MCP HTTP endpoint bound to +loopback that advertises the public gateway-tool specs and, on `tools/call`, runs +`runResolvedTool(spec, args, { relayDir })` — the exact dispatch the old stdio `mcp-server.ts` +used. Claude consumes `type: "http"` MCP natively (verified in #4834 research). + +Why HTTP, not the old stdio bridge: + +- **No runner-host child process.** This is the property #4831 cared about. The old stdio + bridge spawned `tsx mcp-server.ts`; the new channel does not spawn anything — it is an + endpoint the runner already-running process serves. It cannot be the "arbitrary host process" + hole, because there is no author command anywhere near it. +- **Reuses #4834's proven path.** The `McpServerHttp` ACP shape and Claude's native http-MCP + support are already exercised. We add one internal server entry alongside the user ones. +- **Symmetric with the user http path.** One transport for all MCP delivery; stdio stays the + one disabled transport, for users only. + +Sketch (illustrative, not final code): + +- New `tools/tool-mcp-http.ts` (or fold into `mcp-bridge.ts`): start an HTTP server on + `127.0.0.1:` that speaks MCP (`initialize` / `tools/list` / `tools/call`) over the + streamable-HTTP/SSE shape Claude expects. `tools/list` returns `publicToolSpecs(executable)` + (metadata only, `client` tools filtered). `tools/call` runs `runResolvedTool(spec, args, + { toolCallId, relayDir })`. The server holds **only public specs + relayDir** (same env + surface as before) and is reachable only from loopback. +- `buildToolMcpServers(specs, toolCallback, relayDir)`: if there are executable specs, start + that server and return `[{ type: "http", name: "agenta-tools", url, headers: [] }]`. No + secret header — the channel is unauthenticated on loopback and carries no credentials. Empty + / all-`client` specs still return `[]` (the no-tools path stays untouched). +- The runner-side relay loop (`startToolRelay`) is already started by `engines/sandbox_agent.ts` + when `plan.useToolRelay` is true; the http server's `tools/call` feeds the same relay dir, so + the server-side credential path is unchanged. + +Open implementation detail to pin at build time: the exact ACP/Claude HTTP-MCP variant (SSE vs +streamable-HTTP, the `tools/list` + `tools/call` JSON-RPC framing over HTTP). #4834 delivers the +ACP `type: "http"` entry to the harness but did not itself implement an MCP HTTP *server*; this +project implements that minimal server. Pin the framing against the installed ACP / +`@zed-industries/claude-agent-acp` version, mirroring #4834's same caveat. + +### Slice A, option 2 — restore the stdio bridge but ONLY for the internal channel (fallback) + +If the in-process HTTP MCP server proves more work than its worth for a first cut, restore the +OLD stdio `mcp-server.ts` + the `buildToolMcpServers` launcher **verbatim from before #4831**, +but ONLY for the internal gateway-tool channel — the user stdio path stays gated. This brings +back a runner-host child process for the bridge. It is acceptable on the same argument the user +makes (the bridge holds no secrets; execution relays server-side), but it re-introduces a host +process, so it is the fallback, not the recommendation. If chosen, document loudly that the +internal bridge child is metadata-only and distinct from a user stdio server. + +**Recommendation: option 1 (internal HTTP MCP).** It satisfies #4831's "no runner-host +process" intent AND restores delivery, and it reuses #4834's transport. + +## Slice B — make the layering un-conflatable in code + +Regardless of transport, do these so the next security pass cannot re-break delivery: + +1. Rename the internal-channel error/skip so it is never the user-facing + `MCP_UNSUPPORTED_MESSAGE`. `MCP_UNSUPPORTED_MESSAGE` should mean **"user MCP servers are not + supported"** only (used by `run-plan.ts` `hasStdioMcpServer` and `toAcpMcpServers`'s stdio + branch). The internal channel must not borrow it. +2. Comment both terms in `buildSessionMcpServers` with the layer they belong to (internal + gateway-tool vs user MCP capability) and a one-line "do not merge these gates" note, citing + this project. +3. Keep `assertRequiredCapabilities` as-is — once the internal channel is restored, its + `mcpTools:true` assertion for Claude is true again and consistent with delivery. + +## Slice C — tests + +Re-target the tests #4831 inverted, plus add the separability assertions: + +- **Internal channel restored (delivery + callable):** a non-Pi harness (Claude) + a gateway + `callback` toolSpec → `buildSessionMcpServers` returns the internal `agenta-tools` server + (http `type: "http"` per option 1), `tools/list` advertises the public spec, `tools/call` + routes through `runResolvedTool` → relay → `/tools/call`. Assert the advertised shape carries + **no** `callRef` / `code` / scoped `env` / callback auth (the public-spec guarantee). +- **User stdio MCP still refused:** a run with `mcpServers: [{ transport: "stdio", command }]` + → `run-plan.ts` rejects with `MCP_UNSUPPORTED_MESSAGE`; `toAcpMcpServers` still throws on it. +- **Separability (the regression guard):** (a) gateway toolSpecs + NO user MCP → internal + channel present, no throw; (b) user stdio MCP + NO gateway tools → refused; (c) gateway + toolSpecs + user http MCP → internal channel AND the user http server both delivered, stdio + still refused. This is the test that fails if the two layers are ever re-merged. +- **Credentials stay server-side:** assert the channel advertisement and any child env contain + only public metadata + relayDir; no secret, no callRef, no callback bearer. +- **Pi unaffected:** a Pi run still gets `[]` from `buildSessionMcpServers` and tools via the + native extension. +- **Fail-loud consistency:** a Claude run with gateway tools now returns `ok: true` (delivery + works), not the `MCP_UNSUPPORTED_MESSAGE` failure. + +Run `pnpm test` + `pnpm run typecheck` in `services/agent/`. No golden-fixture change expected +(no wire change); confirm the golden contract tests stay green. + +## Live verification (post-implementation, per the QA skill) + +Use `agent-workflows-qa` / `agent-replay-test`: run Claude with a real gateway tool (the +github tool in the `pi-agents` project worked historically) on the local sidecar and confirm +the tool is delivered and callable end-to-end, and that a user stdio MCP server is still +refused. Capture a replay test to pin it. + +## What this explicitly does NOT change + +- `protocol.ts` / `wire.py` / golden fixtures (no new wire field). +- The gateway / Composio / `/tools/call` contract and Layer-3 tool-permission. +- The user stdio MCP disable, the loopback binding, the `/run` token, and the local-network / + filesystem not-implemented gates from #4831. +- Pi's native tool delivery. + +## Smallest-correct-fix, in one paragraph + +Re-populate `buildToolMcpServers` (the INTERNAL channel only) to advertise the run's gateway +tools to Claude over an internal loopback HTTP MCP endpoint that the runner serves, feeding the +already-running relay for execution; rename the user-facing `MCP_UNSUPPORTED_MESSAGE` so the +internal channel never reuses it; leave `toAcpMcpServers` (stdio off, http on) and the +`run-plan.ts` stdio gate exactly as #4831 left them. No wire change. Tests assert the two layers +are independently toggled. diff --git a/docs/design/agent-workflows/projects/gateway-tool-mcp/research.md b/docs/design/agent-workflows/projects/gateway-tool-mcp/research.md new file mode 100644 index 0000000000..1d70f397f3 --- /dev/null +++ b/docs/design/agent-workflows/projects/gateway-tool-mcp/research.md @@ -0,0 +1,137 @@ +# Research + +Verified against the working tree on 2026-06-25. Paths are repo-relative. The relevant runner +code lives under `services/agent/src/`. + +## 1. How Claude got gateway/callback tools BEFORE #4831 + +Two delivery paths existed, branching on harness: + +- **Pi** — native, via the bundled extension. `extensions/agenta.ts` `registerTools` reads + `AGENTA_TOOL_PUBLIC_SPECS` (public metadata only) + `AGENTA_TOOL_RELAY_DIR`, calls + `pi.registerTool(...)` for each, and each tool's `execute` relays back to the runner via + `tools/dispatch.ts` `runResolvedTool`. **No MCP involved.** This path is untouched by #4831. + +- **Claude (and any non-Pi, MCP-only harness)** — an internal stdio MCP bridge. The chain: + 1. `engines/sandbox_agent.ts` (~L284) calls `buildSessionMcpServers(...)`. + 2. `engines/sandbox_agent/mcp.ts` `buildSessionMcpServers` (for a non-Pi harness with + `capabilities.mcpTools`) returned + `[...buildToolMcpServers(toolSpecs, toolCallback, relayDir), ...toAcpMcpServers(userMcpServers)]`. + The **first** term is the internal gateway-tool channel; the **second** is the user's MCP + servers. They were already two distinct terms — the layering existed; #4831 collapsed both. + 3. `tools/mcp-bridge.ts` `buildToolMcpServers` (before #4831) returned ONE ACP + `McpServerStdio` entry: `{ name: "agenta-tools", command: , args: [mcp-server.ts], + env: [AGENTA_TOOL_PUBLIC_SPECS, AGENTA_TOOL_RELAY_DIR] }`. The env carried **only public + specs + the relay dir** — never scoped env, code, callback auth, or callback endpoints. + 4. `tools/mcp-server.ts` (before #4831) was a dependency-free JSON-RPC-over-stdio MCP server. + The daemon launched it as a session MCP server. It served `initialize` / `tools/list` + (advertising the public specs, filtering out `client` tools) / `tools/call`. On a call it + invoked `runResolvedTool(spec, args, { toolCallId, relayDir })`, which writes a relay + request file the runner watches. + 5. The runner-side relay loop (`tools/relay.ts` `startToolRelay`) reads the request, executes + the **private** resolved spec in memory (`executeRelayedTool` → `callAgentaTool` → + `/tools/call`), and writes the response file. This is where the Composio key / connection + auth / callback bearer are applied — **server-side, in runner memory.** + +**Key property:** even in the OLD stdio design, the bridge child process was a dumb protocol +translator. It held only public metadata and a relay-dir path. Every credentialed action +happened back on the runner via the relay. The sandbox/harness never saw a secret. This is why +the user calls the internal channel secure regardless of where the bridge process runs. + +### Where the gateway secret actually lives (server-side, a layer above) + +`tools/callback.ts` `callAgentaTool` POSTs the OpenAI-style envelope to Agenta's `/tools/call` +with the `toolCallback.authorization` bearer. The Composio key and connection auth resolve on +the **platform** side of `/tools/call`. The runner never holds the Composio key; the +harness/sandbox never holds even the callback bearer. The whole point of `callRef` + the +callback is that the credential stays server-side (see `tools/dispatch.ts` header comment: +"POST back through Agenta's /tools/call so the Composio key and connection auth stay +server-side"). + +## 2. EXACTLY what #4831 disabled + +PR #4831 (feat commit `b0ad4ed500`) made the MCP delivery throw a single named constant for +EVERYTHING. The current working-tree state: + +- `tools/mcp-bridge.ts` — `buildToolMcpServers` now: returns `[]` for an empty/all-`client` + spec list, otherwise **throws `MCP_UNSUPPORTED_MESSAGE`** ("MCP servers are not supported by + the sidecar."). The launcher, the env construction, and the `agenta-tools` server entry were + all deleted. The `mcp-server.ts` stdio implementation is gone. +- `tools/mcp-server.ts` — reduced to a refusing stub: it writes `MCP_UNSUPPORTED_MESSAGE` to + stderr and `process.exit(1)`. +- `engines/sandbox_agent/mcp.ts` — `toAcpMcpServers` (the **user** MCP path) throws + `MCP_UNSUPPORTED_MESSAGE` for any real stdio server (`command` present); it delivers HTTP + servers (added by #4834) and skips command-less/url-less entries. +- `engines/sandbox_agent/run-plan.ts` — `buildRunPlan` rejects up front any run whose + `request.mcpServers` contains a stdio server with a `command` (`hasStdioMcpServer` → + `MCP_UNSUPPORTED_MESSAGE`). This is the user-facing gate and is correct. + +So #4831 conflated two things into `buildToolMcpServers` throwing: + +- **Correct:** user stdio MCP delivery (`toAcpMcpServers` stdio branch + `run-plan.ts` + `hasStdioMcpServer`) — keep. +- **Collateral:** the internal gateway-tool channel (`buildToolMcpServers`) — wrongly disabled. + +Note `buildSessionMcpServers` still calls `buildToolMcpServers(toolSpecs, ...)` first, so any +non-Pi run carrying gateway `toolSpecs` hits the throw. + +## 3. The fail-loud interaction (why it is a HARD failure today, not a silent drop) + +`engines/sandbox_agent/capabilities.ts` `assertRequiredCapabilities` (added by commit +`5170e577de`) runs in the engine BEFORE `buildSessionMcpServers`. For a non-Pi harness with +tool specs it checks the probed `mcpTools` + `toolCalls` flags. Claude probes **`mcpTools:true`, +`toolCalls:true`**, so the capability gate **passes** — the runner believes Claude can receive +tools. Then `buildSessionMcpServers` → `buildToolMcpServers` **throws**. The engine catch turns +the throw into `{ ok: false, error: "MCP servers are not supported by the sidecar." }`. + +Result: a Claude run with gateway tools fails the whole run. This is the visible regression and +also an internal contradiction — the capability gate asserts the channel exists, then the +delivery says it does not. The QA matrix corroborates the historical expectation that gateway +tools worked on Claude (`docs/design/agent-workflows/feature-matrix-test.md`, +`project_agent_workflows_qa` memory: "gateway PASS (github via pi-agents project)"). + +## 4. What #4834 (HTTP MCP) gives us to reuse + +PR #4834 (`http-mcp-transport/`) enabled user-declared `transport: "http"` MCP delivery in +`engines/sandbox_agent/mcp.ts`: + +- `McpServerHttp` interface: `{ type: "http"; name; url; headers: Array<{name, value}> }`, + mirroring the ACP `McpServer` `type: "http"` variant. +- `toAcpMcpServers` builds that entry for an http server and maps each resolved `env` entry to + an HTTP header. **No runner-host process launches** — the harness opens the connection and + the secret rides in a header. +- The research notes (verified there) that **Claude Code supports `type: "http"|"sse"` MCP + with a `headers` map natively**, and the bundled `@zed-industries/claude-agent-acp` documents + custom-header injection. So Claude can consume an HTTP MCP server today. + +This is the proven, safe transport. The recommended fix delivers the internal gateway-tool +channel over the SAME HTTP transport (an internal MCP endpoint the runner serves on loopback), +so we never re-introduce a stdio child and we reuse a path already exercised by #4834. + +## 5. Call sites and seams (where a fix lands) + +- `engines/sandbox_agent/mcp.ts` — `buildSessionMcpServers` is the one composition point. It + already separates the internal channel (`buildToolMcpServers`) from the user channel + (`toAcpMcpServers`). The fix re-populates the internal term without touching the user term. +- `engines/sandbox_agent.ts` (~L284-299) — passes `toolSpecs`, `toolCallback`, `relayDir`, + `capabilities` into `buildSessionMcpServers`, then hands `mcpServers` to + `sandbox.createSession({ sessionInit: { cwd, mcpServers } })`. The relay loop + (`startToolRelay`) is already started when `plan.useToolRelay` is true (it is, whenever there + are executable tool specs). So the runner-side execution half is already in place and + untouched; only the **delivery** half (advertising the tools to Claude) needs restoring. +- `tools/dispatch.ts` `runResolvedTool` + `tools/relay.ts` `startToolRelay` / + `executeRelayedTool` — the execution + server-side credential path. Unchanged; reused. +- `tools/public-spec.ts` `publicToolSpecs` / `executableToolSpecs` — produce the + metadata-only advertisement. Reused (this is the "no secrets cross the boundary" guarantee). +- Tests: `tests/unit/mcp-servers.test.ts`, `tests/unit/tool-bridge.test.ts`, + `tests/unit/sandbox-agent-orchestration.test.ts` were rewritten by #4831 to assert the throw; + they are where the restored behavior is re-asserted. + +## 6. Constraint: the wire contract is mirrored + +`protocol.ts` is the `/run` wire source, hand-mirrored in +`sdks/python/agenta/sdk/agents/utils/wire.py` and pinned by golden fixtures (see +`services/agent/CLAUDE.md`). The internal gateway-tool channel is **synthesized by the runner +from `customTools`** — it is not a new wire field. So the recommended fix needs **no +`protocol.ts` / `wire.py` / golden change** (a strong reason to prefer it). `customTools` and +`toolCallback` already ride the wire. This mirrors #4834's "no-SDK-change" outcome. diff --git a/docs/design/agent-workflows/projects/gateway-tool-mcp/status.md b/docs/design/agent-workflows/projects/gateway-tool-mcp/status.md new file mode 100644 index 0000000000..85ab752b1c --- /dev/null +++ b/docs/design/agent-workflows/projects/gateway-tool-mcp/status.md @@ -0,0 +1,86 @@ +# Status + +## State + +DESIGN ONLY — awaiting author review. No code changed. A merge is in progress on the GitButler +tree; these are working-tree planning docs only. The orchestrator commits later. + +## Problem (one line) + +PR #4831 disabled user-facing stdio MCP for security but, via one shared +`MCP_UNSUPPORTED_MESSAGE` gate, ALSO killed the internal channel that delivers Agenta +gateway/callback tools to Claude. Restore the internal channel; keep user stdio MCP disabled. + +## The layering (decision to confirm) + +| Layer | #4831 | Target | +| --- | --- | --- | +| User MCP capability (stdio/NPX) | disabled | **keep disabled** | +| Internal gateway-tool MCP (runner → Claude) | disabled (collateral) | **restore** | +| User HTTP (remote) MCP (#4834) | enabled | unchanged | + +## What #4831 broke (verified) + +- `tools/mcp-bridge.ts` `buildToolMcpServers` now throws `MCP_UNSUPPORTED_MESSAGE` for any + executable tool spec — this is the INTERNAL gateway-tool channel, wrongly disabled. +- `tools/mcp-server.ts` reduced to a refusing stub (the old stdio MCP server deleted). +- Combined with the later fail-loud work (`assertRequiredCapabilities`, commit `5170e577de`), + a Claude run with gateway tools is now a **hard failure** (`ok:false`), not a silent drop: + the capability gate asserts Claude can take tools (`mcpTools:true`), then delivery throws. +- Correctly kept: `toAcpMcpServers` stdio throw + `run-plan.ts` `hasStdioMcpServer` gate (the + USER stdio path). Those stay. + +## Smallest-correct-fix (recommendation) + +Re-populate `buildToolMcpServers` to deliver the internal gateway-tool channel over an internal +loopback **HTTP** MCP endpoint (reusing #4834's `McpServerHttp` shape + Claude's native http-MCP +support), feeding the already-running runner relay for execution. Rename the user-facing +`MCP_UNSUPPORTED_MESSAGE` so the internal channel never reuses it. Leave the user stdio gate +untouched. **No wire/SDK/protocol/golden change** (the channel is synthesized from `customTools`, +not a new wire field). Fallback: restore the pre-#4831 stdio bridge but scope it to the internal +channel only (re-introduces a host process; not preferred). Full detail in [plan.md](plan.md). + +## Decisions + +- Treat MCP as THREE layers, not one: user-stdio (off), internal gateway-tool (on), user-http + (on). Make this explicit in `buildSessionMcpServers` so it cannot be re-conflated. +- Prefer the internal HTTP MCP transport (no runner-host process) over restoring the stdio + bridge — it satisfies #4831's intent and reuses #4834's path. +- No wire change: the internal channel is runner-synthesized from `customTools`. +- Credentials stay server-side; the channel advertises public metadata only and relays + execution to the runner (unchanged from the pre-#4831 design). + +## Open questions + +1. **Transport: internal HTTP MCP (option 1) vs restore the scoped stdio bridge (option 2)?** + Recommendation: option 1. Needs author confirmation — option 1 means implementing a minimal + MCP HTTP *server* in the runner (the framing must be pinned to the installed ACP / Claude + version; #4834 delivered the http *entry* but did not implement a server). +2. **Naming:** rename the user-facing constant to `USER_MCP_UNSUPPORTED_MESSAGE` (or similar) + and reserve a distinct internal-channel concept? Confirm the rename is in scope. +3. **Loopback reachability under Daytona:** the internal HTTP endpoint is on the runner host; + on Daytona the harness runs in the sandbox. Does Claude-on-Daytona reach a runner-loopback + URL, or must the internal channel use the file relay (as today) for the Daytona case and + HTTP only for local? (The file relay already works on Daytona; the delivery/advertisement is + the only open part.) Pin during research-at-implementation. +4. **Codex review:** worth an `ask-codex` pass on the layering/naming before implementation. + +## Next + +- Author reviews this plan and answers the open questions (esp. transport choice + Daytona + reachability). +- If accepted, schedule an `implement-feature` slice (runner-only: `mcp-bridge.ts` / + `mcp.ts` / new internal MCP server + tests) and add it to + [../../scratch/implementation-queue.md](../../scratch/implementation-queue.md). Keep docs in + sync (`keep-docs-in-sync`): the interface inventory's MCP/tool-delivery entries and the + sidecar-trust README's "what it removes" note both need the narrowed-disable correction. + +## Provenance + +- Project: gateway-tool-mcp. Session: 2026-06-25. Design-only. +- Source PRs: #4831 (the disable, `sidecar-trust-and-sandbox-enforcement/`), #4834 + (HTTP MCP, `http-mcp-transport/`), fail-loud commit `5170e577de`. +- Key files: `services/agent/src/tools/mcp-bridge.ts`, `tools/mcp-server.ts`, + `engines/sandbox_agent/mcp.ts`, `engines/sandbox_agent/run-plan.ts`, + `engines/sandbox_agent/capabilities.ts`, `tools/relay.ts`, `tools/dispatch.ts`, + `tools/public-spec.ts`.