diff --git a/litellm/llms/anthropic/chat/handler.py b/litellm/llms/anthropic/chat/handler.py index f8e61d0166a..558662885f2 100644 --- a/litellm/llms/anthropic/chat/handler.py +++ b/litellm/llms/anthropic/chat/handler.py @@ -83,6 +83,7 @@ async def make_call( timeout: Optional[Union[float, httpx.Timeout]], json_mode: bool, speed: Optional[str] = None, + tool_name_reverse_map: Optional[Dict[str, str]] = None, ) -> Tuple[Any, httpx.Headers]: if client is None: client = litellm.module_level_aclient @@ -117,6 +118,7 @@ async def make_call( sync_stream=False, json_mode=json_mode, speed=speed, + tool_name_reverse_map=tool_name_reverse_map, ) # LOGGING @@ -141,6 +143,7 @@ def make_sync_call( timeout: Optional[Union[float, httpx.Timeout]], json_mode: bool, speed: Optional[str] = None, + tool_name_reverse_map: Optional[Dict[str, str]] = None, ) -> Tuple[Any, httpx.Headers]: if client is None: client = litellm.module_level_client # re-use a module level client @@ -183,6 +186,7 @@ def make_sync_call( sync_stream=True, json_mode=json_mode, speed=speed, + tool_name_reverse_map=tool_name_reverse_map, ) # LOGGING @@ -237,6 +241,11 @@ class AnthropicChatCompletion(BaseLLM): timeout=timeout, json_mode=json_mode, speed=optional_params.get("speed") if optional_params else None, + tool_name_reverse_map=( + litellm_params.get("_anthropic_tool_name_map") + if isinstance(litellm_params, dict) + else None + ), ) streamwrapper = CustomStreamWrapper( completion_stream=completion_stream, @@ -462,6 +471,11 @@ class AnthropicChatCompletion(BaseLLM): timeout=timeout, json_mode=json_mode, speed=optional_params.get("speed") if optional_params else None, + tool_name_reverse_map=( + litellm_params.get("_anthropic_tool_name_map") + if isinstance(litellm_params, dict) + else None + ), ) return CustomStreamWrapper( completion_stream=completion_stream, @@ -526,6 +540,7 @@ class ModelResponseIterator: sync_stream: bool, json_mode: Optional[bool] = False, speed: Optional[str] = None, + tool_name_reverse_map: Optional[Dict[str, str]] = None, ): self.streaming_response = streaming_response self.response_iterator = self.streaming_response @@ -533,6 +548,13 @@ class ModelResponseIterator: self.tool_index = -1 self.json_mode = json_mode self.speed = speed + # rewritten-name -> caller's original. Built per-request from the + # forward map in AnthropicConfig._build_request_tool_name_maps; only + # contains entries we actually rewrote, so a tool legitimately named + # `foo_bar` is *not* reverse-mapped just because some other tool was + # rewritten to `foo_bar` in a different request. Empty/None is the + # common case (no '/' or other invalid chars in any tool name). + self.tool_name_reverse_map: Dict[str, str] = tool_name_reverse_map or {} # Generate response ID once per stream to match OpenAI-compatible behavior self.response_id = _generate_id() @@ -792,6 +814,16 @@ class ModelResponseIterator: or content_block_start["content_block"]["type"] == "server_tool_use" ): self.tool_index += 1 + # Reverse-map the (sanitized) tool name back to the + # caller's original. No-op when the map is empty. + _stream_tool_name = content_block_start["content_block"]["name"] + if ( + self.tool_name_reverse_map + and _stream_tool_name in self.tool_name_reverse_map + ): + _stream_tool_name = self.tool_name_reverse_map[ + _stream_tool_name + ] # Use empty string for arguments in content_block_start - actual arguments # come in subsequent content_block_delta chunks and get accumulated. # Using str(input) here would prepend '{}' causing invalid JSON accumulation. @@ -799,7 +831,7 @@ class ModelResponseIterator: id=content_block_start["content_block"]["id"], type="function", function=ChatCompletionToolCallFunctionChunk( - name=content_block_start["content_block"]["name"], + name=_stream_tool_name, arguments="", ), index=self.tool_index, diff --git a/litellm/llms/anthropic/chat/transformation.py b/litellm/llms/anthropic/chat/transformation.py index cd5bb731717..d8da91d1dc7 100644 --- a/litellm/llms/anthropic/chat/transformation.py +++ b/litellm/llms/anthropic/chat/transformation.py @@ -92,6 +92,106 @@ else: LoggingClass = Any +# Anthropic requires tool names to match ^[a-zA-Z0-9_-]{1,128}$. Any other +# character (commonly '/' or '.' from OpenAPI-derived MCP tools, e.g. +# "actions/download-job-logs-for-workflow-run") must be replaced before +# the request is sent. +# +# A naive "replace [^a-zA-Z0-9_-] with _" is unsafe because it's lossy: +# `foo/bar` and `foo_bar` both collapse to `foo_bar`. Two tools with the +# same sanitized name would either 400 at Anthropic (duplicate) or, worse, +# cause the response side to mis-translate `foo_bar` (a name the caller +# really did register) back to `foo/bar`. +# +# Instead we build a *per-request* forward map (original -> sanitized) +# whose codomain is unique within the request: when two originals collapse +# to the same candidate, or when a sanitized name collides with an already- +# valid name elsewhere in the request, we append numeric suffixes +# (`_2`, `_3`, ...) until the result is free. +# +# The reverse map (sanitized -> original) only contains entries where the +# original was actually rewritten. So a tool whose name is already valid +# round-trips identically and is *never* mistakenly re-mapped on the +# response side. +_ANTHROPIC_TOOL_NAME_INVALID_CHARS = re.compile(r"[^a-zA-Z0-9_-]") +_ANTHROPIC_TOOL_NAME_MAX_LEN = 128 +ANTHROPIC_TOOL_NAME_FORWARD_MAP_KEY = "_anthropic_tool_name_forward_map" +ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY = "_anthropic_tool_name_map" + + +def _basic_sanitize_anthropic_tool_name(name: str) -> str: + """Lossy: replace [^a-zA-Z0-9_-] with '_' and truncate to 128. + + Used as a candidate generator for the per-request forward map. + Callers should NOT use this directly for translation -- always go + through the forward map so collisions are resolved. + """ + if not isinstance(name, str) or not name: + return name + return _ANTHROPIC_TOOL_NAME_INVALID_CHARS.sub("_", name)[ + :_ANTHROPIC_TOOL_NAME_MAX_LEN + ] + + +def _build_anthropic_tool_name_maps( + original_names: List[str], +) -> Tuple[Dict[str, str], Dict[str, str]]: + """Build (forward, reverse) tool-name maps for a single request. + + forward[original] = sanitized -- only present when name was rewritten + reverse[sanitized] = original -- inverse of `forward` + + Properties: + - All sanitized names satisfy ^[a-zA-Z0-9_-]{1,128}$. + - Sanitized names are unique within the request (no two originals + collide on the wire). + - A name that's already valid AND doesn't collide with another tool's + sanitized form passes through untouched and is absent from the maps. + That's the key correctness property: response-side translation only + runs on entries we actually rewrote, so a tool legitimately named + `foo_bar` is never incorrectly retyped to `foo/bar` just because + some *other* request had that pair. + - Order-dependent: when two originals would clash, the *second* one + seen gets the disambiguating suffix. Callers should preserve the + caller's tool order (we do). + """ + forward: Dict[str, str] = {} + used: set = set() + for original in original_names: + if not isinstance(original, str) or not original: + continue + candidate = _basic_sanitize_anthropic_tool_name(original) + if candidate == original: + # Already valid. Reserve the slot but don't put it in `forward` + # -- we want untouched names to skip translation entirely. + used.add(candidate) + continue + # Disambiguate against names already chosen this request. + unique = candidate + n = 1 + while unique in used: + n += 1 + suffix = f"_{n}" + # Keep within the 128-char cap. + head = candidate[: _ANTHROPIC_TOOL_NAME_MAX_LEN - len(suffix)] + unique = f"{head}{suffix}" + forward[original] = unique + used.add(unique) + reverse = {v: k for k, v in forward.items()} + return forward, reverse + + +def _apply_anthropic_tool_name_forward( + name: str, forward: Optional[Dict[str, str]] +) -> str: + """Look up `name` in the forward map; return as-is if absent.""" + if not isinstance(name, str) or not name: + return name + if forward and name in forward: + return forward[name] + return name + + class AnthropicConfig(AnthropicModelInfo, BaseConfig): """ Reference: https://docs.anthropic.com/claude/reference/messages_post @@ -378,7 +478,10 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): } def _map_tool_choice( - self, tool_choice: Optional[str], parallel_tool_use: Optional[bool] + self, + tool_choice: Optional[str], + parallel_tool_use: Optional[bool], + name_forward_map: Optional[Dict[str, str]] = None, ) -> Optional[AnthropicMessagesToolChoice]: _tool_choice: Optional[AnthropicMessagesToolChoice] = None if tool_choice == "auto": @@ -402,7 +505,12 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): _tool_name = tool_choice.get("function", {}).get("name") if _tool_name is not None: _tool_choice = AnthropicMessagesToolChoice(type="tool") - _tool_choice["name"] = _tool_name + # Apply the per-request forward map. If the original + # name was already valid (and thus not in the map), + # this is a no-op pass-through. + _tool_choice["name"] = _apply_anthropic_tool_name_forward( + _tool_name, name_forward_map + ) if parallel_tool_use is not None: # Anthropic uses 'disable_parallel_tool_use' flag to determine if parallel tool use is allowed @@ -419,7 +527,9 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): return _tool_choice def _map_tool_helper( # noqa: PLR0915 - self, tool: ChatCompletionToolParam + self, + tool: ChatCompletionToolParam, + name_forward_map: Optional[Dict[str, str]] = None, ) -> Tuple[Optional[AllAnthropicToolsValues], Optional[AnthropicMcpServerTool]]: returned_tool: Optional[AllAnthropicToolsValues] = None mcp_server: Optional[AnthropicMcpServerTool] = None @@ -457,7 +567,9 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): ) _tool = AnthropicMessagesTool( - name=tool["function"]["name"], + name=_apply_anthropic_tool_name_forward( + tool["function"]["name"], name_forward_map + ), input_schema=input_anthropic_schema, type="custom", ) @@ -675,7 +787,9 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): return initial_tool def _map_tools( - self, tools: List + self, + tools: List, + name_forward_map: Optional[Dict[str, str]] = None, ) -> Tuple[List[AllAnthropicToolsValues], List[AnthropicMcpServerTool]]: anthropic_tools = [] mcp_servers = [] @@ -683,7 +797,9 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): if "input_schema" in tool: # assume in anthropic format anthropic_tools.append(tool) else: # assume openai tool call - new_tool, mcp_server_tool = self._map_tool_helper(tool) + new_tool, mcp_server_tool = self._map_tool_helper( + tool, name_forward_map=name_forward_map + ) if new_tool is not None: anthropic_tools.append(new_tool) @@ -691,6 +807,92 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): mcp_servers.append(mcp_server_tool) return anthropic_tools, mcp_servers + @staticmethod + def _rewrite_tool_names_in_messages( + messages: List[AllMessageValues], + name_forward_map: Dict[str, str], + ) -> List[AllMessageValues]: + """Return a copy of `messages` with tool_call/function_call names + rewritten using the per-request forward map. + + Only mutates messages whose tool_call/function_call name is *in* the + forward map. Names absent from the map (already valid, no collision) + round-trip untouched. We only deep-copy the entries we actually + change to keep this O(turns-with-rewritten-tools), not O(history). + """ + if not name_forward_map: + return messages + new_messages: List[AllMessageValues] = [] + for msg in messages: + if not isinstance(msg, dict): + new_messages.append(msg) + continue + tool_calls = msg.get("tool_calls") + function_call = msg.get("function_call") + if not tool_calls and not function_call: + new_messages.append(msg) + continue + new_msg = dict(msg) + if tool_calls: + new_calls = [] + for tc in tool_calls: + if not isinstance(tc, dict): + new_calls.append(tc) + continue + fn = tc.get("function") + fn_name = fn.get("name") if isinstance(fn, dict) else None + if ( + isinstance(fn, dict) + and isinstance(fn_name, str) + and fn_name in name_forward_map + ): + new_fn = dict(fn) + new_fn["name"] = name_forward_map[fn_name] + new_tc = dict(tc) + new_tc["function"] = new_fn + new_calls.append(new_tc) + else: + new_calls.append(tc) + new_msg["tool_calls"] = new_calls + fc_name = ( + function_call.get("name") if isinstance(function_call, dict) else None + ) + if ( + isinstance(function_call, dict) + and isinstance(fc_name, str) + and fc_name in name_forward_map + ): + new_fc = dict(function_call) + new_fc["name"] = name_forward_map[fc_name] + new_msg["function_call"] = new_fc + new_messages.append(cast(AllMessageValues, new_msg)) + return new_messages + + @staticmethod + def _build_request_tool_name_maps( + tools: List, + ) -> Tuple[Dict[str, str], Dict[str, str]]: + """Build the (forward, reverse) tool-name maps for an OpenAI tools list. + + See _build_anthropic_tool_name_maps for the rules. Pulls the original + name out of either ``{"function": {"name": ...}}`` (legacy OpenAI shape) + or ``{"name": ...}`` (rare top-level shape). + """ + original_names: List[str] = [] + for tool in tools or []: + if not isinstance(tool, dict): + continue + original = ( + tool.get("function", {}).get("name") + if isinstance(tool.get("function"), dict) + else None + ) + if original is None: + original = tool.get("name") + if isinstance(original, str) and original: + original_names.append(original) + return _build_anthropic_tool_name_maps(original_names) + def _detect_tool_search_tools(self, tools: Optional[List]) -> bool: """Check if tool search tools are present in the tools list.""" if not tools: @@ -997,6 +1199,27 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): non_default_params=non_default_params ) + # Build per-request tool-name maps once, up-front, so both `tools` + # and `tool_choice` see the same forward map regardless of the + # order params arrive in non_default_params. See + # _build_anthropic_tool_name_maps for the collision-handling rules. + _tools_param = non_default_params.get("tools") + _tool_name_forward_map: Dict[str, str] = {} + _tool_name_reverse_map: Dict[str, str] = {} + if _tools_param: + ( + _tool_name_forward_map, + _tool_name_reverse_map, + ) = self._build_request_tool_name_maps(_tools_param) + if _tool_name_forward_map: + optional_params[ANTHROPIC_TOOL_NAME_FORWARD_MAP_KEY] = ( + _tool_name_forward_map + ) + if _tool_name_reverse_map: + optional_params[ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY] = ( + _tool_name_reverse_map + ) + for param, value in non_default_params.items(): if param == "max_tokens": optional_params["max_tokens"] = ( @@ -1007,8 +1230,9 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): value if isinstance(value, int) else max(1, int(round(value))) ) elif param == "tools": - # check if optional params already has tools - anthropic_tools, mcp_servers = self._map_tools(value) + anthropic_tools, mcp_servers = self._map_tools( + value, name_forward_map=_tool_name_forward_map + ) optional_params = self._add_tools_to_optional_params( optional_params=optional_params, tools=anthropic_tools ) @@ -1019,6 +1243,7 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): self._map_tool_choice( tool_choice=non_default_params.get("tool_choice"), parallel_tool_use=non_default_params.get("parallel_tool_calls"), + name_forward_map=_tool_name_forward_map, ) ) @@ -1434,6 +1659,22 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): headers=headers, optional_params=optional_params ) + # Rewrite tool_call names in prior assistant messages using the + # per-request forward map so the `tool_use` blocks we end up sending + # match the (rewritten) tool names in optional_params["tools"]. + # Without this, Anthropic still 400s on tool_use.name even if the + # tools array is clean. + # + # Also propagate the reverse map onto litellm_params so + # transform_response / streaming can translate response tool_use + # names back to the caller's originals. + _forward = optional_params.get(ANTHROPIC_TOOL_NAME_FORWARD_MAP_KEY) + _reverse = optional_params.get(ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY) + if _forward: + messages = self._rewrite_tool_names_in_messages(messages, _forward) + if _reverse and isinstance(litellm_params, dict): + litellm_params[ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY] = _reverse + # Separate system prompt from rest of message anthropic_system_message_list = self.translate_system_message(messages=messages) # Handling anthropic API Prompt Caching @@ -1886,6 +2127,7 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): json_mode: Optional[bool] = None, prefix_prompt: Optional[str] = None, speed: Optional[str] = None, + tool_name_reverse_map: Optional[Dict[str, str]] = None, ): _hidden_params: Dict = {} _hidden_params["additional_headers"] = process_anthropic_headers( @@ -1910,6 +2152,21 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): compaction_blocks, ) = self.extract_response_content(completion_response=completion_response) + # Reverse-map rewritten tool names back to caller's originals so a + # downstream OpenAI-style dispatcher can match on the registered name. + # See _build_anthropic_tool_name_maps for why this is keyed on the + # per-request reverse map (so a tool legitimately named `foo_bar` is + # never incorrectly retyped to `foo/bar`). No-op when the map is + # empty (the common case). + if tool_name_reverse_map and tool_calls: + for tc in tool_calls: + fn = tc.get("function") if isinstance(tc, dict) else None + if fn is None: + continue + _name = fn.get("name") + if isinstance(_name, str) and _name in tool_name_reverse_map: + fn["name"] = tool_name_reverse_map[_name] + if ( prefix_prompt is not None and not text_content.startswith(prefix_prompt) @@ -2028,6 +2285,11 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): prefix_prompt = self.get_prefix_prompt(messages=messages) speed = optional_params.get("speed") + tool_name_reverse_map: Optional[Dict[str, str]] = None + if isinstance(litellm_params, dict): + _candidate = litellm_params.get(ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY) + if isinstance(_candidate, dict): + tool_name_reverse_map = _candidate model_response = self.transform_parsed_response( completion_response=completion_response, @@ -2036,6 +2298,7 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): json_mode=json_mode, prefix_prompt=prefix_prompt, speed=speed, + tool_name_reverse_map=tool_name_reverse_map, ) return model_response diff --git a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py index 3b2fa097b70..8628fbcb793 100644 --- a/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py +++ b/litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py @@ -6,10 +6,35 @@ import asyncio import contextvars import json import os +import re from pathlib import PurePosixPath from typing import Any, Dict, List, Optional from urllib.parse import quote + +# Tool names emitted from OpenAPI specs must work across all major LLM providers. +# OpenAI/Anthropic/Bedrock all enforce a character class roughly equivalent to +# ^[a-zA-Z0-9_-]+$ on tool names. Many specs (notably GitHub's REST API) use +# tag-namespaced operationIds like "actions/download-job-logs-for-workflow-run" +# which include '/'. Sanitize here so the same regex passes everywhere downstream. +_OPENAPI_TOOL_NAME_INVALID_CHARS = re.compile(r"[^a-zA-Z0-9_-]") +_OPENAPI_TOOL_NAME_MAX_LEN = 128 + + +def sanitize_openapi_tool_name(raw_name: str) -> str: + """Map an OpenAPI operationId / fallback to a provider-safe tool name. + + Replaces any character outside ``[a-zA-Z0-9_-]`` with ``_`` and caps the + result at 128 chars (the most restrictive of the major providers). + Lowercased to match the existing convention in + ``register_tools_from_openapi``. + """ + if not raw_name: + return raw_name + sanitized = _OPENAPI_TOOL_NAME_INVALID_CHARS.sub("_", raw_name).lower() + return sanitized[:_OPENAPI_TOOL_NAME_MAX_LEN] + + from litellm._logging import verbose_logger from litellm.llms.custom_httpx.http_handler import ( get_async_httpx_client, @@ -405,11 +430,13 @@ def register_tools_from_openapi(spec: Dict[str, Any], base_url: str): if method in path_item: operation = path_item[method] - # Generate tool name - operation_id = operation.get( - "operationId", f"{method}_{path.replace('/', '_')}" - ) - tool_name = operation_id.replace(" ", "_").lower() + # Generate tool name. Sanitize to ^[a-zA-Z0-9_-]+$ (lowercase) + # so the resulting name is valid across OpenAI/Anthropic/Bedrock. + # Many specs (e.g. GitHub REST) use tag-namespaced operationIds + # like "actions/download-job-logs-for-workflow-run" which + # contain '/' and would 400 at the LLM provider boundary. + operation_id = operation.get("operationId", f"{method}_{path}") + tool_name = sanitize_openapi_tool_name(operation_id) # Get description description = operation.get( diff --git a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py index 8131c040136..d83cdc8ac85 100644 --- a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py @@ -976,6 +976,7 @@ if MCP_AVAILABLE: build_input_schema, load_openapi_spec_async, resolve_operation_params, + sanitize_openapi_tool_name, ) try: @@ -993,7 +994,12 @@ if MCP_AVAILABLE: operation, path_item, components ) - op_id = operation.get("operationId", f"{method}_{path}") + raw_op_id = operation.get("operationId", f"{method}_{path}") + # Match what register_tools_from_openapi does so the preview + # the user sees in the dashboard equals the names that get + # registered (and shipped to LLM providers, which enforce + # ^[a-zA-Z0-9_-]+$). See sanitize_openapi_tool_name docstring. + op_id = sanitize_openapi_tool_name(raw_op_id) summary = operation.get("summary", "") description = operation.get("description", summary) input_schema = build_input_schema(resolved_op) diff --git a/tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py b/tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py index a7f5f92ab05..4c686595418 100644 --- a/tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py +++ b/tests/test_litellm/llms/anthropic/chat/test_anthropic_chat_transformation.py @@ -3607,3 +3607,474 @@ def test_strip_advisor_blocks_no_op_when_no_advisor_blocks(): original_content = [dict(b) for b in messages[1]["content"]] result = strip_advisor_blocks_from_messages(messages) assert result[1]["content"] == original_content + + +# --------------------------------------------------------------------------- +# Tool-name sanitization for Anthropic compatibility (^[a-zA-Z0-9_-]{1,128}$) +# Repro: Slack-bot agent sent an MCP tool named +# "github_openapi_mcp-actions/download-job-logs-for-workflow-run" which 400'd +# with `tools.N.custom.name: String should match pattern`. +# --------------------------------------------------------------------------- + + +def test_basic_sanitize_anthropic_tool_name_replaces_invalid_chars(): + from litellm.llms.anthropic.chat.transformation import ( + _basic_sanitize_anthropic_tool_name, + ) + + assert ( + _basic_sanitize_anthropic_tool_name( + "github_openapi_mcp-actions/download-job-logs-for-workflow-run" + ) + == "github_openapi_mcp-actions_download-job-logs-for-workflow-run" + ) + # other punctuation + assert _basic_sanitize_anthropic_tool_name("foo.bar:baz qux") == "foo_bar_baz_qux" + # already valid -> unchanged + assert _basic_sanitize_anthropic_tool_name("plain_tool-1") == "plain_tool-1" + # empty + assert _basic_sanitize_anthropic_tool_name("") == "" + # 128-char cap + long = "a/" * 200 + out = _basic_sanitize_anthropic_tool_name(long) + assert len(out) <= 128 + + +def test_build_anthropic_tool_name_maps_no_collisions(): + """Names that need rewriting go in the maps; valid names stay out.""" + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps( + [ + "fine_name", + "actions/download-job-logs-for-workflow-run", + "pulls/list-files", + ] + ) + assert forward == { + "actions/download-job-logs-for-workflow-run": ( + "actions_download-job-logs-for-workflow-run" + ), + "pulls/list-files": "pulls_list-files", + } + assert reverse == {v: k for k, v in forward.items()} + # untouched names absent + assert "fine_name" not in forward + assert "fine_name" not in reverse + + +def test_build_anthropic_tool_name_maps_disambiguates_collision_with_existing_valid(): + """If `foo/bar` would collapse to `foo_bar` but `foo_bar` already exists, + the rewritten one must get a unique suffix and only THAT one shows up in + the reverse map. The legitimately-named `foo_bar` round-trips identically.""" + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps(["foo_bar", "foo/bar"]) + # The original valid name keeps its slot. + assert "foo_bar" not in forward # untouched + # The rewritten one gets a disambiguating suffix. + assert forward["foo/bar"] == "foo_bar_2" + # Reverse map only has the rewritten entry. + assert reverse == {"foo_bar_2": "foo/bar"} + # CRITICAL: a legit `foo_bar` returned by the model must NOT round-trip + # to `foo/bar`. + assert "foo_bar" not in reverse + + +def test_build_anthropic_tool_name_maps_disambiguates_two_rewrites_to_same_target(): + """Two different invalid names that collapse to the same candidate must + both end up with unique sanitized forms.""" + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps(["foo/bar", "foo.bar"]) + # First wins the canonical slot, second gets a suffix. + assert forward["foo/bar"] == "foo_bar" + assert forward["foo.bar"] == "foo_bar_2" + # Round-trip is unambiguous. + assert reverse["foo_bar"] == "foo/bar" + assert reverse["foo_bar_2"] == "foo.bar" + + +def test_build_anthropic_tool_name_maps_three_way_collision(): + """`foo/bar`, `foo.bar`, and an existing `foo_bar` must all coexist.""" + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps( + ["foo_bar", "foo/bar", "foo.bar"] + ) + assert "foo_bar" not in forward # untouched + assert forward["foo/bar"] == "foo_bar_2" + assert forward["foo.bar"] == "foo_bar_3" + # All three sanitized names are distinct. + sent_names = {"foo_bar", forward["foo/bar"], forward["foo.bar"]} + assert len(sent_names) == 3 + assert reverse == {"foo_bar_2": "foo/bar", "foo_bar_3": "foo.bar"} + + +def test_map_tools_sanitizes_function_tool_name(): + """Tools sent to Anthropic must have names matching ^[a-zA-Z0-9_-]{1,128}$.""" + import re as _re + + config = AnthropicConfig() + bad_name = "github_openapi_mcp-actions/download-job-logs-for-workflow-run" + tools = [ + { + "type": "function", + "function": { + "name": bad_name, + "description": "desc", + "parameters": { + "type": "object", + "properties": {"x": {"type": "string"}}, + }, + }, + } + ] + forward, _ = config._build_request_tool_name_maps(tools) + + anthropic_tools, _ = config._map_tools(tools, name_forward_map=forward) + + assert len(anthropic_tools) == 1 + sent_name = anthropic_tools[0]["name"] + assert _re.fullmatch( + r"[a-zA-Z0-9_-]{1,128}", sent_name + ), f"sanitized name {sent_name!r} still violates Anthropic's regex" + assert sent_name == "github_openapi_mcp-actions_download-job-logs-for-workflow-run" + + +def test_map_tool_choice_sanitizes_named_tool(): + config = AnthropicConfig() + forward = { + "actions/download-job-logs-for-workflow-run": ( + "actions_download-job-logs-for-workflow-run" + ) + } + tool_choice = { + "type": "function", + "function": {"name": "actions/download-job-logs-for-workflow-run"}, + } + out = config._map_tool_choice( + tool_choice=tool_choice, parallel_tool_use=None, name_forward_map=forward + ) + assert out is not None + assert out["type"] == "tool" + assert out["name"] == "actions_download-job-logs-for-workflow-run" + + +def test_map_tool_choice_no_forward_map_passes_through_valid_name(): + """tool_choice with an already-valid name and no map -> unchanged.""" + config = AnthropicConfig() + tool_choice = {"type": "function", "function": {"name": "plain_tool"}} + out = config._map_tool_choice( + tool_choice=tool_choice, parallel_tool_use=None, name_forward_map=None + ) + assert out is not None + assert out["name"] == "plain_tool" + + +def test_map_openai_params_stashes_forward_and_reverse_maps(): + config = AnthropicConfig() + optional_params: dict = {} + config.map_openai_params( + non_default_params={ + "tools": [ + { + "type": "function", + "function": { + "name": "actions/download-job-logs-for-workflow-run", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + }, + optional_params=optional_params, + model="claude-sonnet-4", + drop_params=False, + ) + # forward map (original -> sanitized) for use by message rewriting + assert "_anthropic_tool_name_forward_map" in optional_params + assert ( + optional_params["_anthropic_tool_name_forward_map"][ + "actions/download-job-logs-for-workflow-run" + ] + == "actions_download-job-logs-for-workflow-run" + ) + # reverse map (sanitized -> original) for response translation + assert "_anthropic_tool_name_map" in optional_params + assert ( + optional_params["_anthropic_tool_name_map"][ + "actions_download-job-logs-for-workflow-run" + ] + == "actions/download-job-logs-for-workflow-run" + ) + + +def test_map_openai_params_no_maps_when_all_names_already_valid(): + config = AnthropicConfig() + optional_params: dict = {} + config.map_openai_params( + non_default_params={ + "tools": [ + { + "type": "function", + "function": { + "name": "plain_tool", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + }, + optional_params=optional_params, + model="claude-sonnet-4", + drop_params=False, + ) + assert "_anthropic_tool_name_forward_map" not in optional_params + assert "_anthropic_tool_name_map" not in optional_params + + +def test_rewrite_tool_names_in_messages_uses_forward_map(): + config = AnthropicConfig() + forward_map = { + "actions/download-job-logs-for-workflow-run": ( + "actions_download-job-logs-for-workflow-run" + ) + } + messages = [ + {"role": "user", "content": "go"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": { + "name": "actions/download-job-logs-for-workflow-run", + "arguments": "{}", + }, + } + ], + }, + {"role": "tool", "tool_call_id": "call_1", "content": "ok"}, + ] + + out = config._rewrite_tool_names_in_messages(messages, forward_map) + + # input list must not be mutated + assert ( + messages[1]["tool_calls"][0]["function"]["name"] + == "actions/download-job-logs-for-workflow-run" + ) + # output rewritten according to forward map + assert ( + out[1]["tool_calls"][0]["function"]["name"] + == "actions_download-job-logs-for-workflow-run" + ) + # non-tool-call messages pass through unchanged (same object) + assert out[0] is messages[0] + assert out[2] is messages[2] + + +def test_rewrite_tool_names_in_messages_leaves_unmapped_names_alone(): + """A tool_call name not in the forward map must NOT be rewritten, + even if it happens to look like a sanitized form of some other tool.""" + config = AnthropicConfig() + # `foo_bar` is NOT in the forward map (only `foo/bar` -> `foo_bar_2` is). + # If we naively re-sanitized, `foo_bar` would stay `foo_bar`, but more + # subtly, in a buggy implementation we might collide it with the codomain + # of some other rewrite. Either way: it must round-trip identically. + forward_map = {"foo/bar": "foo_bar_2"} + messages = [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "foo_bar", "arguments": "{}"}, + } + ], + }, + ] + out = config._rewrite_tool_names_in_messages(messages, forward_map) + assert out[0]["tool_calls"][0]["function"]["name"] == "foo_bar" + # input list must not be mutated either way + assert messages[0]["tool_calls"][0]["function"]["name"] == "foo_bar" + + +def test_transform_parsed_response_reverse_maps_tool_names(): + """End-to-end: rewritten tool name in Anthropic response -> original in OpenAI tool_calls.""" + import json as _json + + config = AnthropicConfig() + raw_response = MagicMock() + raw_response.headers = {} + raw_response.status_code = 200 + + completion_response = { + "id": "msg_x", + "model": "claude-sonnet-4", + "stop_reason": "tool_use", + "usage": {"input_tokens": 1, "output_tokens": 1}, + "content": [ + { + "type": "tool_use", + "id": "toolu_1", + "name": "actions_download-job-logs-for-workflow-run", + "input": {"job_id": 123}, + } + ], + } + from litellm.types.utils import ModelResponse + + model_response = ModelResponse() + + out = config.transform_parsed_response( + completion_response=completion_response, + raw_response=raw_response, + model_response=model_response, + tool_name_reverse_map={ + "actions_download-job-logs-for-workflow-run": "actions/download-job-logs-for-workflow-run", + }, + ) + + tcs = out.choices[0].message.tool_calls + assert tcs is not None and len(tcs) == 1 + assert tcs[0].function.name == "actions/download-job-logs-for-workflow-run" + assert _json.loads(tcs[0].function.arguments) == {"job_id": 123} + + +def test_transform_parsed_response_does_not_rewrite_unmapped_names(): + """CRITICAL: a tool legitimately named `foo_bar` must NOT be rewritten + to `foo/bar` just because some other request had that pair. The reverse + map is per-request -- only entries we actually created go in it.""" + config = AnthropicConfig() + raw_response = MagicMock() + raw_response.headers = {} + raw_response.status_code = 200 + + # Caller registered `foo_bar` (valid) and `foo/bar` (rewrites to foo_bar_2). + # The reverse map only contains the rewrite. + reverse_map = {"foo_bar_2": "foo/bar"} + + completion_response = { + "id": "msg_x", + "model": "claude-sonnet-4", + "stop_reason": "tool_use", + "usage": {"input_tokens": 1, "output_tokens": 1}, + "content": [ + { + "type": "tool_use", + "id": "toolu_1", + "name": "foo_bar", # the legit one, NOT in reverse map + "input": {}, + } + ], + } + from litellm.types.utils import ModelResponse + + model_response = ModelResponse() + out = config.transform_parsed_response( + completion_response=completion_response, + raw_response=raw_response, + model_response=model_response, + tool_name_reverse_map=reverse_map, + ) + # Must come back as-is, not rewritten to "foo/bar". + assert out.choices[0].message.tool_calls[0].function.name == "foo_bar" + + +def test_transform_parsed_response_no_reverse_map_is_noop(): + """When no map is provided, tool name is passed through unchanged.""" + config = AnthropicConfig() + raw_response = MagicMock() + raw_response.headers = {} + raw_response.status_code = 200 + + completion_response = { + "id": "msg_x", + "model": "claude-sonnet-4", + "stop_reason": "tool_use", + "usage": {"input_tokens": 1, "output_tokens": 1}, + "content": [ + { + "type": "tool_use", + "id": "toolu_1", + "name": "plain_tool", + "input": {}, + } + ], + } + from litellm.types.utils import ModelResponse + + model_response = ModelResponse() + out = config.transform_parsed_response( + completion_response=completion_response, + raw_response=raw_response, + model_response=model_response, + ) + assert out.choices[0].message.tool_calls[0].function.name == "plain_tool" + + +def test_streaming_iterator_reverse_maps_tool_use_name(): + """Streaming `content_block_start` for tool_use should reverse-map the name.""" + from litellm.llms.anthropic.chat.handler import ModelResponseIterator + + iterator = ModelResponseIterator( + streaming_response=iter([]), + sync_stream=True, + tool_name_reverse_map={ + "actions_download-job-logs-for-workflow-run": "actions/download-job-logs-for-workflow-run", + }, + ) + + chunk = { + "type": "content_block_start", + "index": 0, + "content_block": { + "type": "tool_use", + "id": "toolu_1", + "name": "actions_download-job-logs-for-workflow-run", + "input": {}, + }, + } + parsed = iterator.chunk_parser(chunk=chunk) + tool_calls = parsed.choices[0].delta.tool_calls + assert tool_calls is not None and len(tool_calls) == 1 + assert ( + tool_calls[0]["function"]["name"] + == "actions/download-job-logs-for-workflow-run" + ) + + +def test_streaming_iterator_passthrough_when_name_not_in_map(): + from litellm.llms.anthropic.chat.handler import ModelResponseIterator + + iterator = ModelResponseIterator( + streaming_response=iter([]), + sync_stream=True, + tool_name_reverse_map=None, + ) + chunk = { + "type": "content_block_start", + "index": 0, + "content_block": { + "type": "tool_use", + "id": "toolu_1", + "name": "plain_tool", + "input": {}, + }, + } + parsed = iterator.chunk_parser(chunk=chunk) + tool_calls = parsed.choices[0].delta.tool_calls + assert tool_calls is not None and len(tool_calls) == 1 + assert tool_calls[0]["function"]["name"] == "plain_tool" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py index efe100a11dc..957dea22f3c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py @@ -868,3 +868,146 @@ class TestResolveOperationParams: assert "per_page" in names assert "sha" in names assert len(names) == 4 # no duplicates + + +# --------------------------------------------------------------------------- +# Tool name sanitization for OpenAPI -> MCP +# Repro: GitHub's REST OpenAPI uses tag-namespaced operationIds like +# "actions/download-job-logs-for-workflow-run". Without sanitization the +# generated MCP tool name contains '/', which Anthropic/OpenAI/Bedrock all +# reject (^[a-zA-Z0-9_-]+$). This block guards the registration + preview +# paths against that. +# --------------------------------------------------------------------------- + + +class TestSanitizeOpenAPIToolName: + def test_replaces_slashes(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + assert ( + sanitize_openapi_tool_name("actions/download-job-logs-for-workflow-run") + == "actions_download-job-logs-for-workflow-run" + ) + + def test_replaces_other_punctuation(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + assert sanitize_openapi_tool_name("foo.bar:baz qux") == "foo_bar_baz_qux" + + def test_lowercases(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + assert sanitize_openapi_tool_name("Pulls/List-Files") == "pulls_list-files" + + def test_already_valid_passes_through(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + assert sanitize_openapi_tool_name("plain-tool_name") == "plain-tool_name" + + def test_empty_string(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + assert sanitize_openapi_tool_name("") == "" + + def test_caps_at_128_chars(self): + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + out = sanitize_openapi_tool_name("a/" * 200) + assert len(out) <= 128 + + +class TestRegisterToolsFromOpenAPI: + """Verify register_tools_from_openapi emits provider-safe tool names.""" + + def test_github_style_operation_ids_are_sanitized(self, monkeypatch): + import re + + from litellm.proxy._experimental.mcp_server import openapi_to_mcp_generator + + registered: list = [] + + def _capture(name, description, input_schema, handler): # noqa: ANN001 + registered.append(name) + + monkeypatch.setattr( + openapi_to_mcp_generator.global_mcp_tool_registry, + "register_tool", + _capture, + ) + + spec = { + "paths": { + "/repos/{owner}/{repo}/actions/jobs/{job_id}/logs": { + "get": { + "operationId": "actions/download-job-logs-for-workflow-run", + "summary": "Download job logs", + } + }, + "/repos/{owner}/{repo}/pulls/{pull_number}/files": { + "get": { + "operationId": "pulls/list-files", + "summary": "List files", + } + }, + } + } + + openapi_to_mcp_generator.register_tools_from_openapi( + spec, base_url="https://api.example.com" + ) + + assert registered, "expected at least one registered tool" + anthropic_re = re.compile(r"^[a-zA-Z0-9_-]{1,128}$") + for name in registered: + assert anthropic_re.match( + name + ), f"tool name {name!r} violates ^[a-zA-Z0-9_-]+$" + assert "actions_download-job-logs-for-workflow-run" in registered + assert "pulls_list-files" in registered + + def test_missing_operation_id_uses_sanitized_method_path_fallback( + self, monkeypatch + ): + import re + + from litellm.proxy._experimental.mcp_server import openapi_to_mcp_generator + + registered: list = [] + + def _capture(name, description, input_schema, handler): # noqa: ANN001 + registered.append(name) + + monkeypatch.setattr( + openapi_to_mcp_generator.global_mcp_tool_registry, + "register_tool", + _capture, + ) + + spec = { + "paths": { + "/foo/{bar}/baz": { + "get": {"summary": "no operationId here"}, + } + } + } + openapi_to_mcp_generator.register_tools_from_openapi( + spec, base_url="https://api.example.com" + ) + + assert registered + for name in registered: + assert re.match( + r"^[a-zA-Z0-9_-]+$", name + ), f"fallback tool name {name!r} not sanitized" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py index 90e504c959c..68787d7a668 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py @@ -1376,3 +1376,74 @@ class TestEndpointRoleChecks: user_api_key_dict=user_key, ) assert result["status"] == "ok" + + +class TestPreviewOpenAPITools: + """Verify the OpenAPI preview endpoint emits provider-safe tool names. + + Regression: GitHub's OpenAPI spec uses tag-namespaced operationIds like + `actions/download-job-logs-for-workflow-run` which contain '/'. The + preview must sanitize so what the dashboard shows matches what gets + registered (and what makes it past LLM provider tool-name validation). + """ + + pytestmark = pytest.mark.asyncio + + async def test_preview_sanitizes_slash_in_operation_id(self, monkeypatch): + import re + + async def fake_load_spec(spec_path): # noqa: ANN001 + return { + "paths": { + "/repos/{owner}/{repo}/actions/jobs/{job_id}/logs": { + "get": { + "operationId": ( + "actions/download-job-logs-for-workflow-run" + ), + "summary": "Download job logs", + } + }, + "/repos/{owner}/{repo}/pulls/{pull_number}/files": { + "get": { + "operationId": "pulls/list-files", + "summary": "List files", + } + }, + } + } + + from litellm.proxy._experimental.mcp_server import ( + openapi_to_mcp_generator, + ) + + monkeypatch.setattr( + openapi_to_mcp_generator, + "load_openapi_spec_async", + fake_load_spec, + raising=False, + ) + + payload = NewMCPServerRequest( + server_name="github_openapi_mcp", + spec_path="https://example.invalid/openapi.json", + transport="http", + ) + request = _build_request() + + from litellm.proxy._types import LitellmUserRoles + + result = await rest_endpoints.test_tools_list( + request, + payload, + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN), + ) + + assert result.get("error") is None, result + names = [t["name"] for t in result["tools"]] + anthropic_re = re.compile(r"^[a-zA-Z0-9_-]{1,128}$") + for name in names: + assert anthropic_re.match( + name + ), f"preview tool name {name!r} violates ^[a-zA-Z0-9_-]+$" + assert "actions_download-job-logs-for-workflow-run" in names + assert "pulls_list-files" in names