Python: Replace create_mcp_http_client with direct httpx.AsyncClient in MCPStreamableHTTPTool

[eavanvalkenburg]

Motivation and Context

When header_provider was set on MCPStreamableHTTPTool, the code used create_mcp_http_client() from the MCP library to create an HTTP client. This function may not be available in all versions of the MCP SDK and introduces an unnecessary dependency on an internal MCP helper, causing failures when trying to pass AgentContext headers.

Fixes #4808

Description

The root cause was that MCPStreamableHTTPTool._get_client() relied on mcp.client.streamable_http.create_mcp_http_client to construct the default httpx.AsyncClient when a header_provider was configured but no explicit httpx_client was supplied. This was replaced with a direct httpx.AsyncClient(follow_redirects=True, timeout=httpx.Timeout(30.0, read=300.0)) construction, which matches the equivalent defaults and eliminates the problematic import. This ensures header injection (used for passing AgentContext) works reliably without depending on the MCP SDK's internal client factory.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

---

Note: PR autogenerated by eavanvalkenburg's agent

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework
_mcp.py	551	73	86%	99–100, 110–115, 126, 131, 174, 183, 195–196, 247, 256, 319, 327, 386, 499, 534, 547, 549–552, 571–572, 585–588, 590–591, 595, 652, 687, 689, 693–694, 696–697, 751, 766, 784, 829, 961, 974–979, 1003, 1062–1063, 1069–1071, 1090, 1115–1116, 1120–1124, 1141–1145, 1292
TOTAL	27319	3218	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
5355	20 💤	0 ❌	0 🔥	1m 30s ⏱️

[eavanvalkenburg]

Automated Code Review

Reviewers: 4 | Confidence: 89%

✓ Correctness

The diff replaces create_mcp_http_client() with an inline httpx.AsyncClient(...) using identical defaults (follow_redirects=True, timeout 30s/read 300s), which is functionally correct. However, the surrounding code has a pre-existing bug that this diff doesn't fix but is worth noting: every call to get_mcp_client() appends a new _inject_headers callback to http_client.event_hooks["request"] (line 1434) on the same cached self._httpx_client. If the tool reconnects multiple times, duplicate hooks accumulate and headers are injected multiple times per request. This isn't introduced by the diff but is a correctness concern in the method this diff touches.

✓ Security Reliability

The diff replaces the MCP SDK's create_mcp_http_client() helper with an equivalent inline httpx.AsyncClient construction. The hardcoded timeout values (30.0s general, 300.0s read) exactly match the SDK's MCP_DEFAULT_TIMEOUT and MCP_DEFAULT_SSE_READ_TIMEOUT constants, so the behavior is preserved. However, this introduces a maintenance coupling: if the MCP SDK updates its defaults in a future version, this code won't track those changes. More critically, the pre-existing event_hooks append at line 1434 (untouched by this diff) causes unbounded growth of _inject_headers callbacks on every get_mcp_client() call, which is a reliability/resource leak concern that this change should address since it's creating the client specifically for this code path.

✓ Test Coverage

The diff replaces create_mcp_http_client() (from the MCP SDK) with an inline httpx.AsyncClient(follow_redirects=True, timeout=httpx.Timeout(30.0, read=300.0)) for the auto-created client when header_provider is set. The existing test test_mcp_streamable_http_tool_header_provider_with_httpx_event_hook exercises the changed code path and verifies that a client is created and the event hook is wired up. However, no test asserts the specific configuration properties (follow_redirects, timeout) of the auto-created client, meaning a regression in these values (or a drift from what the MCP SDK expects) would go undetected. This is a minor gap, not a blocking issue.

✗ Design Approach

The diff replaces a lazy import of create_mcp_http_client() with an equivalent inline httpx.AsyncClient construction using identical default values (follow_redirects=True, timeout=30.0/read=300.0). The values are functionally correct for now, but two concerns exist: (1) hardcoding the timeout values decouples from MCP's own constants so future MCP default changes won't be inherited; (2) the pre-existing http_client.event_hooks["request"].append(_inject_headers) at line 1434 sits outside the if http_client is None: guard, so on every call to get_mcp_client() a new closure is appended to the same persistent AsyncClient, causing the hook list to grow unboundedly — headers get injected N times after N tool calls.

Flagged Issues

• Event hook accumulation: http_client.event_hooks["request"].append(_inject_headers) (line 1434) executes on every get_mcp_client() call, not just on first client creation. Because self._httpx_client is reused across calls, the hook list grows by one closure per invocation, causing headers to be injected N times after N tool calls. The hook registration should be moved inside the if http_client is None: block, immediately after client creation.

Suggestions

• The timeout values (30.0 / 300.0) are hardcoded copies of MCP SDK constants (MCP_DEFAULT_TIMEOUT, MCP_DEFAULT_SSE_READ_TIMEOUT from mcp.shared._httpx_utils). Consider importing and using the SDK constants so the defaults stay in sync if the SDK updates them.
• Consider adding assertions in test_mcp_streamable_http_tool_header_provider_with_httpx_event_hook to verify the auto-created httpx client's configuration (follow_redirects=True, timeout.connect == 30.0, timeout.read == 300.0) since this diff explicitly replaces create_mcp_http_client() with hardcoded values.

---

Automated review by eavanvalkenburg's agents

[Copilot]

Pull request overview

Updates the Python MCP integration so MCPStreamableHTTPTool no longer depends on an MCP-SDK internal HTTP client factory when header_provider is used, enabling reliable per-call header injection (e.g., forwarding AgentContext-derived values) via a standard httpx.AsyncClient.

Changes:

• Add header_provider support to MCPStreamableHTTPTool using a contextvars.ContextVar and an httpx request event hook to inject per-invocation headers.
• Replace MCP SDK internal HTTP client construction with direct httpx.AsyncClient(...) defaults when needed.
• Add unit tests covering contextvar behavior and event-hook-based header injection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
python/packages/core/agent_framework/_mcp.py	Introduces header_provider plumbing, ContextVar-based header scoping, and httpx event hook injection.
python/packages/core/tests/core/test_mcp.py	Adds tests for header provider behavior, contextvar reset, and request hook injection.

[eavanvalkenburg]

Automated Code Review

Reviewers: 4 | Confidence: 89%

✓ Correctness

The diff fixes a real bug where repeated get_mcp_client() calls would accumulate duplicate httpx event hooks, and replaces hardcoded timeout values with MCP SDK constants. The hook deduplication logic using hasattr is correct, and the new tests properly cover the fix. The main concern is importing from mcp.shared._httpx_utils, a private (underscore-prefixed) module of the MCP SDK, which could break on SDK upgrades. All logic changes are correct and the test cleanup improvements are good practice.

✓ Security Reliability

The changes fix a real bug (duplicate event hooks on repeated get_mcp_client calls) and align timeout defaults with the MCP SDK. The implementation is sound: using hasattr to guard against duplicate hook registration, contextvars for async-safe per-call headers, and proper test cleanup with aclose(). One concern is the import from mcp.shared._httpx_utils, which is a private module — but the constants are in its __all__, reducing the risk. The user-provided httpx client test (test_mcp_streamable_http_tool_header_provider_with_user_httpx_client) was not updated with the same try/finally cleanup pattern, leaving it inconsistent, but it does call aclose() at the end. No security, injection, or resource leak issues found in the changed code.

✓ Test Coverage

The diff fixes a hook duplication bug in get_mcp_client() (using hasattr guard), replaces hardcoded timeout values with MCP SDK constants, and adds a new test for the dedup fix plus improves an existing test with proper resource cleanup and timeout assertions. Test coverage for the changed behavior is adequate: the new test_mcp_streamable_http_tool_hook_not_duplicated_on_repeated_get_mcp_client test directly validates the fix, and the updated test_mcp_streamable_http_tool_header_provider_with_httpx_event_hook test verifies the timeout constants and adds proper aclose() cleanup. Minor gaps exist around testing dedup with a user-provided httpx client and error-path behavior of header_provider, but these are not blocking.

✗ Design Approach

The PR introduces two changes: (1) replacing hardcoded httpx timeout literals with constants imported from mcp.shared._httpx_utils, and (2) guarding the event-hook registration with hasattr to prevent duplicate hooks on repeated get_mcp_client() calls. The hook-deduplication fix is correct and the test covers it well. However, importing from a private module (_httpx_utils) is a fragile design choice, and the hasattr-based guard has a structural smell that the two # type: ignore[attr-defined] comments confirm — the attribute is never declared in __init__.

Flagged Issues

• Importing from mcp.shared._httpx_utils (underscore-prefixed = private module) couples the code to MCP SDK internals with no stability guarantees — the module can be renamed, moved, or removed between minor releases. The previous hardcoded literals (30.0 / 300.0) were explicit and stable. At minimum, add a try/except import with literal fallbacks so the code doesn't break on SDK upgrades.

Suggestions

• Consider adding a test that calls get_mcp_client() multiple times when a user-provided http_client is passed, to verify the hook dedup guard also works in that code path.
• Consider adding a test that verifies behavior when header_provider raises an exception — confirming the exception propagates cleanly without leaving the contextvar in a dirty state.
• The _inject_headers_hook attribute is never declared in __init__, which is why hasattr and two # type: ignore[attr-defined] comments are needed. A cleaner design: declare self._inject_headers_hook: Callable | None = None in __init__, then check if self._inject_headers_hook is None in get_mcp_client. This eliminates the hasattr smell, removes the type ignores, and makes the class's full interface visible at construction time.

---

Automated review by eavanvalkenburg's agents