Python: Add regression tests for MCP tool result event projection in GitHubCopilotAgent - #6226
Python: Add regression tests for MCP tool result event projection in GitHubCopilotAgent#6226Giles Odigwe (giles17) wants to merge 2 commits into
Conversation
Giles Odigwe (giles17)
left a comment
Giles Odigwe (giles17)
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 96% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by giles17's agents
There was a problem hiding this comment.
Pull request overview
Adds a targeted regression test to ensure GitHubCopilotAgent streaming mode continues to correctly project multiple consecutive MCP tool execution events into semantic function_call / function_result content items (covering the multi-tool-call scenario from #5897).
Changes:
- Add a new streaming regression test that simulates three consecutive tool calls with JSON string payload results and asserts correct projection (role, content type, call_id, tool name, result).
- Minor formatting-only updates in a sample harness and a couple of core/test files (no behavior changes).
- Lockfile metadata tweak for the Copilot SDK specifier string (no effective version change).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/uv.lock | Minor normalization of the Copilot SDK specifier string in lock metadata. |
| python/samples/02-agents/harness/harness_research.py | Readability-only reformatting of a compound conditional. |
| python/packages/github_copilot/tests/test_github_copilot_agent.py | Adds a regression test for streaming projection across multiple tool calls with JSON results. |
| python/packages/core/tests/core/test_skills.py | Removes unnecessary parentheses in awaited calls (style-only). |
| python/packages/core/agent_framework/_skills.py | String formatting adjustments in tool metadata (no functional change). |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
) Add a streaming test that exercises multiple consecutive MCP tool invocations with JSON payloads, verifying that TOOL_EXECUTION_START and TOOL_EXECUTION_COMPLETE events are projected as function_call and function_result content items with correct call_id, tool name, and result payload. Removes unrelated formatting-only changes from prior iteration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6b1f824 to
41da08b
Compare
Giles Odigwe (giles17)
left a comment
Giles Odigwe (giles17)
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 96%
✓ Correctness
This PR contains only cosmetic/formatting changes: collapsing multi-line strings onto single lines, removing redundant parentheses around await expressions, reformatting a long conditional, and reordering equivalent version specifiers in uv.lock. All changes are semantically identical to the original code. No correctness issues found.
✓ Security Reliability
This PR contains only cosmetic/formatting changes: collapsing multi-line string literals into single lines, removing redundant parentheses around await expressions, reformatting a long conditional, and reordering semantically-equivalent version specifiers in uv.lock. There are no behavioral, security, or reliability concerns.
✓ Test Coverage
This PR contains exclusively cosmetic/formatting changes: collapsing multi-line strings onto single lines in _skills.py, removing redundant parentheses around await expressions in tests, reformatting a long conditional in a sample file, and reordering version specifiers in uv.lock. There are no behavioral changes whatsoever, so no new test coverage is needed.
✓ Design Approach
This patch does not appear to address the Python-side GitHub Copilot tool-event issue it was meant to fix. The only changes here are formatting-only edits in
core/_skills.py, tests that just remove redundant parentheses, a formatting tweak in a sample harness, and a non-functional dependency-specifier reorder inuv.lock. Meanwhile, the actual Python Copilot agent already handlesTOL_EXECUTION_STARTandTOL_EXECUTION_COMPLETEby emitting function-call and function-result content inpython/packages/github_copilot/agent_framework_github_copilot/_agent.py:642-678, so this PR's current design leaves the real issue surface untouched.
Automated review by giles17's agents
429863e to
41da08b
Compare
…ubCopilotAgent silently drops MCP tool result events — projected as opaque AIContent instead of FunctionResultContent, causing models to fabricate timeout errors
Giles Odigwe (giles17)
left a comment
Giles Odigwe (giles17)
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 95% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by giles17's agents
Motivation and Context
The Python GitHubCopilotAgent already correctly projects MCP tool execution events into
FunctionCallContentandFunctionResultContent(fixed in #4734, #4814, #4828), but there was no dedicated test covering the multi-tool-call scenario described in #5897. These tests prevent regression of the projection logic that, when broken, causes models to fabricate timeout errors because tool results appear as opaque content instead of semantically marked function results.Fixes #5897
Description
Adds a streaming test (
test_run_streaming_multiple_tool_calls_with_json_results) that exercises three consecutive MCP tool invocations returning JSON payloads, verifying that eachTOOL_EXECUTION_STARTevent is projected as afunction_callcontent item and eachTOOL_EXECUTION_COMPLETEevent is projected as afunction_resultcontent item with the correctcall_id, tool name, and result payload. This directly covers the scenario from issue #5897 where successful MCP tool returns were silently dropped as opaqueAIContent.Contribution Checklist