Python: Parse MCP CallToolResult.structuredContent field to prevent tool results returning None#6421
Conversation
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>
…ool results returning None Fixes microsoft#3313
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 90%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
No actionable issues found in this dimension.
✓ Failure Modes
No actionable issues found in this dimension.
✓ Design Approach
The added files look like raw runtime checkpoint snapshots rather than source fixtures.
Suggestions
- Prefer generating checkpoint/approval state inside focused tests instead of committing raw
.checkpoints/*.jsonruntime snapshots. The existing repo pattern already does this with tmp_path-backed round-trip tests inpython/packages/foundry_hosting/tests/test_responses.py:2580-2595andpython/packages/foundry_hosting/tests/test_responses.py:2953-2963.
Automated review by giles17's agents
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 88%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
No actionable issues found in this dimension.
✓ Failure Modes
No actionable issues found in this dimension.
✗ Design Approach
The changes shown here add autogenerated workflow checkpoint snapshots into source control. That is the wrong design surface: this repo treats checkpoints as runtime-managed/temp storage, and the checkpoint format itself is documented as unsafe to load from untrusted sources because it embeds pickled state. I would request changes to remove these artifacts from the PR. The MCP fix addresses the right gap, and I did not find a blocking design problem in the added structuredContent handling. The one design mismatch worth calling out is that this new serialization path is stricter than the repo's existing structured-result serialization pattern, which already falls back to
default=strfor non-plain JSON leaves.
Flagged Issues
- Generated
.checkpointssnapshots are being committed even though repo code stores checkpoints in temp/runtime locations (python/samples/03-workflows/declarative/function_tools/main.py:24-25,84-89), and checkpoint storage is explicitly documented as unsafe for untrusted inputs because it uses pickle (python/packages/azure-cosmos/agent_framework_azure_cosmos/_checkpoint_storage.py:46-50).
Automated review by giles17's agents
Python Test Coverage Report •
Python Unit Test Overview
|
|||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR updates the Python MCP tool result parsing so MCP servers that return results via CallToolResult.structuredContent (instead of content) no longer surface as empty/None results to the agent framework, addressing #3313.
Changes:
- Parse
CallToolResult.structuredContentinMCPTool._parse_tool_result_from_mcp()by serializing it to JSON text and appending it to the returnedContentlist. - Add unit tests covering structured-content-only, structured-content-with-text, and structuredContent=
None. - Includes assorted formatting-only edits in samples/tests, plus adds a large set of
.checkpoints/JSON artifacts.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/core/agent_framework/_mcp.py | Appends serialized structuredContent to parsed tool result content. |
| python/packages/core/tests/core/test_mcp.py | Adds tests validating parsing behavior for structuredContent. |
| python/packages/core/tests/core/test_mcp_observability.py | Updates mock CallToolResult to include structuredContent attribute. |
| python/samples/02-agents/harness/console/observers/planning_models.py | Formatting-only change to a field description string. |
| python/samples/02-agents/harness/console/components/scroll_panel.py | Formatting-only change to slice spacing. |
| python/samples/02-agents/harness/console/commands/todo_handler.py | Formatting-only change to a method call layout. |
| python/samples/02-agents/harness/console/app.py | Formatting-only changes to handler initialization and help text list comprehension. |
| python/samples/02-agents/harness/console/agent_runner.py | Formatting-only change to an awaited call layout. |
| python/packages/mem0/tests/test_mem0_context_provider.py | Formatting-only change to Mem0ContextProvider(...) construction. |
| python/packages/core/tests/core/test_mcp_skills.py | Formatting-only changes to JSON/dict literals and helper invocations. |
| python/packages/core/agent_framework/_skills.py | Formatting-only changes to exception raise and resource-name validation condition. |
| python/.checkpoints/caresp_f72f0fd80e1808ff00FRXuXWMScBycluobpWG1l3AReZiQ2UEK/53a7ef14-fa8b-4437-b7ec-a33555397bbc.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_f72f0fd80e1808ff00cxIUE0eRxKOSm7HoGdNVXAG0ng26oSoW/94b5ecdf-19bc-4e4e-a6ed-df274bf659ab.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_dfafdcd4ffba831300URfFCzeXLRiJXTyRz6t04UM6vRXp6x2P/86966611-910c-4c96-aa9f-cb6202e4368f.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_dfafdcd4ffba831300r5xZ6507717wMZVgNoc41WumqTo8iNNi/5902177b-c769-47ff-8ab6-2631cf777781.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_dfafdcd4ffba831300r5xZ6507717wMZVgNoc41WumqTo8iNNi/07245648-d153-4fd5-93b0-3c61eda95cf2.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_cb7365c2cef6b04500uSCowRcBfxa8phXzmfLVZHOdhJ7s1fXC/d81078aa-2b22-4b20-9902-236583647b03.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_ca50e1b2c9ff05d100bbRLtgKnH4j915D9jIgWO9IYYq3x8IdT/8ccc9bbd-e6ae-4383-b39e-964e1785af0f.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_ca50e1b2c9ff05d1000PSontW63f8PXEndam2gBFROmw5938iK/17d51626-f276-4bcc-8994-9fde313584fb.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_bdbebefdf0a05c5d006AA7IfCq30IQCNYNHCiQ0MlCftCNXeq6/81bc4053-f285-4ece-8604-e7bb56a96fe4.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_a84ccdd71bbf480300zf28dbQzRxNmPIrwmTn1PNrvAjOgmcUH/318f6fb8-a0e1-4201-bd95-bde96f93469f.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_a84ccdd71bbf480300t1G1lEKVTW3azGOEorZQGamRZTn72nGW/1b31ff92-3fef-4547-83cf-bfb03105d6a4.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_a0b79877ce3595ee00LyDFpUYcquhIX8qCzHHMSBymPfY61we7/c3bb0819-377d-4ed9-9920-6da1f01a0ad8.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_a0b79877ce3595ee00LV36P5yjGfhhWEu5MXbge0l66XfSeqHv/7ea60219-b9be-4ab2-844d-3f623fc2f419.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_a0937fbe81e3a97900PVAswKJXZdcFBGccdsHQFlb4942lC0lU/9d32085b-afa4-4543-a135-4ba53625cd7d.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_934e6136e0cefafd00U7b2SBMgbV1YMmm8XX1iJAcdSAyPz31G/4936c6c0-3f52-4584-a050-d7eaa98423a4.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_934e6136e0cefafd00NNfGgFgXKqJ37lxbRPQitofDFnbSC98F/a63d8166-d9f6-4ee3-8bd6-3c97b315e540.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_8a8d874b769d678500hsbDkH42vDc6WBiQngjQZ1PJI81IIRrh/b7ad378a-aac7-4f37-a483-69ca68ce9de9.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_50cd309017ae4d0f00jqiJtuAOYEL24nAUip4R2tSoxuObIFkl/a0e3343a-57b7-4c0e-bed6-9633b8dc9cd0.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_3c4ff94f8314b21a00TsXVES3g5gKfYBHkkVIvfdmzHHZMr6Rt/4dc5220a-f736-4e94-92fc-bf3e0977c910.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_37653587e12fc3b500Faj7f4nPwEGaNTq2kmvotmGAIu6CeecN/e37abb98-2944-46ed-b43a-2c9330651448.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_37653587e12fc3b500b20JrfYcDDgRaC8g8UYw28UGn8jeMlBn/e27562bc-bcbb-428b-87a1-0b2014c104b2.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_260c0bbb30763bf500To5wDgXPWUKWfof6rw5SfOoeEiLDmhdI/5a642b40-3b6f-4a15-918c-f8a4994dd158.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_0853a4b45a741e9b00O2Ev0CTglZgKE490gyQ41HhLGQguZsY7/d66bb5b3-604e-439b-9d42-b13505397240.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_05bbaeeb885c4ab500uw86yTp0J7zAAq7zczFpQHtogJ4O0dJb/d9419c4d-84fa-40bd-a50a-8885272319ac.json | Added checkpoint artifact (should not be committed). |
| python/.checkpoints/caresp_05bbaeeb885c4ab500ACHoMMgZHPjIKoOAteCW1ukyxRGcw8Yo/d322b9d7-2fae-4e3c-bf94-8f39f9bdaf90.json | Added checkpoint artifact (should not be committed). |
…eckpoints/ - 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 microsoft#3313 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t.structuredContent field is not parsed, causing tool results to return None
giles17
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 97% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach
Automated review by giles17's agents
Motivation and Context
MCP servers that return JSON data via the
structuredContentfield (e.g., Power BI MCP) produce empty/None tool results because the framework's result parser only processes thecontentfield, silently discarding structured data.Fixes #3313
Description
The root cause is that
_parse_tool_result_from_mcpin_mcp.pyonly iterates overmcp_type.contentand never inspectsmcp_type.structuredContent. The fix checks whetherstructuredContentis not None after processing the content list, and if present, serializes it as a JSON text content item appended to the results. This ensures MCP servers usingstructuredContentfor structured JSON responses are properly surfaced to the LM. Tests cover structured content alone, alongside regular content, and when set to None.Contribution Checklist