diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index a7d32007846..d9f7899898b 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -1138,6 +1138,8 @@ def __init__( ) if max_host_payload_size_bytes is not None and max_host_payload_size_bytes <= 0: raise ValueError("max_host_payload_size_bytes must be positive or None.") + if request_timeout is not None and request_timeout <= 0: + raise ValueError("request_timeout must be positive or None.") allowed_modes = { "structured_first", "content_first", @@ -2351,6 +2353,7 @@ async def _connect_on_owner( reset_discovery: If True, removes state derived from the previous session after its transport closes. Raises: + ValueError: If ``request_timeout`` was set to a non-positive value after construction. ToolException: If connection or session initialization fails. """ log_level = _to_mcp_logging_level(logger.level) @@ -2369,6 +2372,8 @@ async def _connect_on_owner( self._exit_stack = AsyncExitStack() connection = self._connection if connection is None: + if self.request_timeout is not None and self.request_timeout <= 0: + raise ValueError("request_timeout must be positive or None.") try: try: from mcp import Client, types diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 3128023fffc..20d111c836b 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -1426,6 +1426,17 @@ def test_mcp_host_payload_size_limit_must_be_positive_or_none() -> None: assert unlimited.max_host_payload_size_bytes is None +def test_mcp_request_timeout_must_be_positive_or_none() -> None: + with pytest.raises(ValueError, match="positive or None"): + MCPTool(name="invalid", request_timeout=0) # type: ignore[abstract] # ty: ignore[call-non-callable] + + with pytest.raises(ValueError, match="positive or None"): + MCPStdioTool(name="invalid", command="test-command", request_timeout=-1) + + unlimited = MCPTool(name="unlimited", request_timeout=None) # type: ignore[abstract] # ty: ignore[call-non-callable] + assert unlimited.request_timeout is None + + def test_mcp_host_payload_size_boundary_uri_serialization_and_early_abort( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -5421,6 +5432,25 @@ async def test_connect_preserves_request_timeout_over_24_hours() -> None: assert mock_client_class.call_args.kwargs["read_timeout_seconds"] == 90_000 +async def test_connect_revalidates_request_timeout_cleared_after_construction() -> None: + tool = MCPStdioTool( + name="test", + command="test-command", + load_tools=False, + load_prompts=False, + request_timeout=60, + ) + tool.request_timeout = 0 + + with patch("mcp.Client") as mock_client_class: + mock_client_class.return_value = _mock_sdk_client(protocol_version="2026-07-28") + + with pytest.raises(ValueError, match="request_timeout must be positive or None."): + await tool.connect() + + assert mock_client_class.call_count == 0 + + async def test_connect_retains_and_close_clears_sdk_client() -> None: """Test a framework-owned Client and its session share one lifecycle.""" tool = MCPStdioTool(name="test", command="test-command", load_tools=False, load_prompts=False)