Python: Parse MCP CallToolResult.structuredContent field to prevent tool results returning None (#6421)

* Parse structuredContent from MCP CallToolResult (#3313)

The _parse_tool_result_from_mcp method only iterated over the content
field from CallToolResult, ignoring the structuredContent field entirely.
MCP servers that return JSON data via structuredContent (e.g., Power BI
MCP) appeared to return None.

Add handling for structuredContent: when present, serialize it as JSON
text and append it to the result list. This preserves the data for the
LLM while maintaining backward compatibility with existing behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Python: Parse MCP CallToolResult.structuredContent field to prevent tool results returning None

Fixes #3313

* Address review feedback: add default=str to json.dumps and remove .checkpoints/

- Add default=str to json.dumps for structuredContent serialization so
  non-JSON-serializable values (e.g. bytes) degrade gracefully instead
  of raising TypeError
- Remove all .checkpoints/ runtime artifacts from the repository
- Add **/.checkpoints/ to .gitignore to prevent future accidental commits
- Add test for non-serializable structuredContent values

Fixes #3313

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address review feedback for #3313: Python: MCP CallToolResult.structuredContent field is not parsed, causing tool results to return None

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Giles Odigwe
2026-06-10 05:51:09 -07:00
committed by GitHub
Unverified
parent 9a56bc9f16
commit 93cbf6b3f0
12 changed files with 158 additions and 144 deletions
+1
View File
@@ -206,6 +206,7 @@ temp*/
.temp/
# AI
**/.checkpoints/
.claude/
.omc/
.omx/
@@ -577,6 +577,9 @@ class MCPTool:
case _:
result.append(Content.from_text(str(item)))
if mcp_type.structuredContent is not None:
result.append(Content.from_text(json.dumps(mcp_type.structuredContent, default=str)))
if not result:
result.append(Content.from_text("null"))
return result
@@ -3516,9 +3516,7 @@ class MCPSkill(Skill):
result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri))
text = _mcp_join_text(result)
if not text:
raise ValueError(
f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'."
)
raise ValueError(f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'.")
self._content = text
return text
@@ -3572,11 +3570,7 @@ class MCPSkill(Skill):
or ``None`` if the name is unsafe.
"""
normalized = name.replace("\\", "/")
if (
normalized.startswith("/")
or "://" in normalized
or any(seg == ".." for seg in normalized.split("/"))
):
if normalized.startswith("/") or "://" in normalized or any(seg == ".." for seg in normalized.split("/")):
logger.debug("Rejecting resource name with unsafe path components: %r", name)
return None
return normalized
@@ -342,6 +342,69 @@ def test_parse_tool_result_from_mcp_resource_link_text_resource_and_unknown():
assert result[1].text == "Embedded result"
def test_parse_tool_result_from_mcp_structured_content_only():
"""Test that structuredContent is parsed when content list is empty."""
mcp_result = types.CallToolResult(
content=[],
structuredContent={"Tables": [{"Name": "Sales", "Columns": ["Amount", "Date"]}]},
)
result = _HELPER_MCP_TOOL._parse_tool_result_from_mcp(mcp_result)
assert isinstance(result, list)
assert len(result) == 1
assert result[0].type == "text"
parsed = json.loads(result[0].text)
assert parsed == {"Tables": [{"Name": "Sales", "Columns": ["Amount", "Date"]}]}
def test_parse_tool_result_from_mcp_structured_content_with_text():
"""Test that structuredContent is appended alongside regular content items."""
mcp_result = types.CallToolResult(
content=[types.TextContent(type="text", text="Summary")],
structuredContent={"data": [1, 2, 3]},
)
result = _HELPER_MCP_TOOL._parse_tool_result_from_mcp(mcp_result)
assert isinstance(result, list)
assert len(result) == 2
assert result[0].type == "text"
assert result[0].text == "Summary"
assert result[1].type == "text"
parsed = json.loads(result[1].text)
assert parsed == {"data": [1, 2, 3]}
def test_parse_tool_result_from_mcp_structured_content_none():
"""Test that None structuredContent does not affect results."""
mcp_result = types.CallToolResult(
content=[types.TextContent(type="text", text="Hello")],
structuredContent=None,
)
result = _HELPER_MCP_TOOL._parse_tool_result_from_mcp(mcp_result)
assert isinstance(result, list)
assert len(result) == 1
assert result[0].type == "text"
assert result[0].text == "Hello"
def test_parse_tool_result_from_mcp_structured_content_non_serializable():
"""Test that non-JSON-serializable values in structuredContent degrade gracefully."""
mcp_result = types.CallToolResult(
content=[],
structuredContent={"data": b"raw bytes", "count": 42},
)
result = _HELPER_MCP_TOOL._parse_tool_result_from_mcp(mcp_result)
assert isinstance(result, list)
assert len(result) == 1
assert result[0].type == "text"
parsed = json.loads(result[0].text)
assert parsed["count"] == 42
# bytes should be converted to string representation via default=str
assert "raw bytes" in parsed["data"]
def test_mcp_content_types_to_ai_content_text():
"""Test conversion of MCP text content to AI content."""
mcp_content = types.TextContent(type="text", text="Sample text")
@@ -76,6 +76,7 @@ def _make_call_tool_result(text: str = "result", is_error: bool = False) -> Mock
result = Mock()
result.isError = is_error
result.content = [types.TextContent(type="text", text=text)]
result.structuredContent = None
return result
@@ -281,9 +282,7 @@ async def test_mcp_prompts_get_creates_client_span(span_exporter: InMemorySpanEx
async def test_mcp_prompts_get_mcp_error_sets_error_type(span_exporter: InMemorySpanExporter):
"""When session.get_prompt() raises McpError, the span should have error.type and ERROR status."""
tool = _make_connected_mcp_tool()
tool.session.get_prompt = AsyncMock(
side_effect=McpError(ErrorData(code=-32602, message="prompt not found"))
)
tool.session.get_prompt = AsyncMock(side_effect=McpError(ErrorData(code=-32602, message="prompt not found")))
span_exporter.clear()
with pytest.raises(ToolExecutionException):
@@ -35,26 +35,22 @@ description: Convert between common units.
Body content here.
"""
SAMPLE_SKILL_INDEX = json.dumps(
{
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "unit-converter",
"type": "skill-md",
"description": "Convert between common units.",
"url": "skill://unit-converter/SKILL.md",
}
],
}
)
SAMPLE_SKILL_INDEX = json.dumps({
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "unit-converter",
"type": "skill-md",
"description": "Convert between common units.",
"url": "skill://unit-converter/SKILL.md",
}
],
})
def _make_text_result(text: str, uri: str = "skill://test") -> ReadResourceResult:
"""Create a ReadResourceResult with a single TextResourceContents."""
return ReadResourceResult(
contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")]
)
return ReadResourceResult(contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")])
def _make_blob_result(
@@ -230,12 +226,10 @@ class TestMCPSkill:
@pytest.mark.asyncio
async def test_get_resource_text(self) -> None:
client = _make_client(
**{
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
}
)
client = _make_client(**{
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
})
from agent_framework import SkillFrontmatter
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
@@ -249,12 +243,10 @@ class TestMCPSkill:
@pytest.mark.asyncio
async def test_get_resource_binary(self) -> None:
data = bytes([0x01, 0x02, 0x03, 0x04])
client = _make_client(
**{
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
}
)
client = _make_client(**{
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
})
from agent_framework import SkillFrontmatter
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
@@ -345,12 +337,10 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_index_based_discovery_returns_skill(self) -> None:
client = _make_client(
**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
}
)
client = _make_client(**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -373,9 +363,7 @@ class TestMCPSkillsSource:
async def test_does_not_read_skill_md_during_discovery(self) -> None:
# Index points to a skill, but SKILL.md is not registered on the server.
# Discovery should succeed because it only reads the index.
client = _make_client(
**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")}
)
client = _make_client(**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -384,19 +372,17 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_invalid_name_is_skipped(self) -> None:
index_json = json.dumps(
{
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "UnitConverter", # Invalid: uppercase
"type": "skill-md",
"description": "Convert between common units.",
"url": "skill://UnitConverter/SKILL.md",
}
],
}
)
index_json = json.dumps({
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "UnitConverter", # Invalid: uppercase
"type": "skill-md",
"description": "Convert between common units.",
"url": "skill://UnitConverter/SKILL.md",
}
],
})
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -404,18 +390,16 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_missing_required_fields_is_skipped(self) -> None:
index_json = json.dumps(
{
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "unit-converter",
"type": "skill-md",
# Missing description and url
}
],
}
)
index_json = json.dumps({
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "unit-converter",
"type": "skill-md",
# Missing description and url
}
],
})
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -423,19 +407,17 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_unsupported_type_is_skipped(self) -> None:
index_json = json.dumps(
{
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "some-skill",
"type": "archive",
"description": "Packaged skill.",
"url": "skill://some-skill.tar.gz",
}
],
}
)
index_json = json.dumps({
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"name": "some-skill",
"type": "archive",
"description": "Packaged skill.",
"url": "skill://some-skill.tar.gz",
}
],
})
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -443,18 +425,16 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_template_type_is_skipped(self) -> None:
index_json = json.dumps(
{
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"type": "mcp-resource-template",
"description": "Per-product documentation skill",
"url": "skill://docs/{product}/SKILL.md",
}
],
}
)
index_json = json.dumps({
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
"skills": [
{
"type": "mcp-resource-template",
"description": "Per-product documentation skill",
"url": "skill://docs/{product}/SKILL.md",
}
],
})
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
@@ -462,31 +442,25 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_empty_index_returns_empty(self) -> None:
client = _make_client(
**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")}
)
client = _make_client(**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
assert skills == []
@pytest.mark.asyncio
async def test_malformed_index_json_returns_empty(self) -> None:
client = _make_client(
**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")}
)
client = _make_client(**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")})
source = MCPSkillsSource(client=client)
skills = await source.get_skills()
assert skills == []
@pytest.mark.asyncio
async def test_sibling_text_resource(self) -> None:
client = _make_client(
**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
}
)
client = _make_client(**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
})
source = MCPSkillsSource(client=client)
skill = (await source.get_skills())[0]
resource = await skill.get_resource("references/checklist.md")
@@ -497,13 +471,11 @@ class TestMCPSkillsSource:
@pytest.mark.asyncio
async def test_sibling_binary_resource(self) -> None:
data = bytes([0x01, 0x02, 0x03, 0x04])
client = _make_client(
**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
}
)
client = _make_client(**{
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
})
source = MCPSkillsSource(client=client)
skill = (await source.get_skills())[0]
resource = await skill.get_resource("assets/icon.bin")
@@ -649,9 +621,7 @@ class TestMCPSkillsSourceErrorCodeBranching:
from agent_framework import SkillFrontmatter
client = AsyncMock()
client.read_resource = AsyncMock(
side_effect=McpError(error=ErrorData(code=0, message="Handler error"))
)
client.read_resource = AsyncMock(side_effect=McpError(error=ErrorData(code=0, message="Handler error")))
fm = SkillFrontmatter(name="test-skill", description="Test.")
skill = MCPSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client)
with pytest.raises(McpError):
@@ -198,12 +198,7 @@ class TestBeforeRun:
"""OSS client with all scoping parameters passes them as isolated concurrent kwargs."""
mock_oss_mem0_client.search.return_value = []
provider = Mem0ContextProvider(
source_id="mem0",
mem0_client=mock_oss_mem0_client,
user_id="u1",
agent_id="a1"
)
provider = Mem0ContextProvider(source_id="mem0", mem0_client=mock_oss_mem0_client, user_id="u1", agent_id="a1")
mock_context = MagicMock(spec=SessionContext)
mock_msg = MagicMock()
@@ -313,9 +313,7 @@ class HarnessAgentRunner:
"""
actions: list[FollowUpAction] = []
for observer in self._observers:
observer_actions = await observer.on_stream_complete(
self._ux, self._agent, session
)
observer_actions = await observer.on_stream_complete(self._ux, self._agent, session)
if observer_actions:
actions.extend(observer_actions)
return actions
@@ -182,18 +182,12 @@ class HarnessApp(App[None]):
if command_handlers is None:
from .commands import build_default_command_handlers
self._command_handlers = build_default_command_handlers(
agent, mode_colors=mode_colors
)
self._command_handlers = build_default_command_handlers(agent, mode_colors=mode_colors)
else:
self._command_handlers = command_handlers
# Compute help text from command handlers
help_parts = [
h.get_help_text()
for h in self._command_handlers
if h.get_help_text() is not None
]
help_parts = [h.get_help_text() for h in self._command_handlers if h.get_help_text() is not None]
help_text = ", ".join(help_parts) if help_parts else None
# State and driver
@@ -45,9 +45,7 @@ class TodoCommandHandler(CommandHandler):
ux.append_info_line("TodoProvider is not available.")
return True
todos = await self._todo_provider.store.load_items(
session, source_id=self._todo_provider.source_id
)
todos = await self._todo_provider.store.load_items(session, source_id=self._todo_provider.source_id)
if not todos:
ux.append_info_line("No todos yet.")
@@ -72,7 +72,7 @@ class HarnessScrollPanel(RichLog):
# Truncate lines back to where streaming started
if len(self.lines) > self._streaming_line_start:
del self.lines[self._streaming_line_start:]
del self.lines[self._streaming_line_start :]
from textual.geometry import Size
self.virtual_size = Size(self._widest_line_width, len(self.lines))
@@ -41,8 +41,7 @@ class PlanningQuestion(BaseModel):
choices: list[str] | None = Field(
default=None,
description=(
"For clarifications, this has a list of options that the user can "
"choose from. null for approvals."
"For clarifications, this has a list of options that the user can choose from. null for approvals."
),
)