Skip to content

Commit bfcd403

Browse files
committed
Return CallToolResult from convert_result instead of a tuple
`convert_result()` was returning a bare `(unstructured, structured)` tuple when an output schema was present, leaking an internal detail through `MCPServer.call_tool`'s public API. The declared return type `Sequence[ContentBlock] | dict[str, Any]` was also wrong - the `dict` branch was unreachable. Now `convert_result()` returns a `CallToolResult` directly, and `MCPServer.call_tool` is typed as `CallToolResult | Sequence[ContentBlock]`. The tuple-unpacking and dead `dict` branches in `_handle_call_tool` are removed. Github-Issue: #1251
1 parent 62575ed commit bfcd403

File tree

4 files changed

+14
-27
lines changed

4 files changed

+14
-27
lines changed

src/mcp/server/mcpserver/server.py

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import base64
66
import inspect
7-
import json
87
import re
98
from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Sequence
109
from contextlib import AbstractAsyncContextManager, asynccontextmanager
@@ -308,20 +307,6 @@ async def _handle_call_tool(
308307
return CallToolResult(content=[TextContent(type="text", text=str(e))], is_error=True)
309308
if isinstance(result, CallToolResult):
310309
return result
311-
if isinstance(result, tuple) and len(result) == 2:
312-
unstructured_content, structured_content = result
313-
return CallToolResult(
314-
content=list(unstructured_content), # type: ignore[arg-type]
315-
structured_content=structured_content, # type: ignore[arg-type]
316-
)
317-
if isinstance(result, dict): # pragma: no cover
318-
# TODO: this code path is unreachable — convert_result never returns a raw dict.
319-
# The call_tool return type (Sequence[ContentBlock] | dict[str, Any]) is wrong
320-
# and needs to be cleaned up.
321-
return CallToolResult(
322-
content=[TextContent(type="text", text=json.dumps(result, indent=2))],
323-
structured_content=result,
324-
)
325310
return CallToolResult(content=list(result))
326311

327312
async def _handle_list_resources(
@@ -399,7 +384,7 @@ def get_context(self) -> Context[LifespanResultT, Request]:
399384
request_context = None
400385
return Context(request_context=request_context, mcp_server=self)
401386

402-
async def call_tool(self, name: str, arguments: dict[str, Any]) -> Sequence[ContentBlock] | dict[str, Any]:
387+
async def call_tool(self, name: str, arguments: dict[str, Any]) -> CallToolResult | Sequence[ContentBlock]:
403388
"""Call a tool by name with arguments."""
404389
context = self.get_context()
405390
return await self._tool_manager.call_tool(name, arguments, context=context, convert_result=True)

src/mcp/server/mcpserver/utilities/func_metadata.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ async def call_fn_with_arg_validation(
9191
def convert_result(self, result: Any) -> Any:
9292
"""Convert a function call result to the format for the lowlevel tool call handler.
9393
94-
- If output_model is None, return the unstructured content directly.
95-
- If output_model is not None, convert the result to structured output format
96-
(dict[str, Any]) and return both unstructured and structured content.
94+
- If output_model is None, return the unstructured content as a ``Sequence[ContentBlock]``.
95+
- If output_model is not None, return a ``CallToolResult`` with both unstructured
96+
and structured content.
9797
9898
Note: we return unstructured content here **even though the lowlevel server
9999
tool call handler provides generic backwards compatibility serialization of
@@ -120,7 +120,7 @@ def convert_result(self, result: Any) -> Any:
120120
validated = self.output_model.model_validate(result)
121121
structured_content = validated.model_dump(mode="json", by_alias=True)
122122

123-
return (unstructured_content, structured_content)
123+
return CallToolResult(content=list(unstructured_content), structured_content=structured_content)
124124

125125
def pre_parse_json(self, data: dict[str, Any]) -> dict[str, Any]:
126126
"""Pre-parse data from JSON.

tests/server/mcpserver/test_func_metadata.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,7 @@ def func_with_aliases() -> ModelWithAliases: # pragma: no cover
10381038

10391039
# Check that the actual output uses aliases too
10401040
result = ModelWithAliases(**{"first": "hello", "second": "world"})
1041-
_, structured_content = meta.convert_result(result)
1041+
structured_content = meta.convert_result(result).structured_content
10421042

10431043
# The structured content should use aliases to match the schema
10441044
assert "first" in structured_content
@@ -1050,7 +1050,7 @@ def func_with_aliases() -> ModelWithAliases: # pragma: no cover
10501050

10511051
# Also test the case where we have a model with defaults to ensure aliases work in all cases
10521052
result_with_defaults = ModelWithAliases() # Uses default None values
1053-
_, structured_content_defaults = meta.convert_result(result_with_defaults)
1053+
structured_content_defaults = meta.convert_result(result_with_defaults).structured_content
10541054

10551055
# Even with defaults, should use aliases in output
10561056
assert "first" in structured_content_defaults

tests/server/mcpserver/test_tool_manager.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from mcp.server.mcpserver.tools import Tool, ToolManager
1313
from mcp.server.mcpserver.utilities.func_metadata import ArgModelBase, FuncMetadata
1414
from mcp.server.session import ServerSessionT
15-
from mcp.types import TextContent, ToolAnnotations
15+
from mcp.types import CallToolResult, TextContent, ToolAnnotations
1616

1717

1818
class TestAddTools:
@@ -473,7 +473,7 @@ def get_user(user_id: int) -> UserOutput:
473473
manager.add_tool(get_user)
474474
result = await manager.call_tool("get_user", {"user_id": 1}, convert_result=True)
475475
# don't test unstructured output here, just the structured conversion
476-
assert len(result) == 2 and result[1] == {"name": "John", "age": 30}
476+
assert isinstance(result, CallToolResult) and result.structured_content == {"name": "John", "age": 30}
477477

478478
@pytest.mark.anyio
479479
async def test_tool_with_primitive_output(self):
@@ -488,7 +488,8 @@ def double_number(n: int) -> int:
488488
result = await manager.call_tool("double_number", {"n": 5})
489489
assert result == 10
490490
result = await manager.call_tool("double_number", {"n": 5}, convert_result=True)
491-
assert isinstance(result[0][0], TextContent) and result[1] == {"result": 10}
491+
assert isinstance(result, CallToolResult)
492+
assert isinstance(result.content[0], TextContent) and result.structured_content == {"result": 10}
492493

493494
@pytest.mark.anyio
494495
async def test_tool_with_typeddict_output(self):
@@ -528,7 +529,7 @@ def get_person() -> Person:
528529
manager.add_tool(get_person)
529530
result = await manager.call_tool("get_person", {}, convert_result=True)
530531
# don't test unstructured output here, just the structured conversion
531-
assert len(result) == 2 and result[1] == expected_output
532+
assert isinstance(result, CallToolResult) and result.structured_content == expected_output
532533

533534
@pytest.mark.anyio
534535
async def test_tool_with_list_output(self):
@@ -546,7 +547,8 @@ def get_numbers() -> list[int]:
546547
result = await manager.call_tool("get_numbers", {})
547548
assert result == expected_list
548549
result = await manager.call_tool("get_numbers", {}, convert_result=True)
549-
assert isinstance(result[0][0], TextContent) and result[1] == expected_output
550+
assert isinstance(result, CallToolResult)
551+
assert isinstance(result.content[0], TextContent) and result.structured_content == expected_output
550552

551553
@pytest.mark.anyio
552554
async def test_tool_without_structured_output(self):

0 commit comments

Comments
 (0)