fix(python): preserve zero MCP streamable HTTP timeouts - #14288
fix(python): preserve zero MCP streamable HTTP timeouts#14288King Star (jstar0) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes MCP Streamable HTTP client argument forwarding so explicitly provided zero timeouts (0.0) are passed through, and adds a regression test to prevent reintroducing the bug.
Changes:
- Update
MCPStreamableHttpPlugin.get_mcp_client()to forwardtimeout/sse_read_timeoutwhen they are0.0by checkingis not None. - Add a unit test that asserts
timeout=0.0andsse_read_timeout=0.0are forwarded tostreamablehttp_client.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/tests/unit/connectors/mcp/test_mcp.py | Adds regression coverage ensuring zero timeouts are forwarded to the underlying client factory. |
| python/semantic_kernel/connectors/mcp.py | Fixes argument construction to include explicit zero values for timeouts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @patch("semantic_kernel.connectors.mcp.streamablehttp_client") | ||
| def test_streamable_http_client_forwards_zero_timeouts(mock_client): |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): a05c6a15f445
Model: claude-opus-4.8
Overview
This is a minimal, correctly-scoped bug fix that changes two guards in
MCPStreamableHttpPlugin.get_mcp_client() from truthiness (if self.timeout:)
to explicit is not None, so an explicitly-configured timeout=0.0 /
sse_read_timeout=0.0 is forwarded to the MCP client instead of being dropped in
favor of the SDK defaults. The change brings the streamable-HTTP plugin into
parity with the pre-existing SSE plugin behavior (mcp.py:804-806), preserves the
None "omit-and-use-defaults" path (guarded by the existing
test_with_kwargs_streamablehttp), and is locked by a new focused test. No
untrusted input can reach these developer-supplied configuration values, and in
the pinned mcp->httpx chain a forwarded 0.0 yields an immediate timeout rather
than an infinite one, so there is no denial-of-service exposure. No publishable
defect was found.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Description
Preserve explicit zero timeout values for streamable HTTP MCP connections in the Python connector instead of falling back to the default timeout.
Validation