diff --git a/src/adcp/protocols/mcp.py b/src/adcp/protocols/mcp.py index 251172d8a..9837101ff 100644 --- a/src/adcp/protocols/mcp.py +++ b/src/adcp/protocols/mcp.py @@ -353,13 +353,17 @@ async def _get_session(self) -> ClientSession: if self.agent_config.extra_headers: headers.update(self.agent_config.extra_headers) - # Try the user's exact URL first - urls_to_try = [self.agent_config.agent_uri] - - # If URL doesn't end with /mcp, also try with /mcp suffix - if not self.agent_config.agent_uri.rstrip("/").endswith("/mcp"): - base_uri = self.agent_config.agent_uri.rstrip("/") - urls_to_try.append(f"{base_uri}/mcp") + # Try the user's exact URL first, then the alternate slash form, then + # /mcp discovery paths. MCP servers disagree on whether their endpoint + # is at /mcp or /mcp/ — try both rather than silently normalizing. + uri = self.agent_config.agent_uri + base = uri.rstrip("/") + urls_to_try = [uri] + if base.endswith("/mcp"): + # User pointed at the MCP endpoint; also try the other slash form. + urls_to_try.append(f"{base}/" if not uri.endswith("/") else base) + else: + urls_to_try.extend([f"{base}/mcp", f"{base}/mcp/"]) # RFC 9421 auto-signing: if ADCPClient installed a signing request # hook, wire it into streamable_http via a custom httpx client diff --git a/src/adcp/signing/capability_cache.py b/src/adcp/signing/capability_cache.py index 96b233f6c..728b3f60d 100644 --- a/src/adcp/signing/capability_cache.py +++ b/src/adcp/signing/capability_cache.py @@ -148,10 +148,17 @@ def build_capability_cache_key( still transmit only the original caller's token (the cache key isn't the auth credential itself). - Format matches the JS SDK exactly: + Format matches the JS SDK: ``agent_uri[::sha256(auth_token)[:16]][::sig=signer_fingerprint]`` + + Slash handling: ``agent_uri`` is rstripped of trailing slashes before being + used as the key prefix. The Python ``AgentConfig`` validator preserves the + caller-supplied URI form, so this normalization happens at the cache layer + to ensure ``http://host/mcp`` and ``http://host/mcp/`` resolve to the same + entry. JS SDK callers normalize at upstream call sites; the resulting + cache-key string for a given logical agent is identical across SDKs. """ - parts = [agent_uri] + parts = [agent_uri.rstrip("/")] if auth_token: token_digest = hashlib.sha256(auth_token.encode("utf-8")).hexdigest()[:16] parts.append(f"::{token_digest}") diff --git a/src/adcp/types/core.py b/src/adcp/types/core.py index 863f82397..98e0a0bff 100644 --- a/src/adcp/types/core.py +++ b/src/adcp/types/core.py @@ -61,8 +61,7 @@ def validate_agent_uri(cls, v: str) -> str: "Example: https://agent.example.com" ) - # Remove trailing slash for consistency - return v.rstrip("/") + return v @field_validator("timeout") @classmethod diff --git a/tests/test_capability_cache.py b/tests/test_capability_cache.py index 83f3e5b2c..865709c4b 100644 --- a/tests/test_capability_cache.py +++ b/tests/test_capability_cache.py @@ -128,6 +128,31 @@ def test_cache_key_distinguishes_different_tokens() -> None: assert a != b +@pytest.mark.parametrize( + "uri_with_slash,uri_without_slash", + [ + ("https://x/mcp/", "https://x/mcp"), + ("https://x/", "https://x"), + ("https://x/api/mcp/", "https://x/api/mcp"), + ], +) +def test_cache_key_normalizes_trailing_slash(uri_with_slash: str, uri_without_slash: str) -> None: + """Trailing-slash variants of the same agent_uri must produce the same cache key. + + AgentConfig.validate_agent_uri preserves the caller-supplied URI form (including + trailing slash) so MCP transport can try both /mcp and /mcp/ on connect. Without + normalization here, a single logical agent would split-brain across two cache + entries depending on which slash form the caller passed. + """ + assert build_capability_cache_key(uri_with_slash) == build_capability_cache_key(uri_without_slash) + assert build_capability_cache_key(uri_with_slash, auth_token="t") == build_capability_cache_key( + uri_without_slash, auth_token="t" + ) + assert build_capability_cache_key( + uri_with_slash, signer_fingerprint="fp" + ) == build_capability_cache_key(uri_without_slash, signer_fingerprint="fp") + + # ----- _unwrap_response (transport shape unwrapping) ----- diff --git a/tests/test_client.py b/tests/test_client.py index d486686af..f8794465c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -20,6 +20,20 @@ def test_agent_config_creation(): assert config.protocol == Protocol.A2A +@pytest.mark.parametrize( + "input_uri,expected_uri", + [ + ("https://example.com/mcp/", "https://example.com/mcp/"), + ("https://example.com/mcp", "https://example.com/mcp"), + ("https://example.com", "https://example.com"), + ("https://example.com/", "https://example.com/"), + ], +) +def test_agent_uri_preserves_user_supplied_form(input_uri: str, expected_uri: str) -> None: + cfg = AgentConfig(id="x", agent_uri=input_uri, protocol=Protocol.MCP) + assert cfg.agent_uri == expected_uri + + def test_agent_config_extra_headers_default_empty(): config = AgentConfig( id="test_agent", diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 33fc2be16..49bfcdb57 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -62,8 +62,7 @@ def test_mcp_config_structure(): """Test TEST_AGENT_MCP_CONFIG has correct structure.""" assert TEST_AGENT_MCP_CONFIG.id == "test-agent-mcp" assert TEST_AGENT_MCP_CONFIG.protocol == Protocol.MCP - # AgentConfig validator strips trailing slashes for consistency - assert TEST_AGENT_MCP_CONFIG.agent_uri == "https://test-agent.adcontextprotocol.org/mcp" + assert TEST_AGENT_MCP_CONFIG.agent_uri == "https://test-agent.adcontextprotocol.org/mcp/" assert TEST_AGENT_MCP_CONFIG.auth_token is not None @@ -195,7 +194,7 @@ def test_mcp_no_auth_config_structure(): """Test TEST_AGENT_MCP_NO_AUTH_CONFIG has correct structure.""" assert TEST_AGENT_MCP_NO_AUTH_CONFIG.id == "test-agent-mcp-no-auth" assert TEST_AGENT_MCP_NO_AUTH_CONFIG.protocol == Protocol.MCP - assert TEST_AGENT_MCP_NO_AUTH_CONFIG.agent_uri == "https://test-agent.adcontextprotocol.org/mcp" + assert TEST_AGENT_MCP_NO_AUTH_CONFIG.agent_uri == "https://test-agent.adcontextprotocol.org/mcp/" assert TEST_AGENT_MCP_NO_AUTH_CONFIG.auth_token is None diff --git a/tests/test_protocols.py b/tests/test_protocols.py index 3f2bf8ec9..8e4a16d5e 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -1832,6 +1832,66 @@ async def test_cleanup_handles_exception_group_with_cancelled_error(self, mcp_co assert adapter._session is None +class TestMCPUrlFallback: + """Tests for the MCP URL fallback list built in _get_session.""" + + @pytest.mark.parametrize( + "agent_uri,expected_urls", + [ + # Slash-terminated /mcp/ — also try no-slash form + ( + "https://host/mcp/", + ["https://host/mcp/", "https://host/mcp"], + ), + # No-slash /mcp — also try slash form + ( + "https://host/mcp", + ["https://host/mcp", "https://host/mcp/"], + ), + # Bare host — discovery: try both /mcp and /mcp/ + ( + "https://host", + ["https://host", "https://host/mcp", "https://host/mcp/"], + ), + # Host with trailing slash — discovery: try both /mcp and /mcp/ + ( + "https://host/", + ["https://host/", "https://host/mcp", "https://host/mcp/"], + ), + ], + ) + @pytest.mark.asyncio + async def test_urls_to_try(self, agent_uri: str, expected_urls: list[str]) -> None: + from unittest.mock import patch + + from adcp.protocols.mcp import MCPAdapter + from adcp.types.core import AgentConfig, Protocol + + cfg = AgentConfig(id="t", agent_uri=agent_uri, protocol=Protocol.MCP) + adapter = MCPAdapter(cfg) + + real_urls: list[str] = [] + + class _FakeCM: + async def __aenter__(self) -> None: + raise ConnectionError("abort") + + async def __aexit__(self, *_: object) -> None: + pass + + def capture_url(url: str, **_kw: object) -> _FakeCM: + real_urls.append(url) + return _FakeCM() + + with patch("adcp.protocols.mcp.streamablehttp_client", side_effect=capture_url): + try: + await adapter._get_session() + except Exception: + pass + + assert real_urls == expected_urls + + class TestFromMcpClientFactory: """Tests for ADCPClient.from_mcp_client() factory method."""