From d40d00ea26102d73685f423346efadb7bd9ae254 Mon Sep 17 00:00:00 2001 From: Sarvesh Date: Tue, 16 Jun 2026 11:04:35 +0530 Subject: [PATCH 1/4] fix: tighten mcpserver tool result typing --- src/mcp/server/mcpserver/server.py | 11 +-------- tests/server/mcpserver/test_func_metadata.py | 24 ++++++++++++++++++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index fdb69571d8..f4c971ff7c 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -4,7 +4,6 @@ import base64 import inspect -import json import re from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Sequence from contextlib import AbstractAsyncContextManager, asynccontextmanager @@ -322,14 +321,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,7 +390,7 @@ 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]: + ) -> CallToolResult | Sequence[ContentBlock] | tuple[Sequence[ContentBlock], dict[str, Any]]: """Call a tool by name with arguments.""" if context is None: context = Context(mcp_server=self) diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index c57d1ee9f0..f9eaaa2596 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -13,8 +13,8 @@ from pydantic import BaseModel, Field from mcp.server.mcpserver.exceptions import InvalidSignature -from mcp.server.mcpserver.utilities.func_metadata import func_metadata -from mcp.types import CallToolResult +from mcp.server.mcpserver.utilities.func_metadata import FuncMetadata, func_metadata +from mcp.types import CallToolResult, TextContent class SomeInputModelA(BaseModel): @@ -203,6 +203,26 @@ def func_with_many_params(keep_this: int, skip_this: str, also_keep: float, also assert model.also_keep == 2.5 # type: ignore +def test_convert_result_serializes_unstructured_dict_to_text_content(): + """Unstructured dict returns are content blocks, not raw dict values.""" + + def func_dict(): # pragma: no cover + return {"ok": True, "count": 2} + + meta: FuncMetadata = func_metadata(func_dict) + + assert meta.output_schema is None + assert meta.convert_result({"ok": True, "count": 2}) == [ + TextContent( + type="text", + text="""{ + "ok": true, + "count": 2 +}""", + ) + ] + + def test_structured_output_dict_str_types(): """Test that dict[str, T] types are handled without wrapping.""" From 5a470d7cfd5684741ea18f33e4134fb7fbb0b32d Mon Sep 17 00:00:00 2001 From: Sarvesh Date: Thu, 18 Jun 2026 19:19:25 +0530 Subject: [PATCH 2/4] refactor(server): always return CallToolResult from MCPServer.call_tool --- src/mcp/server/mcpserver/server.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index f4c971ff7c..ab93d31d4e 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -7,7 +7,7 @@ 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, TypeVar, cast, overload import anyio import pydantic_core @@ -308,20 +308,11 @@ 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) + return await self.call_tool(params.name, params.arguments or {}, context) except MCPError: raise except Exception as e: return CallToolResult(content=[TextContent(type="text", text=str(e))], is_error=True) - if isinstance(result, CallToolResult): - return result - if isinstance(result, tuple) and len(result) == 2: - unstructured_content, structured_content = result - return CallToolResult( - content=list(unstructured_content), # type: ignore[arg-type] - structured_content=structured_content, # type: ignore[arg-type] - ) - return CallToolResult(content=list(result)) async def _handle_list_resources( self, ctx: ServerRequestContext[LifespanResultT], params: PaginatedRequestParams | None @@ -390,11 +381,22 @@ async def list_tools(self) -> list[MCPTool]: async def call_tool( self, name: str, arguments: dict[str, Any], context: Context[LifespanResultT, Any] | None = None - ) -> CallToolResult | Sequence[ContentBlock] | tuple[Sequence[ContentBlock], dict[str, Any]]: + ) -> CallToolResult: """Call a tool by name with arguments.""" if context is None: context = Context(mcp_server=self) - return await self._tool_manager.call_tool(name, arguments, context, convert_result=True) + result = await self._tool_manager.call_tool(name, arguments, context, convert_result=True) + if isinstance(result, CallToolResult): + return result + if isinstance(result, tuple) and len(result) == 2: # type: ignore[arg-type] + unstructured_content, structured_content = cast( + tuple[Sequence[ContentBlock], dict[str, Any]], result + ) + return CallToolResult( + content=list(unstructured_content), + structured_content=structured_content, + ) + return CallToolResult(content=list(result)) # type: ignore[arg-type] async def list_resources(self) -> list[MCPResource]: """List all available resources.""" From efa82ca682aa8b2c798f07dd64360c5bb64108ed Mon Sep 17 00:00:00 2001 From: Sarvesh Date: Fri, 19 Jun 2026 12:48:00 +0530 Subject: [PATCH 3/4] style: ruff format fix --- pr_body.md | 15 +++++++++++++++ src/mcp/server/mcpserver/server.py | 4 +--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 pr_body.md diff --git a/pr_body.md b/pr_body.md new file mode 100644 index 0000000000..31ad88f984 --- /dev/null +++ b/pr_body.md @@ -0,0 +1,15 @@ +## Summary +- Changes the default `encoding_error_handler` in `StdioServerParameters` from `"strict"` to `"replace"`. +- Malformed UTF-8 bytes from a child server stdout previously crashed the client transport with `UnicodeDecodeError`. +- With `"replace"`, invalid bytes become U+FFFD and the line fails JSON-RPC validation, which is surfaced as an in-stream `Exception` that the session can handle. +- This mirrors the server-side stdio hardening from PR #2302. + +## Test Plan +- [x] Reproduced the crash with a child process emitting `\xff\xfe\n` followed by valid JSON-RPC. +- [x] Verified the fix: first item is a `ValidationError`, second item is the valid `SessionMessage`. +- [x] Added regression test `test_invalid_utf8_from_the_server_surfaces_as_an_in_stream_exception`. +- [x] Full `tests/client/test_stdio.py` suite passes (34 passed, 1 skipped). + +## Notes +- Fixes #2454 +- Scope intentionally small. diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index ab93d31d4e..46c4181f08 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -389,9 +389,7 @@ async def call_tool( if isinstance(result, CallToolResult): return result if isinstance(result, tuple) and len(result) == 2: # type: ignore[arg-type] - unstructured_content, structured_content = cast( - tuple[Sequence[ContentBlock], dict[str, Any]], result - ) + unstructured_content, structured_content = cast(tuple[Sequence[ContentBlock], dict[str, Any]], result) return CallToolResult( content=list(unstructured_content), structured_content=structured_content, From 13d16e677a9b83b62c15f04367046f578f5918c1 Mon Sep 17 00:00:00 2001 From: Sarvesh Date: Fri, 19 Jun 2026 13:11:57 +0530 Subject: [PATCH 4/4] chore: remove accidentally committed pr_body.md --- pr_body.md | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 pr_body.md diff --git a/pr_body.md b/pr_body.md deleted file mode 100644 index 31ad88f984..0000000000 --- a/pr_body.md +++ /dev/null @@ -1,15 +0,0 @@ -## Summary -- Changes the default `encoding_error_handler` in `StdioServerParameters` from `"strict"` to `"replace"`. -- Malformed UTF-8 bytes from a child server stdout previously crashed the client transport with `UnicodeDecodeError`. -- With `"replace"`, invalid bytes become U+FFFD and the line fails JSON-RPC validation, which is surfaced as an in-stream `Exception` that the session can handle. -- This mirrors the server-side stdio hardening from PR #2302. - -## Test Plan -- [x] Reproduced the crash with a child process emitting `\xff\xfe\n` followed by valid JSON-RPC. -- [x] Verified the fix: first item is a `ValidationError`, second item is the valid `SessionMessage`. -- [x] Added regression test `test_invalid_utf8_from_the_server_surfaces_as_an_in_stream_exception`. -- [x] Full `tests/client/test_stdio.py` suite passes (34 passed, 1 skipped). - -## Notes -- Fixes #2454 -- Scope intentionally small.