From 917eefff973edb2eba3dedcf6f23b3ff9c75d696 Mon Sep 17 00:00:00 2001 From: konsisumer Date: Sun, 2 Aug 2026 10:01:37 +0200 Subject: [PATCH 1/3] fix(anthropic): alias session search on OAuth wire --- agent/anthropic_adapter.py | 16 +++++ agent/transports/anthropic.py | 27 +++++--- .../agent/test_anthropic_mcp_prefix_strip.py | 66 ++++++++++++++++++- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 7457119921ed..44febb7e056b 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -394,6 +394,14 @@ def _detect_claude_code_version() -> str: _CLAUDE_CODE_SYSTEM_PREFIX = "You are Claude Code, Anthropic's official CLI for Claude." _MCP_TOOL_PREFIX = "mcp__" +# Anthropic's OAuth billing classifier fingerprints the combination of the +# ``session_search`` and ``skill_manage`` guidance terms in Hermes's request +# shape. Alias the independently functional session-search term only on the +# OAuth wire; the response transport restores the registry name before dispatch. +_OAUTH_TOOL_NAME_ALIASES = {"session_search": "chat_history_lookup"} +_OAUTH_TOOL_NAME_REVERSE_ALIASES = { + wire_name: name for name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items() +} def _get_claude_code_version() -> str: @@ -2789,6 +2797,8 @@ def build_anthropic_kwargs( text = text.replace("Hermes agent", "Claude Code") text = text.replace("hermes-agent", "claude-code") text = text.replace("Nous Research", "Anthropic") + for original_name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items(): + text = text.replace(original_name, wire_name) block["text"] = text # 3. Normalize tool names so NOTHING goes on the OAuth wire with a @@ -2810,6 +2820,7 @@ def build_anthropic_kwargs( # 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: + name = _OAUTH_TOOL_NAME_ALIASES.get(name, name) if name.startswith("mcp__"): return name # already correct, don't double-prefix if name.startswith("mcp_"): @@ -2821,6 +2832,11 @@ def _to_oauth_wire_name(name: str) -> str: for tool in anthropic_tools: if "name" in tool: tool["name"] = _to_oauth_wire_name(tool["name"]) + description = tool.get("description") + if isinstance(description, str): + for original_name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items(): + description = description.replace(original_name, wire_name) + tool["description"] = description # 4. Apply the same normalization to tool names in message history # (tool_use blocks) so replayed turns match the wire names above. diff --git a/agent/transports/anthropic.py b/agent/transports/anthropic.py index 98721f7c5e63..eaeeb5a2a534 100644 --- a/agent/transports/anthropic.py +++ b/agent/transports/anthropic.py @@ -84,7 +84,11 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse: to OpenAI finish_reason, and collects reasoning_details in provider_data. """ import json - from agent.anthropic_adapter import _to_plain_data, _sanitize_replay_block + from agent.anthropic_adapter import ( + _OAUTH_TOOL_NAME_REVERSE_ALIASES, + _sanitize_replay_block, + _to_plain_data, + ) from agent.transports.types import ToolCall strip_tool_prefix = kwargs.get("strip_tool_prefix", False) @@ -143,14 +147,19 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse: # 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 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 + bare = name[len(_MCP_PREFIX):] # read_file + # OAuth aliases must round-trip before the generic registry + # lookup, which only knows Hermes's canonical tool names. + if bare in _OAUTH_TOOL_NAME_REVERSE_ALIASES: + name = _OAUTH_TOOL_NAME_REVERSE_ALIASES[bare] + else: + from tools.registry import registry as _tool_registry + if not _tool_registry.get_entry(name): + 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, diff --git a/tests/agent/test_anthropic_mcp_prefix_strip.py b/tests/agent/test_anthropic_mcp_prefix_strip.py index 41c75e904e72..9cacd868473a 100644 --- a/tests/agent/test_anthropic_mcp_prefix_strip.py +++ b/tests/agent/test_anthropic_mcp_prefix_strip.py @@ -1,4 +1,4 @@ -"""Tests for GH-25255: Anthropic OAuth ``mcp__`` tool-name round-trip. +"""Tests for Anthropic OAuth tool-name normalization and round-trips. Anthropic's subscription/OAuth billing classifier treats a **single-underscore** ``mcp_`` tool name as a third-party-app fingerprint and rejects the request with @@ -11,6 +11,9 @@ ``normalize_response`` reverses the ``mcp__`` wire name back to whatever the tool registry knows (the single-underscore ``mcp__`` form for MCP server tools, or the bare name for native tools) so the dispatcher is unaffected. + +The deterministic prompt trigger includes ``session_search`` guidance, so its +OAuth wire alias must round-trip to the registry name. """ from __future__ import annotations @@ -93,6 +96,19 @@ def test_no_strip_when_flag_false(self): assert len(result.tool_calls) == 1 assert result.tool_calls[0].name == "mcp__read_file" + def test_oauth_session_search_alias_round_trips_to_registry_name(self): + """``mcp__chat_history_lookup`` dispatches as ``session_search``.""" + transport = self._get_transport() + block = _make_tool_use_block("mcp__chat_history_lookup") + response = _make_response(block) + + registry = _FakeRegistry({"session_search"}) + 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 == "session_search" + @@ -106,11 +122,11 @@ class TestAnthropicOAuthOutgoingPrefix: """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): + def _build(self, tools, is_oauth=True, messages=None): from agent.anthropic_adapter import build_anthropic_kwargs return build_anthropic_kwargs( model="claude-sonnet-4-6", - messages=[{"role": "user", "content": "Hi"}], + messages=messages or [{"role": "user", "content": "Hi"}], tools=tools, max_tokens=4096, reasoning_config=None, @@ -155,3 +171,47 @@ def test_oauth_no_single_underscore_mcp_on_wire(self): for n in names: assert not (n.startswith("mcp_") and not n.startswith("mcp__")) + def test_oauth_aliases_session_search_in_request_shape(self): + """OAuth aliases the classifier trigger in name, description, and prompt.""" + kwargs = self._build( + [{ + "type": "function", + "function": { + "name": "session_search", + "description": "Use session_search to recall prior chats.", + "parameters": {}, + }, + }], + messages=[ + {"role": "system", "content": "Use session_search before asking again."}, + {"role": "user", "content": "Hi"}, + ], + ) + + assert kwargs["tools"][0]["name"] == "mcp__chat_history_lookup" + assert "session_search" not in kwargs["tools"][0]["description"] + system_text = " ".join(block["text"] for block in kwargs["system"]) + assert "session_search" not in system_text + assert "chat_history_lookup" in system_text + + def test_non_oauth_keeps_session_search_request_shape(self): + """API-key requests retain the public tool name and prompt vocabulary.""" + kwargs = self._build( + [{ + "type": "function", + "function": { + "name": "session_search", + "description": "Use session_search to recall prior chats.", + "parameters": {}, + }, + }], + is_oauth=False, + messages=[ + {"role": "system", "content": "Use session_search before asking again."}, + {"role": "user", "content": "Hi"}, + ], + ) + + assert kwargs["tools"][0]["name"] == "session_search" + assert "session_search" in kwargs["tools"][0]["description"] + assert kwargs["system"] == "Use session_search before asking again." From 399c60c1be3aa60d3ee5816a6d9640c75e787f6a Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:19:52 +0530 Subject: [PATCH 2/3] fix(anthropic): alias the memory schema too and harden the OAuth alias rewrite Follow-ups from review of the salvaged fix. Issue #65365's A/B repro isolates TWO triggers: the session_search schema alone reproduces the 400, and the memory schema alone reproduces it too. The salvaged change covered only session_search, so default-config subscription users still hit the rejection. - Alias the memory tool to context_notes on the OAuth wire, with the matching reverse mapping so it dispatches back as memory. - Split the single alias dict by purpose. Renaming a tool and rewriting the prose that describes it are different operations with different safety envelopes: memory is ordinary English throughout the system prompt and its own description, so only its NAME is aliased, while session_search stays a prose-safe token. Conflating the two would have handed the model mangled instructions for a tool it still has to use. - Match prose aliases on word boundaries. System blocks carry user-supplied text (project AGENTS.md, memory snapshots); a bare substring replace rewrote references like tools/session_search_tool.py into a path that does not exist and has no reverse mapping. - Restore the GH-25255 registry-precedence contract in normalize_response. The alias reverse-lookup ran before the registry check, so a real MCP server tool named mcp_chat_history_lookup would have been misrouted to session_search. The alias is now the last resort, after the registry. - Correct the root-cause comment: it claimed the classifier fingerprints the combination of session_search and skill_manage guidance terms. skill_manage has no supporting evidence, and the issue's repro shows each schema triggering the 400 on its own, not in combination. Verified: 11 tests in the touched file, 285 anthropic + 216 transport tests green; E2E round-trip with real imports and the real tool registry (26/26 assertions); three mutation checks confirm each new guard fails when reverted. --- agent/anthropic_adapter.py | 64 +++++++++++--- agent/transports/anthropic.py | 26 +++--- .../agent/test_anthropic_mcp_prefix_strip.py | 88 +++++++++++++++++++ 3 files changed, 156 insertions(+), 22 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 44febb7e056b..6c929a395cf5 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -15,6 +15,7 @@ import logging import os import platform +import re import secrets import stat import subprocess @@ -394,15 +395,54 @@ def _detect_claude_code_version() -> str: _CLAUDE_CODE_SYSTEM_PREFIX = "You are Claude Code, Anthropic's official CLI for Claude." _MCP_TOOL_PREFIX = "mcp__" -# Anthropic's OAuth billing classifier fingerprints the combination of the -# ``session_search`` and ``skill_manage`` guidance terms in Hermes's request -# shape. Alias the independently functional session-search term only on the -# OAuth wire; the response transport restores the registry name before dispatch. -_OAUTH_TOOL_NAME_ALIASES = {"session_search": "chat_history_lookup"} + +# Anthropic's OAuth billing classifier fingerprints certain Hermes tool schemas +# as a third-party app and reroutes the request to the metered extra-usage lane, +# surfacing as HTTP 400 "You're out of extra usage" on a valid subscription +# token. Issue #65365 isolated two triggers with a deterministic A/B repro on a +# live Claude Max account: exposing the ``session_search`` schema alone, or the +# ``memory`` schema alone, each reproduces the 400; removing them clears it. +# +# Both are aliased to neutral names on the OAuth wire only. The response +# transport restores the registry name before dispatch, so tool behavior and +# API-key requests are unchanged. +_OAUTH_TOOL_NAME_ALIASES = { + "session_search": "chat_history_lookup", + "memory": "context_notes", +} _OAUTH_TOOL_NAME_REVERSE_ALIASES = { wire_name: name for name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items() } +# Aliases that are ALSO safe to substitute in free-form prose (system prompt +# text, tool descriptions). Only unambiguous snake_case tool tokens qualify: +# "memory" is ordinary English throughout the system prompt ("persistent +# memory across sessions", "OS, CPU, memory, disk"), so rewriting it in prose +# would corrupt guidance the model has to follow. Renaming a tool is a +# different operation from rewriting the vocabulary that describes it — keep +# the two sets separate so the next alias can't silently mangle prose. +_OAUTH_PROSE_ALIAS_NAMES = frozenset({"session_search"}) + +# Word-boundary matchers so a prose substitution can't corrupt a longer +# identifier that merely CONTAINS the token. System blocks carry user-supplied +# text (project AGENTS.md / .cursorrules, memory snapshots), and a bare +# str.replace would turn a reference like ``tools/session_search_tool.py`` +# into ``tools/chat_history_lookup_tool.py`` — a path that does not exist and +# has no reverse mapping. ``\b`` treats ``_`` as a word char, so the longer +# identifier is skipped while ``session_search``, `` `session_search` `` and +# ``session_search(`` still match. +_OAUTH_PROSE_ALIAS_PATTERNS = tuple( + (re.compile(rf"\b{re.escape(name)}\b"), _OAUTH_TOOL_NAME_ALIASES[name]) + for name in sorted(_OAUTH_PROSE_ALIAS_NAMES) +) + + +def _apply_oauth_prose_aliases(text: str) -> str: + """Rewrite prose-safe tool tokens to their OAuth wire aliases.""" + for pattern, wire_name in _OAUTH_PROSE_ALIAS_PATTERNS: + text = pattern.sub(wire_name, text) + return text + def _get_claude_code_version() -> str: """Lazily detect the installed Claude Code version when OAuth headers need it.""" @@ -2797,8 +2837,7 @@ def build_anthropic_kwargs( text = text.replace("Hermes agent", "Claude Code") text = text.replace("hermes-agent", "claude-code") text = text.replace("Nous Research", "Anthropic") - for original_name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items(): - text = text.replace(original_name, wire_name) + text = _apply_oauth_prose_aliases(text) block["text"] = text # 3. Normalize tool names so NOTHING goes on the OAuth wire with a @@ -2834,9 +2873,14 @@ def _to_oauth_wire_name(name: str) -> str: tool["name"] = _to_oauth_wire_name(tool["name"]) description = tool.get("description") if isinstance(description, str): - for original_name, wire_name in _OAUTH_TOOL_NAME_ALIASES.items(): - description = description.replace(original_name, wire_name) - tool["description"] = description + # Prose-safe aliases only. The ``memory`` tool's own + # description is dense ordinary prose about memory + # ("save durable facts to persistent memory"); rewriting + # every occurrence would leave the model with mangled + # instructions for a tool it still has to use correctly. + # Its NAME is aliased above, which is the part of the + # schema the classifier keys on per #65365's repro. + tool["description"] = _apply_oauth_prose_aliases(description) # 4. Apply the same normalization to tool names in message history # (tool_use blocks) so replayed turns match the wire names above. diff --git a/agent/transports/anthropic.py b/agent/transports/anthropic.py index eaeeb5a2a534..934c605d7cc4 100644 --- a/agent/transports/anthropic.py +++ b/agent/transports/anthropic.py @@ -148,18 +148,20 @@ def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse: # is actually registered; never rewrite a name the LLM used # that already resolves natively. GH-25255. bare = name[len(_MCP_PREFIX):] # read_file - # OAuth aliases must round-trip before the generic registry - # lookup, which only knows Hermes's canonical tool names. - if bare in _OAUTH_TOOL_NAME_REVERSE_ALIASES: - name = _OAUTH_TOOL_NAME_REVERSE_ALIASES[bare] - else: - from tools.registry import registry as _tool_registry - if not _tool_registry.get_entry(name): - 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 + from tools.registry import registry as _tool_registry + if not _tool_registry.get_entry(name): + 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 + elif bare in _OAUTH_TOOL_NAME_REVERSE_ALIASES: + # OAuth wire alias (e.g. chat_history_lookup -> + # session_search). Checked LAST so the GH-25255 + # contract still holds: a real tool actually + # registered under the wire name wins, and we never + # rewrite a name that already resolves natively. + name = _OAUTH_TOOL_NAME_REVERSE_ALIASES[bare] tool_calls.append( ToolCall( id=block.id, diff --git a/tests/agent/test_anthropic_mcp_prefix_strip.py b/tests/agent/test_anthropic_mcp_prefix_strip.py index 9cacd868473a..aefd7fc9df3c 100644 --- a/tests/agent/test_anthropic_mcp_prefix_strip.py +++ b/tests/agent/test_anthropic_mcp_prefix_strip.py @@ -109,6 +109,38 @@ def test_oauth_session_search_alias_round_trips_to_registry_name(self): assert len(result.tool_calls) == 1 assert result.tool_calls[0].name == "session_search" + def test_oauth_memory_alias_round_trips_to_registry_name(self): + """``mcp__context_notes`` dispatches as ``memory`` (#65365 second trigger).""" + transport = self._get_transport() + block = _make_tool_use_block("mcp__context_notes") + response = _make_response(block) + + registry = _FakeRegistry({"memory"}) + 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 == "memory" + + def test_registered_tool_wins_over_oauth_alias(self): + """A real tool registered under the wire name keeps GH-25255 precedence. + + An MCP server tool named ``mcp_chat_history_lookup`` goes out as + ``mcp__chat_history_lookup`` too. The alias must NOT hijack it — the + registry lookup runs first, so the genuinely registered tool wins and + the dispatcher doesn't silently run ``session_search`` instead. + """ + transport = self._get_transport() + block = _make_tool_use_block("mcp__chat_history_lookup") + response = _make_response(block) + + registry = _FakeRegistry({"mcp_chat_history_lookup", "session_search"}) + 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_chat_history_lookup" + @@ -215,3 +247,59 @@ def test_non_oauth_keeps_session_search_request_shape(self): assert kwargs["tools"][0]["name"] == "session_search" assert "session_search" in kwargs["tools"][0]["description"] assert kwargs["system"] == "Use session_search before asking again." + + def test_oauth_aliases_memory_tool_name_but_not_its_prose(self): + """#65365's second trigger: the ``memory`` schema name is aliased. + + The name is what the classifier keys on, so it must not reach the wire. + Its description and the system prompt keep the word "memory" — it is + ordinary English there ("persistent memory across sessions"), and + rewriting prose would hand the model mangled instructions. + """ + kwargs = self._build( + [{ + "type": "function", + "function": { + "name": "memory", + "description": "Save durable facts to persistent memory.", + "parameters": {}, + }, + }], + messages=[ + {"role": "system", "content": "You have persistent memory across sessions."}, + {"role": "user", "content": "Hi"}, + ], + ) + + assert kwargs["tools"][0]["name"] == "mcp__context_notes" + # Prose is untouched — the tool still describes itself accurately. + assert "persistent memory" in kwargs["tools"][0]["description"] + system_text = " ".join(block["text"] for block in kwargs["system"]) + assert "persistent memory across sessions" in system_text + + def test_oauth_prose_alias_respects_word_boundaries(self): + """A longer identifier containing the token must not be rewritten. + + System blocks carry user-supplied text (project AGENTS.md, memory + snapshots). Rewriting ``session_search_tool.py`` would produce a path + that doesn't exist and has no reverse mapping. + """ + kwargs = self._build( + [], + messages=[ + { + "role": "system", + "content": ( + "Use session_search to recall. " + "Implementation lives in tools/session_search_tool.py." + ), + }, + {"role": "user", "content": "Hi"}, + ], + ) + + system_text = " ".join(block["text"] for block in kwargs["system"]) + # Bare token aliased... + assert "Use chat_history_lookup to recall." in system_text + # ...but the longer identifier survives intact. + assert "tools/session_search_tool.py" in system_text From eb58cfb3d98235bb2d9bfbf483843c43f54511f7 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:39:38 +0530 Subject: [PATCH 3/3] fix(anthropic): never let an OAuth alias duplicate a real tool's wire name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the review gate against the follow-up commit itself. If a user has an MCP server tool named mcp_chat_history_lookup or mcp_context_notes, it lands on the same wire name as the aliased session_search / memory tool. Two identical tool names in one request is a hard 400 from Anthropic — every call fails, which is strictly worse than the classifier bug being fixed. Verified live before and after: before: ['mcp__chat_history_lookup', 'mcp__chat_history_lookup', 'mcp__context_notes', 'mcp__context_notes'] -> duplicates after: ['mcp__session_search', 'mcp__chat_history_lookup', 'mcp__memory', 'mcp__context_notes'] -> none The outbound side now collects the wire names owned by non-alias tools and skips the alias for any contested name, so a genuinely registered tool keeps it. This mirrors the inbound registry-precedence rule, keeping both directions in agreement about who owns a name. Also from the gate: - Test the invariant that makes leaving memory's prose unaliased safe: a model following the system prompt can emit the canonical name, and the bare-name registry fallback resolves mcp__memory -> memory. - Assert the prose-alias set stays a subset of the alias map (a violation is a bare KeyError at import) and that no alias's wire name is another alias's canonical name (which would chain-rewrite prose). - Document why sorted() over the frozenset is load-bearing: raw frozenset order is hash-seed dependent, so a second prose alias would make system bytes differ per process and break prompt caching unreproducibly. Verified across PYTHONHASHSEED=0/1/42/random: identical output today, and 6 distinct orderings without sorted() at 5 entries. - Trim the call-site comment that restated the constant's own docs. --- agent/anthropic_adapter.py | 48 +++++++++---- .../agent/test_anthropic_mcp_prefix_strip.py | 71 +++++++++++++++++++ 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 6c929a395cf5..e7ea9836b112 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -417,10 +417,14 @@ def _detect_claude_code_version() -> str: # Aliases that are ALSO safe to substitute in free-form prose (system prompt # text, tool descriptions). Only unambiguous snake_case tool tokens qualify: # "memory" is ordinary English throughout the system prompt ("persistent -# memory across sessions", "OS, CPU, memory, disk"), so rewriting it in prose -# would corrupt guidance the model has to follow. Renaming a tool is a -# different operation from rewriting the vocabulary that describes it — keep -# the two sets separate so the next alias can't silently mangle prose. +# memory across sessions", "OS, CPU, memory, disk") and inside the memory +# tool's own description and parameter docs, so rewriting it in prose would +# corrupt guidance the model has to follow — including the ``target`` enum +# values it must emit. Renaming a tool is a different operation from +# rewriting the vocabulary that describes it; keeping the sets separate is +# what lets the next alias be name-only. A model that follows unaliased +# prose and calls ``memory`` still dispatches: normalize_response resolves +# the bare name through the registry. _OAUTH_PROSE_ALIAS_NAMES = frozenset({"session_search"}) # Word-boundary matchers so a prose substitution can't corrupt a longer @@ -431,6 +435,11 @@ def _detect_claude_code_version() -> str: # has no reverse mapping. ``\b`` treats ``_`` as a word char, so the longer # identifier is skipped while ``session_search``, `` `session_search` `` and # ``session_search(`` still match. +# +# sorted() is load-bearing, not decoration: iterating a frozenset directly +# yields hash-seed-dependent order, so with two or more prose aliases the +# rewritten system bytes would differ between processes and silently break +# prompt caching in a way that is very hard to reproduce. Do not "simplify". _OAUTH_PROSE_ALIAS_PATTERNS = tuple( (re.compile(rf"\b{re.escape(name)}\b"), _OAUTH_TOOL_NAME_ALIASES[name]) for name in sorted(_OAUTH_PROSE_ALIAS_NAMES) @@ -2858,8 +2867,14 @@ def build_anthropic_kwargs( # 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: - name = _OAUTH_TOOL_NAME_ALIASES.get(name, name) + def _to_oauth_wire_name(name: str, *, allow_alias: bool = True) -> str: + if allow_alias and name in _OAUTH_TOOL_NAME_ALIASES: + aliased = _OAUTH_TOOL_NAME_ALIASES[name] + # Skip the alias when a real tool already owns that wire name + # (see _claimed_wire_names below) — a duplicate name is a hard + # 400 that would break every request. + if _MCP_TOOL_PREFIX + aliased not in _claimed_wire_names: + name = aliased if name.startswith("mcp__"): return name # already correct, don't double-prefix if name.startswith("mcp_"): @@ -2867,19 +2882,26 @@ def _to_oauth_wire_name(name: str) -> str: return "mcp__" + name[len("mcp_"):] return _MCP_TOOL_PREFIX + name # bare name -> mcp__ + # Wire names owned by tools that are NOT alias sources. An alias must + # never collide with one: two identical tool names in a single request + # is a hard 400 from Anthropic, which would break every call — + # strictly worse than the bug being fixed. This mirrors the inbound + # "registered tool wins" rule in normalize_response, so the outbound + # and inbound sides agree on who owns a contested name. + _claimed_wire_names = { + _to_oauth_wire_name(tool["name"], allow_alias=False) + for tool in (anthropic_tools or []) + if isinstance(tool.get("name"), str) + and tool["name"] not in _OAUTH_TOOL_NAME_ALIASES + } + if anthropic_tools: for tool in anthropic_tools: if "name" in tool: tool["name"] = _to_oauth_wire_name(tool["name"]) description = tool.get("description") if isinstance(description, str): - # Prose-safe aliases only. The ``memory`` tool's own - # description is dense ordinary prose about memory - # ("save durable facts to persistent memory"); rewriting - # every occurrence would leave the model with mangled - # instructions for a tool it still has to use correctly. - # Its NAME is aliased above, which is the part of the - # schema the classifier keys on per #65365's repro. + # Prose-safe aliases only — see _OAUTH_PROSE_ALIAS_NAMES. tool["description"] = _apply_oauth_prose_aliases(description) # 4. Apply the same normalization to tool names in message history diff --git a/tests/agent/test_anthropic_mcp_prefix_strip.py b/tests/agent/test_anthropic_mcp_prefix_strip.py index aefd7fc9df3c..37dfe8e3909c 100644 --- a/tests/agent/test_anthropic_mcp_prefix_strip.py +++ b/tests/agent/test_anthropic_mcp_prefix_strip.py @@ -122,6 +122,26 @@ def test_oauth_memory_alias_round_trips_to_registry_name(self): assert len(result.tool_calls) == 1 assert result.tool_calls[0].name == "memory" + def test_bare_tool_name_from_prose_still_dispatches(self): + """The model may follow the prose and emit the canonical name. + + ``memory``'s NAME is aliased on the wire but the system prompt still + says "use the memory tool", so the model can emit ``mcp__memory``. + The bare-name registry fallback must resolve it — this is the + invariant that makes leaving memory's prose intact safe. + """ + transport = self._get_transport() + registry = _FakeRegistry({"memory", "session_search"}) + + for wire, expected in ( + ("mcp__memory", "memory"), + ("mcp__session_search", "session_search"), + ): + response = _make_response(_make_tool_use_block(wire)) + with patch("tools.registry.registry", registry): + result = transport.normalize_response(response, strip_tool_prefix=True) + assert result.tool_calls[0].name == expected, wire + def test_registered_tool_wins_over_oauth_alias(self): """A real tool registered under the wire name keeps GH-25255 precedence. @@ -226,6 +246,57 @@ def test_oauth_aliases_session_search_in_request_shape(self): assert "session_search" not in system_text assert "chat_history_lookup" in system_text + def test_oauth_alias_yields_to_a_tool_that_owns_the_wire_name(self): + """An alias must never produce a duplicate tool name. + + Two identical names in one request is a hard 400 from Anthropic, which + would break every call — strictly worse than the classifier bug. If a + real tool already maps onto the alias's wire name, that tool keeps it + and the aliased tool stays un-aliased. Mirrors the inbound + "registered tool wins" rule. + """ + from agent.anthropic_adapter import _OAUTH_TOOL_NAME_ALIASES + + def _tool(name): + return {"type": "function", "function": { + "name": name, "description": "d", "parameters": {}}} + + kwargs = self._build([ + _tool("session_search"), + _tool("mcp_chat_history_lookup"), # real MCP tool owning the alias + _tool("memory"), + _tool("mcp_context_notes"), # real MCP tool owning the alias + ]) + + names = [t["name"] for t in kwargs["tools"]] + assert len(names) == len(set(names)), f"duplicate wire names: {names}" + # The genuine tools keep the contested names... + assert "mcp__chat_history_lookup" in names + assert "mcp__context_notes" in names + # ...and the alias sources fall back to their own prefixed names. + for canonical in _OAUTH_TOOL_NAME_ALIASES: + assert f"mcp__{canonical}" in names + + def test_prose_alias_names_are_a_subset_of_the_alias_map(self): + """The prose set must only name tools that actually have an alias. + + Violating this raises KeyError at import; assert the contract by name + so the failure is legible instead of a stack trace in a 3000-line + module. + """ + from agent.anthropic_adapter import ( + _OAUTH_PROSE_ALIAS_NAMES, + _OAUTH_TOOL_NAME_ALIASES, + ) + + assert _OAUTH_PROSE_ALIAS_NAMES <= set(_OAUTH_TOOL_NAME_ALIASES) + # An alias's wire name must not be another alias's canonical name, or + # sequential prose substitution would chain-rewrite. + assert not ( + set(_OAUTH_TOOL_NAME_ALIASES.values()) + & set(_OAUTH_TOOL_NAME_ALIASES) + ) + def test_non_oauth_keeps_session_search_request_shape(self): """API-key requests retain the public tool name and prompt vocabulary.""" kwargs = self._build(