feat(claude-agent-sdk): support workflow MCP servers (#335) - #346
Merged
Conversation
The provider declared `mcp_tools=False` and the factory rejected any workflow declaring `runtime.mcp_servers`, so authors had to choose between the built-in `claude_code` tool preset and their own MCP tool servers. That was an unimplemented gap rather than an upstream limitation: the SDK has supported `ClaudeAgentOptions.mcp_servers` since well before the pinned version, and its config shapes map almost 1:1 onto Conductor's MCPServerDef. Translate each server into the SDK's stdio/http/sse config shapes and pass them through. Translation runs in `__init__` so a bad config fails at the factory boundary rather than mid-workflow. Two details are load-bearing: - The config is written to a 0600 temp file and passed by path. Passing the dict would make the SDK serialize it into a `--mcp-config <json>` argv element, publishing resolved stdio `env` values and http/sse `Authorization` headers to anything that can read /proc/<pid>/cmdline. Cleanup hangs off execute()'s `finally`, which also covers the early partial-output returns on interrupt. - `strict_mcp_config=True` keeps an ambient project `.mcp.json` or user-global setting from injecting servers the workflow never declared. This option is why the floor pin moves to >=0.2.82. A narrowing per-server `tools:` filter has no SDK equivalent and is refused rather than ignored -- forwarding the server unfiltered would grant more tools than declared, the same reasoning that justifies refusing the per-agent allowlist. A dropped `timeout` only warns, since losing it cannot widen tool access. Per-agent `tools:` allowlists remain unsupported. The SDK's `tools` option covers built-in tools only and `allowed_tools` is a permission auto-approve list rather than an availability filter, so honoring an allowlist needs a permission-mode redesign, not just a name mapping. Also fixes an over-broad validator check that rejected `tools: []` against any provider with mcp_tools=True and workflow_tools_passthrough=False even when the workflow declared no mcp_servers and had nothing to forward. This affected `aca` already, and would have broken examples/experimental-claude-agent-sdk.yaml once mcp_tools flipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…laims Review follow-ups on #335. The MCP config file was written before `execute`'s `try`, so anything raising in between -- `_build_output_format` on a deeply nested `output:` schema, for instance -- left a file containing resolved MCP credentials on disk, once per retry attempt. Reproduced, then moved the write inside the `try` so the existing `finally` reclaims it on every exit path. Added a regression test that snapshots the temp dir. `strict_mcp_config` was only set when the workflow declared servers, so the majority case -- no `mcp_servers:` -- still inherited project `.mcp.json`, user-global, and plugin-provided servers, with `permission_mode` bypassing approval for whatever they exposed. It is now set unconditionally, which makes the documented guarantee true and the validator relaxation sound. This is a behavior change for workflows relying on ambient Claude Code MCP settings; noted in the changelog. Also from review: - Close the SDK iterator before deleting its config file. The interrupt path abandons the generator, deferring subprocess teardown to the GC; on Windows, unlinking a file the live subprocess holds open raises PermissionError. - Raise `_remove_mcp_config`'s failure log from DEBUG to WARNING. Conductor installs no logging handlers, so DEBUG reached nobody -- and the file holds credentials the user must delete by hand. - Missing `command`/`url` now raise `ProviderError` instead of a bare `KeyError` (`create_provider` is public API). Config errors pass `is_retryable=False` explicitly so user-controlled server names cannot trip the message-sniffing heuristic. - Close the fd if `os.fdopen` itself raises. - Delete `_mcp_config_file`: it had no production caller, so three cleanup tests were exercising a wrapper `execute` never entered. Tests now use the primitives `execute` actually calls. Corrected claims that were false as written: - ">=0.2.82 is the earliest release exposing strict_mcp_config" -- it is present in 0.1.74. The pin stays at >=0.2.82 (the tested line) with an honest justification. - "Fails at the factory boundary rather than mid-workflow" -- providers are constructed lazily, so a bad config surfaces on the first agent that uses the provider, and `conductor validate` does not catch it at all. - Descriptor comment claiming `tools: []` "disables all tools" -- it disables built-ins only; declared MCP servers still attach. - README, comparison.md, aca.md, and two example pins still documented MCP as rejected at the factory. - OAuth auto-auth marked unsupported, though auth resolves in cli/run.py before any provider sees the config. New coverage: the pre-`try` leak, secret-in-file-but-not-in-options, concurrent executions getting independent files, `tools: []` still attaching MCP servers, the `for_each` mirrors of both new validator cases, and a real-capabilities cross-check class for claude-agent-sdk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jrob5756
marked this pull request as ready for review
July 29, 2026 20:13
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #335.
What
The
claude-agent-sdkprovider declaredmcp_tools=Falseand the factory rejected any workflow declaringruntime.mcp_servers, so authors had to choose between the built-inclaude_codetool preset and their own MCP tool servers.That was an unimplemented gap, not an upstream limitation.
ClaudeAgentOptions.mcp_serversexists at the current lock (0.2.87) and as far back as the old floor pin (0.1.64), andMcpStdioServerConfig/McpSSEServerConfig/McpHttpServerConfigmap almost 1:1 onto Conductor'sMCPServerDef.How
_translate_mcp_servers()maps each resolved server config onto the SDK's shapes;execute()passes them viaClaudeAgentOptions.Secrets go in a file, not argv. The SDK serializes a
mcp_serversdict straight into a--mcp-config <json>command-line argument, which would publish resolved stdioenvvalues and http/sseAuthorizationheaders to anything that can read/proc/<pid>/cmdline. Neithercopilotnorclaudeexposes secrets that way. The config is written to a0600temp file and passed by path. The write happens insideexecute'stry, so thefinallyreclaims it on every exit path; thefinallyalso closes the SDK iterator first, so theclaudesubprocess is gone before its config file is.A narrowing per-server
tools:filter is refused, not ignored. The SDK's MCP config has no equivalent field, and forwarding the server unfiltered would grant more tools than declared - the same reasoning that already justifies refusing the per-agent allowlist.Behavior change: ambient MCP config is no longer inherited
Conductor now sets
strict_mcp_configunconditionally, including for workflows that declare nomcp_servers. Without this, theclaudeCLI still loads project.mcp.json, user-global settings, and plugin-provided servers - andpermission_mode="bypassPermissions"auto-approves whatever they expose. Only servers declared inruntime.mcp_serversnow attach.Workflows that relied on Claude Code's own MCP settings must declare those servers in the workflow. Called out in the CHANGELOG.
This also makes the validator relaxation below sound: it permits
tools: []when no MCP servers are declared, on the premise that nothing attaches - which ambient config would otherwise falsify.Out of scope
Per-agent
tools:allowlists (workflow_tools_passthrough) stayFalse. The SDK'stoolsoption governs built-in tools only, andallowed_toolsis a permission auto-approve list rather than an availability filter - withbypassPermissionsit is a no-op. Honoring an allowlist needs a permission-mode redesign, not just a name mapping. Follow-up issue.Same for
working_dir, whose in-code justification citedmcp_tools=Falseand had to be rewritten here.Note the refusals fire when the first agent on this provider runs, not at
conductor validate- providers are constructed lazily, and the validator does not inspect per-servertools:filters. Documented as such rather than overclaimed; adding a validator check would need a new capability field.Drive-by fix
config/validator.pyrejectedtools: []against any provider withmcp_tools=Trueandworkflow_tools_passthrough=False, keyed off the capability rather than whether the workflow actually declared anymcp_servers- so it rejected an empty allowlist even when there was nothing to forward. This already affectedaca, and would have brokenexamples/experimental-claude-agent-sdk.yamlthe momentmcp_toolsflipped. Separate CHANGELOG entry.Review round
The second commit is review follow-up. Most significant: the config file was originally written before
execute'stry, so an ordinary deepoutput:schema leaked a credential-bearing file - once per retry attempt. Reproduced, fixed, and pinned with a regression test.Also corrected several claims that were false as written: the
>=0.2.82pin justification (strict_mcp_configactually landed in 0.1.74 - the pin stays, the reasoning is now honest), "fails at the factory boundary", thetools: []descriptor comment, and stale MCP-is-rejected text in README / comparison.md / aca.md / two example pins.Verification
make checkclean; full suite 4565 passed;make validate-examplesclean.claudeCLI: mode0600, accepted with--strict-mcp-config, no secret leakage.{"mcpServers": {...}}envelope - a bare mapping is rejected withmcpServers: Invalid input: expected record, received undefined.Note for reviewers: two
ty: ignore[unresolved-import]comments inclaude_agent_sdk.pylook unused if you install theclaude-agent-sdkextra locally. They are required - CI typechecks without the extra (ci.yml:52/78).New example
examples/claude-agent-sdk-mcp.yaml- the CLI preset and a custom MCP server in one agent.