scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-11-14 09:03:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 214

fix(mcp): keep MCPToolResult's original keys required via NotRequired

Browse source 
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>
This commit is contained in:
lester076 2026-11-02 18:20:38 +08:00
parent d08b259ce9
commit 97cce25d4e
2 changed files with 25 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
litellm/responses/mcp/litellm_proxy_mcp_handler.py
View file

@ -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):

21
tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py
View file

@ -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)