feat(acp): register client-provided MCP servers as agent tools#4705
Merged
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.
The PR changed prev_tools from list[str] to list[dict] with name/result keys. The gateway's _step_callback_sync passed this directly to hooks as 'tool_names', breaking user-authored hooks that call ', '.join(tool_names). Now: - 'tool_names' always contains strings (backward-compatible) - 'tools' carries the enriched dicts for hooks that want results Also adds summary logging to register_mcp_servers() and comprehensive tests for all three PR changes: - sanitize_mcp_name_component edge cases - register_mcp_servers public API - _register_session_mcp_servers ACP integration - step_callback result forwarding - gateway normalization backward compat
Tests the full ACP flow: - new_session with mcpServers → config conversion → register_mcp_servers - prompt → tool_progress_callback → ToolCallStart events - step_callback with results → ToolCallUpdate with rawOutput - toolCallId pairing between start and completion events - server names with slashes/dots sanitized correctly - all session lifecycle methods (load/resume/fork) register MCP
Contributor
|
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
2 participants
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.
Salvage of #4496 (by @Jckwind / TheProxyCompany) with fixes and tests.
What it does
ACP clients pass MCP server definitions (stdio and HTTP/SSE) in session/new, load_session, resume_session, and fork_session. Previously these were silently dropped. This wires them into the MCP registration pipeline so client-provided tools are discovered and available before the first prompt.
Changes from original PR
sanitize_mcp_name_component()that replaces all non-[A-Za-z0-9_]characters, fixing server names likeai.exa/exathat crash Anthropic's APIstep_callbacknow includes tool results sotool_call_updateevents have populatedrawOutputregister_mcp_servers()public API fromdiscover_mcp_tools()Fixes added during salvage
tool_namesinagent:stephook events stayslist[str](newtoolsfield carries enriched dicts)register_mcp_servers()so ACP callers get visibilityTest results
Closes #4496