Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions src/adcp/protocols/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,24 +245,12 @@ async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskRe
# Call the tool using MCP client session
result = await session.call_tool(tool_name, params)

# This SDK requires MCP tools to return structuredContent
# The content field may contain human-readable messages but the actual
# response data must be in structuredContent
if not hasattr(result, "structuredContent") or result.structuredContent is None:
raise ValueError(
f"MCP tool {tool_name} did not return structuredContent. "
f"This SDK requires MCP tools to provide structured responses. "
f"Got content: {result.content if hasattr(result, 'content') else 'none'}"
)
# Check if this is an error response
is_error = hasattr(result, "isError") and result.isError

# Extract the structured data (required)
data_to_return = result.structuredContent

# Extract human-readable message from content (optional)
# This is typically a status message like "Found 42 creative formats"
# Extract human-readable message from content
message_text = None
if hasattr(result, "content") and result.content:
# Serialize content using the same method used for backward compatibility
serialized_content = self._serialize_mcp_content(result.content)
if isinstance(serialized_content, list):
for item in serialized_content:
Expand All @@ -271,14 +259,48 @@ async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskRe
message_text = item["text"]
break

# Handle error responses
if is_error:
# For error responses, structuredContent is optional
# Use the error message from content as the error
error_message = message_text or "Tool execution failed"
if self.agent_config.debug and start_time:
duration_ms = (time.time() - start_time) * 1000
debug_info = DebugInfo(
request=debug_request,
response={
"error": error_message,
"is_error": True,
},
duration_ms=duration_ms,
)
return TaskResult[Any](
status=TaskStatus.FAILED,
error=error_message,
success=False,
debug_info=debug_info,
)

# For successful responses, structuredContent is required
if not hasattr(result, "structuredContent") or result.structuredContent is None:
raise ValueError(
f"MCP tool {tool_name} did not return structuredContent. "
f"This SDK requires MCP tools to provide structured responses "
f"for successful calls. "
f"Got content: {result.content if hasattr(result, 'content') else 'none'}"
)

# Extract the structured data (required for success)
data_to_return = result.structuredContent

if self.agent_config.debug and start_time:
duration_ms = (time.time() - start_time) * 1000
debug_info = DebugInfo(
request=debug_request,
response={
"data": data_to_return,
"message": message_text,
"is_error": result.isError if hasattr(result, "isError") else False,
"is_error": False,
},
duration_ms=duration_ms,
)
Expand Down
30 changes: 27 additions & 3 deletions tests/test_protocols.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ async def test_call_tool_success(self, mcp_config):
# Mock MCP result with structuredContent (required for AdCP)
mock_result.content = [{"type": "text", "text": "Success"}]
mock_result.structuredContent = {"products": [{"id": "prod1"}]}
mock_result.isError = False
mock_session.call_tool.return_value = mock_result

with patch.object(adapter, "_get_session", return_value=mock_session):
Expand Down Expand Up @@ -193,6 +194,7 @@ async def test_call_tool_with_structured_content(self, mcp_config):
# Mock MCP result with structuredContent (preferred over content)
mock_result.content = [{"type": "text", "text": "Found 42 creative formats"}]
mock_result.structuredContent = {"formats": [{"id": "format1"}, {"id": "format2"}]}
mock_result.isError = False
mock_session.call_tool.return_value = mock_result

with patch.object(adapter, "_get_session", return_value=mock_session):
Expand All @@ -207,24 +209,46 @@ async def test_call_tool_with_structured_content(self, mcp_config):

@pytest.mark.asyncio
async def test_call_tool_missing_structured_content(self, mcp_config):
"""Test tool call fails when structuredContent is missing."""
"""Test tool call fails when structuredContent is missing on successful response."""
adapter = MCPAdapter(mcp_config)

mock_session = AsyncMock()
mock_result = MagicMock()
# Mock MCP result WITHOUT structuredContent (invalid for AdCP)
# Mock MCP result WITHOUT structuredContent and isError=False (invalid)
mock_result.content = [{"type": "text", "text": "Success"}]
mock_result.structuredContent = None
mock_result.isError = False
mock_session.call_tool.return_value = mock_result

with patch.object(adapter, "_get_session", return_value=mock_session):
result = await adapter._call_mcp_tool("get_products", {"brief": "test"})

# Verify error handling for missing structuredContent
# Verify error handling for missing structuredContent on success
assert result.success is False
assert result.status == TaskStatus.FAILED
assert "did not return structuredContent" in result.error

@pytest.mark.asyncio
async def test_call_tool_error_without_structured_content(self, mcp_config):
"""Test tool call handles error responses without structuredContent gracefully."""
adapter = MCPAdapter(mcp_config)

mock_session = AsyncMock()
mock_result = MagicMock()
# Mock MCP error response WITHOUT structuredContent (valid for errors)
mock_result.content = [{"type": "text", "text": "brand_manifest must provide brand information"}]
mock_result.structuredContent = None
mock_result.isError = True
mock_session.call_tool.return_value = mock_result

with patch.object(adapter, "_get_session", return_value=mock_session):
result = await adapter._call_mcp_tool("get_products", {"brief": "test"})

# Verify error is handled gracefully
assert result.success is False
assert result.status == TaskStatus.FAILED
assert result.error == "brand_manifest must provide brand information"

@pytest.mark.asyncio
async def test_call_tool_error(self, mcp_config):
"""Test tool call error via MCP."""
Expand Down