fix: handle MCP error responses without structuredContent (#34)

* fix: handle MCP error responses without structuredContent gracefully

When an MCP tool returns an error (isError=True), it may only populate
the content field with an error message and leave structuredContent as
None. The SDK now handles this case gracefully by:

1. Checking isError flag before requiring structuredContent
2. Extracting error message from content field for error responses
3. Returning a proper TaskResult with success=False and the error message
4. Only requiring structuredContent for successful responses (isError=False)

This allows the CLI to display agent-provided error messages instead of
raising SDK validation errors.

Updated tests to cover both cases:
- Error responses without structuredContent (now valid)
- Success responses without structuredContent (still invalid)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: split long error message to satisfy line length limit

Split the structuredContent error message across multiple lines
to keep each line under 100 characters for linter compliance.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: explicitly set isError=False in test mocks for successful responses

MagicMock objects return truthy MagicMock instances for any attribute access,
so mock_result.isError was returning a truthy value even when not explicitly
set. This caused the code to incorrectly treat successful test responses as
errors. Now explicitly setting isError=False for all successful response mocks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: bokelley

src/adcp/protocols/mcp.py

@@ -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:
@@ -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,
                 )

tests/test_protocols.py

@@ -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):
@@ -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):
@@ -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."""