From 457365490b4fe584f9da0136bbc113be58b36316 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 23 Jul 2026 17:44:24 -0700 Subject: [PATCH 1/4] Python: Apply header_provider headers to ambient MCP requests 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 --- python/packages/core/agent_framework/_mcp.py | 21 ++++++- python/packages/core/tests/core/test_mcp.py | 64 ++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 9071deafcf3..b8da43c57eb 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -3125,8 +3125,25 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async # The transport may send this request from a task whose context was # captured before call_tool set the ContextVar; fall back to the # instance-level snapshot of the active call's headers. - headers = _mcp_call_headers.get({}) or self._active_call_headers or {} - for key, value in headers.items(): + headers = _mcp_call_headers.get({}) or self._active_call_headers + if not headers: + # Ambient request made outside call_tool (the initialize handshake, + # load_tools/load_prompts discovery, or background pings). Invoke the + # provider with empty kwargs so static providers can authenticate these + # requests too; providers that require per-call kwargs raise here, so we + # log and proceed unauthenticated (preserving prior behavior). + assert self._header_provider is not None # nosec B101 # ruff:ignore[assert] + try: + headers = self._header_provider({}) + except Exception: + logger.warning( + "header_provider raised for MCP server %r on an ambient request; " + "proceeding without headers.", + self.name, + exc_info=True, + ) + headers = {} + for key, value in (headers or {}).items(): request.headers[key] = value self._inject_headers_hook = _inject_headers diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 4890c0eccb4..5b932b43282 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6027,6 +6027,70 @@ async def test_mcp_streamable_http_tool_header_provider_with_httpx_event_hook(): await tool._httpx_client.aclose() # type: ignore[union-attr] +async def test_mcp_streamable_http_tool_header_provider_injects_on_ambient_request(): + """Regression test for #7079: static header_provider must authenticate ambient requests. + + The initialize handshake, load_tools/load_prompts discovery, and background pings run + outside call_tool, so the contextvar/snapshot are unset. A static header_provider should + still be invoked (with empty kwargs) so these requests carry auth headers. + """ + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + header_provider=lambda kw: {"Authorization": "******"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + # No contextvar set and no active call snapshot: simulates the initialize handshake. + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert request.headers.get("Authorization") == "******" + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + +async def test_mcp_streamable_http_tool_header_provider_ambient_request_tolerates_kwargs_provider(): + """A header_provider that requires per-call kwargs must not crash ambient requests. + + Providers like the mcp_api_key_auth.py sample index runtime kwargs (e.g. kw["mcp_api_key"]) + that are absent at connect time. The hook should swallow the error and proceed without + headers rather than failing the initialize handshake. + """ + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + header_provider=lambda kw: {"Authorization": f"Bearer {kw['mcp_api_key']}"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + # No kwargs available at connect time -> provider raises KeyError -> hook swallows it. + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert "Authorization" not in request.headers + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redirect(): """The request hook must not re-add caller headers after a cross-origin redirect.""" import httpx From 1b19d5c970866e7c5137380b4da05ffa323e2d69 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 23 Jul 2026 18:00:10 -0700 Subject: [PATCH 2/4] Python: address review - distinguish unset vs empty headers, warn once 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 --- python/packages/core/agent_framework/_mcp.py | 40 +++++++--- python/packages/core/tests/core/test_mcp.py | 83 ++++++++++++++++++++ 2 files changed, 113 insertions(+), 10 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index b8da43c57eb..f8563759b46 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -3080,6 +3080,9 @@ def __init__( # otherwise overwrite each other's snapshot and attach the wrong per-call headers. self._active_call_headers: dict[str, str] | None = None self._call_headers_lock = asyncio.Lock() + # Guards the ambient-request warning (see get_mcp_client): a kwargs-dependent + # header_provider raises on every ambient request, so warn once then drop to DEBUG. + self._ambient_header_warning_emitted = False def _mcp_base_span_attributes(self) -> dict[str, Any]: attrs = super()._mcp_base_span_attributes() @@ -3124,9 +3127,14 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async return # The transport may send this request from a task whose context was # captured before call_tool set the ContextVar; fall back to the - # instance-level snapshot of the active call's headers. - headers = _mcp_call_headers.get({}) or self._active_call_headers - if not headers: + # instance-level snapshot of the active call's headers. Both are None + # only when this is an ambient request outside call_tool; an active + # call that legitimately produced no headers yields an empty dict and + # must not trigger the ambient fallback below. + headers = _mcp_call_headers.get(None) + if headers is None: + headers = self._active_call_headers + if headers is None: # Ambient request made outside call_tool (the initialize handshake, # load_tools/load_prompts discovery, or background pings). Invoke the # provider with empty kwargs so static providers can authenticate these @@ -3136,14 +3144,26 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async try: headers = self._header_provider({}) except Exception: - logger.warning( - "header_provider raised for MCP server %r on an ambient request; " - "proceeding without headers.", - self.name, - exc_info=True, - ) + # A kwargs-dependent provider raises on every ambient request + # (initialize, discovery, and recurring pings); warn once per + # instance with a traceback and stay quiet at DEBUG afterwards. + if not self._ambient_header_warning_emitted: + self._ambient_header_warning_emitted = True + logger.warning( + "header_provider raised for MCP server %r on an ambient request; " + "proceeding without headers.", + self.name, + exc_info=True, + ) + else: + logger.debug( + "header_provider raised again for MCP server %r on an ambient request; " + "proceeding without headers.", + self.name, + exc_info=True, + ) headers = {} - for key, value in (headers or {}).items(): + for key, value in headers.items(): request.headers[key] = value self._inject_headers_hook = _inject_headers diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 5b932b43282..8a15bb3dcc1 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6091,6 +6091,89 @@ async def test_mcp_streamable_http_tool_header_provider_ambient_request_tolerate await tool._httpx_client.aclose() # type: ignore[union-attr] +async def test_mcp_streamable_http_tool_header_provider_empty_active_call_skips_ambient_fallback(): + """Regression test: an active call that yields no headers must not trigger the ambient fallback. + + When header_provider legitimately returns {} for a real call_tool invocation, the empty dict + is a valid "set" value. The hook must treat it as set-but-empty (leave the request unchanged) + rather than as "unset", which would re-invoke header_provider({}) mid-call and inject headers + the caller deliberately omitted. + """ + import httpx + + from agent_framework._mcp import _mcp_call_headers + + call_count = 0 + + def provider(kw: dict[str, Any]) -> dict[str, str]: + nonlocal call_count + call_count += 1 + # Non-empty for ambient ({}), empty for the active call below. + return {} if kw.get("suppress") else {"Authorization": "******"} + + tool = MCPStreamableHTTPTool(name="test", url="http://example.com/mcp", header_provider=provider) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + # Simulate an active call whose provider returned {} (both ContextVar and snapshot set). + token = _mcp_call_headers.set({}) + tool._active_call_headers = {} + try: + call_count = 0 + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert "Authorization" not in request.headers + assert call_count == 0, "ambient fallback must not run during a set-but-empty call" + finally: + tool._active_call_headers = None + _mcp_call_headers.reset(token) + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + +async def test_mcp_streamable_http_tool_header_provider_ambient_error_warns_once(caplog): + """A kwargs-dependent provider must warn only once on repeated ambient requests. + + Ambient requests (initialize/discovery plus recurring pings) each raise, so the full WARNING + with a traceback is emitted once per tool instance and subsequent occurrences drop to DEBUG. + """ + import logging + + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + header_provider=lambda kw: {"Authorization": "Bearer " + kw["mcp_api_key"]}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + with caplog.at_level(logging.WARNING, logger="agent_framework._mcp"): + for _ in range(3): + await hooks[0](httpx.Request("POST", "http://example.com/mcp")) + + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert len(warnings) == 1, "expected exactly one WARNING across repeated ambient requests" + assert tool._ambient_header_warning_emitted is True + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redirect(): """The request hook must not re-add caller headers after a cross-origin redirect.""" import httpx From 2fa3b7f75ddd55afe604d9d6410aa8dba8385123 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 23 Jul 2026 18:25:10 -0700 Subject: [PATCH 3/4] Python: narrow ambient header_provider catch to KeyError 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 --- python/packages/core/agent_framework/_mcp.py | 17 ++++++----- python/packages/core/tests/core/test_mcp.py | 32 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index f8563759b46..1e086a12577 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -3138,27 +3138,30 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async # Ambient request made outside call_tool (the initialize handshake, # load_tools/load_prompts discovery, or background pings). Invoke the # provider with empty kwargs so static providers can authenticate these - # requests too; providers that require per-call kwargs raise here, so we - # log and proceed unauthenticated (preserving prior behavior). + # requests too. A provider that indexes required per-call kwargs raises + # KeyError on the empty dict (e.g. the mcp_api_key_auth.py sample); that + # specific case is tolerated so connect still succeeds. Any other error + # is a genuine provider failure and is left to propagate, matching the + # call_tool path which does not catch header_provider exceptions. assert self._header_provider is not None # nosec B101 # ruff:ignore[assert] try: headers = self._header_provider({}) - except Exception: + except KeyError: # A kwargs-dependent provider raises on every ambient request # (initialize, discovery, and recurring pings); warn once per # instance with a traceback and stay quiet at DEBUG afterwards. if not self._ambient_header_warning_emitted: self._ambient_header_warning_emitted = True logger.warning( - "header_provider raised for MCP server %r on an ambient request; " - "proceeding without headers.", + "header_provider raised KeyError for MCP server %r on an ambient " + "request (missing per-call kwargs); proceeding without headers.", self.name, exc_info=True, ) else: logger.debug( - "header_provider raised again for MCP server %r on an ambient request; " - "proceeding without headers.", + "header_provider raised KeyError again for MCP server %r on an " + "ambient request; proceeding without headers.", self.name, exc_info=True, ) diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 8a15bb3dcc1..57dd07b0a11 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6174,6 +6174,38 @@ async def test_mcp_streamable_http_tool_header_provider_ambient_error_warns_once await tool._httpx_client.aclose() # type: ignore[union-attr] +async def test_mcp_streamable_http_tool_header_provider_ambient_non_keyerror_propagates(): + """A genuine header_provider failure on an ambient request must surface, not be swallowed. + + Only the missing-kwargs case (KeyError) is tolerated. Any other error - e.g. a token-refresh + failure or a provider bug - propagates so it is not silently converted into unauthenticated + traffic, matching the call_tool path which does not catch header_provider exceptions. + """ + import httpx + + class TokenRefreshError(RuntimeError): + pass + + def failing_provider(kw: dict[str, Any]) -> dict[str, str]: + raise TokenRefreshError("token endpoint unreachable") + + tool = MCPStreamableHTTPTool(name="test", url="http://example.com/mcp", header_provider=failing_provider) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + with pytest.raises(TokenRefreshError): + await hooks[0](httpx.Request("POST", "http://example.com/mcp")) + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redirect(): """The request hook must not re-add caller headers after a cross-origin redirect.""" import httpx From faef8dbe796bfc1141a8ffb23315acebad4b60f7 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Fri, 24 Jul 2026 11:08:36 -0700 Subject: [PATCH 4/4] Python: address review - raise instead of assert, simplify ambient logging - 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 --- python/packages/core/agent_framework/_mcp.py | 38 +++++++------------- python/packages/core/tests/core/test_mcp.py | 26 +++++++------- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 1e086a12577..d0dc62e91cd 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -3080,9 +3080,6 @@ def __init__( # otherwise overwrite each other's snapshot and attach the wrong per-call headers. self._active_call_headers: dict[str, str] | None = None self._call_headers_lock = asyncio.Lock() - # Guards the ambient-request warning (see get_mcp_client): a kwargs-dependent - # header_provider raises on every ambient request, so warn once then drop to DEBUG. - self._ambient_header_warning_emitted = False def _mcp_base_span_attributes(self) -> dict[str, Any]: attrs = super()._mcp_base_span_attributes() @@ -3138,33 +3135,24 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async # Ambient request made outside call_tool (the initialize handshake, # load_tools/load_prompts discovery, or background pings). Invoke the # provider with empty kwargs so static providers can authenticate these - # requests too. A provider that indexes required per-call kwargs raises - # KeyError on the empty dict (e.g. the mcp_api_key_auth.py sample); that - # specific case is tolerated so connect still succeeds. Any other error - # is a genuine provider failure and is left to propagate, matching the + # requests too. A provider that indexes a required per-call kwarg (e.g. + # kwargs["api_key"]) raises KeyError on the empty dict; that specific + # case is tolerated so connect still succeeds. Any other error is a + # genuine provider failure and is left to propagate, matching the # call_tool path which does not catch header_provider exceptions. - assert self._header_provider is not None # nosec B101 # ruff:ignore[assert] + if self._header_provider is None: + raise RuntimeError("Header injection hook invoked without a header_provider.") try: headers = self._header_provider({}) except KeyError: # A kwargs-dependent provider raises on every ambient request - # (initialize, discovery, and recurring pings); warn once per - # instance with a traceback and stay quiet at DEBUG afterwards. - if not self._ambient_header_warning_emitted: - self._ambient_header_warning_emitted = True - logger.warning( - "header_provider raised KeyError for MCP server %r on an ambient " - "request (missing per-call kwargs); proceeding without headers.", - self.name, - exc_info=True, - ) - else: - logger.debug( - "header_provider raised KeyError again for MCP server %r on an " - "ambient request; proceeding without headers.", - self.name, - exc_info=True, - ) + # (initialize, discovery, and recurring pings). + logger.debug( + "header_provider raised KeyError for MCP server %r on an ambient " + "request (missing per-call kwargs); proceeding without headers.", + self.name, + exc_info=True, + ) headers = {} for key, value in headers.items(): request.headers[key] = value diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 57dd07b0a11..028cac027b3 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6062,9 +6062,9 @@ async def test_mcp_streamable_http_tool_header_provider_injects_on_ambient_reque async def test_mcp_streamable_http_tool_header_provider_ambient_request_tolerates_kwargs_provider(): """A header_provider that requires per-call kwargs must not crash ambient requests. - Providers like the mcp_api_key_auth.py sample index runtime kwargs (e.g. kw["mcp_api_key"]) - that are absent at connect time. The hook should swallow the error and proceed without - headers rather than failing the initialize handshake. + Providers that index runtime kwargs (e.g. kw["mcp_api_key"]) which are absent at connect + time raise KeyError. The hook should swallow that specific error and proceed without headers + rather than failing the initialize handshake. """ import httpx @@ -6138,11 +6138,12 @@ def provider(kw: dict[str, Any]) -> dict[str, str]: await tool._httpx_client.aclose() # type: ignore[union-attr] -async def test_mcp_streamable_http_tool_header_provider_ambient_error_warns_once(caplog): - """A kwargs-dependent provider must warn only once on repeated ambient requests. +async def test_mcp_streamable_http_tool_header_provider_ambient_kwarg_error_is_benign(caplog): + """A kwargs-dependent provider that raises KeyError on ambient requests must not fail them. - Ambient requests (initialize/discovery plus recurring pings) each raise, so the full WARNING - with a traceback is emitted once per tool instance and subsequent occurrences drop to DEBUG. + Ambient requests (initialize/discovery plus recurring pings) call the provider with empty + kwargs, so a provider indexing a required per-call kwarg raises KeyError. That is tolerated: + the request proceeds without headers and only a DEBUG line is logged (no WARNING spam). """ import logging @@ -6162,13 +6163,14 @@ async def test_mcp_streamable_http_tool_header_provider_ambient_error_warns_once hooks = tool._httpx_client.event_hooks.get("request", []) assert len(hooks) == 1 - with caplog.at_level(logging.WARNING, logger="agent_framework._mcp"): + with caplog.at_level(logging.DEBUG, logger="agent_framework._mcp"): for _ in range(3): - await hooks[0](httpx.Request("POST", "http://example.com/mcp")) + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert "Authorization" not in request.headers - warnings = [r for r in caplog.records if r.levelno == logging.WARNING] - assert len(warnings) == 1, "expected exactly one WARNING across repeated ambient requests" - assert tool._ambient_header_warning_emitted is True + # The benign kwargs case must not escalate to WARNING/ERROR. + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] finally: if getattr(tool, "_httpx_client", None) is not None: await tool._httpx_client.aclose() # type: ignore[union-attr]