refactor: move MCP serialization to protocol boundary

This refactor addresses the architectural issue identified in code review:
the response parser was handling both parsing AND protocol translation,
violating single responsibility principle.

Changes:
1. Added _serialize_mcp_content() to MCPAdapter to convert Pydantic
   objects to dicts at the protocol boundary
2. Simplified parse_mcp_content() to only handle plain dicts
3. Removed get_field() helper function (no longer needed)
4. Removed Pydantic mock tests from response_parser tests
5. Added serialization tests to protocol adapter tests

Benefits:
- Clean separation: adapters translate, parsers parse
- Simpler type signatures: list[dict[str, Any]] instead of | Any
- Tests use plain dicts (no MCP SDK dependency in parser tests)
- More obvious code (removed clever helper function)
- Follows adapter pattern correctly

The fix maintains backward compatibility while establishing proper
architectural boundaries for future MCP content types (images, resources).

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

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

Author: bokelley

src/adcp/protocols/mcp.py

@@ -186,6 +186,40 @@ async def _get_session(self) -> ClientSession:
         else:
             raise ValueError(f"Unsupported transport scheme: {parsed.scheme}")
 
+    def _serialize_mcp_content(self, content: list[Any]) -> list[dict[str, Any]]:
+        """
+        Convert MCP SDK content objects to plain dicts.
+
+        The MCP SDK returns Pydantic objects (TextContent, ImageContent, etc.)
+        but the rest of the ADCP client expects protocol-agnostic dicts.
+        This method handles the translation at the protocol boundary.
+
+        Args:
+            content: List of MCP content items (may be dicts or Pydantic objects)
+
+        Returns:
+            List of plain dicts representing the content
+        """
+        result = []
+        for item in content:
+            # Already a dict, pass through
+            if isinstance(item, dict):
+                result.append(item)
+            # Pydantic v2 model with model_dump()
+            elif hasattr(item, "model_dump"):
+                result.append(item.model_dump())
+            # Pydantic v1 model with dict()
+            elif hasattr(item, "dict") and callable(item.dict):
+                result.append(item.dict())  # type: ignore[attr-defined]
+            # Fallback: try to access __dict__
+            elif hasattr(item, "__dict__"):
+                result.append(dict(item.__dict__))
+            # Last resort: serialize as unknown type
+            else:
+                logger.warning(f"Unknown MCP content type: {type(item)}, serializing as string")
+                result.append({"type": "unknown", "data": str(item)})
+        return result
+
     async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskResult[Any]:
         """Call a tool using MCP protocol."""
         start_time = time.time() if self.agent_config.debug else None
@@ -205,12 +239,15 @@ 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)
 
+            # Serialize MCP SDK types to plain dicts at protocol boundary
+            serialized_content = self._serialize_mcp_content(result.content)
+
             if self.agent_config.debug and start_time:
                 duration_ms = (time.time() - start_time) * 1000
                 debug_info = DebugInfo(
                     request=debug_request,
                     response={
-                        "content": result.content,
+                        "content": serialized_content,
                         "is_error": result.isError if hasattr(result, "isError") else False,
                     },
                     duration_ms=duration_ms,
@@ -220,7 +257,7 @@ async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskRe
             # For AdCP, we expect the data in the content
             return TaskResult[Any](
                 status=TaskStatus.COMPLETED,
-                data=result.content,
+                data=serialized_content,
                 success=True,
                 debug_info=debug_info,
             )

src/adcp/utils/response_parser.py

@@ -13,20 +13,20 @@
 T = TypeVar("T", bound=BaseModel)
 
 
-def parse_mcp_content(content: list[dict[str, Any] | Any], response_type: type[T]) -> T:
+def parse_mcp_content(content: list[dict[str, Any]], response_type: type[T]) -> T:
     """
     Parse MCP content array into structured response type.
 
     MCP tools return content as a list of content items:
     [{"type": "text", "text": "..."}, {"type": "resource", ...}]
 
-    Content items may be either plain dicts or Pydantic objects (TextContent, etc.)
-    depending on the MCP SDK version and usage pattern.
+    The MCP adapter is responsible for serializing MCP SDK Pydantic objects
+    to plain dicts before calling this function.
 
     For AdCP, we expect JSON data in text content items.
 
     Args:
-        content: MCP content array (list of dicts or Pydantic objects)
+        content: MCP content array (list of plain dicts)
         response_type: Expected Pydantic model type
 
     Returns:
@@ -38,19 +38,10 @@ def parse_mcp_content(content: list[dict[str, Any] | Any], response_type: type[T
     if not content:
         raise ValueError("Empty MCP content array")
 
-    # Helper to get field value from dict or object
-    def get_field(item: Any, field: str, default: Any = None) -> Any:
-        """Get field from dict or Pydantic object."""
-        if isinstance(item, dict):
-            return item.get(field, default)
-        return getattr(item, field, default)
-
     # Look for text content items that might contain JSON
     for item in content:
-        item_type = get_field(item, "type")
-
-        if item_type == "text":
-            text = get_field(item, "text", "")
+        if item.get("type") == "text":
+            text = item.get("text", "")
             if not text:
                 continue
 
@@ -67,7 +58,7 @@ def get_field(item: Any, field: str, default: Any = None) -> Any:
                     f"MCP content doesn't match expected schema {response_type.__name__}: {e}"
                 )
                 raise ValueError(f"MCP response doesn't match expected schema: {e}") from e
-        elif item_type == "resource":
+        elif item.get("type") == "resource":
             # Resource content might have structured data
             try:
                 return response_type.model_validate(item)
@@ -81,12 +72,9 @@ def get_field(item: Any, field: str, default: Any = None) -> Any:
     if len(content_preview) > 500:
         content_preview = content_preview[:500] + "..."
 
-    # Extract types for error message
-    content_types = [get_field(item, "type") for item in content]
-
     raise ValueError(
         f"No valid {response_type.__name__} data found in MCP content. "
-        f"Content types: {content_types}. "
+        f"Content types: {[item.get('type') for item in content]}. "
         f"Content preview:\n{content_preview}"
     )
 

tests/test_protocols.py

@@ -240,3 +240,59 @@ async def test_close_session(self, mcp_config):
         mock_exit_stack.aclose.assert_called_once()
         assert adapter._exit_stack is None
         assert adapter._session is None
+
+    def test_serialize_mcp_content_with_dicts(self, mcp_config):
+        """Test serializing MCP content that's already dicts."""
+        adapter = MCPAdapter(mcp_config)
+
+        content = [
+            {"type": "text", "text": "Hello"},
+            {"type": "resource", "uri": "file://test.txt"},
+        ]
+
+        result = adapter._serialize_mcp_content(content)
+
+        assert result == content  # Pass through unchanged
+        assert len(result) == 2
+
+    def test_serialize_mcp_content_with_pydantic_v2(self, mcp_config):
+        """Test serializing MCP content with Pydantic v2 objects."""
+        from pydantic import BaseModel
+
+        adapter = MCPAdapter(mcp_config)
+
+        class MockTextContent(BaseModel):
+            type: str
+            text: str
+
+        content = [
+            MockTextContent(type="text", text="Pydantic v2"),
+        ]
+
+        result = adapter._serialize_mcp_content(content)
+
+        assert len(result) == 1
+        assert result[0] == {"type": "text", "text": "Pydantic v2"}
+        assert isinstance(result[0], dict)
+
+    def test_serialize_mcp_content_mixed(self, mcp_config):
+        """Test serializing mixed MCP content (dicts and Pydantic objects)."""
+        from pydantic import BaseModel
+
+        adapter = MCPAdapter(mcp_config)
+
+        class MockTextContent(BaseModel):
+            type: str
+            text: str
+
+        content = [
+            {"type": "text", "text": "Plain dict"},
+            MockTextContent(type="text", text="Pydantic object"),
+        ]
+
+        result = adapter._serialize_mcp_content(content)
+
+        assert len(result) == 2
+        assert result[0] == {"type": "text", "text": "Plain dict"}
+        assert result[1] == {"type": "text", "text": "Pydantic object"}
+        assert all(isinstance(item, dict) for item in result)

tests/test_response_parser.py

@@ -18,13 +18,6 @@ class SampleResponse(BaseModel):
     items: list[str] = Field(default_factory=list)
 
 
-class MockTextContent(BaseModel):
-    """Mock MCP TextContent object for testing."""
-
-    type: str = "text"
-    text: str
-
-
 class TestParseMCPContent:
     """Tests for parse_mcp_content function."""
 
@@ -99,50 +92,6 @@ def test_empty_text_content_skipped(self):
         result = parse_mcp_content(content, SampleResponse)
         assert result.message == "Found"
 
-    def test_parse_pydantic_text_content(self):
-        """Test parsing MCP Pydantic TextContent objects."""
-        content = [
-            MockTextContent(
-                type="text",
-                text=json.dumps({"message": "Pydantic", "count": 99, "items": ["x", "y"]}),
-            )
-        ]
-
-        result = parse_mcp_content(content, SampleResponse)
-
-        assert isinstance(result, SampleResponse)
-        assert result.message == "Pydantic"
-        assert result.count == 99
-        assert result.items == ["x", "y"]
-
-    def test_parse_mixed_dict_and_pydantic_content(self):
-        """Test parsing MCP content with both dicts and Pydantic objects."""
-        content = [
-            {"type": "text", "text": "Not JSON"},
-            MockTextContent(
-                type="text",
-                text=json.dumps({"message": "Mixed", "count": 15}),
-            ),
-        ]
-
-        result = parse_mcp_content(content, SampleResponse)
-
-        assert result.message == "Mixed"
-        assert result.count == 15
-
-    def test_parse_pydantic_empty_text_skipped(self):
-        """Test that empty Pydantic text content is skipped."""
-        content = [
-            MockTextContent(type="text", text=""),
-            MockTextContent(
-                type="text",
-                text=json.dumps({"message": "Skip", "count": 7}),
-            ),
-        ]
-
-        result = parse_mcp_content(content, SampleResponse)
-        assert result.message == "Skip"
-
 
 class TestParseJSONOrText:
     """Tests for parse_json_or_text function."""