feat(acp): register client-provided MCP servers as agent tools#4496
Closed
Jckwind wants to merge 3 commits into
Closed
feat(acp): register client-provided MCP servers as agent tools#4496Jckwind wants to merge 3 commits into
Jckwind wants to merge 3 commits into
Conversation
ACP clients pass MCP server definitions in session/new, load_session,
resume_session, and fork_session. Previously these were accepted but
silently ignored — the agent never connected to them.
This wires the mcp_servers parameter into the existing MCP registration
pipeline (tools/mcp_tool.py) so client-provided servers are connected,
their tools discovered, and the agent's tool surface refreshed before
the first prompt.
Changes:
tools/mcp_tool.py:
- Extract sanitize_mcp_name_component() to replace all non-[A-Za-z0-9_]
characters (fixes crash when server names contain / or other chars
that violate provider tool-name validation rules)
- Use it in _convert_mcp_schema, _sync_mcp_toolsets, _build_utility_schemas
- Extract register_mcp_servers(servers: dict) as a public API that takes
an explicit {name: config} map. discover_mcp_tools() becomes a thin
wrapper that loads config.yaml and calls register_mcp_servers()
acp_adapter/server.py:
- Add _register_session_mcp_servers() which converts ACP McpServerStdio /
McpServerHttp / McpServerSse objects to Hermes MCP config dicts,
registers them via asyncio.to_thread (avoids blocking the ACP event
loop), then rebuilds agent.tools, valid_tool_names, and invalidates
the cached system prompt
- Call it from new_session, load_session, resume_session, fork_session
Tested with Eden (theproxycompany.com) as ACP client — 5 MCP servers
(HTTP + stdio) registered successfully, 110 tools available to the agent.
…ate events The step_callback previously only forwarded tool names as strings, so build_tool_complete received result=None and ACP tool_call_update events had empty content/rawOutput. Now prev_tools carries dicts with both name and result by pairing each tool_call with its matching tool-role message via tool_call_id.
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
1 participant
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.
Summary
ACP clients pass MCP server definitions (stdio and HTTP/SSE) in
session/new,load_session,resume_session, andfork_session. Currently these are accepted but silently dropped — the agent never connects to them, so client-provided tools are unavailable.This wires the
mcp_serversparameter into the existing MCP registration pipeline so client-provided servers are connected, their tools discovered, and the agent's tool surface refreshed before the first prompt.Partial progress on #569 (ACP server mode).
How it works
mcpServersinsession/new(typed asMcpServerStdio/McpServerHttp/McpServerSse)_register_session_mcp_serversconverts them to Hermes MCP config dicts and callsregister_mcp_servers()viaasyncio.to_thread()(avoids blocking the ACP event loop)_discover_and_register_serverpipeline connects, discovers tools, and injects them intohermes-*umbrella toolsetstools,valid_tool_names, and cached system prompt are refreshed so the new tools are immediately availableIdempotent — already-connected servers (by name) are skipped.
Bug fix: tool name sanitization
_convert_mcp_schema,_sync_mcp_toolsets, and_build_utility_schemaseach independently sanitized-and.but missed/and other characters. Server names likeai.exa/exaproduce tool names containing/, which crashes Anthropic's API (tools.N.custom.name: String should match pattern '^[a-zA-Z0-9_-]{1,128}$').Extracted
sanitize_mcp_name_component()that replaces all non-[A-Za-z0-9_]characters, used in all three locations. Raw server names are preserved as registry keys — sanitization only applies when generating tool name prefixes.Bug fix: tool results missing from ACP events
The
step_callbackpreviously forwarded only tool names (as strings) tobuild_tool_complete, so ACPtool_call_updateevents had emptycontentandrawOutput. Nowprev_toolscarries dicts with bothnameandresultby pairing eachtool_callwith its matching tool-role message viatool_call_id.Changes
tools/mcp_tool.pysanitize_mcp_name_component(),register_mcp_servers()public APIacp_adapter/server.py_register_session_mcp_servers(), wired to all session methodsrun_agent.pystep_callbacknow includes tool resultsTest results
Tested on macOS (Apple Silicon), Python 3.11.