From b96138b88e37dacff138bb1147ee58532c9be800 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Fri, 3 Apr 2026 19:03:15 +0000 Subject: [PATCH] fix(mcp): extract error message from content[0].text per MCP 1.4.0 protocol MCPToolAdapter.invoke() was reading error messages from result.get('error', 'unknown error'), but the MCP 1.4.0 protocol returns errors in the content field as a list of content items with type and text keys. This caused every error from a real MCP 1.4.0-compliant server to be silently replaced with the string 'unknown error'. Changes: - src/cleveragents/mcp/adapter.py: extract error_text from content[0].text with safe guards (isinstance check, length check) and fallback to 'unknown error' when content is absent or empty - features/mocks/mock_mcp_transport.py: return MCP 1.4.0-compliant error responses using content list format instead of the non-standard error key - features/tdd_mcp_error_content_key.feature: Behave scenario verifying correct error extraction from MCP 1.4.0 content arrays (written as TDD issue-capture, @tdd_expected_fail removed after fix applied) - features/steps/tdd_mcp_error_content_key_steps.py: step definitions for the new scenario including _MCP14ErrorTransport mock subclass All 51 MCP adapter scenarios pass. Typecheck: 0 errors. Lint: clean. ISSUES CLOSED: #2158 --- features/mocks/mock_mcp_transport.py | 4 +- .../steps/plan_namespaced_name_tdd_steps.py | 12 +++- .../steps/tdd_mcp_error_content_key_steps.py | 71 +++++++++++++++++++ features/tdd_mcp_error_content_key.feature | 11 +++ src/cleveragents/mcp/adapter.py | 7 +- 5 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 features/steps/tdd_mcp_error_content_key_steps.py create mode 100644 features/tdd_mcp_error_content_key.feature diff --git a/features/mocks/mock_mcp_transport.py b/features/mocks/mock_mcp_transport.py index e40409e7a..759d8b662 100644 --- a/features/mocks/mock_mcp_transport.py +++ b/features/mocks/mock_mcp_transport.py @@ -51,7 +51,9 @@ class MockMCPTransport(MCPTransport): if tool_name in self._invoke_errors: return { "isError": True, - "error": self._invoke_errors[tool_name], + "content": [ + {"type": "text", "text": self._invoke_errors[tool_name]} + ], } if tool_name in self._invoke_results: diff --git a/features/steps/plan_namespaced_name_tdd_steps.py b/features/steps/plan_namespaced_name_tdd_steps.py index 6728627fa..cf9bd8295 100644 --- a/features/steps/plan_namespaced_name_tdd_steps.py +++ b/features/steps/plan_namespaced_name_tdd_steps.py @@ -8,7 +8,9 @@ from cleveragents.domain.models.core.plan import NamespacedName @when('I parse the namespaced name "{full_name}" expecting an error') -def step_when_parse_namespaced_name_expecting_error(context: Context, full_name: str) -> None: +def step_when_parse_namespaced_name_expecting_error( + context: Context, full_name: str +) -> None: """Parse a namespaced name string and capture any error.""" try: context.namespaced_name = NamespacedName.parse(full_name) @@ -18,8 +20,12 @@ def step_when_parse_namespaced_name_expecting_error(context: Context, full_name: context.namespaced_name = None -@when('I construct a NamespacedName with namespace "{namespace}" and name "{name}" expecting an error') -def step_when_construct_namespaced_name_expecting_error(context: Context, namespace: str, name: str) -> None: +@when( + 'I construct a NamespacedName with namespace "{namespace}" and name "{name}" expecting an error' +) +def step_when_construct_namespaced_name_expecting_error( + context: Context, namespace: str, name: str +) -> None: """Create a NamespacedName and capture any error.""" try: context.namespaced_name = NamespacedName(namespace=namespace, name=name) diff --git a/features/steps/tdd_mcp_error_content_key_steps.py b/features/steps/tdd_mcp_error_content_key_steps.py new file mode 100644 index 000000000..dfdbd4c86 --- /dev/null +++ b/features/steps/tdd_mcp_error_content_key_steps.py @@ -0,0 +1,71 @@ +"""Step definitions for features/tdd_mcp_error_content_key.feature. + +TDD issue-capture scenario for #2158: MCPToolAdapter.invoke() must extract +error messages from content[0].text per MCP 1.4.0 protocol. +""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then +from behave.runner import Context + +from cleveragents.mcp.adapter import MCPServerConfig, MCPToolAdapter +from features.mocks.mock_mcp_transport import MockMCPTransport + + +class _MCP14ErrorTransport(MockMCPTransport): + """Mock transport that returns MCP 1.4.0-compliant error responses. + + Returns ``{"isError": True, "content": [{"type": "text", "text": "..."}]}`` + instead of the non-standard ``{"isError": True, "error": "..."}`` format. + """ + + def __init__(self, tool_name: str, error_text: str) -> None: + tools = [ + { + "name": tool_name, + "description": f"Mock tool {tool_name}", + "inputSchema": {}, + } + ] + super().__init__(tools=tools) + self._mcp14_error_tool = tool_name + self._mcp14_error_text = error_text + + def call(self, method: str, params: dict[str, Any]) -> dict[str, Any]: + if method == "tools/call": + tool_name = params.get("name", "") + if tool_name == self._mcp14_error_tool: + return { + "isError": True, + "content": [{"type": "text", "text": self._mcp14_error_text}], + } + return super().call(method, params) + + +@given( + 'a connected MCP adapter with a tool returning MCP 1.4.0 content-based error "{tool_name}"' +) +def step_mcp14_error_tool(context: Context, tool_name: str) -> None: + """Set up a connected MCP adapter whose tool returns a MCP 1.4.0 content-based error.""" + transport = _MCP14ErrorTransport( + tool_name=tool_name, + error_text="File not found: /path/to/file", + ) + config = MCPServerConfig(name="test-server", transport="stdio", command="echo") + context.mcp_adapter = MCPToolAdapter(config=config, transport=transport) + context.mcp_adapter.connect() + context.mcp_adapter.discover_tools() + + +@then('the invocation error should contain "{expected}"') +def step_invoke_error_contains(context: Context, expected: str) -> None: + """Assert the invocation error message contains the expected substring.""" + result = context.mcp_invoke_result + assert result is not None, "No invocation result found" + assert result.error is not None, "Expected an error but got None" + assert expected in result.error, ( + f"Expected error to contain {expected!r}, got: {result.error!r}" + ) diff --git a/features/tdd_mcp_error_content_key.feature b/features/tdd_mcp_error_content_key.feature new file mode 100644 index 000000000..ee35b04e9 --- /dev/null +++ b/features/tdd_mcp_error_content_key.feature @@ -0,0 +1,11 @@ +@tdd_issue @tdd_issue_2158 +Feature: TDD Issue #2158 — MCPToolAdapter.invoke() error extraction uses non-standard key + MCPToolAdapter.invoke() currently reads error messages from result.get('error', 'unknown error'), + but the MCP 1.4.0 protocol returns errors in content[0].text. This means every error from a + real MCP 1.4.0 server is silently replaced with "unknown error". + + Scenario: invoke() extracts error message from MCP 1.4.0 content array + Given a connected MCP adapter with a tool returning MCP 1.4.0 content-based error "read_file" + When I invoke "read_file" with arguments {"path": "/path/to/file"} + Then the invocation should fail + And the invocation error should contain "File not found: /path/to/file" diff --git a/src/cleveragents/mcp/adapter.py b/src/cleveragents/mcp/adapter.py index bc36e85c8..5d20c5438 100644 --- a/src/cleveragents/mcp/adapter.py +++ b/src/cleveragents/mcp/adapter.py @@ -514,9 +514,14 @@ class MCPToolAdapter: elapsed = (time.monotonic() - start) * 1000 if result.get("isError"): + content = result.get("content", []) + if content and isinstance(content, list) and len(content) > 0: + error_text = content[0].get("text", "unknown error") + else: + error_text = "unknown error" return MCPToolResult( success=False, - error=f"MCP server error: {result.get('error', 'unknown error')}", + error=f"MCP server error: {error_text}", duration_ms=elapsed, ) -- 2.52.0