mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
fix(mcp): propagate is_error from MCP tool execution to Anthropic tool_result blocks
When an MCP tool call fails — the server returns CallToolResult.isError=True, a guardrail blocks the call, or execution raises — execute_tool_calls drops the failure flag, so _build_tool_result_message renders the error text as a successful tool return and Anthropic never triggers its native error-handling or retry logic. Add an optional is_error key to MCPToolResult, set it on the isError result path (accepting both is_error and isError attribute names across mcp SDK versions) and on all four exception paths, and pass is_error=True through to the AnthropicMessagesToolResultParam in the native /v1/messages MCP handler. Fixes #49359 Co-authored-by: chauvuusvn <58875435+chauvuusvn@users.noreply.github.com>
This commit is contained in:
parent
60fc121ce8
commit
d08b259ce9
4 changed files with 166 additions and 27 deletions
|
|
@ -48,17 +48,20 @@ def _get_stop_reason(response: AnthropicMessagesResponse) -> str | None:
|
|||
|
||||
def _build_tool_result_message(tool_results: Sequence[Mapping[str, object]]) -> AnthropicMessagesUserMessageParam:
|
||||
"""Turn executed tool results into the user message Anthropic expects."""
|
||||
return AnthropicMessagesUserMessageParam(
|
||||
role="user",
|
||||
content=[
|
||||
AnthropicMessagesToolResultParam(
|
||||
type="tool_result",
|
||||
tool_use_id=str(result.get("tool_call_id") or ""),
|
||||
content=str(result.get("result") or ""),
|
||||
)
|
||||
for result in tool_results
|
||||
],
|
||||
)
|
||||
content: list[AnthropicMessagesToolResultParam] = []
|
||||
for result in tool_results:
|
||||
tool_result = AnthropicMessagesToolResultParam(
|
||||
type="tool_result",
|
||||
tool_use_id=str(result.get("tool_call_id") or ""),
|
||||
content=str(result.get("result") or ""),
|
||||
)
|
||||
if result.get("is_error"):
|
||||
# Propagate MCP failures (CallToolResult.isError or an execution
|
||||
# exception) so Anthropic's native error-handling triggers instead
|
||||
# of reading the failure string as a successful tool return (#49359).
|
||||
tool_result["is_error"] = True
|
||||
content.append(tool_result)
|
||||
return AnthropicMessagesUserMessageParam(role="user", content=content)
|
||||
|
||||
|
||||
async def anthropic_messages_with_mcp(
|
||||
|
|
|
|||
|
|
@ -58,10 +58,15 @@ ToolParam: TypeAlias = Mapping[str, object]
|
|||
SplitTools: TypeAlias = tuple[list[ToolParam], list[Any]]
|
||||
|
||||
|
||||
class MCPToolResult(TypedDict):
|
||||
class MCPToolResult(TypedDict, total=False):
|
||||
tool_call_id: str | None
|
||||
result: str
|
||||
name: str | None
|
||||
# True when the MCP server returned CallToolResult.isError or execution
|
||||
# raised (guardrail block, HTTP failure, unexpected exception). Anthropic
|
||||
# native MCP consumers pass is_error=True on the tool_result block so the
|
||||
# model can react to the failure instead of reading it as a success (#49359).
|
||||
is_error: bool
|
||||
|
||||
|
||||
class _AresponsesOptions(ExtraItemsTypedDict, total=False, extra_items=object):
|
||||
|
|
@ -948,13 +953,16 @@ class LiteLLM_Proxy_MCP_Handler:
|
|||
|
||||
# Format result for inclusion in response
|
||||
result_text = LiteLLM_Proxy_MCP_Handler._parse_mcp_result(result)
|
||||
tool_results.append(
|
||||
{
|
||||
"tool_call_id": tool_call_id,
|
||||
"result": result_text,
|
||||
"name": tool_name,
|
||||
}
|
||||
)
|
||||
tool_result: MCPToolResult = {
|
||||
"tool_call_id": tool_call_id,
|
||||
"result": result_text,
|
||||
"name": tool_name,
|
||||
}
|
||||
# mcp>=2.3.0 names the flag is_error (constructor alias isError);
|
||||
# accept both so older SDK versions keep working.
|
||||
if getattr(result, "is_error", False) or getattr(result, "isError", False):
|
||||
tool_result["is_error"] = True
|
||||
tool_results.append(tool_result)
|
||||
|
||||
except BlockedPiiEntityError as e:
|
||||
await LiteLLM_Proxy_MCP_Handler._log_mcp_tool_failure(
|
||||
|
|
@ -970,6 +978,7 @@ class LiteLLM_Proxy_MCP_Handler:
|
|||
"tool_call_id": tool_call_id,
|
||||
"result": error_message,
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
)
|
||||
except GuardrailRaisedException as e:
|
||||
|
|
@ -988,6 +997,7 @@ class LiteLLM_Proxy_MCP_Handler:
|
|||
"tool_call_id": tool_call_id,
|
||||
"result": error_message,
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
)
|
||||
except HTTPException as e:
|
||||
|
|
@ -1004,6 +1014,7 @@ class LiteLLM_Proxy_MCP_Handler:
|
|||
"tool_call_id": tool_call_id,
|
||||
"result": error_message,
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
)
|
||||
except Exception as e:
|
||||
|
|
@ -1019,6 +1030,7 @@ class LiteLLM_Proxy_MCP_Handler:
|
|||
"tool_call_id": tool_call_id,
|
||||
"result": f"Error executing tool: {e}",
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -258,9 +258,7 @@ async def test_anthropic_messages_with_mcp_hands_execution_the_requests_served_t
|
|||
]
|
||||
|
||||
process = AsyncMock(return_value=(served, {"read_wiki_structure": "deepwiki"}))
|
||||
execute = AsyncMock(
|
||||
return_value=[{"tool_call_id": "toolu_1", "result": "ok", "name": "read_wiki_structure"}]
|
||||
)
|
||||
execute = AsyncMock(return_value=[{"tool_call_id": "toolu_1", "result": "ok", "name": "read_wiki_structure"}])
|
||||
responses = [
|
||||
{
|
||||
"stop_reason": "tool_use",
|
||||
|
|
@ -454,3 +452,41 @@ async def test_anthropic_messages_with_mcp_loads_a_filtered_routes_beta_map_off_
|
|||
@pytest.mark.asyncio
|
||||
async def test_anthropic_messages_with_mcp_never_loads_the_beta_map_for_first_party_anthropic():
|
||||
assert await _mcp_safeguards_turn_recording_beta_map_loads("anthropic", "claude-sonnet-4-6") == ()
|
||||
|
||||
|
||||
def test_build_tool_result_message_marks_is_error_results():
|
||||
"""
|
||||
Regression test (#49359): a tool result flagged is_error by the MCP gateway
|
||||
must reach Anthropic as is_error=True so the model reacts to the failure.
|
||||
|
||||
Given: tool results where one carries is_error=True (MCP CallToolResult.isError
|
||||
or an execution exception path in execute_tool_calls)
|
||||
When: _build_tool_result_message renders them
|
||||
Then: Only the flagged result gets is_error; the success stays clean.
|
||||
"""
|
||||
message = _build_tool_result_message(
|
||||
[
|
||||
{"tool_call_id": "call_ok", "result": "9 sections", "name": "read_wiki"},
|
||||
{"tool_call_id": "call_bad", "result": "Error executing tool: boom", "name": "read_wiki", "is_error": True},
|
||||
]
|
||||
)
|
||||
|
||||
assert message["role"] == "user"
|
||||
assert message["content"][0] == {"type": "tool_result", "tool_use_id": "call_ok", "content": "9 sections"}
|
||||
assert message["content"][1] == {
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "call_bad",
|
||||
"content": "Error executing tool: boom",
|
||||
"is_error": True,
|
||||
}
|
||||
|
||||
|
||||
def test_build_tool_result_message_omits_is_error_for_successes():
|
||||
"""
|
||||
Successful tool results must not carry is_error at all (Anthropic treats
|
||||
is_error: false the same, but omitting keeps the payload minimal and the
|
||||
chat-completions bridge unaffected).
|
||||
"""
|
||||
message = _build_tool_result_message([{"tool_call_id": "call_1", "result": "5", "name": "add", "is_error": False}])
|
||||
|
||||
assert message["content"][0] == {"type": "tool_result", "tool_use_id": "call_1", "content": "5"}
|
||||
|
|
|
|||
|
|
@ -49,7 +49,14 @@ class _DummyMCPResult:
|
|||
|
||||
def _setup_mcp_call_environment(monkeypatch: pytest.MonkeyPatch) -> AsyncMock:
|
||||
"""Patch MCP globals so _execute_tool_calls can run in tests."""
|
||||
proxy_module = types.SimpleNamespace(proxy_logging_obj=object(), prisma_client=None)
|
||||
proxy_logging_obj = MagicMock()
|
||||
|
||||
async def _passthrough_post_mcp_call_hook(response, **_):
|
||||
return response
|
||||
|
||||
proxy_logging_obj.post_mcp_call_hook = AsyncMock(side_effect=_passthrough_post_mcp_call_hook)
|
||||
proxy_logging_obj.post_call_failure_hook = AsyncMock()
|
||||
proxy_module = types.SimpleNamespace(proxy_logging_obj=proxy_logging_obj, prisma_client=None)
|
||||
monkeypatch.setitem(sys.modules, "litellm.proxy.proxy_server", proxy_module)
|
||||
|
||||
fake_manager = types.SimpleNamespace(
|
||||
|
|
@ -146,9 +153,7 @@ def test_chat_follow_up_answers_only_tool_calls_in_its_assistant_turn():
|
|||
"message": {
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [
|
||||
{"id": call_id, "type": "function", "function": {"name": "foo", "arguments": "{}"}}
|
||||
],
|
||||
"tool_calls": [{"id": call_id, "type": "function", "function": {"name": "foo", "arguments": "{}"}}],
|
||||
},
|
||||
"finish_reason": "tool_calls",
|
||||
}
|
||||
|
|
@ -621,7 +626,9 @@ async def test_execute_tool_calls_applies_post_call_hook_content(monkeypatch):
|
|||
user_api_key_auth=None,
|
||||
)
|
||||
|
||||
assert results == [{"tool_call_id": "call-1", "result": "[REDACTED]", "name": tool_name}]
|
||||
# The mocked async_post_mcp_tool_call_hook returns a result flagged is_error=True,
|
||||
# so the gateway must propagate that flag (#49359) alongside the redacted text.
|
||||
assert results == [{"tool_call_id": "call-1", "result": "[REDACTED]", "name": tool_name, "is_error": True}]
|
||||
assert logging_obj.async_success_handler.await_args.kwargs["result"].content[0].text == "[REDACTED]"
|
||||
assert logging_obj.async_success_handler.await_args.kwargs["result"].structured_content is None
|
||||
|
||||
|
|
@ -1909,3 +1916,84 @@ async def test_apply_toolset_permissions_pins_the_auth_to_explicit_grants_only(m
|
|||
assert scoped.object_permission is not None
|
||||
assert scoped.object_permission.mcp_servers == ["srv-1"]
|
||||
assert scoped.object_permission.mcp_tool_permissions == {"srv-1": ["add"]}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_tool_calls_propagates_mcp_is_error_flag(monkeypatch):
|
||||
"""
|
||||
Regression test (#49359): CallToolResult.isError=True must surface as
|
||||
is_error=True on the MCPToolResult so Anthropic's native MCP loop can mark
|
||||
the tool_result block as failed.
|
||||
"""
|
||||
call_tool_mock = _setup_mcp_call_environment(monkeypatch)
|
||||
call_tool_mock.return_value = CallToolResult(
|
||||
content=[TextContent(type="text", text="Database connection refused")],
|
||||
isError=True,
|
||||
)
|
||||
tool_name = "read_wiki_structure"
|
||||
tool_calls = [{"id": "call-err", "function": {"name": tool_name, "arguments": "{}"}}]
|
||||
|
||||
results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls(
|
||||
tool_server_map={tool_name: "deepwiki"},
|
||||
tool_calls=tool_calls,
|
||||
user_api_key_auth=None,
|
||||
)
|
||||
|
||||
assert results == [
|
||||
{
|
||||
"tool_call_id": "call-err",
|
||||
"result": "Database connection refused",
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_tool_calls_success_has_no_is_error(monkeypatch):
|
||||
"""A successful CallToolResult must not carry the is_error key at all."""
|
||||
call_tool_mock = _setup_mcp_call_environment(monkeypatch)
|
||||
call_tool_mock.return_value = CallToolResult(
|
||||
content=[TextContent(type="text", text="9 sections")],
|
||||
isError=False,
|
||||
)
|
||||
tool_name = "read_wiki_structure"
|
||||
tool_calls = [{"id": "call-ok", "function": {"name": tool_name, "arguments": "{}"}}]
|
||||
|
||||
results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls(
|
||||
tool_server_map={tool_name: "deepwiki"},
|
||||
tool_calls=tool_calls,
|
||||
user_api_key_auth=None,
|
||||
)
|
||||
|
||||
assert results == [
|
||||
{
|
||||
"tool_call_id": "call-ok",
|
||||
"result": "9 sections",
|
||||
"name": tool_name,
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_tool_calls_marks_exception_results_as_error(monkeypatch):
|
||||
"""An unexpected exception in tool execution must set is_error=True."""
|
||||
call_tool_mock = _setup_mcp_call_environment(monkeypatch)
|
||||
call_tool_mock.side_effect = RuntimeError("boom")
|
||||
tool_name = "read_wiki_structure"
|
||||
tool_calls = [{"id": "call-boom", "function": {"name": tool_name, "arguments": "{}"}}]
|
||||
|
||||
results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls(
|
||||
tool_server_map={tool_name: "deepwiki"},
|
||||
tool_calls=tool_calls,
|
||||
user_api_key_auth=None,
|
||||
)
|
||||
|
||||
assert results == [
|
||||
{
|
||||
"tool_call_id": "call-boom",
|
||||
"result": "Error executing tool: boom",
|
||||
"name": tool_name,
|
||||
"is_error": True,
|
||||
}
|
||||
]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue