Skip to content

fix: correct MCPServer.call_tool return type and remove dead code#2700

Closed
Ar-maan05 wants to merge 5 commits into
modelcontextprotocol:mainfrom
Ar-maan05:fix/call-tool-return-type
Closed

fix: correct MCPServer.call_tool return type and remove dead code#2700
Ar-maan05 wants to merge 5 commits into
modelcontextprotocol:mainfrom
Ar-maan05:fix/call-tool-return-type

Conversation

@Ar-maan05

Copy link
Copy Markdown

Description

This PR addresses the dead-code path and incorrect return type annotation on MCPServer.call_tool() described in #2695.

  1. Removed Dead Code: In src/mcp/server/mcpserver/server.py, the isinstance(result, dict) check inside _handle_call_tool() is unreachable. As traced in the issue, self.call_tool() invokes the tool manager with convert_result=True, which delegates to FuncMetadata.convert_result. This converter function never returns a raw dict (returning CallToolResult, Sequence[ContentBlock], or tuple[Sequence[ContentBlock], dict[str, Any]] instead).
  2. Removed Unused Import: Removed the now-unused import json from src/mcp/server/mcpserver/server.py.
  3. Corrected Type Annotation: Updated MCPServer.call_tool()'s return type signature to accurately reflect the actual runtime types:
    CallToolResult | Sequence[ContentBlock] | tuple[Sequence[ContentBlock], dict[str, Any]]
  4. Migration Guide: Added breaking change documentation to docs/migration.md to assist developers migrating to v2.

Related Issues

Closes #2695

Verification

• Verified that the test suite passes successfully (1,194 passed).
• Verified type checking passes cleanly with pyright (0 errors, 0 warnings).
• Ran coverage and strict-no-cover to verify that code coverage remains at 100.00% and no unused coverage pragmas exist.

Armaan Sandhu added 4 commits May 28, 2026 12:36
Define the call_tool return union once as the ToolResult type alias and
reference it from _handle_call_tool, call_tool, and the migration guide so
the signature can't drift across locations.

Add a regression test that drives a tool with an output schema through
_handle_call_tool and asserts the resulting CallToolResult has both content
and structured_content populated, pinning the reachable tuple path.
@Ar-maan05

This comment was marked as spam.

@Ar-maan05

This comment was marked as spam.

@pranjalbhatia710

Copy link
Copy Markdown

I checked this against #2695 while looking for non-duplicative work. The shape cleanup looks consistent with convert_result=True: the new tests cover all three reachable outputs (CallToolResult, bare Sequence[ContentBlock], and (content, structured_content)), and the removed raw-dict branch stays unreachable.

Local validation on this PR head:

uv run --frozen pytest tests/server/mcpserver/test_server.py -q

Result: 92 passed.

So from a regression-coverage angle this looks well pinned down to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dead code path in MCPServer._handle_call_tool and incorrect call_tool return type

2 participants