feat: Add custom MCP server support#150
Conversation
📝 WalkthroughWalkthroughAdds runtime support for user-configurable "Custom MCP Servers" with org- and project-level config loading/validation/merging, pattern-based blocked tools, variable-substitution helpers, client integration to register custom servers/tools, example configs, tests, and documentation. Duplicate MCP-related definitions were introduced. Changes
Sequence DiagramsequenceDiagram
participant Client as create_client()
participant OrgCfg as Org Config
participant ProjCfg as Project Config
participant Security as Security Module
participant Subst as Substitution Helpers
participant MCPMap as Runtime MCP Map
Client->>OrgCfg: read org config (mcp_servers, blocked_mcp_tools)
Client->>ProjCfg: read project config (mcp_servers)
Client->>Security: get_effective_mcp_servers(project_dir)
Security->>Security: validate & merge org + project servers
Security-->>Client: merged servers[], blocked_tools set
Client->>Security: get_effective_mcp_tools(servers, blocked_tools)
Security->>Security: expand patterns → allowed tool names & permissions
Security-->>Client: allowed_tools[], permissions
Client->>Subst: substitute_variables_in_dict(command/args/env) for each custom server
Subst-->>Client: substituted server configs
Client->>MCPMap: register substituted custom MCP servers
MCPMap-->>Client: updated runtime MCP servers list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
derhally
force-pushed
the
zd/extra-tools
branch
from
bcf082a to
74883a4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@client.py`:
- Around line 369-373: The reported issue is a 3-space indentation on the line
that calls get_effective_mcp_servers; change the indentation to standard 4
spaces so the block aligns with surrounding code (ensure the lines initializing
user_mcp_servers, blocked_mcp_tools and the subsequent call to
get_effective_mcp_tools all use 4-space indentation). Locate the lines with
user_mcp_servers, blocked_mcp_tools and user_mcp_tools/user_mcp_permissions and
adjust whitespace to 4 spaces to conform to Python style.
- Around line 2-3: The module docstring at the top of client.py contains a
duplicated line ("Claude SDK Client Configuration")—remove the redundant second
occurrence so the file retains a single top-level docstring line; locate the
duplicate in the module docstring and delete the extra line, leaving one clear
docstring entry for the module.
- Line 350: Remove the trailing whitespace on the blank line reported in
client.py (around line 350) so the line is a true empty line; edit the file to
delete any spaces or tabs on that blank line and re-run the linter (ruff) to
confirm the pipeline error is resolved.
🧹 Nitpick comments (2)
CLAUDE.md (1)
377-381: Add language specifier to fenced code block.The example output code block is missing a language specifier. For consistency and to satisfy markdownlint (MD040), add a language hint.
📝 Suggested fix
-``` +```text Created security settings at /path/to/project/.claude_settings.json - MCP servers: playwright (browser), features (database), filesystem (custom) - Blocked MCP tools (org): filesystem__write_file, filesystem__delete_file</details> </blockquote></details> <details> <summary>security.py (1)</summary><blockquote> `19-26`: **Consider using `Required[]` for mandatory TypedDict fields.** The `MCPServerConfig` TypedDict uses `total=False` which makes all fields optional at the type level, but the docstring indicates `name`, `command`, and `allowed_tools` are required. For better type safety, consider using `Required[]` (Python 3.11+) for mandatory fields. <details> <summary>♻️ Suggested improvement</summary> ```diff -from typing import Any, Optional, TypedDict +from typing import Any, Optional, Required, TypedDict -class MCPServerConfig(TypedDict, total=False): +class MCPServerConfig(TypedDict, total=False): """Configuration for a custom MCP server.""" - name: str # Required: unique identifier for the server - command: str # Required: command to run (e.g., "npx", "python") + name: Required[str] # Required: unique identifier for the server + command: Required[str] # Required: command to run (e.g., "npx", "python") args: list[str] # Optional: command arguments env: dict[str, str] # Optional: environment variables - allowed_tools: list[str] # Required: explicit list of tools to allow + allowed_tools: Required[list[str]] # Required: explicit list of tools to allow
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client.py (1)
369-418:⚠️ Potential issue | 🟠 MajorPrevent custom configs from expanding built‑in MCP tool access.
get_effective_mcp_toolsruns before built‑in name conflicts are filtered, so a user can define a server named"features"/"playwright"and inject extra tool names intoallowed_toolsandpermissions_list, bypassing the agent‑type tool restrictions even though the server is later skipped. Filter out reserved names before computing tools, or recompute after conflict handling.✅ Suggested fix (filter before computing tools)
# Load user-configured MCP servers from org and project configs user_mcp_servers, blocked_mcp_tools = get_effective_mcp_servers(project_dir) + reserved_server_names = {"features"} + if not yolo_mode: + reserved_server_names.add("playwright") + user_mcp_servers_for_tools = [ + s for s in user_mcp_servers if s["name"] not in reserved_server_names + ] user_mcp_tools, user_mcp_permissions = get_effective_mcp_tools( - user_mcp_servers, blocked_mcp_tools + user_mcp_servers_for_tools, blocked_mcp_tools )
🤖 Fix all issues with AI agents
In `@client.py`:
- Around line 533-538: Update the comment to "# Add env with variable
substitution (defensive merge with parent)"; then defensively merge the
substituted env into the existing parent env instead of replacing it — call
substitute_variables_in_dict(server_config["env"], project_dir) to get
substituted, obtain the parent env from server_entry.get("env",
os.environ.copy()) (import os if not already imported), update a copy of that
parent with the substituted values (so substituted values override), and assign
the merged dict back to server_entry["env"]; reference server_config,
server_entry, substitute_variables_in_dict, and project_dir when making the
change.
| # Add env with variable substitution, merging with parent environment | ||
| if "env" in server_config: | ||
| server_entry["env"] = substitute_variables_in_dict( | ||
| server_config["env"], project_dir | ||
| ) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine client.py to understand the context
fd -t f "client.py" | head -20Repository: leonvanzyl/autocoder
Length of output: 88
🏁 Script executed:
# Once found, let's examine the relevant section with more context
cat -n client.py | sed -n '520,550p'Repository: leonvanzyl/autocoder
Length of output: 1468
🌐 Web query:
Claude Agent SDK MCP environment variables merge parent environment
💡 Result:
In the Claude Agent SDK, the env objects you set:
- at the query level (
options.env, e.g.ENABLE_TOOL_SEARCH) and - per MCP stdio server (
options.mcpServers.<name>.env, e.g.GITHUB_TOKEN)
are intended to be additive overrides on top of the runtime’s existing environment (so you generally don’t need to restate things like PATH; you just set/override the specific variables you need). The Agent SDK docs show using env specifically to pass credentials and feature flags this way. [1]
Notes / gotchas:
- Claude Code may still adjust some variables (people have reported extra path segments being prepended rather than the environment being replaced wholesale). [2]
- In some Claude Desktop/Claude Code integration scenarios, there have been bugs where variables from an
envsection were not passed through to MCP servers as expected. [3]
Sources:
[1] Anthropic “Connect to external tools with MCP” (Agent SDK) docs https://platform.claude.com/docs/en/agent-sdk/mcp
[2] anthropics/claude-code issue #5202 (env vars like PATH/PYTHONPATH not preserved exactly) anthropics/claude-code#5202
[3] anthropics/claude-code issue #1254 (env vars in config not passed to MCP servers) anthropics/claude-code#1254
🌐 Web query:
MCP Model Context Protocol environment configuration SDK implementation
💡 Result:
MCP environment configuration (what to implement)
1) Configure MCP servers + per-server environment (.mcp.json)
In Claude/Agent SDK flows, MCP servers are commonly declared in a project-root .mcp.json, and each server can receive an env object (passed to the MCP server process). Example shown in the Claude docs: env: { "ALLOWED_PATHS": "..." }. [1]
2) Allow shared configs with environment-variable expansion
If you want portable configs (no secrets committed), implement/expect ${VAR} and ${VAR:-default} expansion in .mcp.json fields including command, args, env, url, and headers (Claude Code supports this). [2]
3) SDK-side “in code” configuration (instead of a file)
Claude’s Agent SDK allows passing MCP server definitions directly via an options.mcpServers object when calling query(). [1]
4) Application environment loading (Node/TS example)
A typical Node implementation loads environment variables (e.g., from .env) before constructing clients/transports (the MCP Node client tutorial uses dotenv.config() and reads ANTHROPIC_API_KEY from .env). [3]
Minimal reference implementations
A) .mcp.json (stdio server with env)
{
"mcpServers": {
"filesystem": {
"command": "npx",
"args": ["-y", "@modelcontextprotocol/server-filesystem"],
"env": { "ALLOWED_PATHS": "/Users/me/projects" }
}
}
}[1]
B) .mcp.json with env-var expansion (remote HTTP/SSE auth)
{
"mcpServers": {
"api-server": {
"type": "http",
"url": "${API_BASE_URL:-https://api.example.com}/mcp",
"headers": { "Authorization": "Bearer ${API_KEY}" }
}
}
}[2]
C) Node client setup (dotenv + MCP TS SDK transport)
import dotenv from "dotenv";
dotenv.config(); // loads ANTHROPIC_API_KEY from .env[3]
SDKs you can base your implementation on
- Official MCP Swift SDK (client + server, transports, spec-versioned). [4]
Sources: [1] Claude Agent SDK “MCP in the SDK” docs, [2] Claude Code MCP docs (env expansion), [3] modelcontextprotocol.io Node client tutorial, [4] modelcontextprotocol/swift-sdk README.
🏁 Script executed:
# Search for where mcp_servers is used after construction
cat -n client.py | sed -n '539,600p'Repository: leonvanzyl/autocoder
Length of output: 3494
🏁 Script executed:
# Look for SDK initialization and how mcp_servers is passed
rg -n "mcp_servers" client.py -A 3 -B 3Repository: leonvanzyl/autocoder
Length of output: 2911
🏁 Script executed:
# Check if there are any imports or SDK references that show how env is handled
head -100 client.py | cat -nRepository: leonvanzyl/autocoder
Length of output: 4333
Update comment accuracy and apply defensive env merge to handle known SDK issues.
The comment at line 533 claims "merging with parent environment" but the code only performs variable substitution without actually merging. The Claude Agent SDK is documented to treat env as additive overrides, however there are known issues in the SDK (#5202, #1254) where environment variables from MCP server configs are not correctly passed through. To ensure custom servers receive required variables like PATH, apply the defensive merge:
Fix: Explicitly merge env with parent
if "env" in server_config:
- server_entry["env"] = substitute_variables_in_dict(
- server_config["env"], project_dir
- )
+ server_entry["env"] = {
+ **os.environ,
+ **substitute_variables_in_dict(server_config["env"], project_dir),
+ }Also update the comment on line 533 to: # Add env with variable substitution (defensive merge with parent)
🤖 Prompt for AI Agents
In `@client.py` around lines 533 - 538, Update the comment to "# Add env with
variable substitution (defensive merge with parent)"; then defensively merge the
substituted env into the existing parent env instead of replacing it — call
substitute_variables_in_dict(server_config["env"], project_dir) to get
substituted, obtain the parent env from server_entry.get("env",
os.environ.copy()) (import os if not already imported), update a copy of that
parent with the substituted values (so substituted values override), and assign
the merged dict back to server_entry["env"]; reference server_config,
server_entry, substitute_variables_in_dict, and project_dir when making the
change.
There was a problem hiding this comment.
@leonvanzyl would you be ok with this suggest change?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client.py`:
- Around line 1-8: The module-level docstring in client.py is malformed: there's
an early closing triple-quote that breaks the header (the text including "Claude
SDK Client Configuration" and the separators should be inside one continuous
triple-quoted string). Fix by making a single valid triple-quoted module
docstring at the top of client.py containing the header text (remove the stray
closing """ and delete the duplicated separator line) so the entire
title/description is enclosed correctly.
Introduces the ability to configure additional MCP (Model Context Protocol) servers through organization and project configuration files. This feature extends the agent's capabilities by allowing users to define custom tools. Implements a robust configuration system that: - Loads and validates MCP server definitions and org-level blocked tools. - Supports variable substitution in commands, arguments, and environment variables. - Resolves configuration hierarchy, allowing project-level definitions to override organization-level ones. - Integrates custom tools into the agent's security context, enforcing explicit tool allowlists and organization-wide blocks. Updates `CLAUDE.md` with detailed documentation on configuring and managing custom MCP servers, including schema and key behaviors. Provides example configuration files.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
derhally
force-pushed
the
zd/extra-tools
branch
from
fc6d5fa to
0cd7177
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client.py`:
- Around line 367-371: The comment preceding the MCP server loading calls is
misaligned (tab + 3 spaces) and should match the indentation of the following
function calls (tab + 4 spaces); update the comment line above the calls to
get_effective_mcp_servers and get_effective_mcp_tools so it has one additional
space of indentation to align with those lines.
🧹 Nitpick comments (1)
CLAUDE.md (1)
424-428: Add language specification to fenced code block.The example output code block is missing a language specification, which triggers a markdownlint warning (MD040). Since this is plain text output, use
textorplaintext.-``` +```text Created security settings at /path/to/project/.claude_settings.json
AutoForgeAI#150) Previously, HarnessKernel's _record_tool_call_event() and _record_tool_result_event() truncated payloads over 4KB inline without creating artifact records, causing data loss. The EventRecorder correctly created artifacts, but the kernel's own truncation path did not. Changes: - Added _create_payload_artifact() helper to HarnessKernel that creates an Artifact record with content_inline storing the full payload - Modified _record_tool_call_event() to create artifact and set artifact_ref when truncating large payloads - Modified _record_tool_result_event() with the same fix - Truncated payloads now include _artifact_ref and _note fields indicating the full content is in the referenced artifact - Added hashlib import for SHA256 content hashing Tests: 31 new tests, 56 existing kernel tests pass (0 regressions) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Introduces the ability to configure additional MCP (Model Context Protocol) servers through organization and project configuration files. This feature extends the agent's capabilities by allowing users to define custom tools.
Implements a robust configuration system that:
Updates
CLAUDE.mdwith detailed documentation on configuring and managing custom MCP servers, including schema and key behaviors. Provides example configuration files.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.