Python: [Bug]: A non-positive MCP request timeout silently disables the timeout #9312

Observed Behavior

MCPTool accepts a non-positive request_timeout without complaint, and the only place that value is read treats it as "not set", so the request timeout is switched off:

# python/packages/core/agent_framework/_mcp.py:2007 (main @ 726dcff8)
read_timeout_seconds=(
    timedelta(seconds=self.request_timeout) if self.request_timeout else None
),

With request_timeout=0, if self.request_timeout is falsy, so read_timeout_seconds=None is passed to ClientSession. In the MCP SDK that is the "never time out" setting, not the strictest one — mcp/shared/session.py:192 documents the parameter as # If none, reading will never time out and :284-291 feeds it to anyio.fail_after(timeout), which is a no-op for None.

So 0 — the value a caller reaches for when they want the tightest possible bound — produces the exact opposite: an unbounded wait. A negative value (request_timeout=-5) is likewise accepted and also lands in the same None branch.

This is not confined to the base class: MCPStdioTool, MCPStreamableHTTPTool and MCPWebsocketTool all forward their own request_timeout argument into MCPTool.__init__ unchanged (:3684, :3963, :4497), so every transport inherits the same behavior.

None is already the documented way to ask for no timeout ("Timeout in seconds for MCP requests", :953), so the 0 path has no useful meaning to preserve.

Expected Behavior

A non-positive request_timeout should be rejected at construction time, the same way the sibling knob right next to it already is:

# python/packages/core/agent_framework/_mcp.py:999
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.")

Failing fast also keeps the None meaning intact: callers who genuinely want unbounded requests pass None explicitly instead of discovering, in production, that 0 silently did that for them.

The practical impact is a hang rather than a wrong answer: an agent wired to an MCP server that accepts the connection but never answers a tools/call blocks forever on that request, with no timeout error to catch and no retry to fall back on.

Steps to Reproduce

1. Install from main (726dcff8): agent-framework-core 1.21.0 with the repo's own mcp>=1.24.0,<2 pin (1.30.0 here).
2. Construct a wrapper with an explicit zero timeout — no error is raised: MCPStdioTool(name="x", command="server", request_timeout=0).
3. Call connect() and inspect the keyword argument passed to ClientSession: read_timeout_seconds is None.
4. Point it at a server that never replies to a tool call; the call_tool await never raises McpError and never returns.

Minimal Reproduction

Both reproduce with the MCP test module's existing harness, no server or API keys needed. Adding these two tests to python/packages/core/tests/core/test_mcp.py reproduces the defect on main:

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]

    with pytest.raises(ValueError, match="positive or None"):
        MCPStdioTool(name="invalid", command="test-command", request_timeout=-1)


async def test_connect_passes_positive_request_timeout_to_session() -> None:
    tool = MCPStdioTool(name="test", command="test-command", request_timeout=5, load_tools=False, load_prompts=False)

    mock_transport = (Mock(), Mock())
    mock_context_manager = Mock()
    mock_context_manager.__aenter__ = AsyncMock(return_value=mock_transport)
    mock_context_manager.__aexit__ = AsyncMock(return_value=None)
    tool.get_mcp_client = Mock(return_value=mock_context_manager)  # type: ignore[method-assign]

    with patch("mcp.client.session.ClientSession") as mock_session_class:
        mock_session = AsyncMock()
        mock_session._request_id = 1

        session_cm = AsyncMock()
        session_cm.__aenter__ = AsyncMock(return_value=mock_session)
        session_cm.__aexit__ = AsyncMock(return_value=None)
        mock_session_class.return_value = session_cm

        await tool.connect()

        assert mock_session_class.call_args.kwargs["read_timeout_seconds"] == timedelta(seconds=5)

The first test fails on main; the second already passes and pins the positive-value path so the fix cannot regress it.

Error Messages and Stack Traces

Measured on main (726dcff8), the first test fails and nothing is raised:

>       with pytest.raises(ValueError, match="positive or None"):
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E       Failed: DID NOT RAISE ValueError

tests\core\test_mcp.py:1055: Failed
FAILED tests/core/test_mcp.py::test_mcp_request_timeout_must_be_positive_or_none

Package Versions

agent-framework-core: 1.21.0 (source main @ 726dcff8); mcp: 1.30.0 (repo pin mcp>=1.24.0,<2)

Python Version

Python 3.12.11

Operating System

Windows