From d817ea29122d22eea2e4462aa27f00ae54a71fcc Mon Sep 17 00:00:00 2001 From: poshinchen Date: Fri, 7 Aug 2026 13:03:11 -0400 Subject: [PATCH 1/4] feat(mcp): add compatibility layer for the mcp 2.x line Resolve every mcp name that was renamed or relocated in mcp 2.0 through a single _compat module so the package imports cleanly on both major lines: - MCPError (spelled McpError in 1.x) - streamable_http_transport adapter (2.x streamable_http_client takes a pre-configured HTTPX client instead of loose header kwargs) - GetSessionIdCallback (removed in 2.x along with protocol sessions) - ProgressFnT now imported from mcp.client.session (available on both lines) The MCP_V2 flag is feature-probed via ClientSession.discover rather than version-parsed, and replaces mcp_instrumentation's _is_mcp_v1 probe so the whole codebase branches on one source of truth. Related to #1659 --- strands-py/src/strands/tools/mcp/_compat.py | 67 +++++++++++++++++++ .../src/strands/tools/mcp/mcp_client.py | 12 ++-- .../strands/tools/mcp/mcp_instrumentation.py | 26 ++----- strands-py/src/strands/tools/mcp/mcp_types.py | 2 +- .../tests/strands/tools/mcp/test__compat.py | 54 +++++++++++++++ .../strands/tools/mcp/test_mcp_client.py | 30 +++++---- .../strands/tools/mcp/test_mcp_client_auth.py | 4 +- .../tools/mcp/test_mcp_client_load_servers.py | 2 +- .../tools/mcp/test_mcp_instrumentation.py | 48 ++----------- 9 files changed, 157 insertions(+), 88 deletions(-) create mode 100644 strands-py/src/strands/tools/mcp/_compat.py create mode 100644 strands-py/tests/strands/tools/mcp/test__compat.py diff --git a/strands-py/src/strands/tools/mcp/_compat.py b/strands-py/src/strands/tools/mcp/_compat.py new file mode 100644 index 0000000000..29139dd26e --- /dev/null +++ b/strands-py/src/strands/tools/mcp/_compat.py @@ -0,0 +1,67 @@ +"""Compatibility layer over the `mcp` 1.x and 2.x lines. + +The official `mcp` package renamed and relocated several public names in 2.0. +Import any version-dependent name from this module instead of from `mcp` +directly so the rest of the codebase stays version-agnostic. Branch on +`MCP_V2` only where behavior differs between the two lines; pure renames are +resolved here once. +""" + +from contextlib import AbstractAsyncContextManager +from typing import Any + +import httpx +from mcp import ClientSession + +__all__ = ["MCP_V2", "GetSessionIdCallback", "MCPError", "streamable_http_transport"] + +# Feature-probed rather than version-parsed so pre-releases and backports +# resolve by capability: `ClientSession.discover` is the 2.x replacement for +# the removed initialize handshake. +MCP_V2: bool = hasattr(ClientSession, "discover") + +try: + from mcp.shared.exceptions import MCPError +except ImportError: + # mcp 1.x spells the class McpError + from mcp.shared.exceptions import McpError as MCPError # type: ignore[attr-defined, no-redef] + +try: + from mcp.client.streamable_http import GetSessionIdCallback +except ImportError: + # mcp 2.x removed protocol sessions, so its transports never yield a + # session-id callback; the alias survives only to type 1.x transports. + from collections.abc import Callable + + GetSessionIdCallback = Callable[[], str | None] # type: ignore[misc, assignment] + + +def streamable_http_transport( + url: str, headers: dict[str, Any] | None = None, auth: httpx.Auth | None = None +) -> AbstractAsyncContextManager[Any]: + """Open a streamable HTTP client transport on either `mcp` major line. + + `mcp` 2.x replaced `streamablehttp_client(url, headers=..., auth=...)` with + `streamable_http_client(url, http_client=...)`, which takes a + pre-configured HTTPX client instead of loose header and auth kwargs. This + adapter keeps the 1.x-style call shape for both lines. + + The imports are resolved at call time because each name exists on only + one major line. + + Args: + url: The MCP server endpoint URL. + headers: Optional HTTP headers to send with each request. + auth: Optional HTTPX authentication handler for each request. + + Returns: + An async context manager yielding the transport's read/write streams. + """ + if MCP_V2: + from mcp.client.streamable_http import create_mcp_http_client, streamable_http_client + + return streamable_http_client(url=url, http_client=create_mcp_http_client(headers=headers, auth=auth)) + + from mcp.client.streamable_http import streamablehttp_client + + return streamablehttp_client(url=url, headers=headers, auth=auth) diff --git a/strands-py/src/strands/tools/mcp/mcp_client.py b/strands-py/src/strands/tools/mcp/mcp_client.py index 04468be6b5..a37814acf3 100644 --- a/strands-py/src/strands/tools/mcp/mcp_client.py +++ b/strands-py/src/strands/tools/mcp/mcp_client.py @@ -32,12 +32,9 @@ import httpx from mcp import ClientSession, ListToolsResult, StdioServerParameters, stdio_client from mcp.client.auth.extensions.client_credentials import ClientCredentialsOAuthProvider -from mcp.client.session import ElicitationFnT +from mcp.client.session import ElicitationFnT, ProgressFnT from mcp.client.sse import sse_client -from mcp.client.streamable_http import streamablehttp_client from mcp.shared.auth import OAuthClientInformationFull, OAuthToken -from mcp.shared.exceptions import McpError -from mcp.shared.session import ProgressFnT from mcp.types import ( BlobResourceContents, CancelledNotification, @@ -64,6 +61,7 @@ from ...types.media import ImageFormat from ...types.tools import AgentTool, ToolResultContent, ToolResultStatus from ..tool_provider import ToolProvider +from ._compat import MCPError, streamable_http_transport from .mcp_agent_tool import MCPAgentTool from .mcp_instrumentation import inject_trace_context, mcp_instrumentation from .mcp_tasks import DEFAULT_TASK_CONFIG, DEFAULT_TASK_POLL_TIMEOUT, DEFAULT_TASK_TTL, TasksConfig @@ -982,7 +980,7 @@ def _handle_tool_execution_error(self, tool_use_id: str, exception: Exception) - MCPToolResult: Error result containing either the elicitation data or the original exception message. """ - if isinstance(exception, McpError) and exception.error.code == -32042: + if isinstance(exception, MCPError) and exception.error.code == -32042: try: error_data = ElicitationRequiredErrorData.model_validate(exception.error.data) elicitations = [e.model_dump(exclude_none=True) for e in error_data.elicitations] @@ -1759,7 +1757,7 @@ def _resolve_transport_callable( if scheme == "http" and (auth is not None or auth_provider is not None): logger.warning("url=<%s> | sending oauth credentials over plaintext http", server_url) resolved_auth = _build_client_credentials_auth(server_url, auth) if auth is not None else auth_provider - return lambda: streamablehttp_client(url=server_url, headers=headers, auth=resolved_auth) + return lambda: streamable_http_transport(url=server_url, headers=headers, auth=resolved_auth) # Matches ${VAR} and ${env:VAR} where VAR is a valid environment variable identifier. @@ -1849,7 +1847,7 @@ def _config_transport_callable(name: str, transport: str, server: dict[str, Any] raise ValueError(f"server '{name}': streamable-http transport requires 'url'") headers = server.get("headers") resolved_auth = _parse_config_auth(name, cast(str, url), server.get("auth")) - return lambda: streamablehttp_client(url=cast(str, url), headers=headers, auth=resolved_auth) + return lambda: streamable_http_transport(url=cast(str, url), headers=headers, auth=resolved_auth) case "sse": url = server.get("url") diff --git a/strands-py/src/strands/tools/mcp/mcp_instrumentation.py b/strands-py/src/strands/tools/mcp/mcp_instrumentation.py index 85e6d16059..4dc1fa9f05 100644 --- a/strands-py/src/strands/tools/mcp/mcp_instrumentation.py +++ b/strands-py/src/strands/tools/mcp/mcp_instrumentation.py @@ -20,8 +20,6 @@ from collections.abc import AsyncGenerator, Callable from contextlib import _AsyncGeneratorContextManager, asynccontextmanager from dataclasses import dataclass -from importlib.metadata import PackageNotFoundError -from importlib.metadata import version as _package_version from typing import Any from mcp.shared.message import SessionMessage @@ -29,11 +27,13 @@ from opentelemetry import context, propagate from wrapt import ObjectProxy, register_post_import_hook, wrap_function_wrapper +from ._compat import MCP_V2 + logger = logging.getLogger(__name__) # Module-level flag to ensure instrumentation is applied only once. The lock -# makes the check-and-set atomic: `_is_mcp_v1` performs GIL-releasing I/O, so -# without it concurrent MCPClient construction could stack duplicate wrappers. +# makes the check-and-set atomic so concurrent MCPClient construction cannot +# stack duplicate wrappers. _instrumentation_applied = False _instrumentation_lock = threading.Lock() @@ -64,20 +64,6 @@ def inject_trace_context(meta: dict[str, Any] | None) -> dict[str, Any] | None: return carrier or None -def _is_mcp_v1() -> bool: - """Report whether the installed `mcp` package is on the 1.x line. - - The server-side patches wrap private `mcp` internals that are only stable - within 1.x. When the version cannot be determined, the patches are skipped - rather than applied to an unknown internal surface. - """ - try: - major_version = int(_package_version("mcp").split(".")[0]) - except (PackageNotFoundError, ValueError): - return False - return major_version == 1 - - @dataclass(slots=True, frozen=True) class ItemWithContext: """Wrapper for items that need to carry OpenTelemetry context. @@ -120,11 +106,9 @@ def mcp_instrumentation() -> None: # Return early if instrumentation has already been applied if _instrumentation_applied: return - # Set before the version probe: it leaves the GIL, and a concurrent - # caller checking under the lock must see the flag. _instrumentation_applied = True - if not _is_mcp_v1(): + if MCP_V2: return def transport_wrapper() -> Callable[ diff --git a/strands-py/src/strands/tools/mcp/mcp_types.py b/strands-py/src/strands/tools/mcp/mcp_types.py index 34ad921b66..777d548cbd 100644 --- a/strands-py/src/strands/tools/mcp/mcp_types.py +++ b/strands-py/src/strands/tools/mcp/mcp_types.py @@ -4,12 +4,12 @@ from typing import Any, Literal from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream -from mcp.client.streamable_http import GetSessionIdCallback from mcp.shared.memory import MessageStream from mcp.shared.message import SessionMessage from typing_extensions import NotRequired, TypedDict from ...types.tools import ToolResult +from ._compat import GetSessionIdCallback class MCPClientCredentials(TypedDict): diff --git a/strands-py/tests/strands/tools/mcp/test__compat.py b/strands-py/tests/strands/tools/mcp/test__compat.py new file mode 100644 index 0000000000..4f9259ec3b --- /dev/null +++ b/strands-py/tests/strands/tools/mcp/test__compat.py @@ -0,0 +1,54 @@ +"""Unit tests for the mcp 1.x/2.x compatibility layer.""" + +from unittest.mock import MagicMock, patch + +from strands.tools.mcp import _compat +from strands.tools.mcp._compat import MCPError, streamable_http_transport + + +def test_mcp_v2_flag_matches_discover_capability(): + """Test that the flag reflects whether ClientSession has the 2.x discover API.""" + from mcp import ClientSession + + assert _compat.MCP_V2 is hasattr(ClientSession, "discover") + + +def test_mcp_error_resolves_to_installed_exception(): + """Test that MCPError is the mcp package's error type regardless of its spelling.""" + import mcp.shared.exceptions as mcp_exceptions + + installed = getattr(mcp_exceptions, "MCPError", None) or mcp_exceptions.McpError + assert MCPError is installed + + +def test_streamable_http_transport_v1_call_shape(): + """Test that the 1.x transport receives url, headers, and auth as loose kwargs.""" + headers = {"Authorization": "Bearer token"} + auth = MagicMock() + + with ( + patch.object(_compat, "MCP_V2", False), + patch("mcp.client.streamable_http.streamablehttp_client", create=True) as mock_client, + ): + result = streamable_http_transport("https://example.com/mcp", headers=headers, auth=auth) + + mock_client.assert_called_once_with(url="https://example.com/mcp", headers=headers, auth=auth) + assert result is mock_client.return_value + + +def test_streamable_http_transport_v2_call_shape(): + """Test that the 2.x transport receives a pre-configured HTTP client carrying the headers and auth.""" + headers = {"Authorization": "Bearer token"} + auth = MagicMock() + http_client = MagicMock() + + with ( + patch.object(_compat, "MCP_V2", True), + patch("mcp.client.streamable_http.streamable_http_client", create=True) as mock_client, + patch("mcp.client.streamable_http.create_mcp_http_client", create=True, return_value=http_client) as mock_http, + ): + result = streamable_http_transport("https://example.com/mcp", headers=headers, auth=auth) + + mock_http.assert_called_once_with(headers=headers, auth=auth) + mock_client.assert_called_once_with(url="https://example.com/mcp", http_client=http_client) + assert result is mock_client.return_value diff --git a/strands-py/tests/strands/tools/mcp/test_mcp_client.py b/strands-py/tests/strands/tools/mcp/test_mcp_client.py index 777b6307a4..7d9fd3ccc8 100644 --- a/strands-py/tests/strands/tools/mcp/test_mcp_client.py +++ b/strands-py/tests/strands/tools/mcp/test_mcp_client.py @@ -1406,9 +1406,10 @@ async def test_handle_error_message_with_percent_in_message(): def test_call_tool_sync_elicitation_error(mock_transport, mock_session): """Test that call_tool_sync correctly handles elicitation required errors.""" - from mcp.shared.exceptions import McpError from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams + from strands.tools.mcp._compat import MCPError + elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1417,7 +1418,7 @@ def test_call_tool_sync_elicitation_error(mock_transport, mock_session): ] ) - error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1434,9 +1435,10 @@ def test_call_tool_sync_elicitation_error(mock_transport, mock_session): def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_session): """Test that call_tool_sync correctly handles elicitation errors with multiple elicitations.""" - from mcp.shared.exceptions import McpError from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams + from strands.tools.mcp._compat import MCPError + elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1448,7 +1450,7 @@ def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_ses ] ) - error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1468,13 +1470,14 @@ def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_ses def test_call_tool_sync_elicitation_error_no_urls(mock_transport, mock_session): """Test that -32042 error with empty URL still returns generic elicitation result.""" - from mcp.shared.exceptions import McpError from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams + from strands.tools.mcp._compat import MCPError + elicitation_data = ElicitationRequiredErrorData( elicitations=[ElicitRequestURLParams(url="", message="No URL provided", elicitationId="elicit-1")] ) - error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1486,10 +1489,10 @@ def test_call_tool_sync_elicitation_error_no_urls(mock_transport, mock_session): def test_call_tool_sync_other_mcp_error_code(mock_transport, mock_session): - """Test that non-32042 McpError falls through to generic error.""" - from mcp.shared.exceptions import McpError + """Test that non-32042 MCPError falls through to generic error.""" + from strands.tools.mcp._compat import MCPError - error = McpError(error=MagicMock(code=-32600, message="Invalid request")) + error = MCPError(error=MagicMock(code=-32600, message="Invalid request")) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1500,9 +1503,9 @@ def test_call_tool_sync_other_mcp_error_code(mock_transport, mock_session): def test_call_tool_sync_elicitation_error_malformed_data(mock_transport, mock_session): """Test that -32042 with unparseable data falls through to generic error.""" - from mcp.shared.exceptions import McpError + from strands.tools.mcp._compat import MCPError - error = McpError(error=MagicMock(code=-32042, data={"garbage": True})) + error = MCPError(error=MagicMock(code=-32042, data={"garbage": True})) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1514,9 +1517,10 @@ def test_call_tool_sync_elicitation_error_malformed_data(mock_transport, mock_se @pytest.mark.asyncio async def test_call_tool_async_elicitation_error(mock_transport, mock_session): """Test that call_tool_async correctly handles elicitation required errors.""" - from mcp.shared.exceptions import McpError from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams + from strands.tools.mcp._compat import MCPError + elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1525,7 +1529,7 @@ async def test_call_tool_async_elicitation_error(mock_transport, mock_session): ] ) - error = McpError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) with MCPClient(mock_transport["transport_callable"]) as client: with ( diff --git a/strands-py/tests/strands/tools/mcp/test_mcp_client_auth.py b/strands-py/tests/strands/tools/mcp/test_mcp_client_auth.py index 49a5328b4b..5f0402727e 100644 --- a/strands-py/tests/strands/tools/mcp/test_mcp_client_auth.py +++ b/strands-py/tests/strands/tools/mcp/test_mcp_client_auth.py @@ -12,8 +12,8 @@ @pytest.fixture def streamablehttp_transport(): - """Patch streamablehttp_client as imported into mcp_client.""" - with patch("strands.tools.mcp.mcp_client.streamablehttp_client") as http: + """Patch streamable_http_transport as imported into mcp_client.""" + with patch("strands.tools.mcp.mcp_client.streamable_http_transport") as http: yield http diff --git a/strands-py/tests/strands/tools/mcp/test_mcp_client_load_servers.py b/strands-py/tests/strands/tools/mcp/test_mcp_client_load_servers.py index 5e38562045..d4d68553cc 100644 --- a/strands-py/tests/strands/tools/mcp/test_mcp_client_load_servers.py +++ b/strands-py/tests/strands/tools/mcp/test_mcp_client_load_servers.py @@ -28,7 +28,7 @@ def transports(): """Patch the three transport constructors as imported into mcp_client.""" with ( patch("strands.tools.mcp.mcp_client.stdio_client") as stdio, - patch("strands.tools.mcp.mcp_client.streamablehttp_client") as http, + patch("strands.tools.mcp.mcp_client.streamable_http_transport") as http, patch("strands.tools.mcp.mcp_client.sse_client") as sse, patch("strands.tools.mcp.mcp_client.StdioServerParameters") as params, ): diff --git a/strands-py/tests/strands/tools/mcp/test_mcp_instrumentation.py b/strands-py/tests/strands/tools/mcp/test_mcp_instrumentation.py index e35300217f..aa7217d4e5 100644 --- a/strands-py/tests/strands/tools/mcp/test_mcp_instrumentation.py +++ b/strands-py/tests/strands/tools/mcp/test_mcp_instrumentation.py @@ -1,6 +1,4 @@ import threading -import time -from importlib.metadata import PackageNotFoundError from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -8,7 +6,6 @@ from mcp.types import JSONRPCMessage, JSONRPCRequest from opentelemetry import context, propagate -import strands.tools.mcp.mcp_instrumentation as mcp_instrumentation_module from strands.tools.mcp.mcp_client import MCPClient from strands.tools.mcp.mcp_instrumentation import ( ItemWithContext, @@ -31,35 +28,6 @@ def reset_mcp_instrumentation(): mcp_inst._instrumentation_applied = False -class TestIsMcpV1: - @pytest.mark.parametrize( - ("installed_version", "expected"), - [ - ("1.23.0", True), - ("1.30.0", True), - ("2.0.0", False), - ("2.0.0b1", False), - ], - ) - def test_version_detection(self, installed_version, expected): - """Test that the major version of the installed mcp package is detected correctly.""" - with patch("strands.tools.mcp.mcp_instrumentation._package_version", return_value=installed_version): - assert mcp_instrumentation_module._is_mcp_v1() is expected - - def test_unparseable_version_skips(self): - """Test that an unparseable version reports non-1.x so no patches are applied.""" - with patch("strands.tools.mcp.mcp_instrumentation._package_version", return_value="unknown"): - assert mcp_instrumentation_module._is_mcp_v1() is False - - def test_missing_package_skips(self): - """Test that a missing mcp distribution reports non-1.x so no patches are applied.""" - with patch( - "strands.tools.mcp.mcp_instrumentation._package_version", - side_effect=PackageNotFoundError("mcp"), - ): - assert mcp_instrumentation_module._is_mcp_v1() is False - - class TestInjectTraceContext: def test_injects_context_into_empty_meta(self): """Test that trace context is injected when no metadata is supplied.""" @@ -465,7 +433,7 @@ def test_mcp_instrumentation_registers_server_side_hooks(self): def test_mcp_instrumentation_skips_patches_on_mcp_v2(self): """Test that the server-side patches are not applied when mcp 2.x is installed.""" with ( - patch("strands.tools.mcp.mcp_instrumentation._is_mcp_v1", return_value=False), + patch("strands.tools.mcp.mcp_instrumentation.MCP_V2", True), patch("strands.tools.mcp.mcp_instrumentation.register_post_import_hook") as mock_register, ): mcp_instrumentation() @@ -475,17 +443,11 @@ def test_mcp_instrumentation_skips_patches_on_mcp_v2(self): def test_mcp_instrumentation_applies_once_under_concurrency(self): """Test that concurrent callers cannot apply the patches more than once. - Guards https://github.com/strands-agents/harness-sdk/pull/3611#discussion_r3706469411: the - version probe does GIL-releasing I/O, so an unlocked check-and-set let concurrent MCPClient - construction stack duplicate wrappers. + Guards https://github.com/strands-agents/harness-sdk/pull/3611#discussion_r3706469411: an + unlocked check-and-set let concurrent MCPClient construction stack duplicate wrappers. """ - - def slow_is_mcp_v1(): - time.sleep(0.01) - return True - with ( - patch("strands.tools.mcp.mcp_instrumentation._is_mcp_v1", side_effect=slow_is_mcp_v1), + patch("strands.tools.mcp.mcp_instrumentation.MCP_V2", False), patch("strands.tools.mcp.mcp_instrumentation.register_post_import_hook") as mock_register, ): threads = [threading.Thread(target=mcp_instrumentation) for _ in range(8)] @@ -499,7 +461,7 @@ def slow_is_mcp_v1(): def test_mcp_instrumentation_skip_is_sticky(self): """Test that a skipped application still marks instrumentation as applied.""" - with patch("strands.tools.mcp.mcp_instrumentation._is_mcp_v1", return_value=False): + with patch("strands.tools.mcp.mcp_instrumentation.MCP_V2", True): mcp_instrumentation() with patch("strands.tools.mcp.mcp_instrumentation.register_post_import_hook") as mock_register: From 6dbc9ca834d09b250a460492bf16c89dddefbc61 Mon Sep 17 00:00:00 2001 From: poshinchen Date: Mon, 10 Aug 2026 11:56:58 -0400 Subject: [PATCH 2/4] fix(mcp): close the owned HTTPX client and type-check _compat on both mcp lines Address review findings on the compat layer: - mcp 2.x's streamable_http_client only closes an HTTPX client it created itself, so the adapter now binds the client's lifetime to the transport's via a wrapping context manager. The 1.x branch is unchanged. - mypy sees only the installed mcp line, so each try/except branch carries its own ignore and a per-module warn_unused_ignores override silences the branch the installed line doesn't take. - Branch tests no longer patch with create=True (which invents missing attributes and passes against any spelling); each branch test patches real attributes and skips on the other line, and a hasattr test checks the installed module directly. - Test error construction goes through a make_mcp_error helper because the two lines have different constructors: 2.x MCPError(code, message, data), 1.x McpError(ErrorData). The .error accessor the production code reads is identical on both. Related to #1659 --- strands-py/pyproject.toml | 6 ++ strands-py/src/strands/tools/mcp/_compat.py | 42 +++++++--- .../tests/strands/tools/mcp/conftest.py | 12 +++ .../tests/strands/tools/mcp/test__compat.py | 84 ++++++++++++++----- .../strands/tools/mcp/test_mcp_client.py | 26 ++---- 5 files changed, 121 insertions(+), 49 deletions(-) diff --git a/strands-py/pyproject.toml b/strands-py/pyproject.toml index 8d69f83186..f24d8ab992 100644 --- a/strands-py/pyproject.toml +++ b/strands-py/pyproject.toml @@ -243,6 +243,12 @@ module = [ ] ignore_missing_imports = true +[[tool.mypy.overrides]] +# _compat branches on which mcp major line is installed; mypy only ever sees +# one, so each branch's ignores are "unused" when checked against the other. +module = ["strands.tools.mcp._compat"] +warn_unused_ignores = false + [tool.ruff] line-length = 120 include = ["examples/**/*.py", "src/**/*.py", "tests/**/*.py", "tests_integ/**/*.py"] diff --git a/strands-py/src/strands/tools/mcp/_compat.py b/strands-py/src/strands/tools/mcp/_compat.py index 29139dd26e..fea9d4af4d 100644 --- a/strands-py/src/strands/tools/mcp/_compat.py +++ b/strands-py/src/strands/tools/mcp/_compat.py @@ -5,9 +5,16 @@ directly so the rest of the codebase stays version-agnostic. Branch on `MCP_V2` only where behavior differs between the two lines; pure renames are resolved here once. + +mypy note: only one `mcp` line is installed at a time, so each try/except +branch below is an `attr-defined` error when checked against the other line. +The branch-level ignores cover whichever line mypy runs under, and the +per-module `warn_unused_ignores` override in pyproject.toml silences the +ignores the installed line doesn't need. """ -from contextlib import AbstractAsyncContextManager +from collections.abc import AsyncIterator +from contextlib import AbstractAsyncContextManager, asynccontextmanager from typing import Any import httpx @@ -21,13 +28,13 @@ MCP_V2: bool = hasattr(ClientSession, "discover") try: - from mcp.shared.exceptions import MCPError + from mcp.shared.exceptions import MCPError # type: ignore[attr-defined] except ImportError: # mcp 1.x spells the class McpError from mcp.shared.exceptions import McpError as MCPError # type: ignore[attr-defined, no-redef] try: - from mcp.client.streamable_http import GetSessionIdCallback + from mcp.client.streamable_http import GetSessionIdCallback # type: ignore[attr-defined] except ImportError: # mcp 2.x removed protocol sessions, so its transports never yield a # session-id callback; the alias survives only to type 1.x transports. @@ -58,10 +65,25 @@ def streamable_http_transport( An async context manager yielding the transport's read/write streams. """ if MCP_V2: - from mcp.client.streamable_http import create_mcp_http_client, streamable_http_client - - return streamable_http_client(url=url, http_client=create_mcp_http_client(headers=headers, auth=auth)) - - from mcp.client.streamable_http import streamablehttp_client - - return streamablehttp_client(url=url, headers=headers, auth=auth) + from mcp.client.streamable_http import ( # type: ignore[attr-defined] + create_mcp_http_client, + streamable_http_client, + ) + + # `streamable_http_client` closes an HTTPX client only when it created + # it (`client_provided` check in mcp 2.x), so a caller-provided client + # must be closed by the caller: enter both context managers together + # so the client's lifetime is bound to the transport's. + @asynccontextmanager + async def _owned_client_transport() -> AsyncIterator[Any]: + async with ( + create_mcp_http_client(headers=headers, auth=auth) as http_client, + streamable_http_client(url=url, http_client=http_client) as transport_streams, + ): + yield transport_streams + + return _owned_client_transport() + + from mcp.client.streamable_http import streamablehttp_client # type: ignore[attr-defined] + + return streamablehttp_client(url=url, headers=headers, auth=auth) # type: ignore[no-any-return] diff --git a/strands-py/tests/strands/tools/mcp/conftest.py b/strands-py/tests/strands/tools/mcp/conftest.py index d0ac46bdc1..528c6ae6dd 100644 --- a/strands-py/tests/strands/tools/mcp/conftest.py +++ b/strands-py/tests/strands/tools/mcp/conftest.py @@ -1,8 +1,20 @@ """Shared fixtures and helpers for MCP client tests.""" +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest +from mcp.types import ErrorData + +from strands.tools.mcp import _compat +from strands.tools.mcp._compat import MCPError + + +def make_mcp_error(code: int, message: str = "", data: Any = None) -> Exception: + """Construct the installed line's MCP error: 2.x takes (code, message, data), 1.x takes ErrorData.""" + if _compat.MCP_V2: + return MCPError(code, message, data=data) + return MCPError(error=ErrorData(code=code, message=message, data=data)) @pytest.fixture diff --git a/strands-py/tests/strands/tools/mcp/test__compat.py b/strands-py/tests/strands/tools/mcp/test__compat.py index 4f9259ec3b..a757ceccc0 100644 --- a/strands-py/tests/strands/tools/mcp/test__compat.py +++ b/strands-py/tests/strands/tools/mcp/test__compat.py @@ -1,16 +1,37 @@ -"""Unit tests for the mcp 1.x/2.x compatibility layer.""" +"""Unit tests for the mcp 1.x/2.x compatibility layer. +The branch-specific tests patch real attributes of the installed `mcp` +package (never `create=True`, which would invent missing names and pass +against any spelling), so each test runs only on the line whose names exist +and is skipped on the other. +""" + +from contextlib import asynccontextmanager from unittest.mock import MagicMock, patch +import pytest + from strands.tools.mcp import _compat from strands.tools.mcp._compat import MCPError, streamable_http_transport +requires_mcp_v1 = pytest.mark.skipif(_compat.MCP_V2, reason="exercises the mcp 1.x branch") +requires_mcp_v2 = pytest.mark.skipif(not _compat.MCP_V2, reason="exercises the mcp 2.x branch") + + +def test_installed_line_exposes_expected_transport_names(): + """Test that the names each branch imports exist on the line the flag selects. -def test_mcp_v2_flag_matches_discover_capability(): - """Test that the flag reflects whether ClientSession has the 2.x discover API.""" - from mcp import ClientSession + Directional on purpose: late 1.x releases backport `streamable_http_client` + as an alias, so only the branch actually taken is asserted against the + installed module. + """ + import mcp.client.streamable_http as streamable_http_module - assert _compat.MCP_V2 is hasattr(ClientSession, "discover") + if _compat.MCP_V2: + assert hasattr(streamable_http_module, "streamable_http_client") + assert hasattr(streamable_http_module, "create_mcp_http_client") + else: + assert hasattr(streamable_http_module, "streamablehttp_client") def test_mcp_error_resolves_to_installed_exception(): @@ -21,34 +42,55 @@ def test_mcp_error_resolves_to_installed_exception(): assert MCPError is installed +@requires_mcp_v1 def test_streamable_http_transport_v1_call_shape(): """Test that the 1.x transport receives url, headers, and auth as loose kwargs.""" headers = {"Authorization": "Bearer token"} auth = MagicMock() - with ( - patch.object(_compat, "MCP_V2", False), - patch("mcp.client.streamable_http.streamablehttp_client", create=True) as mock_client, - ): + with patch("mcp.client.streamable_http.streamablehttp_client") as mock_client: result = streamable_http_transport("https://example.com/mcp", headers=headers, auth=auth) mock_client.assert_called_once_with(url="https://example.com/mcp", headers=headers, auth=auth) assert result is mock_client.return_value -def test_streamable_http_transport_v2_call_shape(): - """Test that the 2.x transport receives a pre-configured HTTP client carrying the headers and auth.""" - headers = {"Authorization": "Bearer token"} +@requires_mcp_v2 +@pytest.mark.asyncio +async def test_streamable_http_transport_v2_owns_client_lifecycle(): + """Test that the 2.x transport closes the HTTPX client it creates. + + Guards https://github.com/strands-agents/harness-sdk/pull/3708: 2.x's + `streamable_http_client` only closes a client it created itself, so the + adapter must bind the client's lifetime to the transport's. + """ + lifecycle_events = [] + transport_streams = MagicMock() auth = MagicMock() - http_client = MagicMock() + @asynccontextmanager + async def fake_http_client(headers=None, auth=None): + lifecycle_events.append(("client_enter", headers, auth)) + yield MagicMock() + lifecycle_events.append(("client_exit", headers, auth)) + + @asynccontextmanager + async def fake_transport(url, http_client): + lifecycle_events.append(("transport_enter", url)) + yield transport_streams + lifecycle_events.append(("transport_exit", url)) + + headers = {"Authorization": "Bearer token"} with ( - patch.object(_compat, "MCP_V2", True), - patch("mcp.client.streamable_http.streamable_http_client", create=True) as mock_client, - patch("mcp.client.streamable_http.create_mcp_http_client", create=True, return_value=http_client) as mock_http, + patch("mcp.client.streamable_http.create_mcp_http_client", fake_http_client), + patch("mcp.client.streamable_http.streamable_http_client", fake_transport), ): - result = streamable_http_transport("https://example.com/mcp", headers=headers, auth=auth) - - mock_http.assert_called_once_with(headers=headers, auth=auth) - mock_client.assert_called_once_with(url="https://example.com/mcp", http_client=http_client) - assert result is mock_client.return_value + async with streamable_http_transport("https://example.com/mcp", headers=headers, auth=auth) as streams: + assert streams is transport_streams + + assert lifecycle_events == [ + ("client_enter", headers, auth), + ("transport_enter", "https://example.com/mcp"), + ("transport_exit", "https://example.com/mcp"), + ("client_exit", headers, auth), + ] diff --git a/strands-py/tests/strands/tools/mcp/test_mcp_client.py b/strands-py/tests/strands/tools/mcp/test_mcp_client.py index 7d9fd3ccc8..df4de80494 100644 --- a/strands-py/tests/strands/tools/mcp/test_mcp_client.py +++ b/strands-py/tests/strands/tools/mcp/test_mcp_client.py @@ -27,6 +27,8 @@ from strands.tools.mcp.mcp_types import MCPToolResult from strands.types.exceptions import MCPClientInitializationError +from .conftest import make_mcp_error + # Fixtures mock_transport and mock_session are imported from conftest.py @@ -1408,8 +1410,6 @@ def test_call_tool_sync_elicitation_error(mock_transport, mock_session): """Test that call_tool_sync correctly handles elicitation required errors.""" from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams - from strands.tools.mcp._compat import MCPError - elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1418,7 +1418,7 @@ def test_call_tool_sync_elicitation_error(mock_transport, mock_session): ] ) - error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = make_mcp_error(code=-32042, data=elicitation_data.model_dump()) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1437,8 +1437,6 @@ def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_ses """Test that call_tool_sync correctly handles elicitation errors with multiple elicitations.""" from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams - from strands.tools.mcp._compat import MCPError - elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1450,7 +1448,7 @@ def test_call_tool_sync_elicitation_error_multiple_urls(mock_transport, mock_ses ] ) - error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = make_mcp_error(code=-32042, data=elicitation_data.model_dump()) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1472,12 +1470,10 @@ def test_call_tool_sync_elicitation_error_no_urls(mock_transport, mock_session): """Test that -32042 error with empty URL still returns generic elicitation result.""" from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams - from strands.tools.mcp._compat import MCPError - elicitation_data = ElicitationRequiredErrorData( elicitations=[ElicitRequestURLParams(url="", message="No URL provided", elicitationId="elicit-1")] ) - error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = make_mcp_error(code=-32042, data=elicitation_data.model_dump()) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1490,9 +1486,7 @@ def test_call_tool_sync_elicitation_error_no_urls(mock_transport, mock_session): def test_call_tool_sync_other_mcp_error_code(mock_transport, mock_session): """Test that non-32042 MCPError falls through to generic error.""" - from strands.tools.mcp._compat import MCPError - - error = MCPError(error=MagicMock(code=-32600, message="Invalid request")) + error = make_mcp_error(code=-32600, message="Invalid request") mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1503,9 +1497,7 @@ def test_call_tool_sync_other_mcp_error_code(mock_transport, mock_session): def test_call_tool_sync_elicitation_error_malformed_data(mock_transport, mock_session): """Test that -32042 with unparseable data falls through to generic error.""" - from strands.tools.mcp._compat import MCPError - - error = MCPError(error=MagicMock(code=-32042, data={"garbage": True})) + error = make_mcp_error(code=-32042, data={"garbage": True}) mock_session.call_tool.side_effect = error with MCPClient(mock_transport["transport_callable"]) as client: @@ -1519,8 +1511,6 @@ async def test_call_tool_async_elicitation_error(mock_transport, mock_session): """Test that call_tool_async correctly handles elicitation required errors.""" from mcp.types import ElicitationRequiredErrorData, ElicitRequestURLParams - from strands.tools.mcp._compat import MCPError - elicitation_data = ElicitationRequiredErrorData( elicitations=[ ElicitRequestURLParams( @@ -1529,7 +1519,7 @@ async def test_call_tool_async_elicitation_error(mock_transport, mock_session): ] ) - error = MCPError(error=MagicMock(code=-32042, data=elicitation_data.model_dump())) + error = make_mcp_error(code=-32042, data=elicitation_data.model_dump()) with MCPClient(mock_transport["transport_callable"]) as client: with ( From 13bca5025431ffa3b645e3ed1e07b8dd329bf437 Mon Sep 17 00:00:00 2001 From: poshinchen Date: Tue, 11 Aug 2026 11:12:02 -0400 Subject: [PATCH 3/4] test(mcp): exercise the 2.x compat branches on the 1.x test line Late 1.x mcp releases backport the 2.x transport names, so the client lifecycle test now gates on name availability instead of the installed line and forces the MCP_V2 flag. A reload test covers the GetSessionIdCallback fallback. This brings _compat.py to full line and branch coverage under the mcp<2 pin CI runs with. --- .../tests/strands/tools/mcp/test__compat.py | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/strands-py/tests/strands/tools/mcp/test__compat.py b/strands-py/tests/strands/tools/mcp/test__compat.py index a757ceccc0..27f19d934d 100644 --- a/strands-py/tests/strands/tools/mcp/test__compat.py +++ b/strands-py/tests/strands/tools/mcp/test__compat.py @@ -2,20 +2,28 @@ The branch-specific tests patch real attributes of the installed `mcp` package (never `create=True`, which would invent missing names and pass -against any spelling), so each test runs only on the line whose names exist -and is skipped on the other. +against any spelling), so each test is gated on the names it patches +actually existing on the installed line. Late 1.x releases backport the +2.x transport names, so the 2.x-branch transport test runs there too with +the `MCP_V2` flag forced. """ +import importlib +from collections.abc import Callable from contextlib import asynccontextmanager from unittest.mock import MagicMock, patch +import mcp.client.streamable_http as streamable_http_module import pytest from strands.tools.mcp import _compat from strands.tools.mcp._compat import MCPError, streamable_http_transport requires_mcp_v1 = pytest.mark.skipif(_compat.MCP_V2, reason="exercises the mcp 1.x branch") -requires_mcp_v2 = pytest.mark.skipif(not _compat.MCP_V2, reason="exercises the mcp 2.x branch") +requires_v2_transport_names = pytest.mark.skipif( + not hasattr(streamable_http_module, "streamable_http_client"), + reason="installed mcp line lacks the 2.x transport names", +) def test_installed_line_exposes_expected_transport_names(): @@ -42,6 +50,22 @@ def test_mcp_error_resolves_to_installed_exception(): assert MCPError is installed +def test_get_session_id_callback_falls_back_to_plain_callable(monkeypatch): + """Test that `GetSessionIdCallback` degrades to a plain callable alias when the name is absent. + + Reloads `_compat` with the name deleted from the installed transport + module to exercise the 2.x fallback on either line, then reloads again so + later tests see the module as built against the real environment. + """ + monkeypatch.delattr(streamable_http_module, "GetSessionIdCallback", raising=False) + try: + reloaded = importlib.reload(_compat) + assert reloaded.GetSessionIdCallback == Callable[[], str | None] + finally: + monkeypatch.undo() + importlib.reload(_compat) + + @requires_mcp_v1 def test_streamable_http_transport_v1_call_shape(): """Test that the 1.x transport receives url, headers, and auth as loose kwargs.""" @@ -55,15 +79,16 @@ def test_streamable_http_transport_v1_call_shape(): assert result is mock_client.return_value -@requires_mcp_v2 +@requires_v2_transport_names @pytest.mark.asyncio -async def test_streamable_http_transport_v2_owns_client_lifecycle(): +async def test_streamable_http_transport_v2_owns_client_lifecycle(monkeypatch): """Test that the 2.x transport closes the HTTPX client it creates. Guards https://github.com/strands-agents/harness-sdk/pull/3708: 2.x's `streamable_http_client` only closes a client it created itself, so the adapter must bind the client's lifetime to the transport's. """ + monkeypatch.setattr(_compat, "MCP_V2", True) lifecycle_events = [] transport_streams = MagicMock() auth = MagicMock() From 53aa5a1d18ae949960c3f044cf145959ef16743f Mon Sep 17 00:00:00 2001 From: poshinchen Date: Tue, 11 Aug 2026 11:23:32 -0400 Subject: [PATCH 4/4] ci(python): verify the package against the mcp 2.x line The unit-test matrix only ever exercises mcp 1.x under the <2 pin, so nothing in CI catches a module-level import of a 1.x-only name breaking import strands on 2.x. This job force-installs mcp 2.0.* over the pin, asserts the package imports with MCP_V2 set, and runs the _compat tests so their 2.x-gated paths execute against the real package instead of mocks. It runs on the PR gate, push to main, and the release gate via python-test-lint.yml, and uploads no coverage. --- .github/workflows/python-test-lint.yml | 43 +++++++++++++++++++++ strands-py/src/strands/tools/mcp/_compat.py | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/.github/workflows/python-test-lint.yml b/.github/workflows/python-test-lint.yml index d0ba1f4689..cfdd731d4e 100644 --- a/.github/workflows/python-test-lint.yml +++ b/.github/workflows/python-test-lint.yml @@ -88,6 +88,49 @@ jobs: uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7 with: token: ${{ secrets.CODECOV_TOKEN }} + mcp-v2-compat: + name: MCP 2.x Compat + # The dependency pin is `mcp<2`, so the unit-test matrix only ever + # exercises the 1.x line and the 2.x branches of `_compat` run against + # mocks there. This job force-installs the 2.x line over the pin to verify + # against the real package that `import strands` succeeds and the compat + # layer resolves the 2.x names. Scoped to the compat tests; the full suite + # is not expected to pass on 2.x while the pin holds. Pinned to `2.0.*` so + # upstream 2.x releases cannot break unrelated PRs; bump deliberately. + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + defaults: + run: + working-directory: strands-py + steps: + - name: Checkout code + uses: actions/checkout@v7 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@v7.0.0 + with: + python-version: '3.10' + + - name: Install package, then force mcp 2.x over the pin + run: | + pip install --no-cache-dir -e . \ + "pytest>=9.0.0,<10.0.0" \ + "pytest-asyncio>=1.0.0,<1.5.0" \ + "pytest-timeout>=2.0.0,<3.0.0" \ + "moto>=5.1.0,<6.0.0" + pip install --no-cache-dir "mcp==2.0.*" + + - name: Verify import and version flag + run: python -c "import strands; from strands.tools.mcp import _compat; assert _compat.MCP_V2" + + - name: Run compat tests + run: pytest tests/strands/tools/mcp/test__compat.py -vv + lint: name: Lint runs-on: ubuntu-latest diff --git a/strands-py/src/strands/tools/mcp/_compat.py b/strands-py/src/strands/tools/mcp/_compat.py index fea9d4af4d..f7309dfc23 100644 --- a/strands-py/src/strands/tools/mcp/_compat.py +++ b/strands-py/src/strands/tools/mcp/_compat.py @@ -44,7 +44,7 @@ def streamable_http_transport( - url: str, headers: dict[str, Any] | None = None, auth: httpx.Auth | None = None + url: str, headers: dict[str, str] | None = None, auth: httpx.Auth | None = None ) -> AbstractAsyncContextManager[Any]: """Open a streamable HTTP client transport on either `mcp` major line.