From 7c2f5a1c76c669d109c5a2227b3729f03f914256 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 04:03:01 +0000 Subject: [PATCH] fix(mcp): correct MCPToolResult.data type annotation for MCP 1.4.0 content list format - What was implemented - Updated MCPToolResult.data docstring to document the normalisation behaviour, clarifying that MCP 1.4.0 list-format content is normalised to a dict with a "content" key. - Updated MCPToolAdapter.invoke() success path to normalise MCP 1.4.0 list-format content: when content is a list (as per MCP 1.4.0 spec), it is wrapped as {"content": [...]} so MCPToolResult.data is always dict[str, Any]. - Updated MockMCPTransport.call() to return MCP 1.4.0-compliant list-format content: invoke_results are now wrapped as [{"type": "text", "text": json.dumps(payload)}] instead of the non-standard dict format. - Added json import to mock_mcp_transport.py to support JSON payload encoding. - Updated existing mcp_adapter.feature scenario "Invoke a discovered tool successfully" to check for "content" key instead of "id" (since mock now returns MCP 1.4.0 list format). - Added two new Behave scenarios: "Invoke tool returns MCP 1.4.0 list-format content normalised to dict" and "Invoke tool with MCP 1.4.0 content list stores content items". - Added corresponding step definitions for the new scenarios. - All 14,418 existing scenarios continue to pass; 2 new scenarios added; typecheck passes with 0 errors; lint passes. - Why this approach - Design decision: Normalize to dict (Option B) to maintain a consistent MCPToolResult.data type of dict[str, Any] and avoid breaking downstream code that accesses result.data["key"]. - Fallback path handles non-standard server responses gracefully, ensuring robustness when servers deviate from MCP 1.4.0 spec. - Technical approach and affected components - Core: MCPToolResult data handling and MCPToolAdapter.invoke() logic - Mocks: mock_mcp_transport.py updated to emit MCP 1.4.0 list-format content - Tests: updated mcp_adapter.feature expectations; added two Behave scenarios with new step definitions - Dependencies: added import json to mock_mcp_transport.py - Verification - Comprehensive test suites: 14,418 existing scenarios pass - 2 new scenarios added and pass - Typechecking: 0 errors - Linting: passes ISSUES CLOSED: #2743 --- features/mcp_adapter.feature | 20 +++++++++++- features/mocks/mock_mcp_transport.py | 13 ++++++-- features/steps/mcp_adapter_steps.py | 49 ++++++++++++++++++++++++++++ src/cleveragents/mcp/adapter.py | 19 +++++++++-- 4 files changed, 96 insertions(+), 5 deletions(-) diff --git a/features/mcp_adapter.feature b/features/mcp_adapter.feature index 85c522177..b452fedaa 100644 --- a/features/mcp_adapter.feature +++ b/features/mcp_adapter.feature @@ -83,7 +83,7 @@ Feature: MCP Tool Adapter Given a connected MCP adapter with a callable mock tool "create_issue" When I invoke "create_issue" with arguments {"title": "Bug", "body": "Fix it"} Then the invocation should succeed - And the invocation result should contain key "id" + And the invocation result should contain key "content" Scenario: Invoke with valid input schema passes validation Given a connected MCP adapter with a schema-validated mock tool "create_issue" @@ -209,6 +209,24 @@ Feature: MCP Tool Adapter Then the invocation should fail And the invocation error should mention "server error" + # ------------------------------------------------------------------- + # MCP 1.4.0 Content List Format + # ------------------------------------------------------------------- + + Scenario: Invoke tool returns MCP 1.4.0 list-format content normalised to dict + Given a connected MCP adapter with a callable mock tool "get_status" + When I invoke "get_status" with arguments {} + Then the invocation should succeed + And the invocation result data should have key "content" + And the invocation result content should be a list + + Scenario: Invoke tool with MCP 1.4.0 content list stores content items + Given a connected MCP adapter with a raw list-content mock tool "fetch_data" + When I invoke "fetch_data" with arguments {} + Then the invocation should succeed + And the invocation result data should have key "content" + And the invocation result content list should have 1 item + # ------------------------------------------------------------------- # Error Classification # ------------------------------------------------------------------- diff --git a/features/mocks/mock_mcp_transport.py b/features/mocks/mock_mcp_transport.py index 759d8b662..b87912136 100644 --- a/features/mocks/mock_mcp_transport.py +++ b/features/mocks/mock_mcp_transport.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from typing import Any from cleveragents.mcp.adapter import MCPServerConfig, MCPTransport @@ -57,9 +58,17 @@ class MockMCPTransport(MCPTransport): } if tool_name in self._invoke_results: - return {"content": self._invoke_results[tool_name]} + # MCP 1.4.0: ``content`` is a list of ContentItem dicts. + # Wrap the caller-supplied result dict as a text ContentItem + # so the mock accurately reflects real server behaviour. + payload = self._invoke_results[tool_name] + content_item: dict[str, Any] = { + "type": "text", + "text": json.dumps(payload), + } + return {"content": [content_item]} - return {"content": {"result": "ok"}} + return {"content": [{"type": "text", "text": "ok"}]} return {} diff --git a/features/steps/mcp_adapter_steps.py b/features/steps/mcp_adapter_steps.py index 6b176e153..4aa8a44bc 100644 --- a/features/steps/mcp_adapter_steps.py +++ b/features/steps/mcp_adapter_steps.py @@ -157,6 +157,26 @@ def step_mcp_adapter_close_error(context: Context) -> None: context.mcp_adapter.connect() +@given('a connected MCP adapter with a raw list-content mock tool "{tool_name}"') +def step_mcp_adapter_raw_list_content_tool(context: Context, tool_name: str) -> None: + """Adapter whose transport returns a raw MCP 1.4.0 list-format content response.""" + tools = [_mock_tool(tool_name)] + config = MCPServerConfig(name="test-server", transport="stdio", command="echo") + + class _ListContentTransport(MockMCPTransport): + def call(self, method: str, params: dict[str, Any]) -> dict[str, Any]: + if method == "tools/call": + return { + "content": [{"type": "text", "text": "fetched data"}], + } + return super().call(method, params) + + context.mcp_transport = _ListContentTransport(tools=tools) + context.mcp_adapter = MCPToolAdapter(config=config, transport=context.mcp_transport) + context.mcp_adapter.connect() + context.mcp_adapter.discover_tools() + + @given('a connected MCP adapter with a transport-error mock tool "{tool_name}"') def step_mcp_adapter_transport_error_tool(context: Context, tool_name: str) -> None: tools = [_mock_tool(tool_name)] @@ -411,6 +431,35 @@ def step_invoke_result_key(context: Context, key: str) -> None: ) +@then('the invocation result data should have key "{key}"') +def step_invoke_result_data_key(context: Context, key: str) -> None: + assert isinstance(context.mcp_invoke_result.data, dict), ( + f"Expected data to be a dict, got {type(context.mcp_invoke_result.data)}" + ) + assert key in context.mcp_invoke_result.data, ( + f"Key '{key}' not in result data: {context.mcp_invoke_result.data}" + ) + + +@then("the invocation result content should be a list") +def step_invoke_result_content_is_list(context: Context) -> None: + content = context.mcp_invoke_result.data.get("content") + assert isinstance(content, list), ( + f"Expected 'content' to be a list, got {type(content)}: {content}" + ) + + +@then("the invocation result content list should have {count:d} item") +def step_invoke_result_content_count(context: Context, count: int) -> None: + content = context.mcp_invoke_result.data.get("content") + assert isinstance(content, list), ( + f"Expected 'content' to be a list, got {type(content)}" + ) + assert len(content) == count, ( + f"Expected {count} content item(s), got {len(content)}: {content}" + ) + + @then('the invocation error should mention "{text}"') def step_invoke_error_mention(context: Context, text: str) -> None: assert context.mcp_invoke_result.error is not None diff --git a/src/cleveragents/mcp/adapter.py b/src/cleveragents/mcp/adapter.py index 5d20c5438..7be616705 100644 --- a/src/cleveragents/mcp/adapter.py +++ b/src/cleveragents/mcp/adapter.py @@ -123,7 +123,10 @@ class MCPToolResult(BaseModel): Attributes: success: Whether the invocation succeeded. - data: Result payload from the MCP server. + data: Result payload from the MCP server, normalised to a dict. + On success the MCP 1.4.0 ``content`` list is wrapped under the + ``"content"`` key so callers always receive a ``dict[str, Any]`` + regardless of the raw server response format. error: Error message on failure. duration_ms: Wall-clock execution time in milliseconds. """ @@ -525,9 +528,21 @@ class MCPToolAdapter: duration_ms=elapsed, ) + # MCP 1.4.0 returns ``content`` as a list of ContentItem dicts + # (e.g. ``[{"type": "text", "text": "..."}]``). Normalise to a + # plain dict so that ``MCPToolResult.data`` is always + # ``dict[str, Any]`` and downstream code can rely on dict access. + raw_content = result.get("content", []) + if isinstance(raw_content, list): + normalised: dict[str, Any] = {"content": raw_content} + elif isinstance(raw_content, dict): + normalised = raw_content + else: + # Unexpected type — wrap it so the dict contract holds. + normalised = {"content": raw_content} return MCPToolResult( success=True, - data=result.get("content", result), + data=normalised, duration_ms=elapsed, ) -- 2.52.0