Python: Apply header_provider headers to the MCP initialize handshake and other ambient requests#7305
Python: Apply header_provider headers to the MCP initialize handshake and other ambient requests#7305giles17 wants to merge 4 commits into
Conversation
MCPStreamableHTTPTool.header_provider was only invoked from call_tool(),
so the initialize handshake, load_tools/load_prompts discovery, and
background pings all went out with no headers. MCP servers that require
auth on initialize (e.g. Azure AI Search knowledge-base MCP endpoints)
therefore returned 401 before any tool call could run.
Add an ambient fallback in the _inject_headers httpx request hook: when
neither the per-call ContextVar nor the active-call snapshot is set, the
hook invokes header_provider({}) so every ambient request is
authenticated. Providers that require per-call kwargs raise on the empty
dict; that is caught, logged, and the request proceeds unauthenticated,
preserving prior behavior. Calling the provider on demand also keeps
dynamic token refresh working for post-connect requests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cf0c1dbf-99bc-4f3f-bcf4-7791ce7dbe6a
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
Pull request overview
This PR fixes a gap in Python MCP Streamable HTTP tooling where header_provider headers were not applied to ambient MCP HTTP requests (notably the initialize handshake and other requests outside tools/call), improving compatibility with MCP servers that require authentication on initialization.
Changes:
- Add an ambient fallback in the httpx request hook to invoke
header_provider({})when no per-call headers are available, so ambient requests can be authenticated. - Tolerate kwargs-dependent
header_providerimplementations that raise on{}by catching/logging and proceeding without headers. - Add regression tests covering ambient-request header injection and the “kwargs provider raises” tolerance behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
python/packages/core/agent_framework/_mcp.py |
Adds ambient fallback header injection behavior inside the httpx request hook. |
python/packages/core/tests/core/test_mcp.py |
Adds regression tests validating ambient-request header injection and error-tolerant behavior. |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
Review feedback on the ambient header_provider fallback:
- Distinguish 'unset' (no active call) from 'set but empty' (call_tool
produced no headers). Use _mcp_call_headers.get(None) and the None-ness
of the snapshot instead of a truthiness check, so a provider that
legitimately returns {} during a real call is no longer re-invoked by
the ambient fallback mid-call.
- A kwargs-dependent provider raises on every ambient request (initialize,
discovery, recurring pings). Warn once per tool instance with a
traceback via _ambient_header_warning_emitted and drop subsequent
occurrences to DEBUG to avoid log spam.
Add regression tests for both behaviors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cf0c1dbf-99bc-4f3f-bcf4-7791ce7dbe6a
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✓ Correctness
The PR correctly implements an ambient fallback for header_provider on non-call_tool requests (initialize, discovery, pings). The key change from
_mcp_call_headers.get({}) or self._active_call_headers or {}to explicitis Nonechecks properly distinguishes 'unset' (ambient request) from 'set but empty' (active call that produced no headers). The warn-once pattern for kwargs-dependent providers is safe in asyncio (no await between the flag check and set). The test coverage is thorough and validates all edge cases. No correctness issues found.
✓ Security Reliability
The PR is well-designed from a security/reliability standpoint. The ambient fallback correctly distinguishes 'unset' (None) from 'set-but-empty' ({}), preventing unintended re-invocation of the header_provider during active calls. The broad
except Exceptionis appropriate given the design goal of tolerating kwargs-dependent providers, with proper warn-once behavior. Log formatting uses %r to prevent injection. The single-threaded asyncio model ensures the warning flag check-and-set is race-free. No new security or reliability issues introduced.
✓ Failure Modes
The PR correctly addresses the failure mode where header_provider headers were missing from ambient MCP requests (initialize, discovery, pings). The implementation properly distinguishes 'unset' (None) from 'set but empty' ({}) to avoid re-invoking the provider during active calls, handles provider exceptions with graceful degradation, and rate-limits warnings. The origin-scoped guard remains effective for all paths. No new failure modes are introduced.
✓ Design Approach
I found one design issue in the new ambient-header fallback: it now swallows every exception from
header_provider, which means genuine authentication/header generation failures duringinitializeor background requests are silently converted into unauthenticated traffic. Narrowing that fallback to the missing-runtime-kwargs case would preserve the new regression coverage without masking real provider bugs or token-refresh failures.
Automated review by giles17's agents
Only the missing-per-call-kwargs case (KeyError, e.g. the mcp_api_key_auth.py sample indexing kwargs['mcp_api_key']) is tolerated during ambient requests. Any other exception - a token-refresh failure or a provider bug - now propagates instead of being silently converted into unauthenticated traffic, matching the call_tool path which does not catch header_provider exceptions. Add a regression test asserting a non-KeyError provider failure surfaces from the request hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf0c1dbf-99bc-4f3f-bcf4-7791ce7dbe6a
…gging - Reword the ambient-fallback comment to describe the kwargs-dependent provider pattern generically instead of naming a sample file, which would go stale if the sample is renamed (also in a test docstring). - Replace the type-narrowing assert with a RuntimeError carrying a concise message for the unreachable no-provider state. - Drop the warn-once/_ambient_header_warning_emitted machinery; the KeyError ambient case is expected and benign, so log a single DEBUG line and proceed without headers. Update the corresponding test to assert behavior (request proceeds without an Authorization header and no WARNING is emitted) instead of log-count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf0c1dbf-99bc-4f3f-bcf4-7791ce7dbe6a
Motivation & Context
MCPStreamableHTTPTool.header_providerheaders never reach the MCP server on therequests that happen outside a
tools/call— most importantly theinitializehandshake.
call_tool()stores the provider's output in aContextVar(and, since#7218, an instance-level snapshot), but both are only populated inside
call_tool().initialize,load_tools/load_promptsdiscovery, and background pings all runduring/after
connect(), before any tool call, so the httpx request hook reads anempty dict and sends them unauthenticated.
For MCP servers that require auth on
initialize(for example Azure AI Searchknowledge-base MCP endpoints) this means an immediate 401 and the tool never connects,
so
header_providernever gets a chance to run.Description & Review Guide
_inject_headershttpx request hook inMCPStreamableHTTPTool.get_mcp_client(). When neither the per-callContextVarnor the active-call snapshot is set (i.e. an ambient request such as
initialize,discovery, or a ping), the hook invokes
header_provider({})so the request isauthenticated. A provider that requires per-call kwargs will raise on the empty
dict; that is caught, logged at WARNING, and the request proceeds unauthenticated —
preserving today's behavior for kwargs-dependent providers (e.g. the
mcp_api_key_auth.pysample). Calling the provider on demand also keeps dynamictoken refresh working for post-connect requests.
kwargs-requiring provider that raises on
{}being tolerated without failing.header_providernow authenticates theinitializehandshake and every other ambient HTTP request, not justtools/call.No public API changes; the origin allow-list and per-call behavior are unchanged.
connect-wrapping approach in Python: Apply header_provider headers to the MCP initialize handshake #7080 (see Related Issue): the hook covers all ambient
requests (initialize, discovery, and post-connect pings/refresh), whereas wrapping
_connect_on_owneronly covers the single connect pass. Also confirm the try/exceptis the desired behavior for providers that require per-call kwargs.
Related Issue
Fixes #7079
This supersedes the existing open PR #7080, which also targets #7079 by overriding
_connect_on_ownerto seed theContextVararound the connect pass. This PR insteadadds the fallback in the request hook, which additionally covers post-connect ambient
requests (background pings / dynamic token refresh), not only the initialize/discovery
pass. It is complementary to #7218 (merged), which fixed per-call
tools/callheaders(#7161).
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.