diff --git a/run_agent.py b/run_agent.py index 5f4ac68dc..3f1aff46d 100644 --- a/run_agent.py +++ b/run_agent.py @@ -264,6 +264,19 @@ def _is_destructive_command(cmd: str) -> bool: return False +def _is_mcp_tool_parallel_safe(tool_name: str) -> bool: + """Check if an MCP tool comes from a server with parallel tool calls enabled. + + Lazy-imports from ``tools.mcp_tool`` to avoid circular dependencies. + Returns False if the MCP module is not available. + """ + try: + from tools.mcp_tool import is_mcp_tool_parallel_safe + return is_mcp_tool_parallel_safe(tool_name) + except Exception: + return False + + def _should_parallelize_tool_batch(tool_calls) -> bool: """Return True when a tool-call batch is safe to run concurrently.""" if len(tool_calls) <= 1: @@ -303,7 +316,9 @@ def _should_parallelize_tool_batch(tool_calls) -> bool: continue if tool_name not in _PARALLEL_SAFE_TOOLS: - return False + # Check if it's an MCP tool from a server that opted into parallel calls. + if not _is_mcp_tool_parallel_safe(tool_name): + return False return True diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index d71e6a625..74c966703 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -1622,6 +1622,60 @@ class TestParallelScopePathNormalization: assert not _should_parallelize_tool_batch([tc1, tc2]) +class TestMcpParallelToolBatch: + """Integration test: _should_parallelize_tool_batch respects MCP parallel flag.""" + + def test_mcp_tools_default_sequential(self): + """MCP tools without supports_parallel_tool_calls are sequential.""" + from run_agent import _should_parallelize_tool_batch + tc1 = _mock_tool_call(name="mcp_github_list_repos", arguments='{"org":"openai"}', call_id="c1") + tc2 = _mock_tool_call(name="mcp_github_search_code", arguments='{"q":"test"}', call_id="c2") + assert not _should_parallelize_tool_batch([tc1, tc2]) + + def test_mcp_tools_parallel_when_server_opted_in(self): + """MCP tools from a parallel-safe server can run concurrently.""" + from run_agent import _should_parallelize_tool_batch + from tools.mcp_tool import _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("github") + try: + tc1 = _mock_tool_call(name="mcp_github_list_repos", arguments='{"org":"openai"}', call_id="c1") + tc2 = _mock_tool_call(name="mcp_github_search_code", arguments='{"q":"test"}', call_id="c2") + assert _should_parallelize_tool_batch([tc1, tc2]) + finally: + with _lock: + _parallel_safe_servers.discard("github") + + def test_mixed_mcp_and_builtin_parallel(self): + """MCP parallel tools mixed with built-in parallel-safe tools.""" + from run_agent import _should_parallelize_tool_batch + from tools.mcp_tool import _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("docs") + try: + tc1 = _mock_tool_call(name="mcp_docs_search", arguments='{"query":"api"}', call_id="c1") + tc2 = _mock_tool_call(name="web_search", arguments='{"query":"test"}', call_id="c2") + assert _should_parallelize_tool_batch([tc1, tc2]) + finally: + with _lock: + _parallel_safe_servers.discard("docs") + + def test_mixed_parallel_and_serial_mcp_servers(self): + """One parallel MCP server + one non-parallel MCP server = sequential.""" + from run_agent import _should_parallelize_tool_batch + from tools.mcp_tool import _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("docs") + # "github" is NOT in _parallel_safe_servers + try: + tc1 = _mock_tool_call(name="mcp_docs_search", arguments='{"query":"api"}', call_id="c1") + tc2 = _mock_tool_call(name="mcp_github_list_repos", arguments='{"org":"openai"}', call_id="c2") + assert not _should_parallelize_tool_batch([tc1, tc2]) + finally: + with _lock: + _parallel_safe_servers.discard("docs") + + class TestHandleMaxIterations: def test_returns_summary(self, agent): resp = _mock_response(content="Here is a summary of what I did.") diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 43049c2c1..8d76cfe6a 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -3140,3 +3140,138 @@ class TestRegisterMcpServers: ) _servers.pop("srv", None) + + +# --------------------------------------------------------------------------- +# Tests for parallel tool call support (port from openai/codex#17667) +# --------------------------------------------------------------------------- + +class TestMcpParallelToolCalls: + """Tests for the supports_parallel_tool_calls config option.""" + + def test_is_mcp_tool_parallel_safe_non_mcp_tool(self): + """Non-MCP tool names always return False.""" + from tools.mcp_tool import is_mcp_tool_parallel_safe + assert is_mcp_tool_parallel_safe("web_search") is False + assert is_mcp_tool_parallel_safe("read_file") is False + assert is_mcp_tool_parallel_safe("terminal") is False + assert is_mcp_tool_parallel_safe("") is False + + def test_is_mcp_tool_parallel_safe_no_servers(self): + """MCP tool from unknown server returns False.""" + from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.clear() + assert is_mcp_tool_parallel_safe("mcp_docs_search") is False + + def test_is_mcp_tool_parallel_safe_with_flag(self): + """MCP tool from a parallel-safe server returns True.""" + from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("docs") + try: + assert is_mcp_tool_parallel_safe("mcp_docs_search") is True + assert is_mcp_tool_parallel_safe("mcp_docs_read_file") is True + # Different server should be False + assert is_mcp_tool_parallel_safe("mcp_github_list_repos") is False + finally: + with _lock: + _parallel_safe_servers.discard("docs") + + def test_is_mcp_tool_parallel_safe_server_with_underscores(self): + """Server names containing underscores are correctly matched.""" + from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("my_server") + try: + assert is_mcp_tool_parallel_safe("mcp_my_server_query") is True + finally: + with _lock: + _parallel_safe_servers.discard("my_server") + + def test_is_mcp_tool_parallel_safe_no_tool_suffix(self): + """Tool name that is just 'mcp_{server}' without a tool part returns False.""" + from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock + with _lock: + _parallel_safe_servers.add("docs") + try: + # "mcp_docs" has no tool part after the server name + assert is_mcp_tool_parallel_safe("mcp_docs") is False + # "mcp_docs_" has empty tool part + assert is_mcp_tool_parallel_safe("mcp_docs_") is False + finally: + with _lock: + _parallel_safe_servers.discard("docs") + + def test_register_mcp_servers_tracks_parallel_flag(self): + """register_mcp_servers populates _parallel_safe_servers from config.""" + from tools.mcp_tool import ( + register_mcp_servers, _parallel_safe_servers, _lock, + sanitize_mcp_name_component, + ) + fake_config = { + "parallel_srv": { + "command": "echo", + "supports_parallel_tool_calls": True, + }, + "serial_srv": { + "command": "echo", + "supports_parallel_tool_calls": False, + }, + "default_srv": { + "command": "echo", + # no supports_parallel_tool_calls key + }, + } + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop"), \ + patch("tools.mcp_tool._sync_mcp_toolsets"), \ + patch("tools.mcp_tool._existing_tool_names", return_value=[]): + register_mcp_servers(fake_config) + + with _lock: + assert sanitize_mcp_name_component("parallel_srv") in _parallel_safe_servers + assert sanitize_mcp_name_component("serial_srv") not in _parallel_safe_servers + assert sanitize_mcp_name_component("default_srv") not in _parallel_safe_servers + # Cleanup + _parallel_safe_servers.discard(sanitize_mcp_name_component("parallel_srv")) + + def test_register_mcp_servers_removes_parallel_flag_on_toggle(self): + """Toggling supports_parallel_tool_calls to false removes server from the set.""" + from tools.mcp_tool import ( + register_mcp_servers, _parallel_safe_servers, _lock, + sanitize_mcp_name_component, + ) + + # First registration: parallel enabled + config_on = { + "toggle_srv": { + "command": "echo", + "supports_parallel_tool_calls": True, + }, + } + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop"), \ + patch("tools.mcp_tool._sync_mcp_toolsets"), \ + patch("tools.mcp_tool._existing_tool_names", return_value=[]): + register_mcp_servers(config_on) + with _lock: + assert sanitize_mcp_name_component("toggle_srv") in _parallel_safe_servers + + # Second registration: parallel disabled + config_off = { + "toggle_srv": { + "command": "echo", + "supports_parallel_tool_calls": False, + }, + } + with patch("tools.mcp_tool._MCP_AVAILABLE", True), \ + patch("tools.mcp_tool._ensure_mcp_loop"), \ + patch("tools.mcp_tool._run_on_mcp_loop"), \ + patch("tools.mcp_tool._sync_mcp_toolsets"), \ + patch("tools.mcp_tool._existing_tool_names", return_value=[]): + register_mcp_servers(config_off) + with _lock: + assert sanitize_mcp_name_component("toggle_srv") not in _parallel_safe_servers diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index d6bdc89fa..f1b6e74a5 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -24,6 +24,7 @@ Example config:: args: ["-y", "@modelcontextprotocol/server-github"] env: GITHUB_PERSONAL_ACCESS_TOKEN: "ghp_..." + supports_parallel_tool_calls: true # tools from this server may run concurrently remote_api: url: "https://my-mcp-server.example.com/mcp" headers: @@ -51,6 +52,8 @@ Features: - Thread-safe architecture with dedicated background event loop - Sampling support: MCP servers can request LLM completions via sampling/createMessage (text and tool-use responses) + - Parallel tool call opt-in: per-server ``supports_parallel_tool_calls`` + flag allows concurrent execution of tools from the same server Architecture: A dedicated background event loop (_mcp_loop) runs in a daemon thread. @@ -1167,11 +1170,16 @@ class MCPServerTask: _servers: Dict[str, MCPServerTask] = {} +# Sanitized server names whose ``supports_parallel_tool_calls`` config is True. +# Populated during ``register_mcp_servers()`` and queried by +# ``is_mcp_tool_parallel_safe()`` for the parallel-execution check in run_agent. +_parallel_safe_servers: set = set() + # Dedicated event loop running in a background daemon thread. _mcp_loop: Optional[asyncio.AbstractEventLoop] = None _mcp_thread: Optional[threading.Thread] = None -# Protects _mcp_loop, _mcp_thread, _servers, and _stdio_pids. +# Protects _mcp_loop, _mcp_thread, _servers, _parallel_safe_servers, and _stdio_pids. _lock = threading.Lock() # PIDs of stdio MCP server subprocesses. Tracked so we can force-kill @@ -2047,6 +2055,12 @@ def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: for k, v in servers.items() if k not in _servers and _parse_boolish(v.get("enabled", True), default=True) } + # Track which servers opt-in to parallel tool calls (idempotent). + for srv_name, srv_cfg in servers.items(): + if _parse_boolish(srv_cfg.get("supports_parallel_tool_calls", False), default=False): + _parallel_safe_servers.add(sanitize_mcp_name_component(srv_name)) + else: + _parallel_safe_servers.discard(sanitize_mcp_name_component(srv_name)) if not new_servers: _sync_mcp_toolsets(list(servers.keys())) @@ -2148,6 +2162,29 @@ def discover_mcp_tools() -> List[str]: return tool_names +def is_mcp_tool_parallel_safe(tool_name: str) -> bool: + """Check if an MCP tool belongs to a server that supports parallel tool calls. + + MCP tool names follow the pattern ``mcp_{server}_{tool}``. This extracts + the server component and checks it against the set of servers whose config + includes ``supports_parallel_tool_calls: true``. + + Returns False for non-MCP tools or tools from servers without the flag. + """ + if not tool_name.startswith("mcp_"): + return False + # Strip the "mcp_" prefix and extract the server name. + # Tool names are: mcp_{sanitized_server}_{sanitized_tool} + # We need to check all possible server prefixes because the server name + # itself may contain underscores after sanitization. + rest = tool_name[4:] # strip "mcp_" + with _lock: + for server_name in _parallel_safe_servers: + if rest.startswith(server_name + "_") and len(rest) > len(server_name) + 1: + return True + return False + + def get_mcp_status() -> List[dict]: """Return status of all configured MCP servers for banner display. diff --git a/website/docs/reference/mcp-config-reference.md b/website/docs/reference/mcp-config-reference.md index a87478f91..ecd6ad2c1 100644 --- a/website/docs/reference/mcp-config-reference.md +++ b/website/docs/reference/mcp-config-reference.md @@ -28,6 +28,7 @@ mcp_servers: enabled: true timeout: 120 connect_timeout: 60 + supports_parallel_tool_calls: false tools: include: [] exclude: [] @@ -47,6 +48,7 @@ mcp_servers: | `enabled` | bool | both | Skip the server entirely when false | | `timeout` | number | both | Tool call timeout | | `connect_timeout` | number | both | Initial connection timeout | +| `supports_parallel_tool_calls` | bool | both | Allow tools from this server to run concurrently | | `tools` | mapping | both | Filtering and utility-tool policy | | `auth` | string | HTTP | Authentication method. Set to `oauth` to enable OAuth 2.1 with PKCE | | `sampling` | mapping | both | Server-initiated LLM request policy (see MCP guide) | diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index b136af15c..c1711a9f3 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -105,6 +105,7 @@ Hermes reads MCP config from `~/.hermes/config.yaml` under `mcp_servers`. | `timeout` | number | Tool call timeout | | `connect_timeout` | number | Initial connection timeout | | `enabled` | bool | If `false`, Hermes skips the server entirely | +| `supports_parallel_tool_calls` | bool | If `true`, tools from this server may run concurrently | | `tools` | mapping | Per-server tool filtering and utility policy | ### Minimal stdio example @@ -409,6 +410,23 @@ Because Hermes now only registers those wrappers when both are true: This is intentional and keeps the tool list honest. +## Parallel Tool Calls + +By default, MCP tools run sequentially — one at a time. If your MCP server exposes tools that are safe to run concurrently (e.g. read-only queries, independent API calls), you can opt-in to parallel execution: + +```yaml +mcp_servers: + docs: + command: "docs-server" + supports_parallel_tool_calls: true +``` + +When `supports_parallel_tool_calls` is `true`, Hermes may execute multiple tools from that server at the same time within a single tool-call batch, just like it does for built-in read-only tools (web_search, read_file, etc.). + +:::caution +Only enable parallel calls for MCP servers whose tools are safe to run at the same time. If tools read and write shared state, files, databases, or external resources, review the read/write race conditions before enabling this setting. +::: + ## MCP Sampling Support MCP servers can request LLM inference from Hermes via the `sampling/createMessage` protocol. This allows an MCP server to ask Hermes to generate text on its behalf — useful for servers that need LLM capabilities but don't have their own model access.