Skip to content

Commit 6b60365

Browse files
bokelleyclaude
andauthored
fix: handle Pydantic TextContent objects in MCP response parser (#14)
* fix: handle Pydantic TextContent objects in MCP response parser The MCP SDK returns Pydantic TextContent objects, not plain dicts. The response_parser.py was using .get() method which only works on dicts, causing AttributeError: 'TextContent' object has no attribute 'get'. This change adds a helper function that handles both dict and Pydantic object access patterns, ensuring compatibility with both the MCP SDK's actual return types and the test suite's mock dicts. Tests added for: - Pydantic TextContent objects - Mixed dict and Pydantic content - Empty Pydantic text content handling Fixes regression introduced in v1.0.3 where parsing logic was added that assumed dict objects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * 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> * fix: remove unused type ignore comment Mypy now correctly infers types after hasattr and callable checks, making the type: ignore[attr-defined] comment unnecessary. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f665405 commit 6b60365

File tree

3 files changed

+99
-3
lines changed

3 files changed

+99
-3
lines changed

src/adcp/protocols/mcp.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,40 @@ async def _get_session(self) -> ClientSession:
186186
else:
187187
raise ValueError(f"Unsupported transport scheme: {parsed.scheme}")
188188

189+
def _serialize_mcp_content(self, content: list[Any]) -> list[dict[str, Any]]:
190+
"""
191+
Convert MCP SDK content objects to plain dicts.
192+
193+
The MCP SDK returns Pydantic objects (TextContent, ImageContent, etc.)
194+
but the rest of the ADCP client expects protocol-agnostic dicts.
195+
This method handles the translation at the protocol boundary.
196+
197+
Args:
198+
content: List of MCP content items (may be dicts or Pydantic objects)
199+
200+
Returns:
201+
List of plain dicts representing the content
202+
"""
203+
result = []
204+
for item in content:
205+
# Already a dict, pass through
206+
if isinstance(item, dict):
207+
result.append(item)
208+
# Pydantic v2 model with model_dump()
209+
elif hasattr(item, "model_dump"):
210+
result.append(item.model_dump())
211+
# Pydantic v1 model with dict()
212+
elif hasattr(item, "dict") and callable(item.dict):
213+
result.append(item.dict())
214+
# Fallback: try to access __dict__
215+
elif hasattr(item, "__dict__"):
216+
result.append(dict(item.__dict__))
217+
# Last resort: serialize as unknown type
218+
else:
219+
logger.warning(f"Unknown MCP content type: {type(item)}, serializing as string")
220+
result.append({"type": "unknown", "data": str(item)})
221+
return result
222+
189223
async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskResult[Any]:
190224
"""Call a tool using MCP protocol."""
191225
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
205239
# Call the tool using MCP client session
206240
result = await session.call_tool(tool_name, params)
207241

242+
# Serialize MCP SDK types to plain dicts at protocol boundary
243+
serialized_content = self._serialize_mcp_content(result.content)
244+
208245
if self.agent_config.debug and start_time:
209246
duration_ms = (time.time() - start_time) * 1000
210247
debug_info = DebugInfo(
211248
request=debug_request,
212249
response={
213-
"content": result.content,
250+
"content": serialized_content,
214251
"is_error": result.isError if hasattr(result, "isError") else False,
215252
},
216253
duration_ms=duration_ms,
@@ -220,7 +257,7 @@ async def _call_mcp_tool(self, tool_name: str, params: dict[str, Any]) -> TaskRe
220257
# For AdCP, we expect the data in the content
221258
return TaskResult[Any](
222259
status=TaskStatus.COMPLETED,
223-
data=result.content,
260+
data=serialized_content,
224261
success=True,
225262
debug_info=debug_info,
226263
)

src/adcp/utils/response_parser.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@ def parse_mcp_content(content: list[dict[str, Any]], response_type: type[T]) ->
2020
MCP tools return content as a list of content items:
2121
[{"type": "text", "text": "..."}, {"type": "resource", ...}]
2222
23+
The MCP adapter is responsible for serializing MCP SDK Pydantic objects
24+
to plain dicts before calling this function.
25+
2326
For AdCP, we expect JSON data in text content items.
2427
2528
Args:
26-
content: MCP content array
29+
content: MCP content array (list of plain dicts)
2730
response_type: Expected Pydantic model type
2831
2932
Returns:

tests/test_protocols.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,59 @@ async def test_close_session(self, mcp_config):
240240
mock_exit_stack.aclose.assert_called_once()
241241
assert adapter._exit_stack is None
242242
assert adapter._session is None
243+
244+
def test_serialize_mcp_content_with_dicts(self, mcp_config):
245+
"""Test serializing MCP content that's already dicts."""
246+
adapter = MCPAdapter(mcp_config)
247+
248+
content = [
249+
{"type": "text", "text": "Hello"},
250+
{"type": "resource", "uri": "file://test.txt"},
251+
]
252+
253+
result = adapter._serialize_mcp_content(content)
254+
255+
assert result == content # Pass through unchanged
256+
assert len(result) == 2
257+
258+
def test_serialize_mcp_content_with_pydantic_v2(self, mcp_config):
259+
"""Test serializing MCP content with Pydantic v2 objects."""
260+
from pydantic import BaseModel
261+
262+
adapter = MCPAdapter(mcp_config)
263+
264+
class MockTextContent(BaseModel):
265+
type: str
266+
text: str
267+
268+
content = [
269+
MockTextContent(type="text", text="Pydantic v2"),
270+
]
271+
272+
result = adapter._serialize_mcp_content(content)
273+
274+
assert len(result) == 1
275+
assert result[0] == {"type": "text", "text": "Pydantic v2"}
276+
assert isinstance(result[0], dict)
277+
278+
def test_serialize_mcp_content_mixed(self, mcp_config):
279+
"""Test serializing mixed MCP content (dicts and Pydantic objects)."""
280+
from pydantic import BaseModel
281+
282+
adapter = MCPAdapter(mcp_config)
283+
284+
class MockTextContent(BaseModel):
285+
type: str
286+
text: str
287+
288+
content = [
289+
{"type": "text", "text": "Plain dict"},
290+
MockTextContent(type="text", text="Pydantic object"),
291+
]
292+
293+
result = adapter._serialize_mcp_content(content)
294+
295+
assert len(result) == 2
296+
assert result[0] == {"type": "text", "text": "Plain dict"}
297+
assert result[1] == {"type": "text", "text": "Pydantic object"}
298+
assert all(isinstance(item, dict) for item in result)

0 commit comments

Comments
 (0)