diff --git a/docs/migration.md b/docs/migration.md index 8b70885e8d..328d232d93 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -428,6 +428,30 @@ async def my_tool(x: int, ctx: Context) -> str: The internal layers (`ToolManager.call_tool`, `Tool.run`, `Prompt.render`, `ResourceTemplate.create_resource`, etc.) now require `context` as a positional argument. +### `MCPServer.call_tool()` return type corrected + +`MCPServer.call_tool()`'s return type signature has been corrected from `Sequence[ContentBlock] | dict[str, Any]` to match what the internal tool manager actually returns when converting tool results. The union is now defined once as the `ToolResult` type alias (`mcp.server.mcpserver.server.ToolResult`), so the signature has a single source of truth: + +```python +ToolResult: TypeAlias = CallToolResult | Sequence[ContentBlock] | tuple[Sequence[ContentBlock], dict[str, Any]] +``` + +**Before (v1):** + +```python +async def call_tool( + self, name: str, arguments: dict[str, Any], context: Context[LifespanResultT, Any] | None = None +) -> Sequence[ContentBlock] | dict[str, Any]: +``` + +**After (v2):** + +```python +async def call_tool( + self, name: str, arguments: dict[str, Any], context: Context[LifespanResultT, Any] | None = None +) -> ToolResult: +``` + ### Registering lowlevel handlers on `MCPServer` (workaround) `MCPServer` does not expose public APIs for `subscribe_resource`, `unsubscribe_resource`, or `set_logging_level` handlers. In v1, the workaround was to reach into the private lowlevel server and use its decorator methods: diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index b3471163b7..34e9b1a1d1 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -4,11 +4,10 @@ import base64 import inspect -import json import re from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Sequence from contextlib import AbstractAsyncContextManager, asynccontextmanager -from typing import Any, Generic, Literal, TypeVar, overload +from typing import Any, Generic, Literal, TypeAlias, TypeVar, overload import anyio import pydantic_core @@ -76,6 +75,15 @@ _CallableT = TypeVar("_CallableT", bound=Callable[..., Any]) +ToolResult: TypeAlias = CallToolResult | Sequence[ContentBlock] | tuple[Sequence[ContentBlock], dict[str, Any]] +"""Result of invoking a tool via `MCPServer.call_tool`. One of: + +- `CallToolResult`: the tool returned a `CallToolResult` directly. +- `Sequence[ContentBlock]`: unstructured content from a tool with no output schema. +- `tuple[Sequence[ContentBlock], dict[str, Any]]`: unstructured content paired with + structured content from a tool that has an output schema. +""" + class Settings(BaseSettings, Generic[LifespanResultT]): """MCPServer settings. @@ -309,7 +317,7 @@ async def _handle_call_tool( ) -> CallToolResult: context = Context(request_context=ctx, mcp_server=self) try: - result = await self.call_tool(params.name, params.arguments or {}, context) + result: ToolResult = await self.call_tool(params.name, params.arguments or {}, context) except MCPError: raise except Exception as e: @@ -322,14 +330,6 @@ async def _handle_call_tool( content=list(unstructured_content), # type: ignore[arg-type] structured_content=structured_content, # type: ignore[arg-type] ) - if isinstance(result, dict): # pragma: no cover - # TODO: this code path is unreachable — convert_result never returns a raw dict. - # The call_tool return type (Sequence[ContentBlock] | dict[str, Any]) is wrong - # and needs to be cleaned up. - return CallToolResult( - content=[TextContent(type="text", text=json.dumps(result, indent=2))], - structured_content=result, - ) return CallToolResult(content=list(result)) async def _handle_list_resources( @@ -399,8 +399,15 @@ async def list_tools(self) -> list[MCPTool]: async def call_tool( self, name: str, arguments: dict[str, Any], context: Context[LifespanResultT, Any] | None = None - ) -> Sequence[ContentBlock] | dict[str, Any]: - """Call a tool by name with arguments.""" + ) -> ToolResult: + """Call a tool by name with arguments. + + Returns: + ToolResult: One of a `CallToolResult` (returned directly by the tool), a + `Sequence[ContentBlock]` (unstructured content from a tool with no output schema), + or a `tuple[Sequence[ContentBlock], dict[str, Any]]` (unstructured content paired + with structured content from a tool that has an output schema). + """ if context is None: context = Context(mcp_server=self) return await self._tool_manager.call_tool(name, arguments, context, convert_result=True) diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 3457ec944a..d272d4791e 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -22,6 +22,8 @@ from mcp.types import ( AudioContent, BlobResourceContents, + CallToolRequestParams, + CallToolResult, Completion, CompletionArgument, CompletionContext, @@ -1516,3 +1518,112 @@ async def test_report_progress_passes_related_request_id(): message="halfway", related_request_id="req-abc-123", ) + + +async def test_handle_call_tool_populates_content_and_structured_content(): + """A tool with an output schema flows through the tuple branch of `_handle_call_tool`. + + The resulting `CallToolResult` must have both `content` and `structured_content` + populated. This pins the reachable tuple path: the converter never returns a raw + dict, so the `isinstance(result, dict)` branch removed in #2695 is dead. If that + dead branch is ever reintroduced and starts intercepting this path, the structured + content would be dropped and this test fails. + """ + + class Point(BaseModel): + x: int + y: int + + def make_point(x: int, y: int) -> Point: + return Point(x=x, y=y) + + mcp = MCPServer() + mcp.add_tool(make_point) + + request_context = ServerRequestContext( + session=AsyncMock(), + lifespan_context=None, + experimental=Experimental(), + ) + + result = await mcp._handle_call_tool( + request_context, + CallToolRequestParams(name="make_point", arguments={"x": 1, "y": 2}), + ) + + assert isinstance(result, CallToolResult) + assert result.is_error is False + assert result.structured_content == {"x": 1, "y": 2} + assert len(result.content) == 1 + assert isinstance(result.content[0], TextContent) + + +async def test_handle_call_tool_returns_direct_call_tool_result(): + """A tool that returns `CallToolResult` directly should be returned unchanged. + + This pins the `isinstance(result, CallToolResult)` branch of `_handle_call_tool`. + """ + + def direct_result_tool() -> CallToolResult: + return CallToolResult( + content=[TextContent(type="text", text="Direct content")], + is_error=False, + structured_content={"custom": "metadata"}, + ) + + mcp = MCPServer() + mcp.add_tool(direct_result_tool) + + request_context = ServerRequestContext( + session=AsyncMock(), + lifespan_context=None, + experimental=Experimental(), + ) + + result = await mcp._handle_call_tool( + request_context, + CallToolRequestParams(name="direct_result_tool", arguments={}), + ) + + assert isinstance(result, CallToolResult) + assert result.is_error is False + assert result.structured_content == {"custom": "metadata"} + assert len(result.content) == 1 + assert isinstance(result.content[0], TextContent) + assert result.content[0].text == "Direct content" + + +async def test_handle_call_tool_returns_unstructured_sequence(): + """A tool with no output schema returning a sequence of content blocks flows + + through the fallback branch of `_handle_call_tool` and is wrapped in a CallToolResult. + """ + + def unstructured_tool() -> list[ContentBlock]: + return [ + TextContent(type="text", text="Hello"), + ImageContent(type="image", data="abc", mime_type="image/png"), + ] + + mcp = MCPServer() + mcp.add_tool(unstructured_tool, structured_output=False) + + request_context = ServerRequestContext( + session=AsyncMock(), + lifespan_context=None, + experimental=Experimental(), + ) + + result = await mcp._handle_call_tool( + request_context, + CallToolRequestParams(name="unstructured_tool", arguments={}), + ) + + assert isinstance(result, CallToolResult) + assert result.is_error is False + assert result.structured_content is None + assert len(result.content) == 2 + assert isinstance(result.content[0], TextContent) + assert result.content[0].text == "Hello" + assert isinstance(result.content[1], ImageContent) + assert result.content[1].data == "abc"