Anthropic OAuth mcp_ tool-name prefix and extra-usage errors

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #47723 fix(anthropic): no single-underscore mcp_ tool names on the OAuth wire (plan-limit billing)

  merged to main

  files: agent/anthropic_adapter.py, agent/transports/anthropic.py, tests/agent/test_anthropic_mcp_prefix_strip.py

  Show the diff

  • agent/anthropic_adapter.py

    @@ -372,7 +372,7 @@ def _detect_claude_code_version() -> str:
     
     
     _CLAUDE_CODE_SYSTEM_PREFIX = "You are Claude Code, Anthropic's official CLI for Claude."
    -_MCP_TOOL_PREFIX = "mcp_"
    +_MCP_TOOL_PREFIX = "mcp__"
     
     
     def _get_claude_code_version() -> str:
    @@ -2349,25 +2349,46 @@ def build_anthropic_kwargs(
                     text = text.replace("Nous Research", "Anthropic")
                     block["text"] = text
     
    -        # 3. Prefix tool names with mcp_ (Claude Code convention)
    -        #    Skip names that already begin with the marker — native MCP server
    -        #    tools (from mcp_servers: in config.yaml) are registered under their
    -        #    full mcp_<server>_<tool> name and would double-prefix otherwise,
    -        #    breaking round-trip registry lookup in normalize_response. GH-25255.
    +        # 3. Normalize tool names so NOTHING goes on the OAuth wire with a
    +        #    single-underscore ``mcp_`` prefix.  Anthropic's subscription/OAuth
    +        #    billing classifier treats a single-underscore ``mcp_`` tool name as
    +        #    a third-party-app fingerprint and rejects the request with HTTP 400
    +        #    "Third-party apps now draw from extra usage, not plan limits"
    +        #    (verified empirically: a single ``mcp_foo`` tool flips a request
    +        #    from plan-billing to the extra-usage lane; ``mcp__foo`` is accepted).
    +        #
    +        #    Two cases, both must land on the double-underscore ``mcp__`` form:
    +        #      a) bare Hermes-native tools (``read_file``)  -> ``mcp__read_file``
    +        #      b) native MCP server tools registered under their full
    +        #         single-underscore ``mcp_<server>_<tool>`` name
    +        #         (``mcp_linear_get_issue``) -> ``mcp__linear_get_issue``
    +        #    Case (b) is the gap that the bare ``mcp_``->``mcp__`` constant swap
    +        #    left open: those tools were *skipped* and stayed single-underscore,
    +        #    so any session with an MCP server configured still tripped the
    +        #    classifier. normalize_response reverses both forms via registry
    +        #    lookup so the dispatcher still sees the original name. GH-25255.
    +        def _to_oauth_wire_name(name: str) -> str:
    +            if name.startswith("mcp__"):
    +                return name  # already correct, don't double-prefix
    +            if name.startswith("mcp_"):
    +                # single-underscore native MCP tool -> promote to double
    +                return "mcp__" + name[len("mcp_"):]
    +            return _MCP_TOOL_PREFIX + name  # bare name -> mcp__<name>
    +
             if anthropic_tools:
                 for tool in anthropic_tools:
    -                if "name" in tool and not tool["name"].startswith(_MCP_TOOL_PREFIX):
    -                    tool["name"] = _MCP_TOOL_PREFIX + tool["name"]
    +                if "name" in tool:
    +                    tool["name"] = _to_oauth_wire_name(tool["name"])
     
    -        # 4. Prefix tool names in message history (tool_use and tool_result blocks)
    +        # 4. Apply the same normalization to tool names in message history
    +        #    (tool_use blocks) so replayed turns match the wire names above.
             for msg in anthropic_messages:
                 content = msg.get("content")
                 if isinstance(content, list):
                     for block in content:
                         if isinstance(block, dict):
                             if block.get("type") == "tool_use" and "name" in block:
    -                            if not block["name"].startswith(_MCP_TOOL_PREFIX):
    -                                block["name"] = _MCP_TOOL_PREFIX + block["name"]
    +                            block["name"] = _to_oauth_wire_name(block["name"])
                             elif block.get("type") == "tool_result" and "tool_use_id" in block:
                                 pass  # tool_result uses ID, not name
     

  • agent/transports/anthropic.py

    @@ -88,7 +88,7 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse:
             from agent.transports.types import ToolCall
     
             strip_tool_prefix = kwargs.get("strip_tool_prefix", False)
    -        _MCP_PREFIX = "mcp_"
    +        _MCP_PREFIX = "mcp__"
     
             text_parts = []
             reasoning_parts = []
    @@ -132,17 +132,25 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse:
                 elif block.type == "tool_use":
                     name = block.name
                     if strip_tool_prefix and name.startswith(_MCP_PREFIX):
    -                    stripped = name[len(_MCP_PREFIX):]
    -                    # Only strip the mcp_ prefix for OAuth-injected tools
    -                    # (where Hermes adds the prefix when sending to Anthropic
    -                    # and must remove it on the way back).  Native MCP server
    -                    # tools (from mcp_servers: in config.yaml) are registered
    -                    # in the tool registry under their FULL mcp_<server>_<tool>
    -                    # name and must NOT be stripped.  GH-25255.
    +                    # On the OAuth wire every tool carries a double-underscore
    +                    # ``mcp__`` prefix (added in build_anthropic_kwargs to avoid
    +                    # Anthropic's single-underscore third-party classifier).
    +                    # Reverse it back to the name the registry/dispatcher knows.
    +                    # Two original forms map onto the same ``mcp__`` wire name:
    +                    #   ``mcp__read_file``       <- bare native tool ``read_file``
    +                    #   ``mcp__linear_get_issue`` <- MCP server tool
    +                    #                                ``mcp_linear_get_issue``
    +                    # Resolve by registry lookup, preferring whichever original
    +                    # is actually registered; never rewrite a name the LLM used
    +                    # that already resolves natively. GH-25255.
                         from tools.registry import registry as _tool_registry
    -                    if (_tool_registry.get_entry(stripped)
    -                            and not _tool_registry.get_entry(name)):
    -                        name = stripped
    +                    if not _tool_registry.get_entry(name):
    +                        bare = name[len(_MCP_PREFIX):]            # read_file
    +                        single = "mcp_" + bare                    # mcp_read_file / mcp_linear_get_issue
    +                        if _tool_registry.get_entry(single):
    +                            name = single
    +                        elif _tool_registry.get_entry(bare):
    +                            name = bare
                     tool_calls.append(
                         ToolCall(
                             id=block.id,

  • tests/agent/test_anthropic_mcp_prefix_strip.py

    @@ -1,9 +1,16 @@
    -"""Tests for GH-25255: Anthropic OAuth mcp_ prefix stripping.
    +"""Tests for GH-25255: Anthropic OAuth ``mcp__`` tool-name round-trip.
     
    -When strip_tool_prefix=True (Anthropic OAuth path), the transport must only
    -strip the ``mcp_`` prefix from OAuth-injected tools, NOT from Hermes-native
    -MCP server tools that are registered under their full ``mcp_<server>_<tool>``
    -name in the tool registry.
    +Anthropic's subscription/OAuth billing classifier treats a **single-underscore**
    +``mcp_`` tool name as a third-party-app fingerprint and rejects the request with
    +HTTP 400 "Third-party apps now draw from extra usage, not plan limits".  So on
    +the OAuth wire NOTHING may carry a single-underscore ``mcp_`` prefix:
    +
    +  * bare native tools            ``read_file``            -> ``mcp__read_file``
    +  * native MCP server tools      ``mcp_linear_get_issue`` -> ``mcp__linear_get_issue``
    +
    +``normalize_response`` reverses the ``mcp__`` wire name back to whatever the tool
    +registry knows (the single-underscore ``mcp_<server>_<tool>`` form for MCP server
    +tools, or the bare name for native tools) so the dispatcher is unaffected.
     """
     
     from __future__ import annotations
    @@ -12,7 +19,6 @@
     from unittest.mock import patch
     
     
    -
     # ---------------------------------------------------------------------------
     # Helpers
     # ---------------------------------------------------------------------------
    @@ -38,7 +44,7 @@ def _make_response(*blocks, stop_reason="end_turn"):
     
     
     class _FakeRegistry:
    -    """Minimal fake tool registry for testing prefix stripping logic."""
    +    """Minimal fake tool registry for testing prefix round-trip logic."""
     
         def __init__(self, registered_names: set[str]):
             self._names = registered_names
    @@ -50,25 +56,20 @@ def get_entry(self, name: str):
     
     
     # ---------------------------------------------------------------------------
    -# Tests
    +# Response side: mcp__ wire name -> registry name
     # ---------------------------------------------------------------------------
     
     class TestAnthropicMcpPrefixStrip:
    -    """Verify that strip_tool_prefix only strips OAuth-injected prefixes."""
    +    """Verify strip_tool_prefix reverses the ``mcp__`` wire prefix correctly."""
     
         def _get_transport(self):
             from agent.transports.anthropic import AnthropicTransport
             return AnthropicTransport()
     
    -    def test_strips_prefix_for_oauth_injected_tool(self):
    -        """OAuth tools: mcp_read_file -> read_file (stripped).
    -
    -        The tool was registered as 'read_file' in the registry.
    -        Anthropic sees 'mcp_read_file' because Hermes adds the prefix.
    -        On response, we must strip it back to 'read_file'.
    -        """
    +    def test_strips_prefix_for_oauth_injected_native_tool(self):
    +        """``mcp__read_file`` -> ``read_file`` (bare native tool)."""
             transport = self._get_transport()
    -        block = _make_tool_use_block("mcp_read_file")
    +        block = _make_tool_use_block("mcp__read_file")
             response = _make_response(block)
     
             registry = _FakeRegistry({"read_file", "terminal", "web_search"})
    @@ -78,31 +79,29 @@ def test_strips_prefix_for_oauth_injected_tool(self):
             assert len(result.tool_calls) == 1
             assert result.tool_calls[0].name == "read_file"
     
    -    def test_preserves_native_mcp_server_tool_name(self):
    -        """Native MCP tools: mcp_composio_SEARCH -> mcp_composio_SEARCH (kept).
    +    def test_restores_single_underscore_mcp_server_tool(self):
    +        """``mcp__linear_get_issue`` -> ``mcp_linear_get_issue`` (MCP server tool).
     
    -        The tool is registered with the full mcp_ prefix in the registry.
    -        Stripping would break registry lookup.
    +        MCP server tools are registered under their full single-underscore
    +        ``mcp_<server>_<tool>`` name, but they MUST go on the OAuth wire as
    +        double-underscore to dodge the classifier.  The response side restores
    +        the single-underscore registry name so dispatch still resolves.
             """
             transport = self._get_transport()
    -        block = _make_tool_use_block("mcp_composio_COMPOSIO_SEARCH_TOOLS")
    +        block = _make_tool_use_block("mcp__linear_get_issue")
             response = _make_response(block)
     
    -        registry = _FakeRegistry({
    -            "mcp_composio_COMPOSIO_SEARCH_TOOLS",
    -            "mcp_composio_COMPOSIO_GET_TOOL_SCHEMAS",
    -            "read_file",
    -        })
    +        registry = _FakeRegistry({"mcp_linear_get_issue", "read_file"})
             with patch("tools.registry.registry", registry):
                 result = transport.normalize_response(response, strip_tool_prefix=True)
     
             assert len(result.tool_calls) == 1
    -        assert result.tool_calls[0].name == "mcp_composio_COMPOSIO_SEARCH_TOOLS"
    +        assert result.tool_calls[0].name == "mcp_linear_get_issue"
     
         def test_no_strip_when_flag_false(self):
             """When strip_tool_prefix=False, names are never modified."""
             transport = self._get_transport()
    -        block = _make_tool_use_block("mcp_read_file")
    +        block = _make_tool_use_block("mcp__read_file")
             response = _make_response(block)
     
             registry = _FakeRegistry({"read_file"})
    @@ -110,10 +109,10 @@ def test_no_strip_when_flag_false(self):
                 result = transport.normalize_response(response, strip_tool_prefix=False)
     
             assert len(result.tool_calls) == 1
    -        assert result.tool_calls[0].name == "mcp_read_file"
    +        assert result.tool_calls[0].name == "mcp__read_file"
     
         def test_no_strip_when_not_mcp_prefixed(self):
    -        """Non-mcp_ names are untouched regardless of strip flag."""
    +        """Non-``mcp__`` names are untouched regardless of strip flag."""
             transport = self._get_transport()
             block = _make_tool_use_block("web_search")
             response = _make_response(block)
    @@ -125,68 +124,61 @@ def test_no_strip_when_not_mcp_prefixed(self):
             assert len(result.tool_calls) == 1
             assert result.tool_calls[0].name == "web_search"
     
    -    def test_preserves_name_when_neither_in_registry(self):
    -        """When neither stripped nor full name is in registry, keep full name.
    +    def test_preserves_name_when_no_original_in_registry(self):
    +        """Neither the single-underscore nor bare original is registered.
     
    -        Safety fallback: if we can't determine the type, prefer the full name
    -        since it's what the LLM was told about.
    +        Safety fallback: keep the full ``mcp__`` name the LLM was told about.
             """
             transport = self._get_transport()
    -        block = _make_tool_use_block("mcp_unknown_tool")
    +        block = _make_tool_use_block("mcp__unknown_tool")
             response = _make_response(block)
     
    -        registry = _FakeRegistry({"read_file"})  # neither name registered
    +        registry = _FakeRegistry({"read_file"})  # no matching original
             with patch("tools.registry.registry", registry):
                 result = transport.normalize_response(response, strip_tool_prefix=True)
     
             assert len(result.tool_calls) == 1
    -        assert result.tool_calls[0].name == "mcp_unknown_tool"
    +        assert result.tool_calls[0].name == "mcp__unknown_tool"
     
    -    def test_mixed_tools_same_response(self):
    -        """Both OAuth and native MCP tools in the same response."""
    +    def test_mixed_native_and_mcp_server_tools_same_response(self):
    +        """A bare native tool and an MCP server tool, both wired as ``mcp__``."""
             transport = self._get_transport()
    -        block1 = _make_tool_use_block("mcp_read_file", block_id="tc_1")
    -        block2 = _make_tool_use_block("mcp_composio_SEARCH", block_id="tc_2")
    -        block3 = _make_tool_use_block("mcp_composio_SEARCH", block_id="tc_3")  # also registered natively
    -        response = _make_response(block1, block2, block3)
    -
    -        registry = _FakeRegistry({
    -            "read_file",  # OAuth-injected
    -            "mcp_composio_SEARCH",  # native MCP
    -        })
    +        block1 = _make_tool_use_block("mcp__read_file", block_id="tc_1")
    +        block2 = _make_tool_use_block("mcp__linear_get_issue", block_id="tc_2")
    +        response = _make_response(block1, block2)
    +
    +        registry = _FakeRegistry({"read_file", "mcp_linear_get_issue"})
             with patch("tools.registry.registry", registry):
                 result = transport.normalize_response(response, strip_tool_prefix=True)
     
    -        assert len(result.tool_calls) == 3
    -        # OAuth tool: stripped
    +        assert len(result.tool_calls) == 2
             assert result.tool_calls[0].name == "read_file"
    -        # Native MCP: preserved (both stripped and full are registered, full wins)
    -        assert result.tool_calls[1].name == "mcp_composio_SEARCH"
    -        assert result.tool_calls[2].name == "mcp_composio_SEARCH"
    +        assert result.tool_calls[1].name == "mcp_linear_get_issue"
     
    -    def test_both_stripped_and_full_registered_prefers_full(self):
    -        """Edge case: both 'foo' and 'mcp_foo' exist in registry.
    +    def test_prefers_full_wire_name_when_it_resolves_directly(self):
    +        """If the ``mcp__`` wire name itself is registered, keep it as-is.
     
    -        Keep 'mcp_foo' (the original name) since it's what the LLM requested.
    +        Defensive: never rewrite a name that already resolves natively.
             """
             transport = self._get_transport()
    -        block = _make_tool_use_block("mcp_foo")
    +        block = _make_tool_use_block("mcp__foo")
             response = _make_response(block)
     
    -        registry = _FakeRegistry({"foo", "mcp_foo"})
    +        registry = _FakeRegistry({"foo", "mcp__foo"})
             with patch("tools.registry.registry", registry):
                 result = transport.normalize_response(response, strip_tool_prefix=True)
     
             assert len(result.tool_calls) == 1
    -        # Both exist — the condition `get_entry(stripped) and not get_entry(name)`
    -        # is False because get_entry(name) IS truthy, so we keep the full name.
    -        assert result.tool_calls[0].name == "mcp_foo"
    +        assert result.tool_calls[0].name == "mcp__foo"
     
     
    +# ---------------------------------------------------------------------------
    +# Request side: registry name -> mcp__ wire name (no single-underscore leaks)
    +# ---------------------------------------------------------------------------
    +
     class TestAnthropicOAuthOutgoingPrefix:
    -    """Verify the outgoing-side companion fix: build_anthropic_kwargs must not
    -    double-prefix tool names that already start with ``mcp_`` (native MCP server
    -    tools registered as ``mcp_<server>_<tool>``). GH-25255."""
    +    """build_anthropic_kwargs must emit ZERO single-underscore ``mcp_`` names on
    +    the OAuth wire — bare names and MCP server names both land on ``mcp__``."""
     
         def _build(self, tools, is_oauth=True):
             from agent.anthropic_adapter import build_anthropic_kwargs
    @@ -199,50 +191,65 @@ def _build(self, tools, is_oauth=True):
                 is_oauth=is_oauth,
             )
     
    -    def test_oauth_adds_prefix_to_bare_tool_name(self):
    -        """OAuth + bare name → prefix added (existing Claude Code convention)."""
    +    def test_oauth_adds_double_prefix_to_bare_tool_name(self):
    +        """OAuth + bare name -> ``mcp__`` prefix added."""
             kwargs = self._build([{
                 "type": "function",
                 "function": {"name": "read_file", "description": "x", "parameters": {}},
             }])
    -        names = [t["name"] for t in kwargs["tools"]]
    -        assert names == ["mcp_read_file"]
    +        assert [t["name"] for t in kwargs["tools"]] == ["mcp__read_file"]
     
    -    def test_oauth_does_not_double_prefix_native_mcp_tool(self):
    -        """OAuth + already-prefixed native MCP name → left alone."""
    +    def test_oauth_promotes_single_underscore_mcp_server_tool(self):
    +        """OAuth + ``mcp_<server>_<tool>`` -> promoted to double underscore.
    +
    +        This is the gap left by the bare constant swap: MCP server tools used
    +        to be *skipped* and went on the wire single-underscore, still tripping
    +        the classifier.  They must become ``mcp__`` and NOT be double-prefixed.
    +        """
             kwargs = self._build([{
                 "type": "function",
                 "function": {
    -                "name": "mcp_composio_COMPOSIO_SEARCH_TOOLS",
    +                "name": "mcp_linear_get_issue",
                     "description": "x",
                     "parameters": {},
                 },
             }])
             names = [t["name"] for t in kwargs["tools"]]
    -        # Must NOT become "mcp_mcp_composio_..." — that breaks the round-trip
    -        # because normalize_response only strips ONE mcp_ prefix.
    -        assert names == ["mcp_composio_COMPOSIO_SEARCH_TOOLS"]
    +        assert names == ["mcp__linear_get_issue"]
    +        # never double-prefixed
    +        assert not any(n.startswith("mcp__mcp_") for n in names)
    +
    +    def test_oauth_already_double_prefixed_left_alone(self):
    +        """OAuth + already-``mcp__`` name -> unchanged (no triple underscore)."""
    +        kwargs = self._build([{
    +            "type": "function",
    +            "function": {"name": "mcp__already", "description": "x", "parameters": {}},
    +        }])
    +        assert [t["name"] for t in kwargs["tools"]] == ["mcp__already"]
     
    -    def test_oauth_mixed_native_and_bare_tools(self):
    -        """Mixed: native MCP preserved, bare names prefixed."""
    +    def test_oauth_no_single_underscore_mcp_on_wire(self):
    +        """Mixed set: every wire name is bare-free of single-underscore mcp_."""
             kwargs = self._build([
                 {"type": "function", "function": {"name": "read_file",
    -                                               "description": "x", "parameters": {}}},
    -            {"type": "function", "function": {"name": "mcp_composio_SEARCH",
    -                                               "description": "y", "parameters": {}}},
    +                                              "description": "x", "parameters": {}}},
    +            {"type": "function", "function": {"name": "mcp_linear_get_issue",
    +                                              "description": "y", "parameters": {}}},
                 {"type": "function", "function": {"name": "terminal",
    -                                               "description": "z", "parameters": {}}},
    +                                              "description": "z", "parameters": {}}},
             ])
             names = sorted(t["name"] for t in kwargs["tools"])
    -        assert names == ["mcp_composio_SEARCH", "mcp_read_file", "mcp_terminal"]
    +        assert names == ["mcp__linear_get_issue", "mcp__read_file", "mcp__terminal"]
    +        # The core invariant: NOTHING single-underscore reaches the wire.
    +        for n in names:
    +            assert not (n.startswith("mcp_") and not n.startswith("mcp__"))
     
         def test_non_oauth_path_untouched(self):
             """Non-OAuth requests never get the prefix — schemas pass through as-is."""
             kwargs = self._build([
                 {"type": "function", "function": {"name": "read_file",
    -                                               "description": "x", "parameters": {}}},
    -            {"type": "function", "function": {"name": "mcp_composio_SEARCH",
    -                                               "description": "y", "parameters": {}}},
    +                                              "description": "x", "parameters": {}}},
    +            {"type": "function", "function": {"name": "mcp_linear_get_issue",
    +                                              "description": "y", "parameters": {}}},
             ], is_oauth=False)
             names = sorted(t["name"] for t in kwargs["tools"])
    -        assert names == ["mcp_composio_SEARCH", "read_file"]
    +        assert names == ["mcp_linear_get_issue", "read_file"]