fix(code): drop duplicate token-request client_id under Basic auth (#4323)

Fix MCP OAuth login failures with servers that reject a duplicate
`client_id` under HTTP Basic client authentication (e.g. Pylon).

---

MCP OAuth login fails against servers like Pylon because the MCP Python
SDK includes `client_id` in the token-request body *and* in the
`Authorization: Basic` header when `token_endpoint_auth_method ==
"client_secret_basic"`. Such servers reject the duplicate client
identity with an `OAuthTokenError`. This wraps `prepare_token_auth` in
`_ExpiryAwareOAuthClientProvider` to strip the redundant body
`client_id` only when a Basic header is present (RFC 6749 §2.3.1),
leaving `client_secret_post`/`none` flows untouched. This replaces the
manual token-file workaround of editing `token_endpoint_auth_method` to
`client_secret_post`.

Made by [Open
SWE](https://openswe.vercel.app/agents/122e4418-1b0a-8162-81dc-9d4b7e953de0)

## References
- Plan:
https://openswe.vercel.app/agents/122e4418-1b0a-8162-81dc-9d4b7e953de0/plan

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

libs/code/deepagents_code/mcp_auth.py

@@ -24,7 +24,7 @@ import stat
 import threading
 import time
 from collections.abc import AsyncGenerator, Awaitable, Callable
-from typing import TYPE_CHECKING, Literal, TypedDict
+from typing import TYPE_CHECKING, Any, Literal, TypedDict
 from urllib.parse import parse_qs, urlparse
 
 import httpx
@@ -47,6 +47,8 @@ from pydantic import BaseModel, ConfigDict, ValidationError
 if TYPE_CHECKING:
     from pathlib import Path
 
+    from mcp.client.auth.oauth2 import OAuthContext
+
     from deepagents_code.mcp_oauth_ui import OAuthInteraction
 
 
@@ -1046,6 +1048,35 @@ def _append_query_params(url: str, params: dict[str, str]) -> str:
     return urlunparse(parsed._replace(query=urlencode(existing, doseq=True)))
 
 
+def _strip_duplicate_client_id_under_basic_auth(context: OAuthContext) -> None:
+    """Drop the redundant body `client_id` when token auth uses HTTP Basic.
+
+    The MCP SDK copies `client_id` into the token-request body (on both the
+    authorization-code exchange and refresh paths) and, for
+    `token_endpoint_auth_method == "client_secret_basic"`, *also* sends it in the
+    `Authorization: Basic` header. RFC 6749 §2.3.1 carries the client identity in
+    the header for Basic auth, so the body copy is redundant; some authorization
+    servers (e.g. Pylon) reject the duplicate identity with an `OAuthTokenError`.
+    Wrapping `prepare_token_auth` strips the body `client_id` only when a Basic
+    header is present, leaving `client_secret_post`/`none` flows untouched.
+    """
+    original = context.prepare_token_auth
+
+    def prepare_token_auth(
+        data: dict[str, str],
+        headers: dict[str, str] | None = None,
+    ) -> tuple[dict[str, str], dict[str, str]]:
+        data, headers = original(data, headers)
+        # RFC 7617 makes the auth-scheme token case-insensitive, so match
+        # `basic` regardless of casing rather than coupling to the SDK's exact
+        # `Basic ` literal.
+        if headers.get("Authorization", "").lower().startswith("basic "):
+            data = {k: v for k, v in data.items() if k != "client_id"}
+        return data, headers
+
+    context.prepare_token_auth = prepare_token_auth  # ty: ignore[invalid-assignment]
+
+
 class _ExpiryAwareOAuthClientProvider(OAuthClientProvider):
     """`OAuthClientProvider` that restores `token_expiry_time` from storage.
 
@@ -1063,6 +1094,10 @@ class _ExpiryAwareOAuthClientProvider(OAuthClientProvider):
     falling back to 401.
     """
 
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        super().__init__(*args, **kwargs)
+        _strip_duplicate_client_id_under_basic_auth(self.context)
+
     async def _initialize(self) -> None:
         # Overrides a leading-underscore SDK method; behavior depends on
         # `super()._initialize()` populating `context.current_tokens` from

libs/code/tests/unit_tests/test_mcp_auth.py

@@ -8,7 +8,7 @@ import logging
 import re
 import time
 from pathlib import Path
-from typing import Any
+from typing import Any, Literal
 from unittest.mock import patch
 
 import pytest
@@ -105,6 +105,23 @@ def _make_oauth_metadata(token_endpoint: str = "https://auth.example/token"):
     )
 
 
+def _make_client_info_with_secret(
+    auth_method: Literal["client_secret_basic", "client_secret_post", "none"],
+):
+    from mcp.shared.auth import AnyUrl, OAuthClientInformationFull
+
+    # Public clients (`none`) carry no secret; confidential clients do.
+    client_secret = None if auth_method == "none" else "client-secret"
+    return OAuthClientInformationFull(
+        client_id="client-id",
+        client_secret=client_secret,
+        token_endpoint_auth_method=auth_method,
+        redirect_uris=[AnyUrl("http://localhost/callback")],
+        grant_types=["authorization_code", "refresh_token"],
+        response_types=["code"],
+    )
+
+
 def _make_client_info_with_loopback(port: int):
     from mcp.shared.auth import AnyUrl, OAuthClientInformationFull
 
@@ -544,6 +561,109 @@ class TestExpiryAwareOAuthClientProvider:
         await flow.aclose()
 
 
+@pytest.mark.usefixtures("fake_home")
+class TestBasicAuthClientIdStripping:
+    """Tests for dropping the duplicate body `client_id` under HTTP Basic auth."""
+
+    def _build_provider(
+        self,
+        auth_method: Literal["client_secret_basic", "client_secret_post", "none"],
+    ):
+        from deepagents_code.mcp_auth import build_oauth_provider
+
+        storage = FileTokenStorage("pylon")
+        provider = build_oauth_provider(
+            server_name="pylon",
+            server_url="https://mcp.usepylon.com/mcp",
+            storage=storage,
+            interactive=False,
+        )
+        provider.context.client_info = _make_client_info_with_secret(auth_method)
+        return provider
+
+    def test_basic_auth_drops_body_client_id(self) -> None:
+        """`client_secret_basic` carries credentials in the header, not the body.
+
+        This wrapper strips the redundant body `client_id`. The SDK itself
+        already strips `client_secret` for Basic auth, which the final
+        assertion pins (see `test_sdk_still_injects_client_id_under_basic_auth`
+        for the contract the wrapper depends on).
+        """
+        provider = self._build_provider("client_secret_basic")
+
+        data, headers = provider.context.prepare_token_auth(
+            {
+                "grant_type": "authorization_code",
+                "client_id": "client-id",
+                "client_secret": "client-secret",
+            },
+            {"Content-Type": "application/x-www-form-urlencoded"},
+        )
+
+        assert headers["Authorization"].startswith("Basic ")
+        assert "client_id" not in data
+        assert "client_secret" not in data  # stripped by the SDK, not this wrapper
+
+    def test_post_auth_retains_body_client_id(self) -> None:
+        """`client_secret_post` keeps both fields in the body and adds no header."""
+        provider = self._build_provider("client_secret_post")
+
+        data, headers = provider.context.prepare_token_auth(
+            {
+                "grant_type": "authorization_code",
+                "client_id": "client-id",
+            },
+            {"Content-Type": "application/x-www-form-urlencoded"},
+        )
+
+        assert "Authorization" not in headers
+        assert data["client_id"] == "client-id"
+        assert data["client_secret"] == "client-secret"
+
+    def test_none_auth_retains_body_client_id(self) -> None:
+        """`none` (public client) sends no header, so the body keeps `client_id`."""
+        provider = self._build_provider("none")
+
+        data, headers = provider.context.prepare_token_auth(
+            {
+                "grant_type": "authorization_code",
+                "client_id": "client-id",
+            },
+            {"Content-Type": "application/x-www-form-urlencoded"},
+        )
+
+        assert "Authorization" not in headers
+        assert data["client_id"] == "client-id"
+
+    def test_sdk_still_injects_client_id_under_basic_auth(self) -> None:
+        """Pin the SDK contract this wrapper depends on.
+
+        Unwrapped, the SDK leaves `client_id` in the token-request body under
+        Basic auth (it strips only `client_secret`). If upstream ever strips
+        `client_id` too, this wrapper becomes a silent no-op; this test fails
+        loudly instead, flagging the workaround as obsolete.
+        """
+        from mcp.client.auth.oauth2 import OAuthContext
+
+        provider = self._build_provider("client_secret_basic")
+
+        # Call the SDK's method via the class to bypass the instance-level wrap
+        # installed in `__init__` and observe the SDK's own behavior.
+        data, headers = OAuthContext.prepare_token_auth(
+            provider.context,
+            {
+                "grant_type": "authorization_code",
+                "client_id": "client-id",
+                "client_secret": "client-secret",
+            },
+            {"Content-Type": "application/x-www-form-urlencoded"},
+        )
+
+        assert headers["Authorization"].startswith("Basic ")
+        assert data["client_id"] == "client-id"
+        assert "client_secret" not in data
+
+
 class TestFindReauthRequired:
     """Tests for unwrapping nested re-auth errors."""