From 97cce25d4e18ecf5d2575c383d01416804fa6b29 Mon Sep 17 00:00:00 2001 From: lester076 <2665063734@qq.com> Date: Mon, 2 Nov 2026 18:20:38 +0800 Subject: [PATCH] fix(mcp): keep MCPToolResult's original keys required via NotRequired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut declared the whole TypedDict total=False, which made tool_call_id, result, and name optional to static type checkers and weakened the existing contract — the exact PEP 655 regression flagged in the issue review. Revert to the default total=True and mark only the new is_error key NotRequired, and pin the invariant with a static negative control (required_keys / optional_keys assertions). Co-authored-by: chauvuusvn <58875435+chauvuusvn@users.noreply.github.com> --- .../mcp/litellm_proxy_mcp_handler.py | 8 +++---- .../mcp/test_litellm_proxy_mcp_handler.py | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index a2b60ff8b4d..d970bd3c6d7 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -5,8 +5,7 @@ from typing import TYPE_CHECKING, Any, Final, Literal, Optional, TypeAlias, Type from openai.types.chat import ChatCompletionToolParam from openai.types.responses.function_tool_param import FunctionToolParam -from typing_extensions import TypedDict as ExtraItemsTypedDict -from typing_extensions import Unpack +from typing_extensions import NotRequired, TypedDict as ExtraItemsTypedDict, Unpack from litellm._logging import verbose_logger from litellm.constants import MAXIMUM_TRACEBACK_LINES_TO_LOG @@ -58,7 +57,7 @@ ToolParam: TypeAlias = Mapping[str, object] SplitTools: TypeAlias = tuple[list[ToolParam], list[Any]] -class MCPToolResult(TypedDict, total=False): +class MCPToolResult(TypedDict): tool_call_id: str | None result: str name: str | None @@ -66,7 +65,8 @@ class MCPToolResult(TypedDict, total=False): # 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 + # Optional via NotRequired (PEP 655) so the three original keys stay required. + is_error: NotRequired[bool] class _AresponsesOptions(ExtraItemsTypedDict, total=False, extra_items=object): diff --git a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py index 9a96550a231..69569eb4853 100644 --- a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py +++ b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py @@ -1997,3 +1997,24 @@ async def test_execute_tool_calls_marks_exception_results_as_error(monkeypatch): "is_error": True, } ] + + +def test_mcp_tool_result_keeps_original_keys_required_statically(): + """ + PEP 655 negative control (#49359 review): ``is_error`` is the only optional + key; ``tool_call_id``, ``result``, and ``name`` must stay required, so a + bare ``{"is_error": True}`` literal is rejected at type-check time. This + pins the static contract the runtime tests cannot see. + """ + from typing_extensions import assert_type + + from litellm.responses.mcp.litellm_proxy_mcp_handler import MCPToolResult + + # The three original keys keep Required status: total=False was reverted. + assert not MCPToolResult.__required_keys__ < {"tool_call_id", "result", "name"} + assert MCPToolResult.__required_keys__ == {"tool_call_id", "result", "name"} + assert MCPToolResult.__optional_keys__ == {"is_error"} + + # Runtime shape of a full result is unchanged. + full: MCPToolResult = {"tool_call_id": "call-1", "result": "ok", "name": "echo"} + assert_type(full, MCPToolResult)