From bd6da7f8d2e3e549fbbba5e9a69a4302446a8dde Mon Sep 17 00:00:00 2001
From: rholinshead <5060851+rholinshead@users.noreply.github.com>
Date: Mon, 3 Nov 2025 18:52:32 -0500
Subject: [PATCH 1/5] Updates & tests for json serialized urls in config

---
 src/mcp_agent/oauth/flow.py            |   2 +-
 src/mcp_agent/oauth/metadata.py        |  11 +-
 src/mcp_agent/server/token_verifier.py |  12 +-
 tests/test_oauth_utils.py              | 172 +++++++++++++++++++++++++
 tests/test_token_verifier.py           | 107 +++++++++++++++
 5 files changed, 295 insertions(+), 9 deletions(-)

diff --git a/src/mcp_agent/oauth/flow.py b/src/mcp_agent/oauth/flow.py
index e2c523044..5488aecb8 100644
--- a/src/mcp_agent/oauth/flow.py
+++ b/src/mcp_agent/oauth/flow.py
@@ -136,7 +136,7 @@ async def authorize(
         import urllib.parse
 
         authorize_url = httpx.URL(
-            str(auth_metadata.authorization_endpoint)
+            str(auth_metadata.authorization_endpoint).rstrip("/")
             + "?"
             + urllib.parse.urlencode(params)
         )
diff --git a/src/mcp_agent/oauth/metadata.py b/src/mcp_agent/oauth/metadata.py
index 7d2c974c4..0b6c62353 100644
--- a/src/mcp_agent/oauth/metadata.py
+++ b/src/mcp_agent/oauth/metadata.py
@@ -69,14 +69,19 @@ def select_authorization_server(
             "Protected resource metadata did not include authorization servers"
         )
 
-    if preferred and preferred in candidates:
-        return preferred
-
     if preferred:
+        preferred_normalized = preferred.rstrip("/")
+        candidates_normalized = [c.rstrip("/") for c in candidates]
+
+        for i, candidate_normalized in enumerate(candidates_normalized):
+            if candidate_normalized == preferred_normalized:
+                return candidates[i]
+
         logger.warning(
             "Preferred authorization server not listed; falling back to first entry",
             data={"preferred": preferred, "candidates": candidates},
         )
+
     return candidates[0]
 
 
diff --git a/src/mcp_agent/server/token_verifier.py b/src/mcp_agent/server/token_verifier.py
index 4fff7fbc9..308db50ac 100644
--- a/src/mcp_agent/server/token_verifier.py
+++ b/src/mcp_agent/server/token_verifier.py
@@ -155,12 +155,14 @@ async def _introspect(self, token: str) -> MCPAccessToken | None:
             return None
 
         if self._settings.issuer_url and payload.get("iss"):
-            if str(payload.get("iss")) != str(self._settings.issuer_url):
+            expected_issuer = str(self._settings.issuer_url).rstrip("/")
+            actual_issuer = str(payload.get("iss")).rstrip("/")
+            if actual_issuer != expected_issuer:
                 logger.warning(
                     "Token issuer mismatch",
                     data={
-                        "expected": str(self._settings.issuer_url),
-                        "actual": payload.get("iss"),
+                        "expected": expected_issuer,
+                        "actual": actual_issuer,
                     },
                 )
                 return None
@@ -241,8 +243,8 @@ def _validate_audiences(self, token_audiences: List[str]) -> bool:
             return False
 
         # RFC 9068: Token MUST contain at least one expected audience
-        valid_audiences = set(self._settings.expected_audiences)
-        token_audience_set = set(token_audiences)
+        valid_audiences = set(aud.rstrip("/") for aud in self._settings.expected_audiences)
+        token_audience_set = set(aud.rstrip("/") for aud in token_audiences)
 
         if not valid_audiences.intersection(token_audience_set):
             logger.warning(
diff --git a/tests/test_oauth_utils.py b/tests/test_oauth_utils.py
index 581608524..28b844f47 100644
--- a/tests/test_oauth_utils.py
+++ b/tests/test_oauth_utils.py
@@ -75,6 +75,67 @@ def test_select_authorization_server_prefers_explicit():
     )
 
 
+def test_select_authorization_server_with_serialized_config():
+    """Test that authorization server selection works after config json serialization.
+
+    When MCPOAuthClientSettings is dumped with mode='json', the authorization_server
+    AnyHttpUrl field gets a trailing slash. This test ensures select_authorization_server
+    (metadata.py:72) handles this correctly.
+    """
+    from mcp_agent.config import MCPOAuthClientSettings
+
+    # Create config with authorization_server
+    oauth_config = MCPOAuthClientSettings(
+        enabled=True,
+        authorization_server="https://auth.example.com",
+        resource="https://api.example.com",
+        client_id="test_client",
+    )
+
+    # Simulate json serialization which adds trailing slash
+    dumped = oauth_config.model_dump(mode="json")
+    reloaded_config = MCPOAuthClientSettings(**dumped)
+
+    # Create metadata where authorization servers have trailing slashes
+    # (Pydantic normalizes them)
+    metadata = ProtectedResourceMetadata(
+        resource="https://api.example.com",
+        authorization_servers=[
+            "https://auth.example.com/",  # With trailing slash
+            "https://other-auth.example.com/",
+        ],
+    )
+
+    # Convert the reloaded config's authorization_server to string
+    preferred = str(reloaded_config.authorization_server) if reloaded_config.authorization_server else None
+
+    # Should match even if trailing slashes don't align perfectly
+    selected = select_authorization_server(metadata, preferred)
+
+    # Should select the preferred one
+    assert selected.rstrip("/") == "https://auth.example.com"
+
+
+def test_select_authorization_server_trailing_slash_mismatch():
+    """Test trailing slash handling in select_authorization_server with various combinations."""
+    # Test case 1: preferred has trailing slash, candidates don't
+    metadata1 = ProtectedResourceMetadata(
+        resource="https://api.example.com",
+        authorization_servers=["https://auth.example.com", "https://other.example.com"],
+    )
+    # Pydantic will add trailing slashes to the candidates, but let's test the logic
+    selected1 = select_authorization_server(metadata1, "https://auth.example.com/")
+    assert selected1.rstrip("/") == "https://auth.example.com"
+
+    # Test case 2: preferred doesn't have trailing slash, candidates do
+    metadata2 = ProtectedResourceMetadata(
+        resource="https://api.example.com",
+        authorization_servers=["https://auth.example.com/", "https://other.example.com/"],
+    )
+    selected2 = select_authorization_server(metadata2, "https://auth.example.com")
+    assert selected2.rstrip("/") == "https://auth.example.com"
+
+
 def test_normalize_resource_with_fallback():
     assert (
         normalize_resource("https://example.com/api", None) == "https://example.com/api"
@@ -99,6 +160,38 @@ def test_oauth_loopback_ports_config_defaults():
     assert 33418 in s.loopback_ports
 
 
+def test_oauth_callback_base_url_with_serialized_config():
+    """Test that callback_base_url works correctly after json serialization.
+
+    When OAuthSettings is dumped with mode='json', the callback_base_url AnyHttpUrl
+    field gets a trailing slash. The code in flow.py:75 uses rstrip('/') to handle this,
+    and this test ensures it works correctly.
+    """
+    from mcp_agent.config import OAuthSettings
+
+    # Create settings with callback_base_url
+    settings = OAuthSettings(
+        callback_base_url="https://callback.example.com"
+    )
+
+    # Simulate json serialization which adds trailing slash
+    dumped = settings.model_dump(mode="json")
+    reloaded = OAuthSettings(**dumped)
+
+    # The code in flow.py uses: f"{str(self._settings.callback_base_url).rstrip('/')}/internal/oauth/callback/{flow_id}"
+    # Let's verify the URL is constructed correctly
+    flow_id = "test_flow_123"
+    if reloaded.callback_base_url:
+        constructed_url = f"{str(reloaded.callback_base_url).rstrip('/')}/internal/oauth/callback/{flow_id}"
+
+        # Should not have double slashes
+        assert "//" not in constructed_url.replace("https://", "")
+        # Should end with the flow_id
+        assert constructed_url.endswith(flow_id)
+        # Should have the correct base
+        assert constructed_url.startswith("https://callback.example.com/")
+
+
 @pytest.mark.asyncio
 async def test_callback_registry_state_mapping():
     from mcp_agent.oauth.callbacks import OAuthCallbackRegistry
@@ -110,3 +203,82 @@ async def test_callback_registry_state_mapping():
     assert delivered is True
     result = await asyncio.wait_for(fut, timeout=0.2)
     assert result["code"] == "abc"
+
+
+@pytest.mark.asyncio
+async def test_authorization_url_construction_with_trailing_slash():
+    """Test that authorization URL is constructed correctly when endpoint has trailing slash."""
+    from mcp_agent.oauth.flow import AuthorizationFlowCoordinator
+    from mcp_agent.config import OAuthSettings, MCPOAuthClientSettings
+    from mcp_agent.core.context import Context
+    from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata
+    from mcp.server.session import ServerSession
+    from unittest.mock import AsyncMock, MagicMock, patch
+    import httpx
+
+    # Create settings
+    oauth_settings = OAuthSettings()
+
+    # Create a mock context
+    context = MagicMock(spec=Context)
+
+    # Create a mock user
+    from mcp_agent.oauth.identity import OAuthUserIdentity
+    user = OAuthUserIdentity(subject="user123", provider="test")
+
+    # Create OAuth config
+    oauth_config = MCPOAuthClientSettings(
+        enabled=True,
+        client_id="test_client",
+        authorization_server="https://auth.example.com",
+        resource="https://api.example.com",
+    )
+
+    # Create metadata with trailing slashes (simulating json serialization)
+    resource_metadata = ProtectedResourceMetadata(
+        resource="https://api.example.com/",
+        authorization_servers=["https://auth.example.com/"],
+    )
+
+    auth_metadata = OAuthMetadata(
+        issuer="https://auth.example.com/",
+        authorization_endpoint="https://auth.example.com/authorize/",
+        token_endpoint="https://auth.example.com/token/",
+    )
+
+    # Create flow coordinator
+    http_client = httpx.AsyncClient()
+    flow = AuthorizationFlowCoordinator(http_client=http_client, settings=oauth_settings)
+
+    # Mock _send_auth_request to capture the request payload
+    captured_payload = None
+    async def mock_send_auth_request(ctx, payload):
+        nonlocal captured_payload
+        captured_payload = payload
+        # Simulate user declining to test the flow without needing real callback
+        raise Exception("test_exception")
+
+    with patch("mcp_agent.oauth.flow._send_auth_request", side_effect=mock_send_auth_request):
+        try:
+            await flow.authorize(
+                context=context,
+                user=user,
+                server_name="test_server",
+                oauth_config=oauth_config,
+                resource="https://api.example.com",
+                authorization_server_url="https://auth.example.com",
+                resource_metadata=resource_metadata,
+                auth_metadata=auth_metadata,
+                scopes=["read"],
+            )
+        except Exception:
+            pass  # Expected to fail due to mock
+
+    await http_client.aclose()
+
+    # Verify the URL doesn't have double slashes before query params
+    assert captured_payload is not None
+    url = captured_payload["url"]
+    assert "authorize/?" not in url
+    assert "authorize?" in url
+    assert url.startswith("https://auth.example.com/authorize?")
diff --git a/tests/test_token_verifier.py b/tests/test_token_verifier.py
index b611687f9..35ccd7042 100644
--- a/tests/test_token_verifier.py
+++ b/tests/test_token_verifier.py
@@ -855,3 +855,110 @@ async def test_audience_validation_failure_through_introspect():
     assert token is None
 
     await verifier.aclose()
+
+
+@pytest.mark.asyncio
+async def test_issuer_comparison_with_trailing_slash_from_token():
+    """Test that issuer comparison works when token has trailing slash.
+
+    When config is loaded/dumped with mode='json', AnyHttpUrl fields may gain
+    trailing slashes. This test ensures the issuer comparison in token_verifier.py:158
+    handles this correctly.
+    """
+    # Create settings and simulate json serialization (which adds trailing slash)
+    settings = MCPAuthorizationServerSettings(
+        enabled=True,
+        issuer_url="https://auth.example.com",
+        resource_server_url="https://api.example.com",
+        expected_audiences=["https://api.example.com"],
+    )
+
+    # Dump with mode="json" and reload to simulate config loading
+    dumped = settings.model_dump(mode="json")
+    reloaded_settings = MCPAuthorizationServerSettings(**dumped)
+
+    verifier = MCPAgentTokenVerifier(reloaded_settings)
+
+    # Mock well-known metadata
+    metadata_response = Mock()
+    metadata_response.status_code = 200
+    metadata_response.json.return_value = {
+        "issuer": "https://auth.example.com",
+        "authorization_endpoint": "https://auth.example.com/authorize",
+        "token_endpoint": "https://auth.example.com/token",
+        "introspection_endpoint": "https://auth.example.com/introspect",
+        "response_types_supported": ["code"],
+    }
+
+    # Mock introspection response where issuer has trailing slash
+    # (OAuth servers often add trailing slashes)
+    introspect_response = Mock()
+    introspect_response.status_code = 200
+    introspect_response.json.return_value = {
+        "active": True,
+        "aud": "https://api.example.com",
+        "sub": "user123",
+        "exp": 9999999999,
+        "iss": "https://auth.example.com/",  # Trailing slash from OAuth server
+    }
+
+    verifier._client.get = AsyncMock(return_value=metadata_response)
+    verifier._client.post = AsyncMock(return_value=introspect_response)
+
+    token = await verifier._introspect("test_token")
+
+    # Should succeed even with trailing slash mismatch
+    assert token is not None
+    assert token.subject == "user123"
+
+    await verifier.aclose()
+
+
+@pytest.mark.asyncio
+async def test_issuer_comparison_config_trailing_slash_token_without():
+    """Test issuer comparison when config has trailing slash but token doesn't."""
+    # Create settings and simulate json serialization
+    settings = MCPAuthorizationServerSettings(
+        enabled=True,
+        issuer_url="https://auth.example.com",
+        resource_server_url="https://api.example.com",
+        expected_audiences=["https://api.example.com"],
+    )
+
+    dumped = settings.model_dump(mode="json")
+    reloaded_settings = MCPAuthorizationServerSettings(**dumped)
+
+    verifier = MCPAgentTokenVerifier(reloaded_settings)
+
+    # Mock responses
+    metadata_response = Mock()
+    metadata_response.status_code = 200
+    metadata_response.json.return_value = {
+        "issuer": "https://auth.example.com",
+        "authorization_endpoint": "https://auth.example.com/authorize",
+        "token_endpoint": "https://auth.example.com/token",
+        "introspection_endpoint": "https://auth.example.com/introspect",
+        "response_types_supported": ["code"],
+    }
+
+    # Token issuer WITHOUT trailing slash
+    introspect_response = Mock()
+    introspect_response.status_code = 200
+    introspect_response.json.return_value = {
+        "active": True,
+        "aud": "https://api.example.com",
+        "sub": "user123",
+        "exp": 9999999999,
+        "iss": "https://auth.example.com",  # No trailing slash
+    }
+
+    verifier._client.get = AsyncMock(return_value=metadata_response)
+    verifier._client.post = AsyncMock(return_value=introspect_response)
+
+    token = await verifier._introspect("test_token")
+
+    # Should succeed
+    assert token is not None
+    assert token.subject == "user123"
+
+    await verifier.aclose()

From fa1d297b1ba50beed77e27e3448083087f57fd78 Mon Sep 17 00:00:00 2001
From: rholinshead <5060851+rholinshead@users.noreply.github.com>
Date: Mon, 3 Nov 2025 21:59:55 -0500
Subject: [PATCH 2/5] make lint and format

---
 src/mcp_agent/server/token_verifier.py |  4 +++-
 tests/test_oauth_utils.py              | 28 +++++++++++++++++---------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/mcp_agent/server/token_verifier.py b/src/mcp_agent/server/token_verifier.py
index 308db50ac..f5637e361 100644
--- a/src/mcp_agent/server/token_verifier.py
+++ b/src/mcp_agent/server/token_verifier.py
@@ -243,7 +243,9 @@ def _validate_audiences(self, token_audiences: List[str]) -> bool:
             return False
 
         # RFC 9068: Token MUST contain at least one expected audience
-        valid_audiences = set(aud.rstrip("/") for aud in self._settings.expected_audiences)
+        valid_audiences = set(
+            aud.rstrip("/") for aud in self._settings.expected_audiences
+        )
         token_audience_set = set(aud.rstrip("/") for aud in token_audiences)
 
         if not valid_audiences.intersection(token_audience_set):
diff --git a/tests/test_oauth_utils.py b/tests/test_oauth_utils.py
index 28b844f47..77aabb81f 100644
--- a/tests/test_oauth_utils.py
+++ b/tests/test_oauth_utils.py
@@ -107,7 +107,11 @@ def test_select_authorization_server_with_serialized_config():
     )
 
     # Convert the reloaded config's authorization_server to string
-    preferred = str(reloaded_config.authorization_server) if reloaded_config.authorization_server else None
+    preferred = (
+        str(reloaded_config.authorization_server)
+        if reloaded_config.authorization_server
+        else None
+    )
 
     # Should match even if trailing slashes don't align perfectly
     selected = select_authorization_server(metadata, preferred)
@@ -130,7 +134,10 @@ def test_select_authorization_server_trailing_slash_mismatch():
     # Test case 2: preferred doesn't have trailing slash, candidates do
     metadata2 = ProtectedResourceMetadata(
         resource="https://api.example.com",
-        authorization_servers=["https://auth.example.com/", "https://other.example.com/"],
+        authorization_servers=[
+            "https://auth.example.com/",
+            "https://other.example.com/",
+        ],
     )
     selected2 = select_authorization_server(metadata2, "https://auth.example.com")
     assert selected2.rstrip("/") == "https://auth.example.com"
@@ -170,9 +177,7 @@ def test_oauth_callback_base_url_with_serialized_config():
     from mcp_agent.config import OAuthSettings
 
     # Create settings with callback_base_url
-    settings = OAuthSettings(
-        callback_base_url="https://callback.example.com"
-    )
+    settings = OAuthSettings(callback_base_url="https://callback.example.com")
 
     # Simulate json serialization which adds trailing slash
     dumped = settings.model_dump(mode="json")
@@ -212,8 +217,7 @@ async def test_authorization_url_construction_with_trailing_slash():
     from mcp_agent.config import OAuthSettings, MCPOAuthClientSettings
     from mcp_agent.core.context import Context
     from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata
-    from mcp.server.session import ServerSession
-    from unittest.mock import AsyncMock, MagicMock, patch
+    from unittest.mock import MagicMock, patch
     import httpx
 
     # Create settings
@@ -224,6 +228,7 @@ async def test_authorization_url_construction_with_trailing_slash():
 
     # Create a mock user
     from mcp_agent.oauth.identity import OAuthUserIdentity
+
     user = OAuthUserIdentity(subject="user123", provider="test")
 
     # Create OAuth config
@@ -248,17 +253,22 @@ async def test_authorization_url_construction_with_trailing_slash():
 
     # Create flow coordinator
     http_client = httpx.AsyncClient()
-    flow = AuthorizationFlowCoordinator(http_client=http_client, settings=oauth_settings)
+    flow = AuthorizationFlowCoordinator(
+        http_client=http_client, settings=oauth_settings
+    )
 
     # Mock _send_auth_request to capture the request payload
     captured_payload = None
+
     async def mock_send_auth_request(ctx, payload):
         nonlocal captured_payload
         captured_payload = payload
         # Simulate user declining to test the flow without needing real callback
         raise Exception("test_exception")
 
-    with patch("mcp_agent.oauth.flow._send_auth_request", side_effect=mock_send_auth_request):
+    with patch(
+        "mcp_agent.oauth.flow._send_auth_request", side_effect=mock_send_auth_request
+    ):
         try:
             await flow.authorize(
                 context=context,

From d647ab441fc5e248f63aa901fd4b3f2e6e21af1a Mon Sep 17 00:00:00 2001
From: rholinshead <5060851+rholinshead@users.noreply.github.com>
Date: Tue, 4 Nov 2025 10:49:14 -0500
Subject: [PATCH 3/5] Fix pylint

---
 tests/test_oauth_utils.py | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tests/test_oauth_utils.py b/tests/test_oauth_utils.py
index 77aabb81f..66b474b4f 100644
--- a/tests/test_oauth_utils.py
+++ b/tests/test_oauth_utils.py
@@ -2,6 +2,7 @@
 import asyncio
 import pathlib
 import sys
+from typing import Any, Dict
 
 import pytest
 
@@ -258,13 +259,13 @@ async def test_authorization_url_construction_with_trailing_slash():
     )
 
     # Mock _send_auth_request to capture the request payload
-    captured_payload = None
+    captured_payload: Dict[str, Any] | None = None
 
-    async def mock_send_auth_request(ctx, payload):
+    async def mock_send_auth_request(_ctx, payload: Dict[str, Any]):
         nonlocal captured_payload
         captured_payload = payload
         # Simulate user declining to test the flow without needing real callback
-        raise Exception("test_exception")
+        raise ConnectionAbortedError("test_exception")
 
     with patch(
         "mcp_agent.oauth.flow._send_auth_request", side_effect=mock_send_auth_request
@@ -281,14 +282,17 @@ async def mock_send_auth_request(ctx, payload):
                 auth_metadata=auth_metadata,
                 scopes=["read"],
             )
-        except Exception:
+        except ConnectionAbortedError:
             pass  # Expected to fail due to mock
 
     await http_client.aclose()
 
     # Verify the URL doesn't have double slashes before query params
-    assert captured_payload is not None
-    url = captured_payload["url"]
-    assert "authorize/?" not in url
-    assert "authorize?" in url
-    assert url.startswith("https://auth.example.com/authorize?")
+    assert captured_payload is not None, "captured_payload should have been set by mock"
+
+    # Type narrowing for Pylint
+    if captured_payload is not None:
+        url = captured_payload["url"]
+        assert "authorize/?" not in url
+        assert "authorize?" in url
+        assert url.startswith("https://auth.example.com/authorize?")

From 67f7790bfed9d681b6b7a29500bdfd743f1f5dc4 Mon Sep 17 00:00:00 2001
From: rholinshead <5060851+rholinshead@users.noreply.github.com>
Date: Tue, 4 Nov 2025 11:14:46 -0500
Subject: [PATCH 4/5] Clean up tests

---
 tests/test_oauth_utils.py    | 51 ++++++++----------------------------
 tests/test_token_verifier.py | 13 ++-------
 2 files changed, 13 insertions(+), 51 deletions(-)

diff --git a/tests/test_oauth_utils.py b/tests/test_oauth_utils.py
index 66b474b4f..6207f9660 100644
--- a/tests/test_oauth_utils.py
+++ b/tests/test_oauth_utils.py
@@ -81,11 +81,10 @@ def test_select_authorization_server_with_serialized_config():
 
     When MCPOAuthClientSettings is dumped with mode='json', the authorization_server
     AnyHttpUrl field gets a trailing slash. This test ensures select_authorization_server
-    (metadata.py:72) handles this correctly.
+    handles this correctly.
     """
     from mcp_agent.config import MCPOAuthClientSettings
 
-    # Create config with authorization_server
     oauth_config = MCPOAuthClientSettings(
         enabled=True,
         authorization_server="https://auth.example.com",
@@ -93,31 +92,23 @@ def test_select_authorization_server_with_serialized_config():
         client_id="test_client",
     )
 
-    # Simulate json serialization which adds trailing slash
-    dumped = oauth_config.model_dump(mode="json")
-    reloaded_config = MCPOAuthClientSettings(**dumped)
+    dumped_config = oauth_config.model_dump(mode="json")
+    reloaded_config = MCPOAuthClientSettings(**dumped_config)
 
-    # Create metadata where authorization servers have trailing slashes
-    # (Pydantic normalizes them)
     metadata = ProtectedResourceMetadata(
         resource="https://api.example.com",
         authorization_servers=[
-            "https://auth.example.com/",  # With trailing slash
-            "https://other-auth.example.com/",
+            "https://auth.example.com",
+            "https://other-auth.example.com",
         ],
     )
 
-    # Convert the reloaded config's authorization_server to string
-    preferred = (
-        str(reloaded_config.authorization_server)
-        if reloaded_config.authorization_server
-        else None
-    )
+    dumped_metadata = metadata.model_dump(mode="json")
+    reloaded_metadata = ProtectedResourceMetadata(**dumped_metadata)
 
-    # Should match even if trailing slashes don't align perfectly
-    selected = select_authorization_server(metadata, preferred)
+    preferred = str(reloaded_config.authorization_server)
+    selected = select_authorization_server(reloaded_metadata, preferred)
 
-    # Should select the preferred one
     assert selected.rstrip("/") == "https://auth.example.com"
 
 
@@ -128,7 +119,7 @@ def test_select_authorization_server_trailing_slash_mismatch():
         resource="https://api.example.com",
         authorization_servers=["https://auth.example.com", "https://other.example.com"],
     )
-    # Pydantic will add trailing slashes to the candidates, but let's test the logic
+
     selected1 = select_authorization_server(metadata1, "https://auth.example.com/")
     assert selected1.rstrip("/") == "https://auth.example.com"
 
@@ -172,29 +163,20 @@ def test_oauth_callback_base_url_with_serialized_config():
     """Test that callback_base_url works correctly after json serialization.
 
     When OAuthSettings is dumped with mode='json', the callback_base_url AnyHttpUrl
-    field gets a trailing slash. The code in flow.py:75 uses rstrip('/') to handle this,
-    and this test ensures it works correctly.
+    field gets a trailing slash.
     """
     from mcp_agent.config import OAuthSettings
 
-    # Create settings with callback_base_url
     settings = OAuthSettings(callback_base_url="https://callback.example.com")
-
-    # Simulate json serialization which adds trailing slash
     dumped = settings.model_dump(mode="json")
     reloaded = OAuthSettings(**dumped)
 
-    # The code in flow.py uses: f"{str(self._settings.callback_base_url).rstrip('/')}/internal/oauth/callback/{flow_id}"
-    # Let's verify the URL is constructed correctly
     flow_id = "test_flow_123"
     if reloaded.callback_base_url:
         constructed_url = f"{str(reloaded.callback_base_url).rstrip('/')}/internal/oauth/callback/{flow_id}"
 
-        # Should not have double slashes
         assert "//" not in constructed_url.replace("https://", "")
-        # Should end with the flow_id
         assert constructed_url.endswith(flow_id)
-        # Should have the correct base
         assert constructed_url.startswith("https://callback.example.com/")
 
 
@@ -221,18 +203,12 @@ async def test_authorization_url_construction_with_trailing_slash():
     from unittest.mock import MagicMock, patch
     import httpx
 
-    # Create settings
     oauth_settings = OAuthSettings()
-
-    # Create a mock context
     context = MagicMock(spec=Context)
-
-    # Create a mock user
     from mcp_agent.oauth.identity import OAuthUserIdentity
 
     user = OAuthUserIdentity(subject="user123", provider="test")
 
-    # Create OAuth config
     oauth_config = MCPOAuthClientSettings(
         enabled=True,
         client_id="test_client",
@@ -240,7 +216,6 @@ async def test_authorization_url_construction_with_trailing_slash():
         resource="https://api.example.com",
     )
 
-    # Create metadata with trailing slashes (simulating json serialization)
     resource_metadata = ProtectedResourceMetadata(
         resource="https://api.example.com/",
         authorization_servers=["https://auth.example.com/"],
@@ -252,13 +227,11 @@ async def test_authorization_url_construction_with_trailing_slash():
         token_endpoint="https://auth.example.com/token/",
     )
 
-    # Create flow coordinator
     http_client = httpx.AsyncClient()
     flow = AuthorizationFlowCoordinator(
         http_client=http_client, settings=oauth_settings
     )
 
-    # Mock _send_auth_request to capture the request payload
     captured_payload: Dict[str, Any] | None = None
 
     async def mock_send_auth_request(_ctx, payload: Dict[str, Any]):
@@ -286,8 +259,6 @@ async def mock_send_auth_request(_ctx, payload: Dict[str, Any]):
             pass  # Expected to fail due to mock
 
     await http_client.aclose()
-
-    # Verify the URL doesn't have double slashes before query params
     assert captured_payload is not None, "captured_payload should have been set by mock"
 
     # Type narrowing for Pylint
diff --git a/tests/test_token_verifier.py b/tests/test_token_verifier.py
index 35ccd7042..9c87e5325 100644
--- a/tests/test_token_verifier.py
+++ b/tests/test_token_verifier.py
@@ -865,7 +865,6 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
     trailing slashes. This test ensures the issuer comparison in token_verifier.py:158
     handles this correctly.
     """
-    # Create settings and simulate json serialization (which adds trailing slash)
     settings = MCPAuthorizationServerSettings(
         enabled=True,
         issuer_url="https://auth.example.com",
@@ -873,13 +872,12 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
         expected_audiences=["https://api.example.com"],
     )
 
-    # Dump with mode="json" and reload to simulate config loading
+    # Dump with mode="json" and reload to simulate config loading (with trailing slashes)
     dumped = settings.model_dump(mode="json")
     reloaded_settings = MCPAuthorizationServerSettings(**dumped)
 
     verifier = MCPAgentTokenVerifier(reloaded_settings)
 
-    # Mock well-known metadata
     metadata_response = Mock()
     metadata_response.status_code = 200
     metadata_response.json.return_value = {
@@ -890,8 +888,6 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
         "response_types_supported": ["code"],
     }
 
-    # Mock introspection response where issuer has trailing slash
-    # (OAuth servers often add trailing slashes)
     introspect_response = Mock()
     introspect_response.status_code = 200
     introspect_response.json.return_value = {
@@ -899,7 +895,7 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
         "aud": "https://api.example.com",
         "sub": "user123",
         "exp": 9999999999,
-        "iss": "https://auth.example.com/",  # Trailing slash from OAuth server
+        "iss": "https://auth.example.com",
     }
 
     verifier._client.get = AsyncMock(return_value=metadata_response)
@@ -907,7 +903,6 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
 
     token = await verifier._introspect("test_token")
 
-    # Should succeed even with trailing slash mismatch
     assert token is not None
     assert token.subject == "user123"
 
@@ -917,7 +912,6 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
 @pytest.mark.asyncio
 async def test_issuer_comparison_config_trailing_slash_token_without():
     """Test issuer comparison when config has trailing slash but token doesn't."""
-    # Create settings and simulate json serialization
     settings = MCPAuthorizationServerSettings(
         enabled=True,
         issuer_url="https://auth.example.com",
@@ -930,7 +924,6 @@ async def test_issuer_comparison_config_trailing_slash_token_without():
 
     verifier = MCPAgentTokenVerifier(reloaded_settings)
 
-    # Mock responses
     metadata_response = Mock()
     metadata_response.status_code = 200
     metadata_response.json.return_value = {
@@ -941,7 +934,6 @@ async def test_issuer_comparison_config_trailing_slash_token_without():
         "response_types_supported": ["code"],
     }
 
-    # Token issuer WITHOUT trailing slash
     introspect_response = Mock()
     introspect_response.status_code = 200
     introspect_response.json.return_value = {
@@ -957,7 +949,6 @@ async def test_issuer_comparison_config_trailing_slash_token_without():
 
     token = await verifier._introspect("test_token")
 
-    # Should succeed
     assert token is not None
     assert token.subject == "user123"
 

From c2c791d4c860249986c027578cc66bca0a51f17b Mon Sep 17 00:00:00 2001
From: rholinshead <5060851+rholinshead@users.noreply.github.com>
Date: Tue, 4 Nov 2025 12:04:34 -0500
Subject: [PATCH 5/5] Fix test

---
 tests/test_token_verifier.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_token_verifier.py b/tests/test_token_verifier.py
index 9c87e5325..3fae706cb 100644
--- a/tests/test_token_verifier.py
+++ b/tests/test_token_verifier.py
@@ -892,10 +892,10 @@ async def test_issuer_comparison_with_trailing_slash_from_token():
     introspect_response.status_code = 200
     introspect_response.json.return_value = {
         "active": True,
-        "aud": "https://api.example.com",
+        "aud": "https://api.example.com/",
         "sub": "user123",
         "exp": 9999999999,
-        "iss": "https://auth.example.com",
+        "iss": "https://auth.example.com/",  # trailing slash
     }
 
     verifier._client.get = AsyncMock(return_value=metadata_response)