forked from cleveragents/cleveragents-core
Merge pull request 'fix(mcp): extract error message from content[0].text per MCP 1.4.0 protocol' (#2600) from fix/mcp-adapter-error-extraction-content-key into master
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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}"
|
||||
)
|
||||
@@ -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"
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user