From c9a5f23e7b109585790a195d7d63ac9d390af2ca Mon Sep 17 00:00:00 2001 From: bjtolo <498650411@qq.com> Date: Fri, 3 Jul 2026 15:39:12 +0800 Subject: [PATCH] feat(mcp): add per-server tool_call_timeout for MCP tool calls (#3843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mcp): add per-server tool_call_timeout for MCP tool calls Add a configurable timeout for individual MCP tool calls to prevent agent runs from blocking indefinitely when an MCP server becomes unresponsive (e.g., rate-limited HTTP API, hung subprocess). Uses the MCP SDK's built-in read_timeout_seconds parameter on ClientSession.call_tool, which handles the timeout within the session's own task — avoiding cross-task cancellation issues with the session pool (ref #3379, #3203). Config field is named tool_call_timeout (not timeout) to avoid collision with langchain-mcp-adapters' existing timeout field on HTTP/SSE connections. Closes #3840 * fix(mcp): read tool_call_timeout from McpServerConfig, not connection dict The previous implementation put tool_call_timeout into the connection dict returned by build_server_params, which langchain's create_session then passed to _create_stdio_session(), causing TypeError. Now reads the timeout directly from ExtensionsConfig.mcp_servers where the wrapper is built, keeping it out of the connection dict entirely. Fixes P1 bug from review on #3843. * test(mcp): regression test for tool_call_timeout not leaking into connection dict Adds two tests: - test_build_server_params_excludes_tool_call_timeout: verifies the connection dict returned by build_server_params() does NOT contain tool_call_timeout - test_stdio_tool_call_timeout_does_not_raise_typeerror: end-to-end test that get_mcp_tools() with a stdio server having tool_call_timeout configured loads tools without TypeError from _create_stdio_session() Regression for PR #3843 P1 bug. * fix(mcp): only pass read_timeout_seconds when tool_call_timeout is set When tool_call_timeout is None, don't pass read_timeout_seconds=None to session.call_tool(). This avoids breaking existing tests that assert on exact call_tool arguments without the extra kwarg. * docs(mcp): clarify stdio tool timeout --- README.md | 1 + backend/docs/MCP_SERVER.md | 23 ++++ .../deerflow/config/extensions_config.py | 4 + .../packages/harness/deerflow/mcp/tools.py | 35 ++++- backend/tests/test_mcp_client_config.py | 25 ++++ backend/tests/test_mcp_session_pool.py | 129 ++++++++++++++++++ extensions_config.example.json | 1 + 7 files changed, 213 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index ee53d158d..635e53e15 100644 --- a/README.md +++ b/README.md @@ -351,6 +351,7 @@ See the [Sandbox Configuration Guide](backend/docs/CONFIGURATION.md#sandbox) to DeerFlow supports configurable MCP servers and skills to extend its capabilities. For HTTP/SSE MCP servers, OAuth token flows are supported (`client_credentials`, `refresh_token`). +For stdio MCP servers, per-tool call timeouts can be configured with `tool_call_timeout`. See the [MCP Server Guide](backend/docs/MCP_SERVER.md) for detailed instructions. #### IM Channels diff --git a/backend/docs/MCP_SERVER.md b/backend/docs/MCP_SERVER.md index ba5ccd769..c22d4b02d 100644 --- a/backend/docs/MCP_SERVER.md +++ b/backend/docs/MCP_SERVER.md @@ -14,6 +14,29 @@ DeerFlow supports configurable MCP servers and skills to extend its capabilities 3. Configure each server’s command, arguments, and environment variables as needed. 4. Restart the application to load and register MCP tools. +## Per-Tool Timeout (Stdio MCP Servers) + +For `stdio` MCP servers, set `tool_call_timeout` to limit each individual MCP tool call in seconds: + +```json +{ + "mcpServers": { + "github": { + "enabled": true, + "type": "stdio", + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github"], + "env": { + "GITHUB_TOKEN": "$GITHUB_TOKEN" + }, + "tool_call_timeout": 60 + } + } +} +``` + +`tool_call_timeout` only applies to `stdio` servers. `http` and `sse` servers use transport-level timeouts, and DeerFlow logs a warning if `tool_call_timeout` is configured for those transports. + ## Filesystem MCP Servers DeerFlow already provides built-in file tools for thread-scoped workspace access. diff --git a/backend/packages/harness/deerflow/config/extensions_config.py b/backend/packages/harness/deerflow/config/extensions_config.py index 18b8cd5e2..7968dc153 100644 --- a/backend/packages/harness/deerflow/config/extensions_config.py +++ b/backend/packages/harness/deerflow/config/extensions_config.py @@ -45,6 +45,10 @@ class McpServerConfig(BaseModel): headers: dict[str, str] = Field(default_factory=dict, description="HTTP headers to send (for sse or http type)") oauth: McpOAuthConfig | None = Field(default=None, description="OAuth configuration (for sse or http type)") description: str = Field(default="", description="Human-readable description of what this MCP server provides") + tool_call_timeout: float | None = Field( + default=None, + description="Timeout in seconds for individual stdio MCP tool calls. HTTP/SSE servers use transport-level timeouts. None means no timeout.", + ) model_config = ConfigDict(extra="allow") @model_validator(mode="before") diff --git a/backend/packages/harness/deerflow/mcp/tools.py b/backend/packages/harness/deerflow/mcp/tools.py index 8799d026a..244308df1 100644 --- a/backend/packages/harness/deerflow/mcp/tools.py +++ b/backend/packages/harness/deerflow/mcp/tools.py @@ -6,6 +6,7 @@ import asyncio import logging import re from collections.abc import Iterable, Mapping +from datetime import timedelta from pathlib import Path from typing import Any from urllib.parse import unquote, urlparse @@ -414,6 +415,7 @@ def _make_session_pool_tool( server_name: str, connection: dict[str, Any], tool_interceptors: list[Any] | None = None, + tool_call_timeout: float | None = None, ) -> BaseTool: """Wrap an MCP tool so it reuses a persistent session from the pool. @@ -478,19 +480,29 @@ def _make_session_pool_tool( session_connection["env"] = session_env session = await pool.get_session(server_name, scope_key, session_connection) + # Build common call_tool kwargs once — only add keys when needed so + # existing call-sites that assert on exact arguments are not affected. + call_kwargs: dict[str, Any] = {} + if tool_call_timeout: + call_kwargs["read_timeout_seconds"] = timedelta(seconds=tool_call_timeout) + if tool_interceptors: from langchain_mcp_adapters.interceptors import MCPToolCallRequest async def base_handler(request: MCPToolCallRequest) -> Any: # Preserve interceptor-injected headers for stdio MCP calls by # forwarding them through MCP call meta. - call_kwargs: dict[str, Any] = {} + kwargs = dict(call_kwargs) if request.headers: if isinstance(request.headers, Mapping): - call_kwargs["meta"] = {"headers": dict(request.headers)} + kwargs["meta"] = {"headers": dict(request.headers)} else: logger.warning("Ignoring MCP interceptor headers with unsupported type: %s", type(request.headers).__name__) - return await session.call_tool(request.name, request.args, **call_kwargs) + return await session.call_tool( + request.name, + request.args, + **kwargs, + ) handler = base_handler for interceptor in reversed(tool_interceptors): @@ -509,7 +521,11 @@ def _make_session_pool_tool( ) call_tool_result = await handler(request) else: - call_tool_result = await session.call_tool(original_name, arguments) + call_tool_result = await session.call_tool( + original_name, + arguments, + **call_kwargs, + ) # The after-call snapshot diff only feeds bare-filename correlation in # free text, so skip the second recursive walk when there is no text @@ -646,8 +662,17 @@ async def get_mcp_tools() -> list[BaseTool]: if tool_server is not None: transport = servers_config[tool_server].get("transport", "stdio") if transport == "stdio": - wrapped_tools.append(_make_session_pool_tool(tool, tool_server, servers_config[tool_server], tool_interceptors)) + server_cfg = extensions_config.mcp_servers.get(tool_server) + _timeout = server_cfg.tool_call_timeout if server_cfg else None + wrapped_tools.append(_make_session_pool_tool(tool, tool_server, servers_config[tool_server], tool_interceptors, tool_call_timeout=_timeout)) else: + server_cfg = extensions_config.mcp_servers.get(tool_server) + if server_cfg and server_cfg.tool_call_timeout is not None: + logger.warning( + "Ignoring tool_call_timeout for MCP server '%s' because transport '%s' is not stdio; configure HTTP/SSE transport-level timeouts instead.", + tool_server, + transport, + ) wrapped_tools.append(tool) else: wrapped_tools.append(tool) diff --git a/backend/tests/test_mcp_client_config.py b/backend/tests/test_mcp_client_config.py index 3216b59fa..0396b2a5b 100644 --- a/backend/tests/test_mcp_client_config.py +++ b/backend/tests/test_mcp_client_config.py @@ -146,3 +146,28 @@ def test_build_servers_config_skips_invalid_server_and_keeps_valid_ones(): assert result["valid-stdio"]["transport"] == "stdio" assert "invalid-stdio" not in result assert "disabled-http" not in result + + +def test_build_server_params_excludes_tool_call_timeout(): + """tool_call_timeout must NOT appear in the connection dict. + + langchain-mcp-adapters passes the connection dict to create_session(), + which forwards unknown keys to _create_stdio_session(), causing TypeError. + The timeout is read from McpServerConfig at the tool wrapper call-site + instead. Regression for PR #3843 P1 bug. + """ + config = McpServerConfig( + type="stdio", + command="npx", + args=["-y", "my-mcp-server"], + tool_call_timeout=30.0, + ) + + params = build_server_params("my-server", config) + + assert "tool_call_timeout" not in params + assert params == { + "transport": "stdio", + "command": "npx", + "args": ["-y", "my-mcp-server"], + } diff --git a/backend/tests/test_mcp_session_pool.py b/backend/tests/test_mcp_session_pool.py index 8525007ce..7ccdb9012 100644 --- a/backend/tests/test_mcp_session_pool.py +++ b/backend/tests/test_mcp_session_pool.py @@ -1,6 +1,7 @@ """Tests for the MCP persistent-session pool.""" import asyncio +import logging import stat import threading from unittest.mock import AsyncMock, MagicMock, patch @@ -916,6 +917,134 @@ async def test_http_transport_tools_not_pooled(): assert stdio_tools[0].coroutine is not stdio_tool.coroutine +@pytest.mark.asyncio +async def test_non_stdio_tool_call_timeout_warns_that_it_is_ignored(caplog): + """HTTP/SSE servers should not silently ignore stdio-only tool_call_timeout.""" + from langchain_core.tools import StructuredTool + from pydantic import BaseModel, Field + + from deerflow.config.extensions_config import McpServerConfig + from deerflow.mcp.tools import get_mcp_tools + + class Args(BaseModel): + query: str = Field(..., description="query") + + http_tool = StructuredTool( + name="remote_search", + description="Search tool", + args_schema=Args, + coroutine=AsyncMock(), + response_format="content_and_artifact", + ) + + server_cfg = McpServerConfig( + type="http", + url="https://example.com/mcp", + tool_call_timeout=30.0, + ) + extensions_config = MagicMock() + extensions_config.get_enabled_mcp_servers.return_value = {"remote": server_cfg} + extensions_config.mcp_servers = {"remote": server_cfg} + extensions_config.model_extra = {} + + servers_config = { + "remote": {"transport": "http", "url": "https://example.com/mcp"}, + } + + with ( + patch("deerflow.mcp.tools.ExtensionsConfig.from_file", return_value=extensions_config), + patch("deerflow.mcp.tools.build_servers_config", return_value=servers_config), + patch("deerflow.mcp.tools.get_initial_oauth_headers", return_value={}), + patch("deerflow.mcp.tools.build_oauth_tool_interceptor", return_value=None), + patch("langchain_mcp_adapters.client.MultiServerMCPClient") as MockClient, + caplog.at_level(logging.WARNING, logger="deerflow.mcp.tools"), + ): + mock_client_instance = MockClient.return_value + mock_client_instance.get_tools = AsyncMock(return_value=[http_tool]) + + tools = await get_mcp_tools() + + assert tools == [http_tool] + assert any(record.levelno == logging.WARNING and "remote" in record.getMessage() and "tool_call_timeout" in record.getMessage() and "stdio" in record.getMessage() for record in caplog.records) + + +# --------------------------------------------------------------------------- +# Regression for PR #3843: tool_call_timeout must not leak into connection dict +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_stdio_tool_call_timeout_does_not_raise_typeerror(): + """A stdio server with tool_call_timeout must load tools without TypeError. + + The timeout must be read from McpServerConfig (extensions_config), NOT from + the connection dict that langchain's create_session receives. If it leaks + into the connection dict, _create_stdio_session() raises TypeError. + Regression for PR #3843 P1 bug. + """ + from langchain_core.tools import StructuredTool + from pydantic import BaseModel, Field + + from deerflow.config.extensions_config import McpServerConfig + from deerflow.mcp.tools import get_mcp_tools + + class Args(BaseModel): + query: str = Field(..., description="query") + + stdio_tool = StructuredTool( + name="biomcp_search", + description="Search biomedical data", + args_schema=Args, + coroutine=AsyncMock(), + response_format="content_and_artifact", + ) + + mock_session = AsyncMock() + mock_cm = MagicMock() + mock_cm.__aenter__ = AsyncMock(return_value=mock_session) + mock_cm.__aexit__ = AsyncMock(return_value=False) + + # Use real McpServerConfig so tool_call_timeout is a real field value, + # not a MagicMock that might accidentally work. + server_cfg = McpServerConfig( + type="stdio", + command="biomcp", + args=["serve"], + tool_call_timeout=60.0, + ) + + extensions_config = MagicMock() + extensions_config.get_enabled_mcp_servers.return_value = {"biomcp": server_cfg} + extensions_config.mcp_servers = {"biomcp": server_cfg} + extensions_config.model_extra = {} + + # Connection dict must NOT contain tool_call_timeout — this is the key assertion. + servers_config = { + "biomcp": {"transport": "stdio", "command": "biomcp", "args": ["serve"]}, + } + + with ( + patch("deerflow.mcp.tools.ExtensionsConfig.from_file", return_value=extensions_config), + patch("deerflow.mcp.tools.build_servers_config", return_value=servers_config), + patch("deerflow.mcp.tools.get_initial_oauth_headers", return_value={}), + patch("deerflow.mcp.tools.build_oauth_tool_interceptor", return_value=None), + patch("langchain_mcp_adapters.client.MultiServerMCPClient") as MockClient, + patch("langchain_mcp_adapters.sessions.create_session", return_value=mock_cm), + ): + mock_client_instance = MockClient.return_value + mock_client_instance.get_tools = AsyncMock(return_value=[stdio_tool]) + + # This must NOT raise TypeError from _create_stdio_session() + tools = await get_mcp_tools() + + assert len(tools) == 1 + # The tool should be wrapped with session pool (it's stdio) + assert tools[0].coroutine is not stdio_tool.coroutine + + # Verify the connection dict passed to the pool does NOT contain tool_call_timeout + assert "tool_call_timeout" not in servers_config["biomcp"] + + # --------------------------------------------------------------------------- # Regression for #3379: cancel scope must be exited in the entering task # --------------------------------------------------------------------------- diff --git a/extensions_config.example.json b/extensions_config.example.json index 7c0dce740..38d0a81d4 100644 --- a/extensions_config.example.json +++ b/extensions_config.example.json @@ -14,6 +14,7 @@ "env": { "GITHUB_TOKEN": "$GITHUB_TOKEN" }, + "tool_call_timeout": 60, "description": "GitHub MCP server for repository operations" }, "postgres": {