diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index ba840bc3d8..abe9e016e2 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -2124,27 +2124,62 @@ def anthropic_process_openai_file_message( ) +_EMPTY_TEXT_PLACEHOLDER = ( + "[System: Empty message content sanitised to satisfy protocol]" +) + + def _sanitize_empty_text_content( message: AllMessageValues, ) -> AllMessageValues: """ Case C: Sanitize empty text content - Replace empty or whitespace-only text content with a placeholder message. + - Handles both string content and list-of-blocks content (rewriting only + the empty text blocks in place; non-text blocks like images are left + untouched). Returns: The message with sanitized content if needed, otherwise the original message """ - if message.get("role") in ["user", "assistant"]: - content = message.get("content") - if isinstance(content, str): - if not content or not content.strip(): - message = cast(AllMessageValues, dict(message)) # Make a copy - message["content"] = ( - "[System: Empty message content sanitised to satisfy protocol]" - ) - verbose_logger.debug( - f"_sanitize_empty_text_content: Replaced empty text content in {message.get('role')} message" - ) + if message.get("role") not in ["user", "assistant"]: + return message + + content = message.get("content") + + if isinstance(content, str): + if not content or not content.strip(): + message = cast(AllMessageValues, dict(message)) # Make a copy + message["content"] = _EMPTY_TEXT_PLACEHOLDER + verbose_logger.debug( + f"_sanitize_empty_text_content: Replaced empty text content in {message.get('role')} message" + ) + return message + + if isinstance(content, list): + # Walk the blocks and rewrite any empty text blocks. We rewrite (rather + # than drop) so callers don't end up with an entirely empty content + # list, which Anthropic also rejects. + new_blocks: List[Any] = [] + rewrote_any = False + for block in content: + if isinstance(block, dict) and block.get("type") == "text": + text = block.get("text") + if not isinstance(text, str) or not text or not text.strip(): + new_block = dict(block) + new_block["text"] = _EMPTY_TEXT_PLACEHOLDER + new_blocks.append(new_block) + rewrote_any = True + continue + new_blocks.append(block) + + if rewrote_any: + message = cast(AllMessageValues, dict(message)) # Make a copy + message["content"] = new_blocks # type: ignore + verbose_logger.debug( + f"_sanitize_empty_text_content: Replaced empty text block(s) in {message.get('role')} message" + ) + return message @@ -2427,6 +2462,18 @@ def anthropic_messages_pt( # noqa: PLR0915 # Sanitize messages for tool calling issues when modify_params=True messages = sanitize_messages_for_tool_calling(messages) + # Anthropic rejects empty text content blocks with: + # "messages: text content blocks must be non-empty" + # OpenAI/other providers silently tolerate `{"role": "user", "content": ""}`, + # so callers (and upstream agent frameworks like pydantic-ai) routinely + # send empty user/assistant turns. We always rewrite these to a placeholder + # for Anthropic-shaped requests, independent of `litellm.modify_params`, + # because there is no way to "pass through" an empty text block — the + # request will always 400 otherwise. The richer tool-call sanitization + # (Cases A/B/D in `sanitize_messages_for_tool_calling`) remains gated on + # `modify_params` because it actually mutates conversation structure. + messages = [_sanitize_empty_text_content(m) for m in messages] + # add role=tool support to allow function call result/error submission user_message_types = {"user", "tool", "function"} # reformat messages to ensure user/assistant are alternating, if there's either 2 consecutive 'user' messages or 2 consecutive 'assistant' message, merge them. diff --git a/litellm/llms/anthropic/chat/handler.py b/litellm/llms/anthropic/chat/handler.py index f8e61d0166..672de6a705 100644 --- a/litellm/llms/anthropic/chat/handler.py +++ b/litellm/llms/anthropic/chat/handler.py @@ -65,7 +65,7 @@ from litellm.types.utils import ( from ...base import BaseLLM from ..common_utils import AnthropicError, process_anthropic_headers -from .transformation import AnthropicConfig +from .transformation import ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY, AnthropicConfig if TYPE_CHECKING: from litellm.litellm_core_utils.streaming_handler import CustomStreamWrapper @@ -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_REVERSE_MAP_KEY) + 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_REVERSE_MAP_KEY) + 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 345558a69f..dc3100f467 100644 --- a/litellm/llms/anthropic/chat/transformation.py +++ b/litellm/llms/anthropic/chat/transformation.py @@ -105,6 +105,115 @@ 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 +# Single, internal-only key on ``litellm_params`` used to thread the per- +# request reverse map (sanitized -> original) from request build to response +# parsing. ``litellm_params`` is never serialized to a provider; ``optional_ +# params`` IS (it becomes the JSON body via ``data = {**optional_params}``). +# Keep these two channels strictly separate -- never stash internal +# coordination state in ``optional_params``. +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() + + # First pass: reserve slots for names that are already valid so they + # always have priority regardless of input order. + for original in original_names: + if not isinstance(original, str) or not original: + continue + candidate = _basic_sanitize_anthropic_tool_name(original) + if candidate == original: + used.add(candidate) + + # Second pass: sanitize/disambiguate names that need rewriting. + for original in original_names: + if not isinstance(original, str) or not original: + continue + candidate = _basic_sanitize_anthropic_tool_name(original) + if candidate == original: + continue + # Skip duplicates of the same original name. Without this guard the + # second pass would assign a fresh suffix and overwrite the forward + # map entry, causing every reference to map to the suffixed name and + # leaving the original sanitized slot orphaned in `used` with no + # reverse mapping. + if original in forward: + 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 + + REASONING_EFFORT_TO_OUTPUT_CONFIG_EFFORT: Dict[str, str] = { "low": "low", "minimal": "low", @@ -486,7 +595,9 @@ 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], ) -> Optional[AnthropicMessagesToolChoice]: _tool_choice: Optional[AnthropicMessagesToolChoice] = None if tool_choice == "auto": @@ -527,7 +638,8 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): return _tool_choice def _map_tool_helper( # noqa: PLR0915 - self, tool: ChatCompletionToolParam + self, + tool: ChatCompletionToolParam, ) -> Tuple[Optional[AllAnthropicToolsValues], Optional[AnthropicMcpServerTool]]: returned_tool: Optional[AllAnthropicToolsValues] = None mcp_server: Optional[AnthropicMcpServerTool] = None @@ -783,7 +895,8 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): return initial_tool def _map_tools( - self, tools: List + self, + tools: List, ) -> Tuple[List[AllAnthropicToolsValues], List[AnthropicMcpServerTool]]: anthropic_tools = [] mcp_servers = [] @@ -799,6 +912,174 @@ 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 isinstance(tool_calls, list): + 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. + + Operates on **OpenAI-format** tool dicts (pre-``_map_tools``). The + production sanitization path uses ``_sanitize_tool_names_in_request`` + instead, which operates on **Anthropic-format** tools (post- + ``_map_tools``, where ``type == "custom"``). This helper exists for + callers that need to compute the maps from the raw OpenAI shape -- + e.g. test setup or future pre-mapping consumers. + + See _build_anthropic_tool_name_maps for the collision 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) + + @staticmethod + def _sanitize_tool_names_in_request( + optional_params: Dict[str, Any], + ) -> Tuple[Dict[str, str], Dict[str, str]]: + """Sanitize ``optional_params['tools']`` and ``optional_params['tool_choice']`` + in place so every name matches Anthropic's ``^[a-zA-Z0-9_-]{1,128}$``. + + Returns ``(forward, reverse)`` for use by message-history rewriting + and response translation. ``forward[original] = sanitized`` is only + populated for names that were actually rewritten -- i.e. either + contained an invalid character or collided with another tool's + sanitized form. Names already valid AND unique pass through and are + absent from both maps. + + Only ``type == "custom"`` tools (the OpenAI function-tool shape) are + considered. Hosted tools (``web_search``, ``bash``, ``code_execution``, + ``computer_*``, ``mcp``, ...) own reserved names defined by Anthropic + and must not be touched. + """ + tools = optional_params.get("tools") + if not isinstance(tools, list) or not tools: + return {}, {} + + # 1. Collect originals from the Anthropic-shaped custom-tool entries. + # Order matters: the first occurrence wins the canonical slot; + # later collisions get numeric suffixes (see + # ``_build_anthropic_tool_name_maps``). + original_names: List[str] = [] + for t in tools: + if not isinstance(t, dict): + continue + if t.get("type") != "custom": + continue + name = t.get("name") + if isinstance(name, str) and name: + original_names.append(name) + + if not original_names: + return {}, {} + + forward, reverse = _build_anthropic_tool_name_maps(original_names) + if not forward: + # Every name was already valid -- nothing to do. + return forward, reverse + + # 2. Apply forward map. Build a new list with copy-on-change entries + # so a caller reusing the same tool list/dicts across requests + # doesn't see its inputs permanently rewritten (which would also + # drop the original key from `forward` on the next request). + new_tools: List[Any] = [] + for t in tools: + if ( + isinstance(t, dict) + and t.get("type") == "custom" + and isinstance(t.get("name"), str) + and t["name"] in forward + ): + new_tools.append({**t, "name": forward[t["name"]]}) + else: + new_tools.append(t) + optional_params["tools"] = new_tools + + # 3. Same for ``tool_choice`` when it targets a named tool. Copy + # rather than mutate for the same reason as above. + tool_choice = optional_params.get("tool_choice") + if isinstance(tool_choice, dict) and tool_choice.get("type") == "tool": + tc_name = tool_choice.get("name") + if isinstance(tc_name, str) and tc_name in forward: + optional_params["tool_choice"] = { + **tool_choice, + "name": forward[tc_name], + } + + return forward, reverse + def _detect_tool_search_tools(self, tools: Optional[List]) -> bool: """Check if tool search tools are present in the tools list.""" if not tools: @@ -1125,6 +1406,17 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): non_default_params=non_default_params ) + # NB: ``map_openai_params`` deliberately does NOT sanitize tool names + # here. Names are the *original* OpenAI names at this stage, and must + # remain so until ``transform_request`` -- which is the single + # chokepoint where Anthropic, Bedrock-Anthropic, and Vertex-Anthropic + # all pass through. Doing it there guarantees: + # 1. one source of truth for the per-request forward/reverse maps, + # 2. the maps land on ``litellm_params`` (internal), never on + # ``optional_params`` (which is serialized into the request body + # via ``data = {**optional_params}`` and would 400 with + # ``Extra inputs are not permitted``). + for param, value in non_default_params.items(): if param == "max_tokens": optional_params["max_tokens"] = ( @@ -1135,7 +1427,6 @@ 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) optional_params = self._add_tools_to_optional_params( optional_params=optional_params, tools=anthropic_tools @@ -1565,6 +1856,34 @@ class AnthropicConfig(AnthropicModelInfo, BaseConfig): headers=headers, optional_params=optional_params ) + # === Tool-name sanitization (single chokepoint) === + # Anthropic enforces ^[a-zA-Z0-9_-]{1,128}$ on every tool name. We + # sanitize *here* -- not in map_openai_params -- because: + # + # - This function is the single boundary shared by AnthropicConfig, + # AmazonAnthropicConfig (Bedrock invoke), VertexAIAnthropicConfig, + # and AzureAnthropicConfig (all call ``super().transform_request`` + # or ``AnthropicConfig.transform_request(self, ...)``). Sanitizing + # once here covers every Anthropic-shaped request. + # - The forward/reverse maps are coordination state; they belong on + # ``litellm_params`` (internal-only), never on ``optional_params`` + # (which becomes the JSON body via ``{**optional_params}``). + # - It keeps ``map_openai_params`` a pure param translator with no + # side-channel state. + # + # The reverse map only contains entries for names that were actually + # rewritten -- so a tool legitimately named ``foo_bar`` is never + # incorrectly retyped to ``foo/bar`` on the response side. + # See _build_anthropic_tool_name_maps for the collision-handling + # rules and rationale. + _name_forward_map, _name_reverse_map = self._sanitize_tool_names_in_request( + optional_params=optional_params, + ) + if _name_forward_map: + messages = self._rewrite_tool_names_in_messages(messages, _name_forward_map) + if _name_reverse_map and isinstance(litellm_params, dict): + litellm_params[ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY] = _name_reverse_map + # Separate system prompt from rest of message anthropic_system_message_list = self.translate_system_message(messages=messages) # Handling anthropic API Prompt Caching @@ -2041,6 +2360,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( @@ -2065,6 +2385,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) @@ -2191,6 +2526,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, @@ -2199,6 +2539,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 3b2fa097b7..718435cce6 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,34 @@ 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, @@ -399,17 +423,36 @@ def create_tool_function( def register_tools_from_openapi(spec: Dict[str, Any], base_url: str): """Register MCP tools from OpenAPI specification.""" paths = spec.get("paths", {}) + used_names: set = set() for path, path_item in paths.items(): for method in ["get", "post", "put", "delete", "patch"]: 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) + + # Disambiguate collisions: two operationIds that differ only + # by sanitized characters (e.g. "foo/list" and "foo.list") + # would both become "foo_list". Append _2, _3, … to keep + # every tool reachable, mirroring the Anthropic-side logic + # in _build_anthropic_tool_name_maps. + unique = tool_name + n = 1 + while unique in used_names: + n += 1 + suffix = f"_{n}" + unique = ( + tool_name[: _OPENAPI_TOOL_NAME_MAX_LEN - len(suffix)] + suffix + ) + tool_name = unique + used_names.add(tool_name) # 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 3047fb7332..829863d2db 100644 --- a/litellm/proxy/_experimental/mcp_server/rest_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/rest_endpoints.py @@ -857,6 +857,7 @@ if MCP_AVAILABLE: ######################################################## from litellm.proxy.management_endpoints.mcp_management_endpoints import ( NewMCPServerRequest, + _inherit_credentials_from_existing_server, ) def _extract_credentials( @@ -975,9 +976,11 @@ if MCP_AVAILABLE: async def _preview_openapi_tools(spec_path: str) -> dict: """Generate tool previews from an OpenAPI spec without creating a server.""" from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + _OPENAPI_TOOL_NAME_MAX_LEN, build_input_schema, load_openapi_spec_async, resolve_operation_params, + sanitize_openapi_tool_name, ) try: @@ -985,8 +988,9 @@ if MCP_AVAILABLE: paths = spec.get("paths", {}) components = spec.get("components", {}) tools: List[dict] = [] + used_names: set = set() for path, path_item in paths.items(): - for method in ("get", "post", "put", "patch", "delete"): + for method in ("get", "post", "put", "delete", "patch"): operation = path_item.get(method) if operation is None: continue @@ -995,7 +999,23 @@ 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) + + unique = op_id + n = 1 + while unique in used_names: + n += 1 + suffix = f"_{n}" + unique = ( + op_id[: _OPENAPI_TOOL_NAME_MAX_LEN - len(suffix)] + suffix + ) + op_id = unique + used_names.add(op_id) summary = operation.get("summary", "") description = operation.get("description", summary) input_schema = build_input_schema(resolved_op) @@ -1068,6 +1088,10 @@ if MCP_AVAILABLE: }, ) + new_mcp_server_request = _inherit_credentials_from_existing_server( + new_mcp_server_request + ) + # For OpenAPI spec servers, generate tools from the spec directly if new_mcp_server_request.spec_path: return await _preview_openapi_tools(new_mcp_server_request.spec_path) 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 26ed8d29c1..6f67d5417d 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 @@ -3914,3 +3914,764 @@ 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_build_anthropic_tool_name_maps_reverse_order_collision(): + """REGRESSION: when the invalid name appears *before* the valid name that + its sanitized form collides with, both must still end up with distinct + names on the wire.""" + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps(["foo/bar", "foo_bar"]) + # The valid name keeps its slot untouched. + assert "foo_bar" not in forward + # The rewritten one gets a disambiguating suffix. + assert forward["foo/bar"] == "foo_bar_2" + assert reverse == {"foo_bar_2": "foo/bar"} + assert "foo_bar" not in reverse + + +def test_build_anthropic_tool_name_maps_duplicate_originals(): + """REGRESSION: duplicate originals must not corrupt the forward map. + + Previously, the second occurrence of the same invalid name would + rewrite ``forward[original]`` to a suffixed name (``foo_bar_2``), + leaving ``foo_bar`` orphaned in ``used`` with no reverse mapping — + so when ``_sanitize_tool_names_in_request`` applied the forward + map, *both* tool entries got the suffixed name and Anthropic 400'd + on duplicates. + """ + from litellm.llms.anthropic.chat.transformation import ( + _build_anthropic_tool_name_maps, + ) + + forward, reverse = _build_anthropic_tool_name_maps(["foo/bar", "foo/bar"]) + # Same original sanitizes to the same target — no spurious suffix. + assert forward == {"foo/bar": "foo_bar"} + assert reverse == {"foo_bar": "foo/bar"} + + +def test_map_openai_params_does_not_pollute_optional_params_with_internal_keys(): + """REGRESSION: ``optional_params`` is what becomes the JSON body sent to + Anthropic (``data = {**optional_params}``). It MUST NOT carry LiteLLM- + internal coordination state like the per-request forward/reverse name + maps, or Anthropic 400s with ``Extra inputs are not permitted``. + Sanitization belongs in ``transform_request``, not here.""" + 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, + ) + # No internal keys may appear in optional_params for ANY input. + for key in optional_params: + assert not key.startswith( + "_anthropic_tool_name" + ), f"optional_params leaked internal key {key!r}: {optional_params}" + # And no key starting with `_` either; optional_params should only + # contain documented Anthropic Messages API parameters. + for key in optional_params: + assert not key.startswith("_"), ( + f"optional_params leaked underscore-prefixed key {key!r}: " + f"{optional_params}" + ) + + +def test_map_openai_params_no_maps_when_all_names_already_valid(): + """Sanity check: an all-valid tool list adds nothing weird either.""" + 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, + ) + for key in optional_params: + assert not key.startswith("_anthropic_tool_name") + + +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_rewrite_tool_names_in_messages_with_tool_calls_and_none_function_call(): + """When a message has tool_calls but function_call is explicitly None, + the rewrite must still apply to tool_calls and leave function_call as + None. Pins behavior at the boundary where ``new_msg = dict(msg)`` + copies the explicit-None key forward.""" + config = AnthropicConfig() + forward_map = {"foo/bar": "foo_bar"} + messages = [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "foo/bar", "arguments": "{}"}, + } + ], + "function_call": None, + }, + ] + out = config._rewrite_tool_names_in_messages(messages, forward_map) + assert out[0]["tool_calls"][0]["function"]["name"] == "foo_bar" + assert out[0]["function_call"] is None + # input list must not be mutated + assert messages[0]["tool_calls"][0]["function"]["name"] == "foo/bar" + + +def test_sanitize_tool_names_in_request_does_not_mutate_caller_tool_dicts(): + """REGRESSION: a caller reusing the same tool list/dicts across requests + must not see its inputs permanently rewritten. _sanitize_tool_names_in_request + builds a new list with copy-on-change entries.""" + config = AnthropicConfig() + original_name = "actions/download-job-logs-for-workflow-run" + caller_tool = { + "type": "custom", + "name": original_name, + "input_schema": {"type": "object", "properties": {}}, + } + caller_tools = [caller_tool] + optional_params: dict = {"tools": caller_tools} + + forward, reverse = config._sanitize_tool_names_in_request( + optional_params=optional_params + ) + + assert forward.get(original_name) + sanitized = forward[original_name] + assert optional_params["tools"][0]["name"] == sanitized + # caller's original dict + list must not be touched + assert caller_tool["name"] == original_name + assert caller_tools[0] is caller_tool + + +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" + + +# --------------------------------------------------------------------------- +# transform_request: end-to-end sanitization regression coverage +# --------------------------------------------------------------------------- + + +def _build_optional_params_for_tools(tools): + """Run a tools list through ``map_openai_params`` to get the same shape + ``transform_request`` will see from the router. Keeping this helper local + avoids duplicating the OpenAI->Anthropic param mapping in tests.""" + config = AnthropicConfig() + optional_params: dict = {} + config.map_openai_params( + non_default_params={"tools": tools}, + optional_params=optional_params, + model="claude-sonnet-4", + drop_params=False, + ) + return optional_params + + +def test_transform_request_does_not_leak_internal_keys_into_body(): + """REGRESSION for "_anthropic_tool_name_forward_map: Extra inputs are not + permitted". The dict returned by ``transform_request`` is what becomes + the JSON body POSTed to Anthropic. It must contain ONLY documented + Anthropic Messages fields -- no LiteLLM coordination state.""" + config = AnthropicConfig() + tools = [ + { + "type": "function", + "function": { + "name": "github_openapi_mcp-actions/download-job-logs-for-workflow-run", + "description": "d", + "parameters": {"type": "object", "properties": {}}, + }, + }, + { + "type": "function", + "function": { + "name": "plain_tool", + "description": "d", + "parameters": {"type": "object", "properties": {}}, + }, + }, + ] + optional_params = _build_optional_params_for_tools(tools) + litellm_params: dict = {} + + data = config.transform_request( + model="claude-sonnet-4", + messages=[{"role": "user", "content": "go"}], + optional_params=optional_params, + litellm_params=litellm_params, + headers={}, + ) + + # Body must not contain any LiteLLM-internal keys. + for key in data.keys(): + assert not key.startswith("_"), ( + f"transformed request body leaked underscore-prefixed key {key!r}; " + f"Anthropic will reject this with 'Extra inputs are not permitted'. " + f"body keys: {list(data.keys())}" + ) + + # Tool names in the body match Anthropic's pattern. + import re as _re + + for tool in data.get("tools", []): + name = tool.get("name") + assert isinstance(name, str) + assert _re.fullmatch( + r"[a-zA-Z0-9_-]{1,128}", name + ), f"sanitized tool name {name!r} still violates Anthropic regex" + + # Sent name for the bad tool is the disambiguated form, valid name passes through. + sent_names = {t["name"] for t in data["tools"]} + assert "github_openapi_mcp-actions_download-job-logs-for-workflow-run" in sent_names + assert "plain_tool" in sent_names + + # Reverse map landed on litellm_params (NOT optional_params, NOT body). + rmap = litellm_params["_anthropic_tool_name_map"] + assert ( + rmap["github_openapi_mcp-actions_download-job-logs-for-workflow-run"] + == "github_openapi_mcp-actions/download-job-logs-for-workflow-run" + ) + # The legitimately-named tool is not in the reverse map -- it round-trips + # untouched on the response side. + assert "plain_tool" not in rmap + + +def test_transform_request_no_reverse_map_when_all_names_valid(): + """If every name is already valid, ``litellm_params`` stays clean + (no reverse map key) -- minimizes blast radius for the common case.""" + config = AnthropicConfig() + tools = [ + { + "type": "function", + "function": { + "name": "plain_tool", + "description": "d", + "parameters": {"type": "object", "properties": {}}, + }, + }, + ] + optional_params = _build_optional_params_for_tools(tools) + litellm_params: dict = {} + + data = config.transform_request( + model="claude-sonnet-4", + messages=[{"role": "user", "content": "go"}], + optional_params=optional_params, + litellm_params=litellm_params, + headers={}, + ) + assert data["tools"][0]["name"] == "plain_tool" + assert "_anthropic_tool_name_map" not in litellm_params + + +def test_transform_request_sanitizes_tool_choice_named_tool(): + """``tool_choice={"type": "function", "function": {"name": ""}}`` + must arrive at Anthropic as ``{"type": "tool", "name": ""}``, + matching the sanitized name in the tools array.""" + config = AnthropicConfig() + tools = [ + { + "type": "function", + "function": { + "name": "actions/download-job-logs-for-workflow-run", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + optional_params = AnthropicConfig().map_openai_params( + non_default_params={ + "tools": tools, + "tool_choice": { + "type": "function", + "function": {"name": "actions/download-job-logs-for-workflow-run"}, + }, + }, + optional_params={}, + model="claude-sonnet-4", + drop_params=False, + ) + litellm_params: dict = {} + data = config.transform_request( + model="claude-sonnet-4", + messages=[{"role": "user", "content": "go"}], + optional_params=optional_params, + litellm_params=litellm_params, + headers={}, + ) + assert data["tool_choice"]["type"] == "tool" + assert data["tool_choice"]["name"] == "actions_download-job-logs-for-workflow-run" + assert data["tools"][0]["name"] == "actions_download-job-logs-for-workflow-run" + + +def test_transform_request_rewrites_tool_names_in_history(): + """Historical assistant messages with ``tool_calls`` referencing the bad + name must be rewritten to the sanitized form so Anthropic doesn't 400 on + ``tool_use.name`` mismatching the (sanitized) tools array.""" + config = AnthropicConfig() + tools = [ + { + "type": "function", + "function": { + "name": "actions/download-job-logs-for-workflow-run", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + optional_params = _build_optional_params_for_tools(tools) + messages = [ + {"role": "user", "content": "logs please"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "toolu_old", + "type": "function", + "function": { + "name": "actions/download-job-logs-for-workflow-run", + "arguments": "{}", + }, + } + ], + }, + {"role": "tool", "tool_call_id": "toolu_old", "content": "..."}, + {"role": "user", "content": "again"}, + ] + litellm_params: dict = {} + data = config.transform_request( + model="claude-sonnet-4", + messages=messages, + optional_params=optional_params, + litellm_params=litellm_params, + headers={}, + ) + # Find the assistant tool_use block in the Anthropic-shaped messages. + tool_use_names = [] + for msg in data["messages"]: + content = msg.get("content") + if not isinstance(content, list): + continue + for block in content: + if isinstance(block, dict) and block.get("type") == "tool_use": + tool_use_names.append(block.get("name")) + assert ( + tool_use_names + ), "expected at least one tool_use block in transformed messages" + for name in tool_use_names: + assert name == "actions_download-job-logs-for-workflow-run", ( + f"history tool_use.name {name!r} not rewritten -- Anthropic will " + f"400 because it doesn't match the (sanitized) tools array" + ) + + +def test_sanitize_tool_names_in_request_skips_hosted_tools(): + """Hosted tools (web_search, computer_*, code_execution, ...) own + Anthropic-reserved names. The sanitizer must not enumerate them as + ``custom`` and must not rename them.""" + optional_params = { + "tools": [ + {"type": "web_search_20250305", "name": "web_search"}, + { + "type": "custom", + "name": "actions/download-job-logs-for-workflow-run", + "input_schema": {"type": "object", "properties": {}}, + }, + ], + } + forward, reverse = AnthropicConfig._sanitize_tool_names_in_request(optional_params) + # Only the custom tool was rewritten. + assert forward == { + "actions/download-job-logs-for-workflow-run": "actions_download-job-logs-for-workflow-run" + } + assert reverse == { + "actions_download-job-logs-for-workflow-run": "actions/download-job-logs-for-workflow-run" + } + # Hosted tool's name unchanged. + assert optional_params["tools"][0]["name"] == "web_search" + # Custom tool's name updated in place. + assert ( + optional_params["tools"][1]["name"] + == "actions_download-job-logs-for-workflow-run" + ) + + +def test_sanitize_tool_names_in_request_no_tools_is_noop(): + """Empty / missing tools must not error or pollute return.""" + forward, reverse = AnthropicConfig._sanitize_tool_names_in_request({}) + assert forward == {} + assert reverse == {} + forward, reverse = AnthropicConfig._sanitize_tool_names_in_request({"tools": []}) + assert forward == {} + assert reverse == {} diff --git a/tests/test_litellm/llms/anthropic/test_message_sanitization.py b/tests/test_litellm/llms/anthropic/test_message_sanitization.py index a5f9c479d5..79ed321d0e 100644 --- a/tests/test_litellm/llms/anthropic/test_message_sanitization.py +++ b/tests/test_litellm/llms/anthropic/test_message_sanitization.py @@ -339,6 +339,95 @@ class TestMessageSanitization: assert result[0]["role"] == "user" assert result[1]["role"] == "assistant" + def test_empty_string_content_sanitized_without_modify_params(self): + """ + Regression: An empty user message ({"role": "user", "content": ""}) must + be rewritten to a non-empty placeholder *before* it reaches Anthropic, + even when litellm.modify_params is False. Otherwise Anthropic returns: + "messages: text content blocks must be non-empty" + Reproduces a real failure from the pr-review agent (pydantic-ai). + """ + litellm.modify_params = False + + messages = [ + {"role": "user", "content": "First message"}, + {"role": "user", "content": "please review this"}, + {"role": "user", "content": ""}, + ] + + result = anthropic_messages_pt( + messages=messages, model="claude-sonnet-4-5", llm_provider="anthropic" + ) + + # All three user messages get merged into one user turn for Anthropic. + assert len(result) == 1 + assert result[0]["role"] == "user" + text_blocks = [ + b for b in result[0]["content"] if isinstance(b, dict) and b.get("type") == "text" + ] + assert len(text_blocks) == 3 + # No text block may be empty — that's the contract Anthropic enforces. + for block in text_blocks: + assert block["text"].strip() != "" + assert text_blocks[2]["text"] == ( + "[System: Empty message content sanitised to satisfy protocol]" + ) + + def test_empty_text_block_in_list_content_sanitized(self): + """ + Same regression for the list-of-blocks form: + {"role": "user", "content": [{"type": "text", "text": ""}]} + Empty text *blocks* must be rewritten too, regardless of modify_params. + """ + litellm.modify_params = False + + messages = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "real content"}, + {"type": "text", "text": ""}, + {"type": "text", "text": " \n "}, + ], + }, + ] + + result = anthropic_messages_pt( + messages=messages, model="claude-sonnet-4-5", llm_provider="anthropic" + ) + + assert len(result) == 1 + text_blocks = [ + b for b in result[0]["content"] if isinstance(b, dict) and b.get("type") == "text" + ] + assert len(text_blocks) == 3 + assert text_blocks[0]["text"] == "real content" + for block in text_blocks[1:]: + assert block["text"].strip() != "" + + def test_non_empty_content_unchanged_without_modify_params(self): + """ + Sanity check: when nothing is empty, the messages flow through unchanged + even with modify_params disabled. + """ + litellm.modify_params = False + + messages = [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi there"}, + {"role": "user", "content": "How are you?"}, + ] + + result = anthropic_messages_pt( + messages=messages, model="claude-sonnet-4-5", llm_provider="anthropic" + ) + + # Two user turns + one assistant turn (alternation preserved). + assert len(result) == 3 + assert result[0]["content"][0]["text"] == "Hello" + assert result[1]["content"][0]["text"] == "Hi there" + assert result[2]["content"][0]["text"] == "How are you?" + if __name__ == "__main__": pytest.main([__file__, "-v"]) 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 efe100a11d..957dea22f3 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 90e504c959..f4feac68fc 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,164 @@ 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 + + async def test_preview_method_order_matches_registration(self, monkeypatch): + """Preview must iterate HTTP methods in the same order as + register_tools_from_openapi, otherwise collision-disambiguation + suffixes (_2, _3, ...) get assigned to different operations and the + dashboard shows names that differ from what's actually registered. + """ + from litellm.proxy._experimental.mcp_server import ( + openapi_to_mcp_generator, + ) + + spec = { + "paths": { + "/items/{id}": { + "delete": { + "operationId": "items/delete", + "summary": "Delete item", + }, + "patch": { + "operationId": "items.delete", + "summary": "Soft-delete item", + }, + } + } + } + + async def fake_load_spec(spec_path): # noqa: ANN001 + return spec + + monkeypatch.setattr( + openapi_to_mcp_generator, + "load_openapi_spec_async", + fake_load_spec, + raising=False, + ) + + payload = NewMCPServerRequest( + server_name="collision_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 + preview_summary_to_name = {t["description"]: t["name"] for t in result["tools"]} + + registered_summary_to_name: dict = {} + + def fake_create_tool_function( + path, method, operation, base_url + ): # noqa: ANN001 + def _f(): + return None + + return _f + + monkeypatch.setattr( + openapi_to_mcp_generator, + "create_tool_function", + fake_create_tool_function, + ) + + class _StubRegistry: + def register_tool( + self, name, description, input_schema, handler + ): # noqa: ANN001 + registered_summary_to_name[description] = name + + monkeypatch.setattr( + openapi_to_mcp_generator, + "global_mcp_tool_registry", + _StubRegistry(), + ) + + openapi_to_mcp_generator.register_tools_from_openapi( + spec, base_url="https://example.invalid" + ) + + assert preview_summary_to_name == registered_summary_to_name, ( + f"preview {preview_summary_to_name} != " + f"registered {registered_summary_to_name} — method iteration " + "order is out of sync, so collision suffixes (_2, _3, ...) " + "land on different operations" + ) diff --git a/ui/litellm-dashboard/package-lock.json b/ui/litellm-dashboard/package-lock.json index ca4e469de0..560baedcde 100644 --- a/ui/litellm-dashboard/package-lock.json +++ b/ui/litellm-dashboard/package-lock.json @@ -13242,6 +13242,21 @@ "type": "github", "url": "https://github.com/sponsors/wooorm" } + }, + "node_modules/@next/swc-win32-ia32-msvc": { + "version": "14.2.33", + "resolved": "https://registry.npmjs.org/@next/swc-win32-ia32-msvc/-/swc-win32-ia32-msvc-14.2.33.tgz", + "integrity": "sha512-pc9LpGNKhJ0dXQhZ5QMmYxtARwwmWLpeocFmVG5Z0DzWq5Uf0izcI8tLc+qOpqxO1PWqZ5A7J1blrUIKrIFc7Q==", + "cpu": [ + "ia32" + ], + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } } } } diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index af5bba6dc3..997e76982d 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -1102,10 +1102,6 @@ const MCPServerEdit: React.FC = ({ toolNameToDescription={toolNameToDescription} onToolNameToDisplayNameChange={setToolNameToDisplayName} onToolNameToDescriptionChange={setToolNameToDescription} - externalTools={tools} - externalIsLoading={isLoadingTools} - externalError={toolsError} - externalCanFetch={!!mcpServer.server_id} />