mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-22 22:18:15 +00:00
feat(mcp): add per-server tool_call_timeout for MCP tool calls (#3843)
* 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
This commit is contained in:
parent
629477fd5c
commit
c9a5f23e7b
@ -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
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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")
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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"],
|
||||
}
|
||||
|
||||
@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
"env": {
|
||||
"GITHUB_TOKEN": "$GITHUB_TOKEN"
|
||||
},
|
||||
"tool_call_timeout": 60,
|
||||
"description": "GitHub MCP server for repository operations"
|
||||
},
|
||||
"postgres": {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user