diff --git a/docs/design/agent-workflows/documentation/adapters/claude-code.md b/docs/design/agent-workflows/documentation/adapters/claude-code.md index a3b5f3e5ab..86b2cbe1ab 100644 --- a/docs/design/agent-workflows/documentation/adapters/claude-code.md +++ b/docs/design/agent-workflows/documentation/adapters/claude-code.md @@ -64,10 +64,10 @@ restored over loopback HTTP by the gateway-tool-mcp project, and extended to Day in-sandbox-tool-mcp project. It is independent of the user MCP capability below — the two toggle separately.) -User-declared `mcp_servers` are a separate thing and effectively off today. They would reach -Claude through `toAcpMcpServers` as additional ACP stdio servers, but only when -`AGENTA_AGENT_MCPS_ENABLED` is set (off by default), so in practice no user MCP server is -attached. See [tools.md](../tools.md#status-and-known-gaps). +User-declared `mcp_servers` are separate from `agenta-tools`. Claude accepts external HTTP MCP +servers through ACP session initialization. The service resolves named header-secret references +before the runner builds the ACP HTTP entries. Public stdio configuration is not accepted. See +[tools.md](../tools.md#status-and-known-gaps). ## Permissions diff --git a/docs/design/agent-workflows/documentation/agent-configuration.md b/docs/design/agent-workflows/documentation/agent-configuration.md index 256099110c..802bff70cf 100644 --- a/docs/design/agent-workflows/documentation/agent-configuration.md +++ b/docs/design/agent-workflows/documentation/agent-configuration.md @@ -178,8 +178,8 @@ list of `{pattern, permission}` entries for harness-builtin tools (for example `Bash(rm:*)` set to `ask`). The runner checks rules before falling back to `default`. Tool entries are strict even though the list is lenient. Each tool subclass is `extra="forbid"` -(`sdks/python/agenta/sdk/agents/tools/models.py`). `MCPServerConfig` is also `extra="forbid"` -with a transport validator (`sdks/python/agenta/sdk/agents/mcp/models.py`). +(`sdks/python/agenta/sdk/agents/tools/models.py`). `MCPServerConfig` is also `extra="forbid"`. It accepts only the nested HTTP +connection, credential, and policy roles defined in `sdks/python/agenta/sdk/agents/mcp/models.py`. There is no `ModelRef` type. `model` is a plain string everywhere. There is no provider field. The rich model picker is built only for the UI by `_model_catalog_type()` @@ -217,7 +217,7 @@ Legend: (a) catalog/schema, (b) SDK neutral config, (c) runtime. | --- | --- | --- | --- | --- | | model / provider | yes, `model: str` | yes, `Optional[str]` | wired to the runner | Loose string. No `ModelRef`, no provider enum. There is no separate provider field. | | tools | yes, strict list | yes, lenient coercion | wired, resolved to builtin names + tool specs | Entries strict, list lenient. | -| mcp_servers | yes, strict list | yes | wired, resolved to runner mcp servers | Strict per entry. Gated by `AGENTA_AGENT_MCPS_ENABLED` at the service. | +| mcp_servers | yes, strict list | yes | wired, resolved to runner MCP servers | Strict per entry. Claude supports external HTTP servers; Pi refuses them until its bridge exists. | | skills | yes, embed/inline list | yes | wired | Author-settable (`SkillConfig` inline or `@ag.embed` references). The playground build-kit overlay embeds one skill, the `build-an-agent` playbook; the `pi_agenta` harness additionally force-unions `getting-started`. See below. | | persona | no | no | wired but forced only | Not a config field. The Agenta harness hardcodes an append-system preamble. See below. | | agents_md | yes, `agents_md: str` | yes, as `instructions` | wired to `agentsMd` | The schema names it `agents_md`. The neutral config names it `instructions`. | @@ -259,13 +259,17 @@ This is what the playground saves and the runtime reads: ], "mcp_servers": [ { - "name": "github", - "transport": "stdio", - "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-github"] + "name": "memory", + "connection": { + "type": "http", + "url": "https://memory.example.com/mcp", + "headers": {}, + "credentials": { "type": "none" } + }, + "policy": { "tools": { "mode": "all" }, "permission": "ask" } } ], - "harness": "pi_core", + "harness": "claude", "sandbox": "local", "runner": { "permissions": { "default": "allow_reads" } } } @@ -274,8 +278,8 @@ This is what the playground saves and the runtime reads: With this config, the runtime reads `agents_md`, `model`, `tools`, `mcp_servers`, and the run-selection fields `harness`, `sandbox`, and `runner.permissions.default` through the one neutral `AgentConfig`, resolves the tools and MCP servers server-side, and runs one turn on -the Pi harness in a local sandbox. Under `allow_reads`, `web_search` runs as a read with no -prompt; a write tool would pause for approval on this harness exactly as it would on Claude. +the Claude harness in a local sandbox. Under `allow_reads`, `web_search` runs as a read with no +prompt; a write tool pauses for approval. ## See also diff --git a/docs/design/agent-workflows/documentation/architecture.md b/docs/design/agent-workflows/documentation/architecture.md index 2cdad1e6ad..e07a9d7d09 100644 --- a/docs/design/agent-workflows/documentation/architecture.md +++ b/docs/design/agent-workflows/documentation/architecture.md @@ -111,9 +111,9 @@ Batch `/invoke` follows this path: 2. `_agent` parses one `AgentConfig` from request parameters; it carries the run-selection fields `harness` and `sandbox`, plus the permission policy at `runner.permissions.default`. -3. The service resolves three things independently: tools, MCP servers, and provider-key - secrets. MCP resolution is gated by `AGENTA_AGENT_MCPS_ENABLED` - (`services/oss/src/agent/tools/resolver.py:23`, off by default). +3. The service resolves three things independently: tools, external HTTP MCP servers, and + provider-key secrets. There is no deployment feature flag for MCP resolution. Harness + capabilities control authoring, and unsupported Pi runs fail loudly. 4. The service builds `SessionConfig` and constructs a harness over an `Environment` and `SandboxAgentBackend`. 5. The harness opens a cold session, sends one `/run` request to the sidecar, and tears the diff --git a/docs/design/agent-workflows/documentation/ground-truth.md b/docs/design/agent-workflows/documentation/ground-truth.md index e768e0fa2f..e1b9f5344f 100644 --- a/docs/design/agent-workflows/documentation/ground-truth.md +++ b/docs/design/agent-workflows/documentation/ground-truth.md @@ -56,7 +56,8 @@ this page and the referenced code as the source of truth. on Daytona (`tools/tool-mcp-stdio.ts`; the harness spawns it, its `tools/call` writes relay request files, and the runner-side relay loop executes server-side). The channel's name `agenta-tools` is reserved; a user-declared MCP server claiming it is refused. User-declared - MCP resolution is feature-gated (`AGENTA_AGENT_MCPS_ENABLED`, off by default). + external HTTP MCP configuration is resolved for every run. Harness capabilities expose the + editor for Claude and hide it for Pi until Pi has a delivery bridge. - `client` tools (browser-fulfilled, e.g. `request_connection`) are delivered to Claude too on the local path: advertised over the same internal MCP channel and PAUSED in the `tools/call` handler (no JSON-RPC result + abort the request), then resumed from the browser result next @@ -84,10 +85,10 @@ this page and the referenced code as the source of truth. per option (`HARNESS_IDENTITIES`); the stored/wire harness value stays the bare string. - Per-request model override is not honored on the Pi ACP path. pi-acp accepts only its default model and silently falls back (`projects/qa/findings.md`, F-007). -- User-declared MCP transports split: remote (`http`) servers are delivered by the runner - (`toAcpMcpServers`, #4834); stdio servers are disabled (`USER_MCP_UNSUPPORTED_MESSAGE`) because - they launch a process on the runner host. The runner's own internal gateway-tool channel is a - separate thing, delivered over loopback HTTP locally and the in-sandbox stdio shim on Daytona. +- User-declared MCP servers are HTTP-only. The runner delivers them to Claude through + `toAcpMcpServers`; Pi refuses them until its MCP bridge exists. The runner's own internal + gateway-tool channel is separate and uses loopback HTTP locally or a trusted in-sandbox stdio + shim on Daytona. - Trigger lifecycle, Compose.io trigger integration, and event-to-agent mapping are not implemented in the agent workflow code. - A persisted agent template object that separates `AGENTS.md`, skills, tools, diff --git a/docs/design/agent-workflows/documentation/ports-and-adapters.md b/docs/design/agent-workflows/documentation/ports-and-adapters.md index 86c4d4a5d3..7ee6dea050 100644 --- a/docs/design/agent-workflows/documentation/ports-and-adapters.md +++ b/docs/design/agent-workflows/documentation/ports-and-adapters.md @@ -159,6 +159,7 @@ result fields should update both sides and the wire tests in the same PR. - Session history is not persisted: the runtime is cold and completed `/messages` turns are not stored. - `AgentaHarness` policy content is placeholder product copy. -- MCP server resolution is disabled unless `AGENTA_AGENT_MCPS_ENABLED` is truthy. +- External HTTP MCP servers are resolved without a deployment flag. Harness capability metadata + exposes authoring for Claude and hides it for Pi until Pi has a delivery bridge. - The code still has historical WP labels in some comments. Those labels should not guide new design decisions. diff --git a/docs/design/agent-workflows/documentation/running-the-agent.md b/docs/design/agent-workflows/documentation/running-the-agent.md index faf3875290..c2a085f662 100644 --- a/docs/design/agent-workflows/documentation/running-the-agent.md +++ b/docs/design/agent-workflows/documentation/running-the-agent.md @@ -113,8 +113,7 @@ In the EE dev compose, the relevant services are: - `services`. Runs uvicorn on port `8080` inside the container (`hosting/docker-compose/ee/docker-compose.dev.yml:519`). It hosts the Python agent service. Traefik routes `/services/` to it. It sets `AGENTA_RUNNER_INTERNAL_URL` to - `http://runner:8765` and `AGENTA_AGENT_MCPS_ENABLED` to `false` by default (lines 564 to - 565). It depends on `runner` being healthy (lines 573 to 574). + `http://runner:8765`. It depends on `runner` being healthy. - `runner`. The Node runner (line 588 onward). In dev it runs `tsx src/server.ts` after rebuilding the Pi extension. It listens on `8765`. Its health check hits @@ -167,7 +166,6 @@ sections). `http://runner:8765`. When unset, the Python service spawns the runner CLI locally instead (see `runner_url` in `services/oss/src/agent/config.py` and `select_backend` in `services/oss/src/agent/app.py`). -- `AGENTA_AGENT_MCPS_ENABLED`. Gates MCP server resolution. Default `false`. - `SANDBOX_AGENT_PROVIDER`. `local` or `daytona`. Default `local`. - `SANDBOX_AGENT_DAYTONA_API_KEY`, `_API_URL`, `_TARGET`, `_SNAPSHOT`, `_IMAGE`, `_INSTALL_PI`. Daytona credentials the runner reads for the `daytona` sandbox provider. diff --git a/docs/design/agent-workflows/documentation/tools.md b/docs/design/agent-workflows/documentation/tools.md index c9f93878ab..eee9fe5245 100644 --- a/docs/design/agent-workflows/documentation/tools.md +++ b/docs/design/agent-workflows/documentation/tools.md @@ -112,7 +112,7 @@ Resolution is the service's job, but most of it now lives in the SDK. The servic entrypoints in `services/oss/src/agent/app.py` (`_agent`): `resolve_tools(agent_config.tools)` and `resolve_mcp_servers(agent_config.mcp_servers)`. Both are thin re-exports. The service files under `services/oss/src/agent/tools/` are shims: -`resolver.py` re-exports the SDK's `resolve_tools` and adds the MCP gate; `gateway.py` and +`resolver.py` re-exports the SDK's `resolve_tools`; `gateway.py` and `secrets.py` re-export the SDK platform adapters. The real composition is `resolve_tools` in `sdks/python/agenta/sdk/agents/platform/resolve.py`, which builds a `ToolResolver` (`sdks/python/agenta/sdk/agents/tools/resolver.py`) wired with two @@ -162,14 +162,11 @@ invoke immediately instead of failing the model mid-loop, and the agent only eve name, a schema, and an opaque slug. The Composio key and the connection's auth never leave the service. -MCP servers resolve on the same path but only when `AGENTA_AGENT_MCPS_ENABLED` is truthy. The -gate lives in `resolve_mcp_servers` (`services/oss/src/agent/tools/resolver.py`): when the -flag is off it returns an empty list before the SDK `MCPResolver` ever runs. When on, the -`MCPResolver` injects each server's named secrets into its `env`, the same way code tools get -theirs. By default this is off, so `mcp_servers` is dropped at the service and `mcpServers` is -omitted from the wire. See the [status](#status-and-known-gaps) section: even with the flag on, -user MCP reaches Claude only, not the default Pi harness, so the field is a no-op in the common -case. +External MCP servers resolve on their own path for every run. The `MCPResolver` reads the nested +HTTP connection, fetches named header-secret references, and creates a secret-bearing per-run +server. There is no deployment feature flag. The harness catalog exposes user MCP authoring for +Claude and hides it for Pi until Pi has a delivery bridge. A direct Pi request carrying an +external MCP server fails loudly. The whole resolved bundle then rides the `/run` wire: built-in names in `tools`, resolved specs in `customTools`, the callback in `toolCallback`, and resolved MCP servers in @@ -398,24 +395,17 @@ separately, at session start. The extension edits Pi's active tool set at every non-builtin tool untouched. A builtin outside the grant list is simply absent from the model's active tools, so no call for it ever fires, and the permission hook never sees it. -### MCP servers: a server process the daemon launches - -Execution happens in a separate server process. A declared MCP server is resolved server-side -(secrets injected into its `env`) and, for MCP-capable harnesses, passed to the ACP daemon as a -stdio server (`toAcpMcpServers` in `services/agent/src/engines/sandbox_agent/mcp.ts`). The -daemon launches the server's `command` with the resolved `env`, and the harness talks to it -over the MCP protocol. - -In practice user MCP is dead on the default path, and for two reasons that stack. First, -resolution is gated behind `AGENTA_AGENT_MCPS_ENABLED`, which is off by default, so the servers -never reach the wire. Second, even with the flag on, `buildSessionMcpServers` drops user MCP -for Pi (Pi's ACP adapter does not forward them), so it would reach Claude only. Pi and Agenta -are the default harnesses, so the `mcp_servers` field is accepted and then silently ignored in -the common case. This is the silent-drop that the -[harness-capabilities project](../../projects/harness-capabilities/proposal.md) is built to fix -(fail loud, or deliver MCP on Pi through the extension). The -[removal-and-capability notes](../../scratch/notes-tools-mcp-capabilities.md) lay out the two -options. +### External MCP servers: remote HTTP connections + +A declared user MCP server contains identity, an HTTP connection, credential references, and +policy. The service resolves credential references into request headers for the current run. The +runner validates the remote URL and passes an ACP HTTP MCP entry to Claude. Claude then connects +directly to the external server and discovers its tools through MCP. Public stdio commands and +process environments are not part of the author or runner interface. + +The editor shows this section only when the selected harness publishes `mcp.user_servers`. Claude +publishes it. Pi does not and rejects external MCP servers until its bridge exists. The private +`agenta-tools` server remains a separate trusted delivery mechanism for Agenta tools. ## Approval and rendering @@ -612,7 +602,7 @@ never drift from the files that exist. The canonical playbook format lives in th | Platform | `callback` spec + direct `call` | the Agenta service | the exposed endpoint, called directly (no `/tools/call` hop) | caller credential reused; self-targeting ids bound server-side | | Code | `code` spec + `env` | the runner | a local subprocess | only the tool's own secrets, scoped to the child | | Client | `client` spec | the browser | the user's browser, next turn | none | -| MCP | resolved server + `env` | a server process | a stdio child the daemon launches | secrets injected into the server env | +| MCP | resolved HTTP server + headers | the external MCP server | remote URL reached by Claude | named secret references become per-run request headers | ## Where this lives @@ -626,7 +616,7 @@ never drift from the files that exist. The canonical playbook format lives in th | Platform-op catalog (the `op` table + schema/context-binding resolution) | `sdks/python/agenta/sdk/agents/platform/op_catalog.py` | | Platform tool resolver (catalog → `CallbackToolSpec` + `call`) | `sdks/python/agenta/sdk/agents/platform/platform_tools.py` | | `x-ag-type-ref` schema expansion | `sdks/python/agenta/sdk/agents/platform/_schema.py` | -| Service entrypoints (shims + MCP gate) | `services/oss/src/agent/tools/resolver.py`, `__init__.py` | +| Service MCP composition | `sdks/python/agenta/sdk/agents/platform/resolve.py` | | Gateway resolver (calls `/tools/resolve`) | `sdks/python/agenta/sdk/agents/platform/gateway.py` (shim: `services/oss/src/agent/tools/gateway.py`) | | Named-secret resolution (`/secrets/resolve`) | `sdks/python/agenta/sdk/agents/platform/secrets.py` (shim: `services/oss/src/agent/tools/secrets.py`) | | API resolve + execute | `api/oss/src/core/tools/service.py`, `api/oss/src/apis/fastapi/tools/router.py` | @@ -653,12 +643,10 @@ never drift from the files that exist. The canonical playbook format lives in th ## Status and known gaps -- **User MCP is effectively dead on the default path.** Resolution is off unless - `AGENTA_AGENT_MCPS_ENABLED` is truthy, and even on, the runner drops user MCP for Pi. Pi and - Agenta are the default harnesses, so `mcp_servers` is a silent no-op for most runs. It would - reach Claude only. Do not confuse this with the `agenta-tools` server, which is an internal - tool-delivery vehicle for Claude, not a user MCP server; the name is reserved, and a - user-declared server claiming it is refused. +- **User MCP is harness-capability gated.** Claude receives external HTTP MCP servers through ACP. + Pi hides the editor and refuses external MCP servers until its bridge exists. Do not confuse + user MCP with `agenta-tools`, which is the trusted internal tool-delivery channel; its name is + reserved. - A tool's `permission` is honored on both harnesses now, including Pi's own builtins. Claude checks its rendered settings file first, then the ACP responder. Pi has no separate harness-side settings gate, so the relay decides everything Pi runs: gateway and code tools diff --git a/docs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.md b/docs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.md index ba403ba073..fea8089c4f 100644 --- a/docs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.md +++ b/docs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.md @@ -9,22 +9,16 @@ MCP layers, and they toggle separately (do not merge their gates): endpoint the runner serves (local sandbox; no runner-host child process), or an in-sandbox stdio MCP shim the runner uploads and the harness launches (Daytona). Its server name, `agenta-tools`, is reserved on every transport. -2. **User-declared MCP servers** — the user's own servers on the `/run` payload after the Python - side resolves their secrets. HTTP (remote) servers are delivered; stdio servers are DISABLED - (they launch an arbitrary process on the runner host, outside the sandbox boundary). - -PR #4831 once conflated these into a single `MCP_UNSUPPORTED_MESSAGE` switch, which disabled the -internal channel as collateral with the (correct) user-stdio disable; the gateway-tool-mcp -project split them again. The user-facing constant is now `USER_MCP_UNSUPPORTED_MESSAGE` and means -ONLY "user MCP servers are unsupported"; the internal channel never borrows it. +2. **User-declared MCP servers** - the user's external HTTP servers on the `/run` payload after + the Python side resolves header-secret references. Public stdio is not representable. ## The contract **Gating.** The runner builds MCP servers only when the harness is not Pi and the capability probe reports `mcpTools: true`. Pi always returns an empty MCP set because it gets tools the -native way. Because Pi cannot consume MCP, a USER MCP server (stdio OR http) attached to a Pi run +native way. Because Pi cannot consume MCP, a user HTTP MCP server attached to a Pi run would be dropped silently — so `run-plan.ts` refuses any Pi run carrying `mcpServers` up front -with `PI_USER_MCP_UNSUPPORTED_MESSAGE` (fail loud, the way the stdio-MCP and code-tool gates do), +with `PI_USER_MCP_UNSUPPORTED_MESSAGE`, rather than returning a "successful" empty run. A user http MCP is a Claude-only capability. **Reserved server name.** The channel's name `agenta-tools` is a stable identity on both @@ -137,28 +131,33 @@ the guard passes `ask`. Residual, stated honestly: on that path a forged request 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. -**User-declared servers.** `mcpServers` in `/run` carries each user server with its -transport, command or url, args, env (secrets already injected by the Python resolver), tool -allowlist, and permission. Two transports, opposite states: - -- **HTTP (`transport: "http"` + `url`) is delivered.** A remote server has no child process on - the runner host: the harness connects to the URL and the named secret rides in a request - header, so it does not bypass the sandbox boundary. The resolved secret arrives on the wire - under the server's `env` map (the resolver merges named secrets into `env` regardless of - transport, and the wire has no separate `headers` field), so `toAcpMcpServers` emits each - `env` entry as an HTTP header (`Authorization: `, etc.). The author names the header - via the secret-map key — `secrets: {"Authorization": "linear-mcp-token"}` lands as a header. - The runner builds the ACP `McpServer` `type: "http"` variant (`{name, url, headers}`). Before - attaching the credential, `validateUserMcpUrl` applies an SSRF guard: the `url` must be `https` - and must not target an internal/metadata host (loopback, link-local incl. `169.254.169.254`, - or private literals), else the run fails loud. A host listed in the optional comma-separated - `AGENTA_AGENT_MCPS_HOST_ALLOWLIST` env var opts out of both checks (e.g. a known-safe internal - endpoint). -- **Stdio (`transport: "stdio"` + `command`) is disabled.** A stdio server launches an - arbitrary process on the runner host, outside the sandbox boundary, so the implementation is - disabled (parity with the removed code execution) until its security is fixed. `run-plan.ts` - refuses any run carrying one (`USER_MCP_UNSUPPORTED_MESSAGE`); `toAcpMcpServers` throws the same - as a defense-in-depth backstop. The wire shape is kept; only delivery is off. +**User-declared servers.** `/run.mcpServers` carries only resolved external HTTP servers: + +```jsonc +{ + "name": "memory", + "connection": { + "type": "http", + "url": "https://memory.example.com/mcp", + "headers": { "Authorization": "Bearer resolved-value" } + }, + "policy": { + "tools": { "mode": "all" }, + "permission": "ask" + } +} +``` + +The Python service resolves named header-secret references before constructing this object. The +runner treats `connection.headers` as secret-bearing and converts each entry to the ACP HTTP header +array. `validateUserMcpUrl` requires HTTPS and blocks internal, private, link-local, loopback, and +metadata destinations unless the operator explicitly allowlists the host with +`AGENTA_AGENT_MCPS_HOST_ALLOWLIST`. + +A user HTTP server remains reachable from Daytona because Claude connects from inside the sandbox +to the remote URL. This is independent of the internal `agenta-tools` transport selection. Public +stdio does not exist in the run contract. Pi refuses external user MCP servers with +`PI_USER_MCP_UNSUPPORTED_MESSAGE` until Pi has a delivery bridge. ## Owned by @@ -180,8 +179,9 @@ allowlist, and permission. Two transports, opposite states: - `services/runner/src/engines/sandbox_agent/client-tools.ts`: the shared client-tool seam (`buildClientToolRelay`, `emitClientToolInteraction`, the ACP tool-call correlation index). - `services/runner/src/tools/mcp-bridge.ts`: the internal channel builder (advertises `client` - tools when a relay is wired); the `USER_MCP_UNSUPPORTED_MESSAGE` / - `PI_USER_MCP_UNSUPPORTED_MESSAGE` refusal constants. + tools when a relay is wired). +- `services/runner/src/engines/sandbox_agent/run-plan.ts`: the + `PI_USER_MCP_UNSUPPORTED_MESSAGE` refusal. - `services/runner/src/tools/tool-mcp-http.ts`: the internal loopback HTTP MCP server (the `client` pause: no JSON-RPC result + abort-the-request). - `services/runner/src/tools/tool-mcp-stdio.ts`: the in-sandbox stdio shim (newline-delimited @@ -190,7 +190,6 @@ allowlist, and permission. Two transports, opposite states: module the server-side entry builder shares without importing the bundle entrypoint. - `services/runner/src/tools/spec-schema.ts`: the shared `specInputSchema` accessor + arg validation. -- `services/runner/src/tools/mcp-server.ts`: the removed stdio JSON-RPC server (refusing stub). - `services/runner/src/tools/relay.ts`: the runner-side relay loop and hosts (delete-on-pickup; idle-poll backoff in fallback mode). - `services/runner/src/tools/relay-client.ts` and `relay-protocol.ts`: the bundle-safe @@ -234,11 +233,7 @@ allowlist, and permission. Two transports, opposite states: the shim. Requiring a grant there would refuse every approved call; removing the guard would let a forged file run a denied tool. Keep the residual (forged-file ask without a dialog on the MCP path) documented until the grant-ledger follow-up lands. -- **HTTP MCP delivery.** `toAcpMcpServers` routes the resolved secret from `env` into a - request header and builds the ACP `type: "http"` entry. Changing the env-to-header mapping or - the ACP variant shape changes which auth reaches the remote server. -- **USER stdio MCP stays disabled.** Re-enabling it requires making its runner-host execution - sandbox-safe — a real security change, not a flag flip. The internal in-sandbox stdio shim - is not an exception to this: it runs inside the sandbox, is synthesized from runner - constants, and can never be user-declared. +- **HTTP MCP delivery.** `toAcpMcpServers` routes resolved `connection.headers` into the ACP + `type: "http"` entry. Changing the header mapping or ACP variant changes which authentication + reaches the remote server. - **Per-server permission and allowlist behavior.** diff --git a/docs/design/agent-workflows/interfaces/in-service/mcp-models-and-resolution.md b/docs/design/agent-workflows/interfaces/in-service/mcp-models-and-resolution.md index 3e464a9397..1560e1b2c0 100644 --- a/docs/design/agent-workflows/interfaces/in-service/mcp-models-and-resolution.md +++ b/docs/design/agent-workflows/interfaces/in-service/mcp-models-and-resolution.md @@ -1,71 +1,73 @@ # MCP Models And Resolution -MCP config is parsed and resolved on its own path, separate from tools. The author declares a -server with the secrets it needs by name; the resolver fetches those secrets from the vault -and injects them into the server's env before the runner ever sees it. So the config holds -secret names, and the resolved server holds secret values. Tokens never live in the config. +MCP configuration follows a separate path from Agenta tools. The saved author object contains a +remote HTTP connection, secret references, and policy. The resolver fetches named values from the +project vault and creates a per-run object with resolved headers. Secret values never live in the +saved agent revision. -## The contract +## The contracts -**Declared (`MCPServerConfig`).** What the author writes: +**Declared (`MCPServerConfig`).** What the author saves: ```jsonc { - "name": "files", - "transport": "stdio", // "stdio" (needs command) | "http" (needs url) - "command": "npx", - "args": ["-y", "server-filesystem"], - "env": {}, // non-secret env - "url": null, // http transport only - "secrets": { "TOKEN_ENV": "vault-secret-name" }, // {env var: vault secret name} - "tools": [], // allowlist; empty means all - "permission": null // "allow" | "ask" | "deny" + "name": "memory", + "connection": { + "type": "http", + "url": "https://memory.example.com/mcp", + "headers": {}, + "credentials": { + "type": "header_secret_refs", + "headers": { "Authorization": "memory-mcp-token" } + } + }, + "policy": { + "tools": { "mode": "all" }, + "permission": "ask" + } } ``` -**Resolved (`ResolvedMCPServer`).** Same shape, minus `secrets`, with the named secrets merged -into `env`. The `to_wire()` emits only the keys that are set, so an empty `args`, `env`, -`url`, or `tools` is omitted from the `/run` payload. +The only public connection type is `http`. `credentials.type` is `none` or +`header_secret_refs`. Public stdio, commands, arguments, and environment variables are not +representable. `agenta-tools` is a reserved name. -**Resolution.** The resolver collects every named secret across all servers, fetches them in -one call, and injects each into its server's `env` (the same for both transports — there is no -separate `headers` wire field). A missing secret raises under the error policy. +**Resolved (`ResolvedMCPServer`).** The per-run model contains `name`, `url`, resolved `headers`, +and `policy`. It is secret-bearing and must not be persisted, returned by inspect, or logged. +Serialization emits the runner object as `{name, connection: {type, url, headers}, policy}`. -**Transport delivery (runner side).** HTTP (`transport: "http"` + `url`) servers are delivered: -the runner (`toAcpMcpServers`) reads each resolved `env` entry and emits it as an HTTP request -header (so `secrets: {"Authorization": "vault-name"}` becomes an `Authorization` header on the -remote call). Before attaching the credential, an SSRF guard (`validateUserMcpUrl`) requires the -`url` to be `https` and to not target an internal/metadata host (loopback, `169.254.169.254`, -private literals); `AGENTA_AGENT_MCPS_HOST_ALLOWLIST` opts a host out. Stdio (`transport: "stdio"` -+ `command`) servers are disabled in the sidecar — a stdio server runs an arbitrary process on -the runner host, outside the sandbox boundary — so a run carrying one is refused -(`USER_MCP_UNSUPPORTED_MESSAGE`). On a Pi harness, ANY user MCP server (stdio or http) is refused -up front (`PI_USER_MCP_UNSUPPORTED_MESSAGE`) because Pi delivers tools through its bundled -extension, not MCP — refusing it loudly avoids the silent drop. This is the USER MCP capability and -is distinct from the runner's internal gateway-tool MCP channel (delivered over loopback HTTP on -the local sandbox, and via an in-sandbox stdio shim feeding the file relay on Daytona; see -`runner-to-mcp-server.md`). One naming constraint from that channel reaches user servers: its -name `agenta-tools` is reserved (permission rules are rendered against it), so the runner -refuses a user-declared server with that name (`RESERVED_MCP_SERVER_NAME_MESSAGE`). The SDK -models, resolver, and wire are transport-agnostic; the enable/disable split and the -reserved-name check live entirely in the runner. +**Resolution.** The resolver collects the referenced secret names across all servers, fetches them +in one call, and maps each value onto its declared HTTP header. A missing required secret fails the +run. Public headers and resolved credential headers are merged only in the per-run object. + +## Runtime delivery + +The runner validates each external URL before attaching credentials. HTTPS is required, and +loopback, private, link-local, and metadata destinations are blocked unless an operator explicitly +adds the host to `AGENTA_AGENT_MCPS_HOST_ALLOWLIST`. + +Claude receives each external HTTP server in ACP session initialization and connects to the remote +server directly. The same delivery works when Claude runs locally or in Daytona because the URL is +not runner loopback. Pi refuses any external MCP server with `PI_USER_MCP_UNSUPPORTED_MESSAGE` +until its MCP bridge exists. This user-server path is separate from the trusted internal +`agenta-tools` channel. + +The service has no MCP feature flag. The runtime harness catalog publishes +`mcp.user_servers` only for harnesses that support the path, and the editor follows that +capability. ## Owned by -- `sdks/python/agenta/sdk/agents/mcp/models.py`: the declared and resolved models. -- `sdks/python/agenta/sdk/agents/mcp/resolver.py`: the batch secret resolution. -- `sdks/python/agenta/sdk/agents/mcp/wire.py`: wire serialization. +- `sdks/python/agenta/sdk/agents/mcp/models.py`: declared and resolved models. +- `sdks/python/agenta/sdk/agents/mcp/resolver.py`: batch secret resolution. +- `sdks/python/agenta/sdk/agents/mcp/wire.py`: runner serialization. +- `services/runner/src/engines/sandbox_agent/mcp.ts`: URL validation and ACP entries. +- `sdks/python/agenta/sdk/agents/capabilities.py`: harness capability publication. ## Watch for when changing -- **Stdio versus remote.** HTTP (remote) is delivered; stdio is disabled (runner-host process, - outside the sandbox). The split is in the runner, not the SDK. -- **Secret env resolution.** Secrets are named in config, fetched once, and injected into - `env`. The config never holds a token. For http the runner reads that `env` as request - headers; for stdio it would be process env (if/when re-enabled). -- **Tool allowlists and the permission model.** Per-server gating flows to the runner. -- **The reserved name.** A user server named `agenta-tools` is refused by the runner on every - transport; the SDK models do not need to know, but a doc or UI that suggests server names - must not offer it. -- **Wire serialization.** Empty fields are omitted; that omission is part of the `/run` - contract. +- Keep saved secret references separate from resolved header values. +- Keep public user MCP separate from the trusted internal `agenta-tools` stdio shim. +- Preserve HTTP-only authoring unless a new public execution boundary is explicitly designed. +- Keep the SDK, runner protocol, interface inventory, editor, and harness catalog in sync. +- Enforce policy on both tool listing and calls when tool-selection product work lands. diff --git a/docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md b/docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md index ffad27746f..94dc0f1a44 100644 --- a/docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md +++ b/docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md @@ -22,7 +22,7 @@ The fields and the full schema follow. | `agents_md` | string (textarea) | hello-world prompt | The agent's system prompt, its AGENTS.md. | | `model` | string (`grouped_choice`) | `"gpt-5.5"` | Model the agent runs on. A plain id (`"gpt-5.5"`) or a structured `{provider, connection}` ref. See [Model connection resolution](../in-service/model-connection-resolution.md). | | `tools` | `(ToolConfig \| EmbedRef)[]` | `[]` | Runnable tools: `builtin`, `gateway`, `code`, `client`, `reference` (a workflow referenced as a tool — `type: "reference"` — the service runs server-side as a callback tool), or `platform` (an existing Agenta endpoint exposed to the agent — `type: "platform"` — the runner calls it directly). A workflow value can also be inlined via `@ag.embed`. See [Tool models and resolution](../in-service/tool-models-and-resolution.md). | -| `mcp_servers` | `MCPServerConfig[]` | `[]` | Declared MCP servers; secret env resolved from the vault at run time. See [MCP models and resolution](../in-service/mcp-models-and-resolution.md). | +| `mcp_servers` | `MCPServerConfig[]` | `[]` | External HTTP MCP servers; named header-secret references resolve from the vault per run. See [MCP models and resolution](../in-service/mcp-models-and-resolution.md). | | `harness` | `"pi_core" \| "claude" \| "pi_agenta"` (see slug+name note) | `"pi_core"` | The coding agent to drive. `pi_core` and `pi_agenta` both drive the `pi` ACP agent; `pi_agenta` adds Agenta's forced skills, prompt, and policy. | | `sandbox` | `"local" \| "daytona"` | `"local"` | Where it runs. | | `permissions` | `{default: "allow" \| "ask" \| "deny" \| "allow_reads", rules?: [...]}` | `{default: "allow_reads"}` | The agent-wide policy. `allow_reads` runs read-hinted tools and asks for everything else; `allow` runs everything; `ask` asks for everything; `deny` runs nothing unless a tool explicitly allows it. `rules` are optional authored patterns (for example `Bash(rm:*)`) that override the default for matching harness builtins. | @@ -186,35 +186,37 @@ way. See [Tool models and resolution](../in-service/tool-models-and-resolution.m ### `mcp_servers[]` +External user MCP servers use one HTTP-only, role-based object: + ```jsonc { - "name": "files", - "transport": "stdio", // "stdio" (needs command; DISABLED) | "http" (needs url; delivered) - "command": "npx", "args": ["-y", "server-filesystem"], - "env": {}, // non-secret env - "url": null, // http transport only - "secrets": { "TOKEN_ENV": "vault-secret-name" }, // {env-or-header name: vault secret name} - "tools": [], // allowlist; empty = all - "permission": null // "allow" | "ask" | "deny" + "name": "memory", + "connection": { + "type": "http", + "url": "https://memory.example.com/mcp", + "headers": { "X-Client": "agenta" }, + "credentials": { + "type": "header_secret_refs", + "headers": { "Authorization": "memory-mcp-token" } + } + }, + "policy": { + "tools": { "mode": "all" }, + "permission": "ask" + } } ``` -For an **http** server the resolved secret is sent as a request header named by the secret-map -key, so a bearer token is `secrets: {"Authorization": "vault-name"}` (value `"Bearer ..."`). -**stdio** servers are disabled in the sidecar (they launch a runner-host process); a run -carrying one is refused. +`connection.headers` contains public values. `connection.credentials.headers` maps an HTTP header +name to a project-vault secret name; the saved revision never contains the resolved value. Use +`{"type": "none"}` when the server needs no credential. Public stdio, commands, arguments, and +environment variables are not part of this interface. The name `agenta-tools` is reserved for the +runner's private tool channel. -```jsonc -// http (remote) MCP server example -{ - "name": "linear", - "transport": "http", - "url": "https://mcp.linear.app/sse", - "secrets": { "Authorization": "linear-mcp-token" }, // -> Authorization request header - "tools": [], - "permission": "ask" -} -``` +`policy.tools.mode` is either `all` or `include`. `include` requires a non-empty `names` list. +The initial editor authors `all`; discovered-tool selection is future product work. Claude +publishes the `mcp.user_servers` harness capability and receives the HTTP server through ACP. Pi +does not publish the capability and refuses external MCP configuration until its bridge exists. ### `sandbox_permission` diff --git a/docs/designs/testing/rtm/web-acceptance-rtm.md b/docs/designs/testing/rtm/web-acceptance-rtm.md index 9e941c4937..68c571f613 100644 --- a/docs/designs/testing/rtm/web-acceptance-rtm.md +++ b/docs/designs/testing/rtm/web-acceptance-rtm.md @@ -1468,13 +1468,13 @@ For every RTM entry below: - License: `oss` - Status: active -### WEB-ACC-MCP-001 - MCP server form: switching transport renders the correct fields +### WEB-ACC-MCP-001 - Claude MCP server form shows the HTTP connection fields #### Source - Feature file: `web/oss/tests/playwright/acceptance/features/mcp-servers.feature` (not yet created) - Test file: `web/oss/tests/playwright/acceptance/mcp-servers/mcp-servers.spec.ts` (not yet created) -- Playwright title: `MCP Servers: form > switching transport between stdio and http shows the correct fields` +- Playwright title: `MCP Servers: form > Claude shows HTTP connection and credential fields` #### Markers @@ -1490,23 +1490,23 @@ For every RTM entry below: - License: `oss` - Status: active -### WEB-ACC-MCP-002 - stdio MCP server is blocked at runtime with a clear error message +### WEB-ACC-MCP-002 - Pi hides external MCP authoring #### Source - Feature file: `web/oss/tests/playwright/acceptance/features/mcp-servers.feature` (not yet created) - Test file: `web/oss/tests/playwright/acceptance/mcp-servers/mcp-servers.spec.ts` (not yet created) -- Playwright title: `MCP Servers: runtime > stdio MCP server is refused with USER_MCP_UNSUPPORTED_MESSAGE` +- Playwright title: `MCP Servers: capability > Pi does not render the external MCP editor` #### Markers - Scope: `playground`, `mcp` - Coverage: `light` -- Path: `grumpy` -- Case: `edge` +- Path: `happy` +- Case: `typical` - Lens: `functional` -- Speed: `slow` -- Cost: `paid` +- Speed: `fast` +- Cost: `free` - Role: `owner` - Plan: environment-defined - License: `oss` diff --git a/docs/docs/self-host/02-configuration.mdx b/docs/docs/self-host/02-configuration.mdx index 005c64c8fe..93379e23b8 100644 --- a/docs/docs/self-host/02-configuration.mdx +++ b/docs/docs/self-host/02-configuration.mdx @@ -161,7 +161,6 @@ Services API. `SANDBOX_AGENT_*` variables are read by the separate `runner` serv |---|---|---| | `AGENTA_RUNNER_INTERNAL_URL` | Services API | `agentRunner.externalUrl` or generated from `agentRunner.enabled` | | `AGENTA_RUNNER_TIMEOUT_SECONDS` | Services API / SDK | n/a | -| `AGENTA_AGENT_MCPS_ENABLED` | Services API | `agentRunner.enableMcp` | | `AGENTA_SANDBOX_LOCAL_ALLOWED` | Services API, SDK | `agenta.sandboxLocalAllowed` | | `SANDBOX_AGENT_PROVIDER` | `runner` | `agentRunner.provider` | | `SANDBOX_AGENT_LOG_LEVEL` | `runner` | `agentRunner.logLevel` | diff --git a/hosting/docker-compose/ee/docker-compose.dev.yml b/hosting/docker-compose/ee/docker-compose.dev.yml index a332483382..3c6ad11ad8 100644 --- a/hosting/docker-compose/ee/docker-compose.dev.yml +++ b/hosting/docker-compose/ee/docker-compose.dev.yml @@ -352,7 +352,6 @@ services: environment: DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} AGENTA_RUNNER_INTERNAL_URL: ${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - AGENTA_AGENT_MCPS_ENABLED: ${AGENTA_AGENT_MCPS_ENABLED:-false} # === NETWORK ============================================== # networks: - agenta-network diff --git a/hosting/docker-compose/ee/docker-compose.gh.local.yml b/hosting/docker-compose/ee/docker-compose.gh.local.yml index 055ed748b4..082f9875d3 100644 --- a/hosting/docker-compose/ee/docker-compose.gh.local.yml +++ b/hosting/docker-compose/ee/docker-compose.gh.local.yml @@ -235,7 +235,6 @@ services: environment: - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - - AGENTA_AGENT_MCPS_ENABLED=${AGENTA_AGENT_MCPS_ENABLED:-false} - AGENTA_STORE_ENDPOINT_URL=${AGENTA_STORE_ENDPOINT_URL:-} - AGENTA_STORE_ACCESS_KEY=${AGENTA_STORE_ACCESS_KEY:-} - AGENTA_STORE_SECRET_KEY=${AGENTA_STORE_SECRET_KEY:-} diff --git a/hosting/docker-compose/ee/docker-compose.gh.yml b/hosting/docker-compose/ee/docker-compose.gh.yml index b8cae25d1c..3e76a5a21e 100644 --- a/hosting/docker-compose/ee/docker-compose.gh.yml +++ b/hosting/docker-compose/ee/docker-compose.gh.yml @@ -235,7 +235,6 @@ services: - SCRIPT_NAME=/services - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - - AGENTA_AGENT_MCPS_ENABLED=${AGENTA_AGENT_MCPS_ENABLED:-false} - AGENTA_STORE_ENDPOINT_URL=${AGENTA_STORE_ENDPOINT_URL:-} - AGENTA_STORE_ACCESS_KEY=${AGENTA_STORE_ACCESS_KEY:-} - AGENTA_STORE_SECRET_KEY=${AGENTA_STORE_SECRET_KEY:-} diff --git a/hosting/docker-compose/ee/env.ee.dev.example b/hosting/docker-compose/ee/env.ee.dev.example index da59ad6824..19151c777d 100644 --- a/hosting/docker-compose/ee/env.ee.dev.example +++ b/hosting/docker-compose/ee/env.ee.dev.example @@ -55,7 +55,6 @@ AGENTA_CRYPT_KEY=replace-me # ================================================================== # # AGENTA_RUNNER_HOST=0.0.0.0 # AGENTA_API_KEY= -# AGENTA_AGENT_MCPS_ENABLED=false # AGENTA_AGENT_SANDBOX_PI_DIR=/home/sandbox/.pi/agent # AGENTA_AGENT_SANDBOX_PI_VERSION=0.80.6 # AGENTA_AGENT_SANDBOX_PI_INSTALLED=true diff --git a/hosting/docker-compose/ee/env.ee.gh.example b/hosting/docker-compose/ee/env.ee.gh.example index 259312ad97..aae4b996cb 100644 --- a/hosting/docker-compose/ee/env.ee.gh.example +++ b/hosting/docker-compose/ee/env.ee.gh.example @@ -57,7 +57,6 @@ AGENTA_CRYPT_KEY=replace-me # ================================================================== # # AGENTA_RUNNER_HOST=0.0.0.0 # AGENTA_API_KEY= -# AGENTA_AGENT_MCPS_ENABLED=false # AGENTA_AGENT_SANDBOX_PI_DIR=/home/sandbox/.pi/agent # AGENTA_AGENT_SANDBOX_PI_VERSION=0.80.6 # AGENTA_AGENT_SANDBOX_PI_INSTALLED=true diff --git a/hosting/docker-compose/oss/docker-compose.dev.yml b/hosting/docker-compose/oss/docker-compose.dev.yml index c9a3781a1c..1029355268 100644 --- a/hosting/docker-compose/oss/docker-compose.dev.yml +++ b/hosting/docker-compose/oss/docker-compose.dev.yml @@ -345,7 +345,6 @@ services: environment: DOCKER_NETWORK_MODE: ${DOCKER_NETWORK_MODE:-bridge} AGENTA_RUNNER_INTERNAL_URL: ${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - AGENTA_AGENT_MCPS_ENABLED: ${AGENTA_AGENT_MCPS_ENABLED:-false} # === NETWORK ============================================== # networks: - agenta-network diff --git a/hosting/docker-compose/oss/docker-compose.gh.local.yml b/hosting/docker-compose/oss/docker-compose.gh.local.yml index 76a23bb46f..abca29fb6c 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.local.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.local.yml @@ -233,7 +233,6 @@ services: environment: - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - - AGENTA_AGENT_MCPS_ENABLED=${AGENTA_AGENT_MCPS_ENABLED:-false} - AGENTA_STORE_ENDPOINT_URL=${AGENTA_STORE_ENDPOINT_URL:-} - AGENTA_STORE_ACCESS_KEY=${AGENTA_STORE_ACCESS_KEY:-} - AGENTA_STORE_SECRET_KEY=${AGENTA_STORE_SECRET_KEY:-} diff --git a/hosting/docker-compose/oss/docker-compose.gh.ssl.yml b/hosting/docker-compose/oss/docker-compose.gh.ssl.yml index 206af46b6a..9ec0810e26 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.ssl.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.ssl.yml @@ -252,7 +252,6 @@ services: environment: - SCRIPT_NAME=/services - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - - AGENTA_AGENT_MCPS_ENABLED=${AGENTA_AGENT_MCPS_ENABLED:-false} - AGENTA_STORE_ENDPOINT_URL=${AGENTA_STORE_ENDPOINT_URL:-} - AGENTA_STORE_ACCESS_KEY=${AGENTA_STORE_ACCESS_KEY:-} - AGENTA_STORE_SECRET_KEY=${AGENTA_STORE_SECRET_KEY:-} diff --git a/hosting/docker-compose/oss/docker-compose.gh.yml b/hosting/docker-compose/oss/docker-compose.gh.yml index dd5ef719bc..7f0d32aca7 100644 --- a/hosting/docker-compose/oss/docker-compose.gh.yml +++ b/hosting/docker-compose/oss/docker-compose.gh.yml @@ -252,7 +252,6 @@ services: - SCRIPT_NAME=/services - DOCKER_NETWORK_MODE=${DOCKER_NETWORK_MODE:-bridge} - AGENTA_RUNNER_INTERNAL_URL=${AGENTA_RUNNER_INTERNAL_URL:-http://runner:8765} - - AGENTA_AGENT_MCPS_ENABLED=${AGENTA_AGENT_MCPS_ENABLED:-false} - AGENTA_STORE_ENDPOINT_URL=${AGENTA_STORE_ENDPOINT_URL:-} - AGENTA_STORE_ACCESS_KEY=${AGENTA_STORE_ACCESS_KEY:-} - AGENTA_STORE_SECRET_KEY=${AGENTA_STORE_SECRET_KEY:-} diff --git a/hosting/docker-compose/oss/env.oss.dev.example b/hosting/docker-compose/oss/env.oss.dev.example index 4ae0850b22..41f299964f 100644 --- a/hosting/docker-compose/oss/env.oss.dev.example +++ b/hosting/docker-compose/oss/env.oss.dev.example @@ -55,7 +55,6 @@ AGENTA_CRYPT_KEY=replace-me # ================================================================== # # AGENTA_RUNNER_HOST=0.0.0.0 # AGENTA_API_KEY= -# AGENTA_AGENT_MCPS_ENABLED=false # AGENTA_AGENT_SANDBOX_PI_DIR=/home/sandbox/.pi/agent # AGENTA_AGENT_SANDBOX_PI_VERSION=0.80.6 # AGENTA_AGENT_SANDBOX_PI_INSTALLED=true diff --git a/hosting/docker-compose/oss/env.oss.gh.example b/hosting/docker-compose/oss/env.oss.gh.example index da3450da45..4d0ba12d02 100644 --- a/hosting/docker-compose/oss/env.oss.gh.example +++ b/hosting/docker-compose/oss/env.oss.gh.example @@ -57,7 +57,6 @@ AGENTA_CRYPT_KEY=replace-me # ================================================================== # # AGENTA_RUNNER_HOST=0.0.0.0 # AGENTA_API_KEY= -# AGENTA_AGENT_MCPS_ENABLED=false # AGENTA_AGENT_SANDBOX_PI_DIR=/home/sandbox/.pi/agent # AGENTA_AGENT_SANDBOX_PI_VERSION=0.80.6 # AGENTA_AGENT_SANDBOX_PI_INSTALLED=true diff --git a/hosting/kubernetes/helm/templates/_helpers.tpl b/hosting/kubernetes/helm/templates/_helpers.tpl index 8d75935ebe..7e1637d4c9 100644 --- a/hosting/kubernetes/helm/templates/_helpers.tpl +++ b/hosting/kubernetes/helm/templates/_helpers.tpl @@ -327,8 +327,6 @@ http://{{ include "agenta.agentRunner.serviceName" . }}:{{ include "agenta.agent - name: AGENTA_RUNNER_INTERNAL_URL value: {{ $url | quote }} {{- end }} -- name: AGENTA_AGENT_MCPS_ENABLED - value: {{ default false $runner.enableMcp | quote }} {{- end }} {{/* ================================================================ diff --git a/hosting/kubernetes/helm/values.schema.json b/hosting/kubernetes/helm/values.schema.json index d350665b03..47464c5a5f 100644 --- a/hosting/kubernetes/helm/values.schema.json +++ b/hosting/kubernetes/helm/values.schema.json @@ -304,7 +304,6 @@ "properties": { "enabled": { "type": "boolean", "description": "Deploy the runner sidecar; the Services API derives AGENTA_RUNNER_INTERNAL_URL from it unless externalUrl is set." }, "externalUrl": { "type": "string", "description": "AGENTA_RUNNER_INTERNAL_URL override pointing at an external runner." }, - "enableMcp": { "type": "boolean", "description": "AGENTA_AGENT_MCPS_ENABLED." }, "provider": { "type": "string", "description": "SANDBOX_AGENT_PROVIDER (e.g. local, daytona) read by the runner service." }, "piAgentDir": { "type": "string", "description": "PI_CODING_AGENT_DIR for local Pi runs (default /pi-agent); unset means no Agenta extension for the run (the runner logs a warning)." }, "logLevel": { "type": "string", "description": "SANDBOX_AGENT_LOG_LEVEL read by the runner service." }, diff --git a/hosting/kubernetes/helm/values.yaml b/hosting/kubernetes/helm/values.yaml index b49db7506e..35811d2ced 100644 --- a/hosting/kubernetes/helm/values.yaml +++ b/hosting/kubernetes/helm/values.yaml @@ -131,13 +131,11 @@ redisDurable: # size: 20Gi # ================================================================== # -# agentRunner — agent runner sidecar. enableMcp controls -# AGENTA_AGENT_MCPS_ENABLED (off by default). See values.schema.json for -# the full agentRunner shape. +# agentRunner configures the agent runner sidecar. See values.schema.json for +# the full shape. # ================================================================== # # agentRunner: # enabled: true -# enableMcp: true # daytona: # sessionIdleTtlMs: "" # sessionMaxWarm: "" diff --git a/hosting/railway/oss/scripts/configure.sh b/hosting/railway/oss/scripts/configure.sh index 76fab882c9..8f877e8c3f 100755 --- a/hosting/railway/oss/scripts/configure.sh +++ b/hosting/railway/oss/scripts/configure.sh @@ -288,7 +288,6 @@ main() { POSTGRES_URI_TRACING="$pg_async_tracing" \ POSTGRES_URI_SUPERTOKENS="$pg_sync_supertokens" \ AGENTA_RUNNER_INTERNAL_URL="$agent_runner_url" \ - AGENTA_AGENT_MCPS_ENABLED="${AGENTA_AGENT_MCPS_ENABLED:-false}" \ AGENTA_STORE_ENDPOINT_URL="$seaweedfs_endpoint_url" \ AGENTA_STORE_ACCESS_KEY="$AGENTA_STORE_ACCESS_KEY" \ AGENTA_STORE_SECRET_KEY="$AGENTA_STORE_SECRET_KEY" \ diff --git a/sdks/python/agenta/sdk/agents/__init__.py b/sdks/python/agenta/sdk/agents/__init__.py index 2c4d7a702a..ce28c8c8f3 100644 --- a/sdks/python/agenta/sdk/agents/__init__.py +++ b/sdks/python/agenta/sdk/agents/__init__.py @@ -96,7 +96,6 @@ ) from .mcp import ( MCPConfigurationError, - MCPDisabledError, MCPError, MCPResolver, MCPServerConfig, @@ -221,7 +220,6 @@ "MCPResolver", "MCPError", "MCPConfigurationError", - "MCPDisabledError", "MissingMCPSecretError", # Skills are a sibling subsystem "SkillTemplate", diff --git a/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py b/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py index 0b3b376276..e18a058245 100644 --- a/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py +++ b/sdks/python/agenta/sdk/agents/adapters/agenta_builtins.py @@ -196,9 +196,9 @@ ### mcps -Declared MCP servers. Each: `{ "name": ..., "transport": "stdio"|"http", "command"/"args" -(stdio) or "url" (http), "env", "secrets", "tools", "permission" }`. Secret env resolves from the -vault at run time; tokens never live in the config. +Declared external MCP servers. Each has `name`, an HTTP `connection` with `url`, optional +public `headers`, and discriminated `credentials`, plus a `policy` for tools and permission. +Secret header references resolve from the vault at run time; values never live in the config. ### skills diff --git a/sdks/python/agenta/sdk/agents/adapters/claude_settings.py b/sdks/python/agenta/sdk/agents/adapters/claude_settings.py index deb9a6773f..68176a73fc 100644 --- a/sdks/python/agenta/sdk/agents/adapters/claude_settings.py +++ b/sdks/python/agenta/sdk/agents/adapters/claude_settings.py @@ -113,7 +113,8 @@ def _rules_from_mcp_permissions(mcp_servers: Any) -> Dict[str, List[str]]: deny: List[str] = [] for server in mcp_servers or []: name = _get(server, "name") - permission = _get(server, "permission") + policy = _get(server, "policy") + permission = _get(policy, "permission") if not permission or not name: continue if name == INTERNAL_TOOL_MCP_SERVER: diff --git a/sdks/python/agenta/sdk/agents/capabilities.py b/sdks/python/agenta/sdk/agents/capabilities.py index 9f1e192cc4..1fef2d5a93 100644 --- a/sdks/python/agenta/sdk/agents/capabilities.py +++ b/sdks/python/agenta/sdk/agents/capabilities.py @@ -33,7 +33,7 @@ from __future__ import annotations -from typing import Dict, List +from typing import Dict, List, Optional from pydantic import BaseModel, Field @@ -164,6 +164,17 @@ def _pi_models() -> Dict[str, List[str]]: return models +class UserMCPServerCapabilities(BaseModel): + connection_types: List[str] = Field(default_factory=lambda: ["http"]) + credentials: List[str] = Field( + default_factory=lambda: ["none", "header_secret_refs"] + ) + + +class HarnessMCPCapabilities(BaseModel): + user_servers: Optional[UserMCPServerCapabilities] = None + + class HarnessConnectionCapabilities(BaseModel): """The connection-relevant capabilities of one harness (the ``/inspect`` ``meta`` shape). @@ -192,6 +203,7 @@ class HarnessConnectionCapabilities(BaseModel): # ``models``. Loosely typed as dicts here so this module stays decoupled from the entry schema # in ``model_catalog.py`` and the ``/inspect`` payload stays plain JSON. model_catalog: List[Dict[str, object]] = Field(default_factory=list) + mcp: Optional[HarnessMCPCapabilities] = None HARNESS_CONNECTION_CAPABILITIES: Dict[str, HarnessConnectionCapabilities] = { @@ -218,6 +230,9 @@ class HarnessConnectionCapabilities(BaseModel): model_selection="alias", models={"anthropic": list(CLAUDE_MODEL_ALIASES)}, model_catalog=_model_catalog("claude"), + mcp=HarnessMCPCapabilities( + user_servers=UserMCPServerCapabilities(), + ), ), } @@ -235,7 +250,7 @@ def harness_capabilities_document() -> Dict[str, Dict[str, object]]: type — instead of an inlined, agent-only ``meta`` field on every inspect call. """ return { - harness: caps.model_dump() + harness: caps.model_dump(exclude_none=True) for harness, caps in HARNESS_CONNECTION_CAPABILITIES.items() } @@ -249,7 +264,10 @@ def harness_catalog_document() -> Dict[str, Dict[str, object]]: template's harness field via ``x-ag-harness-ref``. """ return { - harness: {"harness": harness, "capabilities": caps.model_dump()} + harness: { + "harness": harness, + "capabilities": caps.model_dump(exclude_none=True), + } for harness, caps in HARNESS_CONNECTION_CAPABILITIES.items() } diff --git a/sdks/python/agenta/sdk/agents/handler.py b/sdks/python/agenta/sdk/agents/handler.py index a522dceeaf..bf36c4beb2 100644 --- a/sdks/python/agenta/sdk/agents/handler.py +++ b/sdks/python/agenta/sdk/agents/handler.py @@ -2,7 +2,7 @@ Owns stream/trim/force; composition (template, tool/MCP/connection resolvers, backend selector) is injectable via `AgentComposition`, defaulting to env-driven SDK behavior. The -default composition also owns capability gating, degradation policy, and MCP gating: +default composition also owns capability gating and degradation policy: these are protocol-level safety behaviors, not service-specific, so a bare `agent_v0` (no composition override) gets them for free instead of a permissive fallback. """ @@ -33,8 +33,7 @@ from agenta.sdk.agents.tools import ResolvedToolSet from agenta.sdk.agents.adapters import SandboxAgentBackend, make_harness from agenta.sdk.agents.errors import LocalSandboxNotAllowedError -from agenta.sdk.agents.mcp import MCPDisabledError, ResolvedMCPServer -from agenta.sdk.agents.mcp.parsing import parse_mcp_server_configs +from agenta.sdk.agents.mcp import ResolvedMCPServer from agenta.sdk.agents.platform import ( resolve_connection as _platform_resolve_connection, ) @@ -99,25 +98,10 @@ async def _default_resolve_tools(tools, **kwargs) -> ResolvedToolSet: return await _platform_resolve_tools(tools, **kwargs) -def _mcp_enabled() -> bool: - # MCP gating: off by default, deployment opts in via AGENTA_AGENT_MCPS_ENABLED. - return os.getenv("AGENTA_AGENT_MCPS_ENABLED", "").strip().lower() in TRUTHY - - async def _default_resolve_mcp_servers( mcp_servers, **kwargs ) -> List[ResolvedMCPServer]: - """Resolve MCP servers, gated by ``AGENTA_AGENT_MCPS_ENABLED`` (off by default). - - Disabled + no servers declared -> ``[]`` (the common case, unchanged). Disabled + servers - declared -> :class:`MCPDisabledError`, so a caller's ignored MCP config fails loud instead - of silently running with none. - """ - if not _mcp_enabled(): - if not mcp_servers: - return [] - names = [config.name for config in parse_mcp_server_configs(mcp_servers)] - raise MCPDisabledError(server_names=names) + """Resolve external MCP server declarations for one run.""" return await _platform_resolve_mcp(mcp_servers, **kwargs) diff --git a/sdks/python/agenta/sdk/agents/mcp/__init__.py b/sdks/python/agenta/sdk/agents/mcp/__init__.py index 5a8de94d7e..5ade98dbb4 100644 --- a/sdks/python/agenta/sdk/agents/mcp/__init__.py +++ b/sdks/python/agenta/sdk/agents/mcp/__init__.py @@ -2,18 +2,30 @@ from .errors import ( MCPConfigurationError, - MCPDisabledError, MCPError, MissingMCPSecretError, ) from .interfaces import MCPSecretProvider -from .models import MCPServerConfig, ResolvedMCPServer +from .models import ( + MCPConnection, + MCPHeaderSecretRefs, + MCPPolicy, + MCPServerConfig, + MCPToolPolicy, + NoMCPCredentials, + ResolvedMCPServer, +) from .parsing import parse_mcp_server_config, parse_mcp_server_configs from .resolver import MCPResolver from .wire import mcp_server_to_wire, mcp_servers_to_wire __all__ = [ "MCPServerConfig", + "MCPConnection", + "MCPHeaderSecretRefs", + "MCPPolicy", + "MCPToolPolicy", + "NoMCPCredentials", "ResolvedMCPServer", "MCPSecretProvider", "MCPResolver", @@ -23,6 +35,5 @@ "mcp_servers_to_wire", "MCPError", "MCPConfigurationError", - "MCPDisabledError", "MissingMCPSecretError", ] diff --git a/sdks/python/agenta/sdk/agents/mcp/errors.py b/sdks/python/agenta/sdk/agents/mcp/errors.py index 523f98a566..2d2ab05193 100644 --- a/sdks/python/agenta/sdk/agents/mcp/errors.py +++ b/sdks/python/agenta/sdk/agents/mcp/errors.py @@ -22,26 +22,6 @@ def __init__( self.value = value -class MCPDisabledError(MCPError): - """The deployment disabled MCP, but the request still declared ``mcp_servers``. - - Raised instead of silently dropping the servers, so the user learns their MCP config was - ignored rather than wondering why the run behaved as if it had no servers. The fail-loud - MCP guards in the runner never see these servers (resolution strips them before the wire), - so this is the boundary that must surface the disabled state. - """ - - def __init__(self, *, server_names: Sequence[str]) -> None: - names = tuple(server_names) - listed = ", ".join(names) if names else "(unnamed)" - super().__init__( - "MCP servers are disabled on this deployment " - "(set AGENTA_AGENT_MCPS_ENABLED to enable them), but the request declared " - f"{len(names)} MCP server(s): {listed}. Remove them or enable MCP." - ) - self.server_names = names - - class MissingMCPSecretError(MCPError): def __init__(self, *, server_name: str, secret_names: Sequence[str]) -> None: names = tuple(secret_names) diff --git a/sdks/python/agenta/sdk/agents/mcp/models.py b/sdks/python/agenta/sdk/agents/mcp/models.py index 327278b01c..585c430116 100644 --- a/sdks/python/agenta/sdk/agents/mcp/models.py +++ b/sdks/python/agenta/sdk/agents/mcp/models.py @@ -1,94 +1,110 @@ -"""Canonical MCP server declarations and resolved runner configuration.""" +"""MCP author configuration and resolved runner delivery models.""" from __future__ import annotations -from typing import Any, Dict, List, Literal, Optional +from typing import Annotated, Any, Dict, List, Literal, Optional, Union -from pydantic import BaseModel, ConfigDict, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator -# Layer-3 per-server permission (same value set as a tool's): ``allow`` runs with -# no prompt, ``ask`` raises a human-in-the-loop request, ``deny`` never runs. Absent means the -# server inherits the runner policy. Permission = Literal["allow", "ask", "deny"] -# The deleted pre-redesign vocabulary, still present in old dev-DB drafts. Literal on -# purpose so the legacy spelling stays greppable. -_LEGACY_PERMISSION_KEYS = frozenset({"permission_mode", "permissionMode"}) +class NoMCPCredentials(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) -def _drop_legacy_permission_keys(data: Any) -> Any: - if isinstance(data, dict): - return { - key: value - for key, value in data.items() - if key not in _LEGACY_PERMISSION_KEYS - } - return data + type: Literal["none"] = "none" -class MCPServerConfig(BaseModel): +class MCPHeaderSecretRefs(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + + type: Literal["header_secret_refs"] = "header_secret_refs" + headers: Dict[str, str] = Field(default_factory=dict) + + +MCPCredentials = Annotated[ + Union[NoMCPCredentials, MCPHeaderSecretRefs], + Field(discriminator="type"), +] + + +class MCPConnection(BaseModel): + """How Agenta reaches one external MCP server.""" + model_config = ConfigDict(extra="forbid") - name: str = Field(min_length=1) - transport: Literal["stdio", "http"] = "stdio" - command: Optional[str] = None - args: List[str] = Field(default_factory=list) - env: Dict[str, str] = Field(default_factory=dict, repr=False) - url: Optional[str] = None - secrets: Dict[str, str] = Field(default_factory=dict) - tools: List[str] = Field(default_factory=list) - permission: Optional[Permission] = None + type: Literal["http"] = "http" + url: str = Field(min_length=1) + headers: Dict[str, str] = Field(default_factory=dict) + credentials: MCPCredentials = Field(default_factory=NoMCPCredentials) - @model_validator(mode="before") - @classmethod - def _ignore_legacy_permission_keys(cls, data: Any) -> Any: - return _drop_legacy_permission_keys(data) + +class MCPToolPolicy(BaseModel): + model_config = ConfigDict(extra="forbid") + + mode: Literal["all", "include"] = "all" + names: List[str] = Field(default_factory=list) @model_validator(mode="after") - def _validate_transport(self) -> "MCPServerConfig": - if self.transport == "stdio" and not self.command: - raise ValueError("stdio MCP server requires command") - if self.transport == "http" and not self.url: - raise ValueError("http MCP server requires url") + def _validate_names(self) -> "MCPToolPolicy": + if self.mode == "all" and self.names: + raise ValueError("MCP tool policy mode 'all' must not declare names") + if self.mode == "include" and not self.names: + raise ValueError("MCP tool policy mode 'include' requires names") return self +class MCPPolicy(BaseModel): + model_config = ConfigDict(extra="forbid") + + tools: MCPToolPolicy = Field(default_factory=MCPToolPolicy) + permission: Optional[Permission] = None + + +class MCPServerConfig(BaseModel): + """Saved author intent. This model never contains resolved secret values.""" + + model_config = ConfigDict(extra="forbid") + + name: str = Field(min_length=1) + connection: MCPConnection + policy: MCPPolicy = Field(default_factory=MCPPolicy) + + @field_validator("name") + @classmethod + def _reject_reserved_name(cls, value: str) -> str: + if value == "agenta-tools": + raise ValueError("MCP server name 'agenta-tools' is reserved") + return value + + class ResolvedMCPServer(BaseModel): + """Per-run delivery config. Headers may contain resolved secret values.""" + model_config = ConfigDict(extra="forbid", frozen=True) name: str - transport: Literal["stdio", "http"] = "stdio" - command: Optional[str] = None - args: List[str] = Field(default_factory=list) - env: Dict[str, str] = Field(default_factory=dict, repr=False) - url: Optional[str] = None - tools: List[str] = Field(default_factory=list) - permission: Optional[Permission] = None - - @model_validator(mode="after") - def _validate_transport(self) -> "ResolvedMCPServer": - if self.transport == "stdio" and not self.command: - raise ValueError("stdio MCP server requires command") - if self.transport == "http" and not self.url: - raise ValueError("http MCP server requires url") - return self + url: str + headers: Dict[str, str] = Field(default_factory=dict, repr=False) + policy: MCPPolicy = Field(default_factory=MCPPolicy) def to_wire(self) -> Dict[str, Any]: + connection: Dict[str, Any] = { + "type": "http", + "url": self.url, + } + if self.headers: + connection["headers"] = dict(self.headers) + wire: Dict[str, Any] = { "name": self.name, - "transport": self.transport, + "connection": connection, + "policy": { + "tools": self.policy.tools.model_dump(exclude_defaults=True) + or {"mode": "all"}, + }, } - if self.command: - wire["command"] = self.command - if self.args: - wire["args"] = list(self.args) - if self.env: - wire["env"] = dict(self.env) - if self.url: - wire["url"] = self.url - if self.tools: - wire["tools"] = list(self.tools) - if self.permission is not None: - wire["permission"] = self.permission + if self.policy.permission is not None: + wire["policy"]["permission"] = self.policy.permission return wire diff --git a/sdks/python/agenta/sdk/agents/mcp/resolver.py b/sdks/python/agenta/sdk/agents/mcp/resolver.py index 1593f9de8d..27259b34e8 100644 --- a/sdks/python/agenta/sdk/agents/mcp/resolver.py +++ b/sdks/python/agenta/sdk/agents/mcp/resolver.py @@ -8,7 +8,7 @@ from .errors import MissingMCPSecretError from .interfaces import MCPSecretProvider -from .models import MCPServerConfig, ResolvedMCPServer +from .models import MCPHeaderSecretRefs, MCPServerConfig, ResolvedMCPServer class MCPResolver: @@ -29,7 +29,8 @@ async def resolve( { secret_name for server_config in server_configs - for secret_name in server_config.secrets.values() + if isinstance(server_config.connection.credentials, MCPHeaderSecretRefs) + for secret_name in server_config.connection.credentials.headers.values() } ) secret_values: Mapping[str, str] = ( @@ -38,9 +39,15 @@ async def resolve( resolved: list[ResolvedMCPServer] = [] for server_config in server_configs: + credentials = server_config.connection.credentials + secret_refs = ( + credentials.headers + if isinstance(credentials, MCPHeaderSecretRefs) + else {} + ) missing = [ secret_name - for secret_name in server_config.secrets.values() + for secret_name in secret_refs.values() if secret_name not in secret_values ] if missing and self._missing_secret_policy == MissingSecretPolicy.ERROR: @@ -49,21 +56,17 @@ async def resolve( secret_names=missing, ) - env = dict(server_config.env) - for env_var, secret_name in server_config.secrets.items(): + headers = dict(server_config.connection.headers) + for header_name, secret_name in secret_refs.items(): if secret_name in secret_values: - env[env_var] = secret_values[secret_name] + headers[header_name] = secret_values[secret_name] resolved.append( ResolvedMCPServer( name=server_config.name, - transport=server_config.transport, - command=server_config.command, - args=list(server_config.args), - env=env, - url=server_config.url, - tools=list(server_config.tools), - permission=server_config.permission, + url=server_config.connection.url, + headers=headers, + policy=server_config.policy, ) ) return resolved diff --git a/sdks/python/agenta/sdk/agents/wire_models.py b/sdks/python/agenta/sdk/agents/wire_models.py index 60ce74d5fc..038e214bb0 100644 --- a/sdks/python/agenta/sdk/agents/wire_models.py +++ b/sdks/python/agenta/sdk/agents/wire_models.py @@ -285,17 +285,11 @@ class WirePermissions(_WireModel): class WireMcpServer(_WireModel): - """A user-declared MCP server (stdio or http), mirrors ``mcp_servers_to_wire``.""" + """A resolved external HTTP MCP server, mirrors ``mcp_servers_to_wire``.""" name: str - transport: Optional[str] = None - command: Optional[str] = None - args: Optional[List[str]] = None - env: Optional[Dict[str, str]] = None - url: Optional[str] = None - headers: Optional[Dict[str, str]] = None - tools: Optional[List[str]] = None - permission: Optional[str] = None + connection: Dict[str, Any] + policy: Dict[str, Any] class WireSkillFile(_WireModel): diff --git a/sdks/python/oss/tests/pytest/unit/agents/adapters/test_claude_settings.py b/sdks/python/oss/tests/pytest/unit/agents/adapters/test_claude_settings.py index 378954209a..21f8d499de 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/adapters/test_claude_settings.py +++ b/sdks/python/oss/tests/pytest/unit/agents/adapters/test_claude_settings.py @@ -17,7 +17,7 @@ build_claude_settings_files, ) from agenta.sdk.agents.dtos import SandboxPermission -from agenta.sdk.agents.mcp import ResolvedMCPServer +from agenta.sdk.agents.mcp import MCPPolicy, ResolvedMCPServer from agenta.sdk.agents.tools.models import ( CallbackToolSpec, ClientToolSpec, @@ -41,6 +41,14 @@ def _claude(permissions): return permissions +def _mcp(name: str, permission=None) -> ResolvedMCPServer: + return ResolvedMCPServer( + name=name, + url="https://x", + policy=MCPPolicy(permission=permission), + ) + + def test_renders_author_mode_and_rules(): files = build_claude_settings_files( _claude( @@ -96,9 +104,7 @@ def test_filesystem_off_denies_write_edit(): def test_mcp_permission_deny_renders_server_rule(): - server = ResolvedMCPServer( - name="github", transport="http", url="https://x", permission="deny" - ) + server = _mcp("github", "deny") files = build_claude_settings_files(None, None, [server]) perms = _settings(files)["permissions"] assert perms["deny"] == ["mcp__github"] @@ -108,16 +114,10 @@ def test_mcp_permission_deny_renders_server_rule(): def test_mcp_permissions_route_to_their_lists_and_skip_unset(): servers = [ - ResolvedMCPServer( - name="filesystem", transport="http", url="https://x", permission="allow" - ), - ResolvedMCPServer( - name="github", transport="http", url="https://x", permission="ask" - ), - ResolvedMCPServer( - name="shell", transport="http", url="https://x", permission="deny" - ), - ResolvedMCPServer(name="unset", transport="http", url="https://x"), + _mcp("filesystem", "allow"), + _mcp("github", "ask"), + _mcp("shell", "deny"), + _mcp("unset"), ] perms = _settings(build_claude_settings_files(None, None, servers))["permissions"] assert perms["allow"] == ["mcp__filesystem"] @@ -126,12 +126,7 @@ def test_mcp_permissions_route_to_their_lists_and_skip_unset(): def test_reserved_internal_mcp_server_name_does_not_render_whole_server_rule(): - server = ResolvedMCPServer( - name=INTERNAL_TOOL_MCP_SERVER, - transport="http", - url="https://x", - permission="deny", - ) + server = _mcp(INTERNAL_TOOL_MCP_SERVER, "deny") tool = CallbackToolSpec( name="get_user", description="d", call_ref="tools__x", permission="allow" ) @@ -181,7 +176,7 @@ def test_accepts_plain_dicts_for_sandbox_and_mcp(): files = build_claude_settings_files( None, {"network": {"mode": "off"}}, - [{"name": "github", "permission": "deny"}], + [{"name": "github", "policy": {"permission": "deny"}}], ) perms = _settings(files)["permissions"] assert perms["deny"] == ["WebFetch", "WebSearch", "mcp__github"] @@ -199,12 +194,7 @@ def test_empty_inputs_render_nothing(): == [] ) # an MCP server with no permission contributes nothing. - assert ( - build_claude_settings_files( - None, None, [ResolvedMCPServer(name="x", transport="http", url="https://x")] - ) - == [] - ) + assert build_claude_settings_files(None, None, [_mcp("x")]) == [] def test_malformed_permissions_slice_renders_nothing(): @@ -360,9 +350,7 @@ def test_client_tool_deny_renders_deny_rule(): def test_tool_rules_merge_with_author_and_mcp(): # Author allow/deny first, then the per-MCP-server rule, then the per-tool rule append (deduped, # first-seen order preserved). - server = ResolvedMCPServer( - name="github", transport="http", url="https://x", permission="allow" - ) + server = _mcp("github", "allow") allow_tool = CallbackToolSpec( name="capital_lookup", description="d", diff --git a/sdks/python/oss/tests/pytest/unit/agents/connections/test_capabilities.py b/sdks/python/oss/tests/pytest/unit/agents/connections/test_capabilities.py index 1533290c4a..aa8130ae8f 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/connections/test_capabilities.py +++ b/sdks/python/oss/tests/pytest/unit/agents/connections/test_capabilities.py @@ -100,6 +100,14 @@ def test_capabilities_document_shape(): "vertex_ai", "vertex", ] + assert doc["claude"]["mcp"] == { + "user_servers": { + "connection_types": ["http"], + "credentials": ["none", "header_secret_refs"], + } + } + assert "mcp" not in doc["pi_core"] + assert "mcp" not in doc["pi_agenta"] def test_every_harness_publishes_a_models_map(): diff --git a/sdks/python/oss/tests/pytest/unit/agents/mcp/test_resolver.py b/sdks/python/oss/tests/pytest/unit/agents/mcp/test_resolver.py index 7cb093fb13..f562b99cd4 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/mcp/test_resolver.py +++ b/sdks/python/oss/tests/pytest/unit/agents/mcp/test_resolver.py @@ -6,8 +6,12 @@ from pydantic import ValidationError from agenta.sdk.agents.mcp import ( + MCPConnection, + MCPHeaderSecretRefs, + MCPPolicy, MCPResolver, MCPServerConfig, + MCPToolPolicy, MissingMCPSecretError, ) from agenta.sdk.agents.tools import MissingSecretPolicy @@ -21,29 +25,53 @@ async def get_many(self, names: Sequence[str]) -> Mapping[str, str]: return {name: self.values[name] for name in names if name in self.values} -def test_transport_specific_fields_are_required(): - with pytest.raises(ValidationError, match="requires command"): - MCPServerConfig(name="stdio") - with pytest.raises(ValidationError, match="requires url"): - MCPServerConfig(name="remote", transport="http") +def server(**overrides) -> MCPServerConfig: + values = { + "name": "memory", + "connection": {"type": "http", "url": "https://memory.example.com/mcp"}, + } + values.update(overrides) + return MCPServerConfig.model_validate(values) + + +def test_connection_is_required_and_legacy_flat_shape_is_rejected(): + with pytest.raises(ValidationError, match="connection"): + MCPServerConfig(name="memory") + with pytest.raises(ValidationError, match="Extra inputs are not permitted"): + MCPServerConfig.model_validate( + { + "name": "legacy", + "transport": "stdio", + "command": "npx", + "connection": {"type": "http", "url": "https://example.com/mcp"}, + } + ) -async def test_resolves_mcp_environment_in_sibling_subsystem(): - servers = await MCPResolver( - secret_provider=DictSecretProvider({"github_pat": "ghp"}) +async def test_resolves_public_and_secret_headers(): + resolved = await MCPResolver( + secret_provider=DictSecretProvider({"memory_token": "secret-value"}) ).resolve( [ - MCPServerConfig( - name="github", - command="npx", - env={"LOG": "info"}, - secrets={"GITHUB_TOKEN": "github_pat"}, + server( + connection=MCPConnection( + type="http", + url="https://memory.example.com/mcp", + headers={"X-Workspace": "demo"}, + credentials=MCPHeaderSecretRefs( + headers={"Authorization": "memory_token"} + ), + ) ) ] ) - assert servers[0].to_wire()["env"] == { - "LOG": "info", - "GITHUB_TOKEN": "ghp", + assert resolved[0].to_wire()["connection"] == { + "type": "http", + "url": "https://memory.example.com/mcp", + "headers": { + "X-Workspace": "demo", + "Authorization": "secret-value", + }, } @@ -51,51 +79,66 @@ async def test_missing_mcp_secret_is_explicit(): with pytest.raises(MissingMCPSecretError): await MCPResolver(secret_provider=DictSecretProvider({})).resolve( [ - MCPServerConfig( - name="github", - command="npx", - secrets={"GITHUB_TOKEN": "missing"}, + server( + connection=MCPConnection( + type="http", + url="https://memory.example.com/mcp", + credentials=MCPHeaderSecretRefs( + headers={"Authorization": "missing"} + ), + ) ) ] ) -async def test_permission_rides_the_wire_when_set(): - # An author's per-server permission is carried onto the resolved server and serialized. - servers = await MCPResolver(secret_provider=DictSecretProvider({})).resolve( - [MCPServerConfig(name="github", command="npx", permission="ask")] +async def test_policy_rides_the_wire(): + resolved = await MCPResolver(secret_provider=DictSecretProvider({})).resolve( + [ + server( + policy=MCPPolicy( + tools=MCPToolPolicy(mode="include", names=["search"]), + permission="ask", + ) + ) + ] ) - assert servers[0].permission == "ask" - assert servers[0].to_wire()["permission"] == "ask" + assert resolved[0].to_wire()["policy"] == { + "tools": {"mode": "include", "names": ["search"]}, + "permission": "ask", + } -async def test_permission_absent_from_wire_when_unset(): - # No permission declared -> no `permission` key (a server has no read_only to default from). - servers = await MCPResolver(secret_provider=DictSecretProvider({})).resolve( - [MCPServerConfig(name="github", command="npx")] +async def test_default_policy_is_explicit_all(): + resolved = await MCPResolver(secret_provider=DictSecretProvider({})).resolve( + [server()] ) - assert servers[0].permission is None - assert "permission" not in servers[0].to_wire() + assert resolved[0].to_wire()["policy"] == {"tools": {"mode": "all"}} -def test_legacy_permission_mode_alias_is_ignored(): - config = MCPServerConfig.model_validate( - {"name": "github", "command": "npx", "permission_mode": "deny"} - ) - assert config.permission is None +def test_tool_policy_rejects_ambiguous_combinations(): + with pytest.raises(ValidationError, match="must not declare names"): + MCPToolPolicy(mode="all", names=["search"]) + with pytest.raises(ValidationError, match="requires names"): + MCPToolPolicy(mode="include") -async def test_mcp_compatibility_policy_can_omit_missing_secret(): - servers = await MCPResolver( +async def test_omit_missing_secret_keeps_public_headers_only(): + resolved = await MCPResolver( secret_provider=DictSecretProvider({}), missing_secret_policy=MissingSecretPolicy.OMIT, ).resolve( [ - MCPServerConfig( - name="github", - command="npx", - secrets={"GITHUB_TOKEN": "missing"}, + server( + connection=MCPConnection( + type="http", + url="https://memory.example.com/mcp", + headers={"X-Workspace": "demo"}, + credentials=MCPHeaderSecretRefs( + headers={"Authorization": "missing"} + ), + ) ) ] ) - assert "env" not in servers[0].to_wire() + assert resolved[0].to_wire()["connection"]["headers"] == {"X-Workspace": "demo"} diff --git a/sdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.py b/sdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.py index 70ae5cb2d3..2d2eb01dea 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.py +++ b/sdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.py @@ -21,7 +21,6 @@ ) from agenta.sdk.agents.handler import AgentComposition, make_agent_handler from agenta.sdk.agents.interfaces import Backend, Sandbox, Session -from agenta.sdk.agents.mcp import MCPDisabledError from agenta.sdk.agents.streaming import AgentStream from agenta.sdk.models.workflows import WorkflowServiceRequest @@ -276,57 +275,6 @@ async def _bare(model_ref, context): assert len(calls) == 1 -# --------------------------------------------------------------------------- # -# Drift 3: MCP gating is the seam DEFAULT now (previously ungated in handler.py). -# --------------------------------------------------------------------------- # -async def test_default_composition_mcp_disabled_with_no_servers_is_empty(monkeypatch): - from agenta.sdk.agents import handler as handler_module - - monkeypatch.delenv("AGENTA_AGENT_MCPS_ENABLED", raising=False) - backend = _FakeBackend() - comp = AgentComposition( - select_backend=lambda template: backend, - resolve_connection=_no_connection, - ) - handler = make_agent_handler(comp) - - result = await handler( - request=_request(), - messages=[{"role": "user", "content": "hi"}], - parameters=_params(), - ) - assert isinstance(result, dict) - assert handler_module._mcp_enabled() is False - - -async def test_default_composition_mcp_disabled_with_servers_fails_loud(monkeypatch): - monkeypatch.delenv("AGENTA_AGENT_MCPS_ENABLED", raising=False) - backend = _FakeBackend() - comp = AgentComposition( - select_backend=lambda template: backend, - resolve_connection=_no_connection, - ) - handler = make_agent_handler(comp) - - params = _params() - params["agent"]["mcps"] = [{"name": "github", "command": "npx"}] - - with pytest.raises(MCPDisabledError) as excinfo: - await handler( - request=_request(), - messages=[{"role": "user", "content": "hi"}], - parameters=params, - ) - assert "github" in str(excinfo.value) - - -async def test_default_composition_mcp_enabled_resolves_servers(monkeypatch): - from agenta.sdk.agents import handler as handler_module - - monkeypatch.setenv("AGENTA_AGENT_MCPS_ENABLED", "true") - assert handler_module._mcp_enabled() is True - - # --------------------------------------------------------------------------- # # Drift 5: backend/template defaults are per-composition (deployment-specific), # proving the seam is request/composition-aware rather than hardcoding one source. diff --git a/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py b/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py index 7eaba107b5..1bd083a4bb 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py +++ b/sdks/python/oss/tests/pytest/unit/agents/test_wire_contract.py @@ -665,10 +665,9 @@ def test_request_to_wire_carries_code_client_and_mcp_specs(): mcp_servers=[ { "name": "github", - "transport": "stdio", - "command": "npx", - "env": {"GITHUB_TOKEN": "ghp"}, - "tools": ["create_issue"], + "url": "https://mcp.example.com/mcp", + "headers": {"Authorization": "Bearer ghp"}, + "policy": {"tools": {"mode": "include", "names": ["create_issue"]}}, } ], ) @@ -692,10 +691,12 @@ def test_request_to_wire_carries_code_client_and_mcp_specs(): assert payload["mcpServers"] == [ { "name": "github", - "transport": "stdio", - "command": "npx", - "env": {"GITHUB_TOKEN": "ghp"}, - "tools": ["create_issue"], + "connection": { + "type": "http", + "url": "https://mcp.example.com/mcp", + "headers": {"Authorization": "Bearer ghp"}, + }, + "policy": {"tools": {"mode": "include", "names": ["create_issue"]}}, } ] @@ -762,9 +763,8 @@ def test_request_to_wire_claude_renders_settings_from_options_and_boundaries(): mcp_servers=[ { "name": "github", - "transport": "http", "url": "https://x", - "permission": "ask", + "policy": {"permission": "ask"}, } ], ) diff --git a/services/oss/src/agent/tools/resolver.py b/services/oss/src/agent/tools/resolver.py index 0cafdd86b3..6a46fb345d 100644 --- a/services/oss/src/agent/tools/resolver.py +++ b/services/oss/src/agent/tools/resolver.py @@ -1,46 +1,20 @@ -"""Service-side resolution wiring. - -The three resolution entrypoints now live in the SDK (``agenta.sdk.agents.platform``) so the -service and a standalone SDK user share them. ``resolve_tools`` is re-exported as-is; the -service only adds the MCP deployment gate (``AGENTA_AGENT_MCPS_ENABLED``, off by -default) on top of the SDK's ``resolve_mcp``. -""" +"""Service-side tool and MCP resolution wiring.""" from __future__ import annotations -import os from typing import Any, List, Optional, Sequence -from agenta.sdk.agents.mcp import MCPDisabledError, ResolvedMCPServer -from agenta.sdk.agents.mcp.parsing import parse_mcp_server_configs +from agenta.sdk.agents.mcp import ResolvedMCPServer from agenta.sdk.agents.platform import resolve_mcp, resolve_tools from agenta.sdk.agents.tools.interfaces import ToolSecretProvider -from agenta.sdk.utils.constants import TRUTHY __all__ = ["resolve_tools", "resolve_mcp_servers"] -def _mcp_enabled() -> bool: - return os.getenv("AGENTA_AGENT_MCPS_ENABLED", "").strip().lower() in TRUTHY - - async def resolve_mcp_servers( mcp_servers: Sequence[Any], *, secret_provider: Optional[ToolSecretProvider] = None, ) -> List[ResolvedMCPServer]: - """Resolve MCP servers, gated by ``AGENTA_AGENT_MCPS_ENABLED`` (off by default). - - When MCP is enabled, returns the resolved servers. When it is disabled and the request - declared NO servers, returns an empty list (the common case, unchanged). When it is disabled - but the request DID declare servers, raises :class:`MCPDisabledError` instead of silently - stripping them — silent-stripping made the runner's fail-loud MCP guards unreachable via - ``/invoke`` and ``/messages``, so the user got a run that quietly ignored their MCP config. - """ - if not _mcp_enabled(): - if not mcp_servers: - return [] - # Parse only to surface the server names in the error; do not resolve secrets (disabled). - names = [config.name for config in parse_mcp_server_configs(mcp_servers)] - raise MCPDisabledError(server_names=names) + """Resolve external MCP server declarations for one run.""" return await resolve_mcp(mcp_servers, secret_provider=secret_provider) diff --git a/services/oss/tests/pytest/unit/agent/tools/test_resolution.py b/services/oss/tests/pytest/unit/agent/tools/test_resolution.py index 3bd5c59c0e..b6c8a28f28 100644 --- a/services/oss/tests/pytest/unit/agent/tools/test_resolution.py +++ b/services/oss/tests/pytest/unit/agent/tools/test_resolution.py @@ -6,13 +6,11 @@ from agenta.sdk.agents import ( CodeToolSpec, - MCPDisabledError, MissingMCPSecretError, MissingToolSecretError, ) from oss.src.agent.tools import resolve_mcp_servers, resolve_tools -from oss.src.agent.tools import resolver as resolver_module class _FakeSecretProvider: @@ -71,40 +69,24 @@ async def test_missing_tool_secret_is_not_silently_omitted(): ) -async def test_mcp_disabled_with_no_servers_is_an_empty_list(monkeypatch): - # The common case (MCP off, no servers declared): still a clean empty list, unchanged. - monkeypatch.delenv("AGENTA_AGENT_MCPS_ENABLED", raising=False) +async def test_no_mcp_servers_is_an_empty_list(): assert await resolve_mcp_servers([]) == [] -async def test_mcp_disabled_with_servers_fails_loud_instead_of_silent_strip( - monkeypatch, -): - # F-039: disabling MCP must NOT silently drop user-declared servers. The user gets a clear - # "MCP servers are disabled" error naming the servers, not a run that quietly ignored them. - monkeypatch.delenv("AGENTA_AGENT_MCPS_ENABLED", raising=False) - with pytest.raises(MCPDisabledError) as excinfo: - await resolve_mcp_servers( - [ - {"name": "github", "command": "npx"}, - {"name": "filesystem", "command": "npx"}, - ] - ) - assert excinfo.value.server_names == ("github", "filesystem") - message = str(excinfo.value) - assert "disabled" in message - assert "github" in message and "filesystem" in message - - -async def test_missing_mcp_secret_is_explicit_when_enabled(monkeypatch): - monkeypatch.setattr(resolver_module, "_mcp_enabled", lambda: True) +async def test_missing_mcp_secret_is_explicit(): with pytest.raises(MissingMCPSecretError): await resolve_mcp_servers( [ { "name": "github", - "command": "npx", - "secrets": {"GITHUB_TOKEN": "missing"}, + "connection": { + "type": "http", + "url": "https://github.example.com/mcp", + "credentials": { + "type": "header_secret_refs", + "headers": {"Authorization": "missing"}, + }, + }, } ], secret_provider=_FakeSecretProvider({}), diff --git a/services/runner/README.md b/services/runner/README.md index 6092101d24..4c116a3053 100644 --- a/services/runner/README.md +++ b/services/runner/README.md @@ -37,7 +37,6 @@ src/ code.ts execute resolved code tools in a scoped subprocess dispatch.ts dispatch resolved tools by executor kind mcp-bridge.ts the INTERNAL gateway-tool MCP channel (loopback HTTP) — live - mcp-server.ts the OLD stdio MCP bridge — REMOVED (refuses to serve; no longer launched) extensions/ agenta.ts the Pi extension (tracing + tools), bundled into dist/ for Pi to load ``` @@ -89,13 +88,9 @@ Non-Pi harnesses (e.g. Claude) that only accept tools over MCP get these same re through an INTERNAL loopback HTTP MCP channel the runner serves (`tools/mcp-bridge.ts` + `tools/tool-mcp-http.ts`) — this channel is live and is how Claude runs take custom tools. -This internal channel is a different thing from USER-declared MCP servers (a run request's own -`mcpServers`), which stay gated: stdio user MCP servers are refused for every harness -(`tools/mcp-server.ts`, the old stdio bridge, is REMOVED — it launched an unconfined child -process on the runner host, the same execution bypass that had code tools removed), and it is -_Pi_, not the sidecar broadly, that also refuses user _http_ MCP servers, because Pi delivers -tools through its bundled extension rather than over ACP MCP. Claude accepts user http MCP -servers. See `docs/design/agent-workflows/projects/sidecar-trust-and-sandbox-enforcement/`. +This internal channel is different from external user MCP servers in `mcpServers`. The public +contract accepts HTTP only. Pi currently refuses external MCP because it delivers tools through +its bundled extension rather than ACP MCP; Claude accepts the HTTP entries. ## The extension bundle diff --git a/services/runner/src/engines/sandbox_agent.ts b/services/runner/src/engines/sandbox_agent.ts index b65f549faa..f690e6a778 100644 --- a/services/runner/src/engines/sandbox_agent.ts +++ b/services/runner/src/engines/sandbox_agent.ts @@ -203,8 +203,8 @@ function serverPermissionsFromRequest( ): ReadonlyMap { const permissions = new Map(); for (const server of request.mcpServers ?? []) { - if (server.permission !== undefined) { - permissions.set(server.name, server.permission); + if (server.policy?.permission !== undefined) { + permissions.set(server.name, server.policy.permission); } } return permissions; diff --git a/services/runner/src/engines/sandbox_agent/mcp.ts b/services/runner/src/engines/sandbox_agent/mcp.ts index a0874503bf..1c2b716254 100644 --- a/services/runner/src/engines/sandbox_agent/mcp.ts +++ b/services/runner/src/engines/sandbox_agent/mcp.ts @@ -5,7 +5,6 @@ import type { } from "../../protocol.ts"; import { buildToolMcpServers, - USER_MCP_UNSUPPORTED_MESSAGE, } from "../../tools/mcp-bridge.ts"; import type { ClientToolRelay } from "../../tools/client-tool-relay.ts"; import { executableToolSpecs } from "../../tools/public-spec.ts"; @@ -194,63 +193,28 @@ export async function validateUserMcpUrl( return undefined; } -/** - * Convert USER-declared MCP servers into ACP entries. (This is the USER MCP capability layer — - * distinct from the INTERNAL gateway-tool channel below; see `buildSessionMcpServers`.) - * - * - HTTP (`transport: "http"` + `url`) is ENABLED. A remote server has no child process on the - * runner host: the harness connects to the URL and the named secret rides in a request header, - * so it does not bypass the sandbox boundary the way a stdio child does. The resolved secret - * arrives on the `/run` wire in the server's `env` map (the SDK resolver merges named secrets - * into `env` regardless of transport, and the wire has no separate `headers` field), so each - * `env` entry is emitted as an HTTP header (`Authorization: `, etc.). The author names - * the header via the secret-map key, exactly as a stdio server names its env var. - * - STDIO (`transport: "stdio"` + `command`) is DISABLED. A stdio MCP server launches an - * arbitrary process on the runner host, outside the sandbox boundary, so the implementation is - * disabled (parity with the removed code execution) until its security is fixed. The wire shape - * (`McpServerConfig`) is kept, but a stdio server throws `USER_MCP_UNSUPPORTED_MESSAGE` rather - * than being delivered. - * - A server that is neither a valid http (no `url`) nor a valid stdio (no `command`) is skipped - * with a log — it was never deliverable. - * - An http `url` that fails the SSRF guard (`validateUserMcpUrl`: non-https, or an - * internal/metadata host) throws, so a credentialed request is never sent to an internal sink. - */ +/** Convert external HTTP MCP servers into Claude ACP session entries. */ export async function toAcpMcpServers( servers: McpServerConfig[] | undefined, log: Log = () => {}, ): Promise { const out: McpServerEntry[] = []; for (const s of servers ?? []) { - const transport = s.transport ?? "stdio"; - - if (transport === "http") { - if (!s.url) { - log(`skipping http MCP server '${s?.name ?? "?"}' (no url)`); - continue; - } - // SSRF guard: the resolved named secret rides as a header on this author-supplied URL, so - // reject a non-https / internal / metadata target before any credential is attached. - const urlError = await validateUserMcpUrl(s.url); - if (urlError) throw new Error(urlError); - out.push({ - type: "http", - name: s.name, - url: s.url, - headers: Object.entries(s.env ?? {}).map(([name, value]) => ({ - name, - value, - })), - }); + const url = s.connection?.url; + if (!url) { + log(`skipping HTTP MCP server '${s?.name ?? "?"}' (no URL)`); continue; } - - // stdio: a command-less server was never launched, so it stays a skipped no-op; a real - // stdio server is disabled and fails loud. - if (!s.command) { - log(`skipping stdio MCP server '${s?.name ?? "?"}' (no command)`); - continue; - } - throw new Error(USER_MCP_UNSUPPORTED_MESSAGE); + const urlError = await validateUserMcpUrl(url); + if (urlError) throw new Error(urlError); + out.push({ + type: "http", + name: s.name, + url, + headers: Object.entries(s.connection.headers ?? {}).map( + ([name, value]) => ({ name, value }), + ), + }); } return out; } diff --git a/services/runner/src/engines/sandbox_agent/run-plan.ts b/services/runner/src/engines/sandbox_agent/run-plan.ts index 6c7b9c9445..200ad17110 100644 --- a/services/runner/src/engines/sandbox_agent/run-plan.ts +++ b/services/runner/src/engines/sandbox_agent/run-plan.ts @@ -5,7 +5,6 @@ import { basename, join } from "node:path"; import { type AgentRunRequest, - type McpServerConfig, type ResolvedToolSpec, type SandboxPermission, resolvePromptText, @@ -14,7 +13,6 @@ import { executableToolSpecs } from "../../tools/public-spec.ts"; import { CODE_TOOL_UNSUPPORTED_MESSAGE } from "../../tools/code.ts"; import { PI_USER_MCP_UNSUPPORTED_MESSAGE, - USER_MCP_UNSUPPORTED_MESSAGE, } from "../../tools/mcp-bridge.ts"; import { INTERNAL_TOOL_MCP_SERVER_NAME, @@ -149,20 +147,6 @@ export interface BuildRunPlanDeps { log?: Log; } -/** - * True when an MCP server runs as a host command (stdio) rather than a remote URL. Mirrors - * the delivery rule in `mcp.ts` (`toAcpMcpServers`): the default transport is `stdio`, and a - * stdio server only runs when it carries a `command`. Such a server is an arbitrary process - * on the RUNNER HOST, so a network-blocked sandbox does not confine it. HTTP servers - * (`transport: "http"`) are delivered (the harness connects to the remote URL with the secret - * in a header) and are NOT flagged here — they have no runner-host process to confine. - */ -function hasStdioMcpServer(servers: McpServerConfig[] | undefined): boolean { - return (servers ?? []).some( - (s) => (s.transport ?? "stdio") === "stdio" && !!s.command, - ); -} - /** * True when any resolved tool is a `code` tool. Code execution was removed for security * (F-010); the sidecar must refuse a run that carries one rather than advertise it and then @@ -336,22 +320,12 @@ export function buildRunPlan( // Pi delivers tools through its bundled extension, not over ACP MCP, so a user MCP server on // a Pi run is DROPPED by `buildSessionMcpServers` (it returns [] for Pi). Dropping it silently - // (no log, HTTP 200) is the F-032 silent-drop bug. Refuse ANY user MCP server (stdio AND http) - // on Pi up front with a Pi-specific message, the way the stdio-MCP and code-tool gates fail - // loud. This MUST precede the harness-agnostic stdio gate so Pi gets the clearer reason for - // both transports (http MCP is otherwise a Claude-only capability, #4834). + // (no log, HTTP 200) is the F-032 silent-drop bug. Refuse any external user MCP server + // on Pi up front with a Pi-specific message. if (isPi && (request.mcpServers?.length ?? 0) > 0) { return { ok: false, error: PI_USER_MCP_UNSUPPORTED_MESSAGE }; } - // stdio MCP servers run as arbitrary processes on the RUNNER HOST, outside the sandbox - // boundary, and the sidecar's stdio MCP implementation is disabled (parity with the removed - // code execution) until its security is fixed. Refuse any run carrying one, the way code - // tools are gated — keep the wire shape, but the delivery is not supported. - if (hasStdioMcpServer(request.mcpServers)) { - return { ok: false, error: USER_MCP_UNSUPPORTED_MESSAGE }; - } - // The internal gateway-tool channel's name is reserved on every transport: the Python // Claude adapter renders permission rules against `agenta-tools`, so a user server with // that name would collide with the internal channel and inherit/steal its rendered rules. diff --git a/services/runner/src/protocol.ts b/services/runner/src/protocol.ts index 5988654c3f..760ea7e58e 100644 --- a/services/runner/src/protocol.ts +++ b/services/runner/src/protocol.ts @@ -234,25 +234,23 @@ export interface WireSkill { allowExecutableFiles?: boolean; } -/** - * A user-declared MCP server attached to the run. `stdio` launches `command`/`args` with - * `env` (secret env already resolved server-side); `tools` is an optional allowlist (empty = - * all). Remote (`http`) carries no auth on the wire by design. - */ +export interface McpToolPolicy { + mode: "all" | "include"; + names?: string[]; +} + export interface McpServerConfig { name: string; - transport?: "stdio" | "http"; - command?: string; - args?: string[]; - env?: Record; - url?: string; - tools?: string[]; - /** - * Layer-3 permission for the whole server: `allow` / `ask` / `deny`. Absent = - * fall back to the global permission plan. An MCP server has no `readOnly` hint, so there - * is no derived default: an explicit author value or nothing. - */ - permission?: ToolPermission; + connection: { + type: "http"; + url: string; + /** Resolved per-run headers. Values may be secret and must never be logged. */ + headers?: Record; + }; + policy: { + tools: McpToolPolicy; + permission?: ToolPermission; + }; } /** diff --git a/services/runner/src/tools/callback.ts b/services/runner/src/tools/callback.ts index 022347cf57..43cee16e59 100644 --- a/services/runner/src/tools/callback.ts +++ b/services/runner/src/tools/callback.ts @@ -3,7 +3,7 @@ * * One implementation of the tool round-trip used by every delivery path: * - extensions/agenta.ts registerTools (Pi under sandbox-agent/ACP, via the bundled extension) - * - tools/mcp-server.ts (the MCP stdio bridge for non-Pi harnesses) + * - the internal MCP channels used by non-Pi harnesses * * Each call POSTs the OpenAI-style envelope to Agenta's /tools/call, so the Composio key * and connection auth stay server-side. Keeping the request envelope and response parse in diff --git a/services/runner/src/tools/dispatch.ts b/services/runner/src/tools/dispatch.ts index 40ae9d26de..273753cfc7 100644 --- a/services/runner/src/tools/dispatch.ts +++ b/services/runner/src/tools/dispatch.ts @@ -2,8 +2,8 @@ * Shared tool dispatch: execute one backend-resolved tool, branching on its executor `kind`. * * The same "branch on spec.kind to run a resolved tool" logic was duplicated across the - * delivery paths (extensions/agenta.ts Pi-under-sandbox-agent, tools/mcp-server.ts the MCP - * bridge). This module owns that dispatch ONCE so a change to how a kind is executed is a + * delivery paths (the Pi extension and internal MCP channels). This module owns that dispatch + * once so a change to how a kind is executed is a * one-line edit, not several. Each call site still keeps its OWN result-wrapping shape (the Pi * extension's tool details, the MCP `content` envelope) and its OWN advertise/skip behavior * for `client` tools — only the execution itself is shared. diff --git a/services/runner/src/tools/mcp-bridge.ts b/services/runner/src/tools/mcp-bridge.ts index c25706a7ab..2f5e016883 100644 --- a/services/runner/src/tools/mcp-bridge.ts +++ b/services/runner/src/tools/mcp-bridge.ts @@ -13,10 +13,8 @@ * loopback HTTP endpoint launches no process; it is served by the already-running runner. This * is why restoring delivery here does NOT reopen #4831's hole (see project gateway-tool-mcp). * - * IMPORTANT — this is NOT the user MCP capability: - * - USER stdio MCP servers stay DISABLED (`engines/sandbox_agent/mcp.ts` `toAcpMcpServers` + - * `run-plan.ts` `hasStdioMcpServer`, fail with `USER_MCP_UNSUPPORTED_MESSAGE`). - * - USER http MCP servers are delivered unchanged (#4834). + * IMPORTANT: this is not the user MCP capability: + * - External user HTTP MCP servers are delivered separately. * - THIS internal channel is synthesized by the runner from `customTools`; the user never * declares it. The two layers toggle independently — do not merge their gates. */ @@ -28,21 +26,12 @@ import { startInternalToolMcpServer } from "./tool-mcp-http.ts"; export type { ResolvedToolSpec, ToolCallbackContext } from "../protocol.ts"; /** - * USER-facing MCP refusal. Means ONLY "user-declared MCP servers are not supported" — used by - * the user stdio gate (`run-plan.ts` `hasStdioMcpServer`) and the user stdio branch - * (`toAcpMcpServers`). The INTERNAL gateway-tool channel below must NEVER borrow this constant: - * conflating the two is exactly the #4831 regression this project fixed. - */ -export const USER_MCP_UNSUPPORTED_MESSAGE = - "MCPs are not supported by the sidecar."; - -/** - * Pi-family refusal for ANY user-declared MCP server (stdio AND http). The Pi runtime delivers + * Pi-family refusal for an external user-declared HTTP MCP server. The Pi runtime delivers * tools through the bundled Agenta extension, not over ACP MCP (`buildSessionMcpServers` returns * `[]` for Pi), so a user MCP server attached to a Pi run would be DROPPED — silently, with no * log and an HTTP 200. That is exactly the silent-drop F-032 forbids. The `run-plan.ts` gate * refuses it up front (the way the stdio-MCP and code-tool gates do) so the failure is loud - * instead of a "successful" empty run. Http MCP is a Claude-only capability (#4834); pick a + * instead of a "successful" empty run. HTTP MCP is a Claude-only capability for now; pick a * non-Pi harness to use one. */ export const PI_USER_MCP_UNSUPPORTED_MESSAGE = diff --git a/services/runner/src/tools/mcp-server.ts b/services/runner/src/tools/mcp-server.ts deleted file mode 100644 index 707b4a70da..0000000000 --- a/services/runner/src/tools/mcp-server.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Stdio MCP bridge server — REMOVED. - * - * This was a JSON-RPC-over-stdio MCP server that exposed backend-resolved tools to non-Pi - * harnesses and relayed each call back to the runner. It ran as a child process ON THE RUNNER - * HOST, outside the sandbox boundary, so a network-blocked sandbox did not confine it — the - * same runner-host execution bypass that had code execution removed (`tools/code.ts`). - * - * The stdio implementation is removed. The internal gateway-tool channel was RESTORED over an - * internal loopback HTTP MCP endpoint instead (`tools/tool-mcp-http.ts`), which launches no - * runner-host process — so it does not reintroduce this hole (project gateway-tool-mcp). - * Nothing launches this file anymore. If invoked directly it refuses loudly rather than serving. - */ -import { USER_MCP_UNSUPPORTED_MESSAGE } from "./mcp-bridge.ts"; - -process.stderr.write(`[tool-bridge] ${USER_MCP_UNSUPPORTED_MESSAGE}\n`); -process.exit(1); diff --git a/services/runner/src/tools/tool-mcp-http.ts b/services/runner/src/tools/tool-mcp-http.ts index 5f2b06a24a..e38b341ca7 100644 --- a/services/runner/src/tools/tool-mcp-http.ts +++ b/services/runner/src/tools/tool-mcp-http.ts @@ -9,17 +9,15 @@ * the SAME dispatch the Pi path uses (`runResolvedTool` → relay → `/tools/call`), so every * credentialed action still happens server-side in runner memory. * - * Why HTTP-on-loopback and not the old stdio bridge: - * - No runner-host child process. The old `mcp-server.ts` spawned `tsx mcp-server.ts` on the - * runner host, outside the sandbox boundary — the same execution hole PR #4831 closed for - * USER stdio MCP servers. This endpoint launches nothing; it is served by the already-running - * runner process and is reachable only from loopback. + * Why HTTP on loopback: + * - No runner-host child process. This endpoint is served by the already-running runner process + * and is reachable only from loopback. * - Reuses #4834's proven transport. Claude consumes a `type: "http"` MCP server natively (the * bundled `@agentclientprotocol/claude-agent-acp` maps it to the Claude SDK's HTTP MCP client). * * This server is INDEPENDENT of the user MCP capability (`engines/sandbox_agent/mcp.ts` - * `toAcpMcpServers`): user stdio MCP stays disabled, user HTTP MCP is unchanged, and this - * internal channel is its own thing. Do not merge their gates (see project gateway-tool-mcp). + * `toAcpMcpServers`): external HTTP MCP is passed separately and this internal channel remains + * its own thing. * * Transport: MCP Streamable HTTP in stateless JSON mode. The MCP client (`@modelcontextprotocol/sdk` * `StreamableHTTPClientTransport`) POSTs JSON-RPC with `Accept: application/json, text/event-stream`; diff --git a/services/runner/tests/unit/mcp-servers.test.ts b/services/runner/tests/unit/mcp-servers.test.ts index 7af7dc9c22..5b6345e49b 100644 --- a/services/runner/tests/unit/mcp-servers.test.ts +++ b/services/runner/tests/unit/mcp-servers.test.ts @@ -1,64 +1,34 @@ -/** - * Unit tests for the user-declared MCP server conversion — HTTP enabled, stdio disabled. - * - * `resolve_mcp_servers` (Python) emits the McpServerConfig wire shape; the wire is unchanged. - * - * - HTTP (`transport: "http"` + `url`) is DELIVERED. A remote server has no child process on the - * runner host: the harness connects to the URL with the named secret in a request header, so it - * does not bypass the sandbox boundary. The resolved secret arrives on the wire under the - * server's `env` map (the SDK resolver merges named secrets into `env` regardless of transport, - * and the wire has no separate `headers` field), so each `env` entry becomes an HTTP header. - * - STDIO is DISABLED: a stdio server runs an arbitrary process on the RUNNER HOST, outside the - * sandbox boundary, so it throws `USER_MCP_UNSUPPORTED_MESSAGE` (parity with the removed code - * execution) until its security is fixed. - * - A command-less stdio server or a url-less http server was never deliverable and is skipped. - * - * The SSRF guard resolves hostnames via DNS (`tools/ssrf-guard.ts`), so tests here use IP literals - * to stay network-free; the DNS-resolution path itself is covered in `ssrf-guard.test.ts`. - * - * Run: pnpm test (or: pnpm exec vitest run tests/unit/mcp-servers.test.ts) - */ +/** External HTTP MCP conversion and SSRF policy. */ import { afterEach, beforeEach, describe, it } from "vitest"; import assert from "node:assert/strict"; import { toAcpMcpServers } from "../../src/engines/sandbox_agent.ts"; import type { McpServerHttp } from "../../src/engines/sandbox_agent/mcp.ts"; -import { USER_MCP_UNSUPPORTED_MESSAGE } from "../../src/tools/mcp-bridge.ts"; import type { McpServerConfig } from "../../src/protocol.ts"; -describe("toAcpMcpServers (http enabled, stdio disabled)", () => { +const http = ( + url: string, + headers: Record = { Authorization: "Bearer secret" }, +): McpServerConfig[] => [ + { + name: "s", + connection: { type: "http", url, headers }, + policy: { tools: { mode: "all" } }, + }, +]; + +describe("toAcpMcpServers", () => { it("maps empty input to []", async () => { assert.deepEqual(await toAcpMcpServers(undefined), [], "undefined -> []"); assert.deepEqual(await toAcpMcpServers([]), [], "[] -> []"); }); - it("throws the unsupported error for a stdio server", async () => { - const servers: McpServerConfig[] = [ - { - name: "github", - transport: "stdio", - command: "npx", - args: ["-y", "@modelcontextprotocol/server-github"], - env: { GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_x", LOG_LEVEL: "info" }, - }, - ]; - await assert.rejects( - () => toAcpMcpServers(servers), - new RegExp(USER_MCP_UNSUPPORTED_MESSAGE), - ); - }); - - it("delivers an http server, routing the resolved secret into a header", async () => { - const out = await toAcpMcpServers([ - { - name: "linear", - transport: "http", - url: "https://93.184.216.34/sse", - // The SDK resolver put the resolved `linear-mcp-token` secret here under the - // author-chosen header name "Authorization"; the wire has no separate `headers` key. - env: { Authorization: "Bearer secret-token-value" }, - }, - ]); + it("delivers an HTTP server with resolved headers", async () => { + const servers = http("https://93.184.216.34/sse", { + Authorization: "Bearer secret-token-value", + }); + servers[0].name = "linear"; + const out = await toAcpMcpServers(servers); assert.equal(out.length, 1, "one server delivered"); const server = out[0] as McpServerHttp; @@ -68,39 +38,20 @@ describe("toAcpMcpServers (http enabled, stdio disabled)", () => { assert.deepEqual( server.headers, [{ name: "Authorization", value: "Bearer secret-token-value" }], - "env entry -> request header", + "resolved header is delivered to ACP", ); }); it("delivers an http server with no secrets as an empty header list", async () => { - const out = await toAcpMcpServers([ - { name: "public", transport: "http", url: "https://93.184.216.34/sse" }, - ]); + const out = await toAcpMcpServers(http("https://93.184.216.34/sse", {})); const server = out[0] as McpServerHttp; assert.equal(server.type, "http"); - assert.deepEqual(server.headers, [], "no env -> no headers"); - }); - - it("does not throw on a mix of http (delivered) and command-less stdio (skipped)", async () => { - const out = await toAcpMcpServers([ - { - name: "remote", - transport: "http", - url: "https://93.184.216.34/mcp", - env: { "X-Api-Key": "k" }, - }, - { name: "broken", transport: "stdio" }, // no command -> never launched, skipped - { name: "no-url", transport: "http" }, // no url -> never deliverable, skipped - ]); - - assert.equal(out.length, 1, "only the valid http server is delivered"); - assert.equal((out[0] as McpServerHttp).name, "remote"); + assert.deepEqual(server.headers, [], "no headers stays empty"); }); }); describe("toAcpMcpServers SSRF guard (http url scheme/host)", () => { const previousAllowlist = process.env.AGENTA_AGENT_MCPS_HOST_ALLOWLIST; - // Egress defaults permissive (unset -> allowed); this suite exercises the guard, so arm it. beforeEach(() => { process.env.AGENTA_INSECURE_EGRESS_ALLOWED = "false"; }); @@ -111,15 +62,6 @@ describe("toAcpMcpServers SSRF guard (http url scheme/host)", () => { else process.env.AGENTA_AGENT_MCPS_HOST_ALLOWLIST = previousAllowlist; }); - const http = (url: string): McpServerConfig[] => [ - { - name: "s", - transport: "http", - url, - env: { Authorization: "Bearer secret" }, - }, - ]; - it("rejects a non-https url (the secret would ride in clear text)", async () => { await assert.rejects( () => toAcpMcpServers(http("http://93.184.216.34/sse")), diff --git a/services/runner/tests/unit/sandbox-agent-orchestration.test.ts b/services/runner/tests/unit/sandbox-agent-orchestration.test.ts index 6b7b9b389f..6daf667df9 100644 --- a/services/runner/tests/unit/sandbox-agent-orchestration.test.ts +++ b/services/runner/tests/unit/sandbox-agent-orchestration.test.ts @@ -17,7 +17,6 @@ import { import { PendingApprovalPauseController } from "../../src/engines/sandbox_agent/pause.ts"; import { mountStorage } from "../../src/engines/sandbox_agent/mount.ts"; import { buildPiGateEnvelope } from "../../src/engines/sandbox_agent/pi-gate-envelope.ts"; -import { USER_MCP_UNSUPPORTED_MESSAGE } from "../../src/tools/mcp-bridge.ts"; import type { PermissionDecision } from "../../src/responder.ts"; import { runSandboxAgent, @@ -1062,29 +1061,6 @@ describe("runSandboxAgent orchestration", () => { assert.equal(calls.sandboxDestroyed, 1, "sandbox disposed in finally"); }); - it("still refuses a run carrying a USER stdio MCP server (user gate untouched)", async () => { - // The user-facing stdio MCP path stays disabled (parity with removed code execution); only - // the internal gateway-tool channel was restored. A user-declared stdio MCP server is still - // rejected up front with the user-facing message. - const { deps } = fakeHarness(); - - const result = await runSandboxAgent( - { - harness: "claude", - messages: [{ role: "user", content: "go" }], - mcpServers: [{ name: "github", transport: "stdio", command: "npx" }], - } as AgentRunRequest, - undefined, - undefined, - deps, - ); - - assert.deepEqual(result, { - ok: false, - error: USER_MCP_UNSUPPORTED_MESSAGE, - }); - }); - it("fails loud when a non-Pi harness probes mcpTools:false but the run carries tools", async () => { // A7: the capability gate runs BEFORE the MCP bridge, so a harness whose probe reports it // cannot receive tools fails with a SPECIFIC capability error (not the generic MCP-disabled diff --git a/services/runner/tests/unit/sandbox-agent-run-plan.test.ts b/services/runner/tests/unit/sandbox-agent-run-plan.test.ts index cb2bb516ef..212b5c37d2 100644 --- a/services/runner/tests/unit/sandbox-agent-run-plan.test.ts +++ b/services/runner/tests/unit/sandbox-agent-run-plan.test.ts @@ -7,7 +7,6 @@ import { afterEach, describe, it } from "vitest"; import assert from "node:assert/strict"; import type { AgentRunRequest } from "../../src/protocol.ts"; -import { USER_MCP_UNSUPPORTED_MESSAGE } from "../../src/tools/mcp-bridge.ts"; import { buildRunPlan, shouldUploadOwnLogin, @@ -458,24 +457,6 @@ describe("buildRunPlan", () => { assert.equal(result.ok, true); }); - it("errors on any run carrying a stdio MCP server (MCP disabled)", () => { - // The stdio MCP implementation is disabled in the sidecar; a stdio server errors the - // not-implemented way regardless of sandbox/enforcement, even with no network policy. - const result = buildRunPlan( - { - harness: "claude", - sandbox: "daytona", - messages: [{ role: "user", content: "hello" }], - mcpServers: [{ name: "fs", transport: "stdio", command: "mcp-fs" }], - } as AgentRunRequest, - { createDaytonaCwd: () => "/home/sandbox/agenta-fixed" }, - ); - - assert.equal(result.ok, false); - if (result.ok) return; - assert.equal(result.error, USER_MCP_UNSUPPORTED_MESSAGE); - }); - it("refuses a user MCP server that claims the reserved internal name 'agenta-tools'", () => { // The internal gateway-tool channel is keyed by name and claude_settings.py renders // permission rules against it; a user server with the name would collide/steal them. @@ -487,8 +468,8 @@ describe("buildRunPlan", () => { mcpServers: [ { name: "agenta-tools", - transport: "http", - url: "https://mcp.example.com/mcp", + connection: { type: "http", url: "https://mcp.example.com/mcp" }, + policy: { tools: { mode: "all" } }, }, ], } as AgentRunRequest, @@ -499,41 +480,6 @@ describe("buildRunPlan", () => { assert.equal(result.error, RESERVED_MCP_SERVER_NAME_MESSAGE); }); - it("errors on a stdio MCP server on the local sandbox too (non-Pi harness)", () => { - const result = buildRunPlan( - { - harness: "claude", - sandbox: "local", - messages: [{ role: "user", content: "hello" }], - mcpServers: [{ name: "fs", transport: "stdio", command: "mcp-fs" }], - } as AgentRunRequest, - { createLocalCwd: () => "/tmp/local-cwd" }, - ); - - assert.equal(result.ok, false); - if (result.ok) return; - assert.equal(result.error, USER_MCP_UNSUPPORTED_MESSAGE); - }); - - it("errors LOUD on a Pi run carrying a user STDIO MCP server (F-032, Pi-specific)", () => { - // Pi delivers tools through its bundled extension, not MCP, so a user MCP server would be - // dropped silently (the F-032 bug). The Pi gate refuses it up front with a Pi-specific - // message (precedes the harness-agnostic stdio gate). - const result = buildRunPlan( - { - harness: "pi_core", - sandbox: "local", - messages: [{ role: "user", content: "hello" }], - mcpServers: [{ name: "fs", transport: "stdio", command: "mcp-fs" }], - } as AgentRunRequest, - { createLocalCwd: () => "/tmp/local-cwd" }, - ); - - assert.equal(result.ok, false); - if (result.ok) return; - assert.match(result.error, /not supported on the Pi harness/); - }); - it("errors LOUD on a Pi run carrying a user HTTP MCP server (F-032 silent-drop fix)", () => { // The core of F-032: an http user MCP on Pi previously passed the stdio gate, then // buildSessionMcpServers returned [] for Pi -> the server was dropped with NO log and an @@ -547,8 +493,8 @@ describe("buildRunPlan", () => { mcpServers: [ { name: "linear", - transport: "http", - url: "https://mcp.linear.app/sse", + connection: { type: "http", url: "https://mcp.linear.app/sse" }, + policy: { tools: { mode: "all" } }, }, ], } as AgentRunRequest, @@ -579,8 +525,8 @@ describe("buildRunPlan", () => { mcpServers: [ { name: "linear", - transport: "http", - url: "https://mcp.linear.app/sse", + connection: { type: "http", url: "https://mcp.linear.app/sse" }, + policy: { tools: { mode: "all" } }, }, ], } as AgentRunRequest, @@ -860,7 +806,11 @@ describe("buildRunPlan", () => { sandbox: "daytona", messages: [{ role: "user", content: "hello" }], mcpServers: [ - { name: "remote", transport: "http", url: "https://mcp.example" }, + { + name: "remote", + connection: { type: "http", url: "https://mcp.example" }, + policy: { tools: { mode: "all" } }, + }, ], sandboxPermission: { network: { mode: "off" }, diff --git a/services/runner/tests/unit/session-mcp-layering.test.ts b/services/runner/tests/unit/session-mcp-layering.test.ts index b2cba4867b..211092e4a0 100644 --- a/services/runner/tests/unit/session-mcp-layering.test.ts +++ b/services/runner/tests/unit/session-mcp-layering.test.ts @@ -2,16 +2,13 @@ * Layering regression guard for buildSessionMcpServers — the test that FAILS if the internal * gateway-tool channel and the user MCP capability are ever re-merged into one gate. * - * PR #4831 conflated two independent things into a single `MCP_UNSUPPORTED_MESSAGE` switch, which - * disabled gateway-tool delivery to Claude as collateral with the (correct) user stdio MCP - * disable. The fix (project gateway-tool-mcp) split them into: + * The gateway-tool repair split two independent things into: * 1. INTERNAL gateway-tool channel — restored over loopback HTTP MCP; toggles on executable tools. - * 2. USER MCP capability — stdio DISABLED, http delivered (#4834). + * 2. USER MCP capability — external HTTP delivered separately. * - * These three cases pin that the two layers toggle independently: + * These cases pin that the two layers toggle independently: * (a) gateway tools + NO user MCP -> internal channel present, no throw - * (b) user stdio MCP + NO gateway tools -> refused (user gate) - * (c) gateway tools + user http MCP -> BOTH delivered; user stdio still refused + * (b) gateway tools + user HTTP MCP -> both delivered * * Plus: Pi gets [] (native delivery), and the channel never carries a credential. * @@ -27,7 +24,6 @@ import { type SessionMcpServers, } from "../../src/engines/sandbox_agent/mcp.ts"; import type { ToolMcpAssets } from "../../src/engines/sandbox_agent/tool-mcp-assets.ts"; -import { USER_MCP_UNSUPPORTED_MESSAGE } from "../../src/tools/mcp-bridge.ts"; import type { HarnessCapabilities, McpServerConfig, @@ -79,27 +75,7 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = ); }); - it("(b) user stdio MCP + no gateway tools -> refused (user gate untouched)", async () => { - const userMcpServers: McpServerConfig[] = [ - { name: "github", transport: "stdio", command: "npx", args: ["x"] }, - ]; - await assert.rejects( - () => - buildSessionMcpServers({ - isPi: false, - isDaytona: false, - capabilities: mcpCapable, - harness: "claude", - toolSpecs: [], - userMcpServers, - relayDir, - }), - new RegExp(USER_MCP_UNSUPPORTED_MESSAGE), - "a user stdio MCP server is still refused", - ); - }); - - it("(c) gateway tools + user http MCP -> BOTH delivered; user stdio still refused", async () => { + it("(b) gateway tools + user HTTP MCP -> both delivered", async () => { const { servers } = await build({ isPi: false, isDaytona: false, @@ -109,9 +85,12 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = userMcpServers: [ { name: "linear", - transport: "http", - url: "https://mcp.linear.app/sse", - env: { Authorization: "Bearer x" }, + connection: { + type: "http", + url: "https://mcp.linear.app/sse", + headers: { Authorization: "Bearer x" }, + }, + policy: { tools: { mode: "all" } }, }, ], relayDir, @@ -124,20 +103,6 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = const names = servers.map((s) => s.name).sort(); assert.deepEqual(names, ["agenta-tools", "linear"]); - // The user stdio path is still refused even alongside a gateway tool. - await assert.rejects( - () => - buildSessionMcpServers({ - isPi: false, - isDaytona: false, - capabilities: mcpCapable, - harness: "claude", - toolSpecs: [gatewayTool], - userMcpServers: [{ name: "evil", transport: "stdio", command: "rm" }], - relayDir, - }), - new RegExp(USER_MCP_UNSUPPORTED_MESSAGE), - ); }); it("Pi gets [] (native delivery, no MCP channel) even with gateway tools", async () => { @@ -239,10 +204,9 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = ); }); - it("(Daytona, non-Pi, internalToolMcp) one run: user stdio refused, user http delivered, internal stdio entry present", async () => { - // The slice-1 layering pin, all three layers on ONE Daytona run: the uploaded in-sandbox - // shim becomes the internal TYPELESS stdio entry; a user http MCP rides along unchanged; - // a user stdio MCP is still refused. The internal entry carries ONLY the two env names — + it("(Daytona, non-Pi, internalToolMcp) delivers user HTTP and the internal stdio entry", async () => { + // The uploaded in-sandbox shim becomes the internal TYPELESS stdio entry and a user HTTP MCP + // rides along unchanged. The internal entry carries only the two env names: // no credential, no callRef, no loopback URL. The response-watch flag is cleared for the // duration (buildInternalToolMcpEntry forwards it verbatim when the runner env sets it, // which would add a third env entry; the with-flag shape has its own test below). @@ -259,8 +223,8 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = // shape is owned by another seam. const userHttp: McpServerConfig = { name: "linear", - transport: "http", - url: "https://mcp.linear.app/sse", + connection: { type: "http", url: "https://mcp.linear.app/sse" }, + policy: { tools: { mode: "all" } }, }; const { servers } = await build({ isPi: false, @@ -307,23 +271,6 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = ); assert.ok(!serialized.includes("127.0.0.1"), "no loopback URL"); - // The user stdio path is still refused on the same Daytona + internalToolMcp shape. - await assert.rejects( - () => - buildSessionMcpServers({ - isPi: false, - isDaytona: true, - capabilities: mcpCapable, - harness: "claude", - toolSpecs: [gatewayTool], - userMcpServers: [ - { name: "evil", transport: "stdio", command: "rm" }, - ], - relayDir, - internalToolMcp, - }), - new RegExp(USER_MCP_UNSUPPORTED_MESSAGE), - ); } finally { if (savedResponseWatch === undefined) { delete process.env.AGENTA_AGENT_TOOLS_RELAY_RESPONSE_WATCH_ENABLED; @@ -443,8 +390,8 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = userMcpServers: [ { name: "agenta-tools", - transport: "http", - url: "https://mcp.example.com/mcp", + connection: { type: "http", url: "https://mcp.example.com/mcp" }, + policy: { tools: { mode: "all" } }, }, ], relayDir, @@ -466,9 +413,12 @@ describe("buildSessionMcpServers layering (do-not-merge regression guard)", () = userMcpServers: [ { name: "linear", - transport: "http", - url: "https://mcp.linear.app/sse", - env: { Authorization: "Bearer x" }, + connection: { + type: "http", + url: "https://mcp.linear.app/sse", + headers: { Authorization: "Bearer x" }, + }, + policy: { tools: { mode: "all" } }, }, ], relayDir, diff --git a/services/runner/tests/unit/tool-dispatch.test.ts b/services/runner/tests/unit/tool-dispatch.test.ts index db378987fc..13182c9811 100644 --- a/services/runner/tests/unit/tool-dispatch.test.ts +++ b/services/runner/tests/unit/tool-dispatch.test.ts @@ -2,8 +2,9 @@ * Unit tests for the shared tool-dispatch module (tools/dispatch.ts) and its routing. * * The kind-dispatch ("branch on spec.kind to execute a resolved tool") lives once in - * `runResolvedTool`, used by the Pi extension (extensions/agenta.ts) and the MCP server - * (tools/mcp-server.ts). These tests cover the routing into that function and the file relay: + * `runResolvedTool`, used by the Pi extension (extensions/agenta.ts) and the internal MCP tool + * servers (tools/tool-mcp-http.ts and tools/tool-mcp-stdio.ts). These tests cover the routing + * into that function and the file relay: * - runResolvedTool advertises code tools but fails their sidecar execution, and relays `client`. * - relayToolCall reads back the relayed result from the Daytona file relay. * diff --git a/web/packages/agenta-entities/src/workflow/state/inspectMeta.ts b/web/packages/agenta-entities/src/workflow/state/inspectMeta.ts index 0b2f010c18..19e0a13a7c 100644 --- a/web/packages/agenta-entities/src/workflow/state/inspectMeta.ts +++ b/web/packages/agenta-entities/src/workflow/state/inspectMeta.ts @@ -73,6 +73,13 @@ export interface HarnessCapabilities { * and falls back to `models`. Absent on an older backend. */ model_catalog?: ModelCatalogEntry[] + /** External MCP servers the selected harness can consume. */ + mcp?: { + user_servers?: { + connection_types: string[] + credentials: string[] + } | null + } | null } /** The full per-harness capability map (harness type -> capabilities). */ diff --git a/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx b/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx index e967bf260d..7066267dbb 100644 --- a/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx +++ b/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx @@ -89,7 +89,7 @@ import {WorkflowReferenceSelector} from "./WorkflowReferenceSelector" // Tooltip copy for the config-panel draft/validation indicators. const INVALID_ITEM_TIP: Record = { tool: "This tool is missing its name.", - mcp: "This server is missing a required field (name, command, or URL).", + mcp: "This server is missing its name or URL.", skill: "This skill is missing its name.", } const DRAFT_TIP: Record = { @@ -376,7 +376,7 @@ export function AgentTemplateControl({ referenceableWorkflows, } = useAgentTools({config, onChange, configRef, openCreate, workflowReference}) - // MCP servers: a flat array of McpServer shapes (stdio command/args/env or remote url + secrets). + // External HTTP MCP servers from the saved agent template. const mcpServers = useMemo( () => (Array.isArray(config.mcps) ? (config.mcps as unknown[]) : []), [config.mcps], @@ -429,7 +429,7 @@ export function AgentTemplateControl({ const hasInstructions = Boolean(props.instructions) const hasTools = Boolean(props.tools) - const hasMcp = Boolean(props.mcps) + const hasMcp = Boolean(props.mcps) && mh.mcpSupported const hasSkills = Boolean(props.skills) // Per-field section headers read their label from the template schema (`props..title`), @@ -633,7 +633,7 @@ export function AgentTemplateControl({ } if (key === "mcp") return mcpServers.some((m) => ITEM_KINDS.mcp.draftInvalid(m as Record)) - ? "An MCP server is missing a required field." + ? "An MCP server is missing its name or URL." : null if (key === "skills") return skills.some((s) => ITEM_KINDS.skill.draftInvalid(s as Record)) diff --git a/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/McpServerFormView.tsx b/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/McpServerFormView.tsx index fd7007aa9d..841ce9fef5 100644 --- a/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/McpServerFormView.tsx +++ b/web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/McpServerFormView.tsx @@ -1,14 +1,5 @@ /** - * McpServerFormView - * - * Structured form view for an MCP server, the Form side of {@link ConfigItemDrawer}. Mirrors - * the MCPServerConfig shape (sdks/python/agenta/sdk/agents/mcp/models.py): name, transport - * (stdio command/args/env, or http url), vault secret-name map, and an optional exposed-tool - * allowlist. It edits only those known keys so the object stays valid (the backend model is - * `extra="forbid"`). - * - * Re-mount it per server (key on the open item) so the local key/value text state seeds - * cleanly. + * Structured editor for one external HTTP MCP server. */ import {useState} from "react" @@ -16,19 +7,25 @@ import {Input, Select} from "antd" import {RailField, railInfoLabel} from "../../drawers/shared/RailField" -import {CodeEditor} from "./CodeEditor" - type Dict = Record +type CredentialType = "none" | "header_secret_refs" + +interface McpCredentials { + type?: CredentialType + headers?: Dict +} + +interface McpConnection { + type?: "http" + url?: string + headers?: Dict + credentials?: McpCredentials +} interface McpServer { name?: string - transport?: "stdio" | "http" - command?: string - args?: string[] - env?: Dict - url?: string - secrets?: Dict - tools?: string[] + connection?: McpConnection + policy?: Record } export interface McpServerFormViewProps { @@ -37,7 +34,6 @@ export interface McpServerFormViewProps { disabled?: boolean } -/** `KEY=value` per line ↔ a string map. Local text state avoids cursor jumps on edit. */ function KeyValueLines({ value, onChange, @@ -51,27 +47,26 @@ function KeyValueLines({ }) { const [text, setText] = useState(() => Object.entries(value ?? {}) - .map(([k, v]) => `${k}=${v}`) + .map(([key, entryValue]) => `${key}=${entryValue}`) .join("\n"), ) const handle = (next: string) => { setText(next) - const obj: Dict = {} + const entries: Dict = {} next.split("\n").forEach((line) => { - const i = line.indexOf("=") - if (i > 0) { - const k = line.slice(0, i).trim() - if (k) obj[k] = line.slice(i + 1).trim() - } + const separator = line.indexOf("=") + if (separator <= 0) return + const key = line.slice(0, separator).trim() + if (key) entries[key] = line.slice(separator + 1).trim() }) - onChange(obj) + onChange(entries) } return ( handle(e.target.value)} + onChange={(event) => handle(event.target.value)} placeholder={placeholder} disabled={disabled} autoSize={{minRows: 2, maxRows: 6}} @@ -81,17 +76,36 @@ function KeyValueLines({ } export function McpServerFormView({value, onChange, disabled}: McpServerFormViewProps) { - const server = (value ?? {}) as McpServer - const transport = server.transport ?? "stdio" - - const set = (key: keyof McpServer, fieldValue: unknown) => { - const next = {...(value ?? {})} - if (fieldValue === undefined || fieldValue === null || fieldValue === "") { - delete (next as Record)[key] - } else { - ;(next as Record)[key] = fieldValue - } - onChange(next) + const server = value as McpServer + const connection = server.connection ?? { + type: "http" as const, + url: "", + credentials: {type: "none" as const}, + } + const credentials = connection.credentials ?? {type: "none" as const} + const credentialType = credentials.type ?? "none" + + const setServer = (patch: Partial) => { + onChange({...value, ...patch}) + } + + const setConnection = (patch: Partial) => { + setServer({ + connection: { + ...connection, + type: "http", + ...patch, + }, + }) + } + + const setCredentialType = (type: CredentialType) => { + setConnection({ + credentials: + type === "none" + ? {type: "none"} + : {type: "header_secret_refs", headers: credentials.headers ?? {}}, + }) } return ( @@ -99,102 +113,64 @@ export function McpServerFormView({value, onChange, disabled}: McpServerFormView set("name", e.target.value)} - placeholder="my-mcp-server" - disabled={disabled} - /> - - - - set("args", v)} - placeholder="one argument per token" - disabled={disabled} - open={false} - suffixIcon={null} - /> - - - ) : ( - - set("url", e.target.value)} - placeholder="https://example.com/mcp" - disabled={disabled} - /> - - )} - - - set("env", Object.keys(env).length ? env : undefined)} - placeholder={"NODE_ENV=production"} + + setConnection({url: event.target.value})} + placeholder="https://example.com/mcp" disabled={disabled} /> - + - set("secrets", Object.keys(secrets).length ? secrets : undefined) + value={connection.headers} + onChange={(headers) => + setConnection({headers: Object.keys(headers).length ? headers : undefined}) } - placeholder={"API_KEY=my_api_key"} + placeholder="X-Workspace=my-workspace" disabled={disabled} /> - -