[BREAKING] Python: Update github-copilot-sdk integration to use ToolInvocation/ToolResult types#4551
Conversation
…ft#4549) - Update requires-python from >=3.10 to >=3.11 - Remove Python 3.10 classifier - Update mypy python_version to 3.11 - Update dependency to github-copilot-sdk>=0.1.32 - Fix ToolResult API: use snake_case kwargs (text_result_for_llm, result_type) instead of camelCase (textResultForLlm, resultType) - Update test assertions to use attribute access on ToolResult - Add ToolResult type assertions to tool handler tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…osoft#4549) Update test_github_copilot_agent.py to pass ToolInvocation objects to tool handlers instead of plain dicts, matching the github-copilot-sdk>=0.1.32 API where ToolInvocation is a dataclass with an .arguments attribute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
moonbox3
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✓ Correctness
The diff updates two test call sites to pass a
ToolInvocationobject instead of a plain dictionary tocopilot_tool.handler(), aligning the tests with a likely signature change in the handler that now expects a typedToolInvocationrather than a raw dict. The import is correctly added and the constructionToolInvocation(arguments={"arg": "test"})matches the previous dict shape. No correctness issues found.
✓ Security Reliability
This diff is a minor test-only change that replaces raw dictionary arguments with typed ToolInvocation objects when calling copilot tool handlers. This improves type safety in the tests and aligns them with an updated handler signature. There are no security or reliability concerns.
✓ Test Coverage
The diff updates two existing tests to pass a
ToolInvocationobject instead of a raw dict tocopilot_tool.handler(), aligning tests with a signature change. The existing assertions (checkingresult_typefor success and failure) remain meaningful. However, there are no new tests covering edge cases of theToolInvocationtype — e.g., missing arguments, extra fields, orNonearguments — and no test verifies that passing a raw dict now raises an error (if that was the intent of the breaking change).
✓ Design Approach
The diff updates two test call sites to pass a proper
ToolInvocationobject instead of a raw dictionary tocopilot_tool.handler(). This is a correct and straightforward change that aligns tests with a typed interface, replacing duck-typed dictionaries with the actual domain object. No design concerns.
Suggestions
- Consider adding a test that verifies passing a raw dict (the old calling convention) to
copilot_tool.handler()now raises aTypeErroror similar error, to lock in the new contract. - Consider adding a test with
ToolInvocation(arguments={})(empty arguments) to cover the edge case where no arguments are provided.
Automated review by moonbox3's agents
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub Copilot Python integration to align with github-copilot-sdk>=0.1.32’s runtime types, and adds/adjusts tests so future regressions (back to dict-based tool invocation/results) are caught.
Changes:
- Update
GitHubCopilotAgenttool handler to consumeToolInvocationand returnToolResultwith snake_case fields. - Update regression tests to pass
ToolInvocation(...)and assert onToolResultattributes. - Bump
agent-framework-github-copilotto Python>=3.11and upgrade dependency togithub-copilot-sdk>=0.1.32.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/github_copilot/tests/test_github_copilot_agent.py | Updates tool handler tests to use ToolInvocation and validate ToolResult attributes. |
| python/packages/github_copilot/pyproject.toml | Raises minimum Python to 3.11, removes 3.10 classifier, updates mypy target, and bumps github-copilot-sdk dependency. |
| python/packages/github_copilot/agent_framework_github_copilot/_agent.py | Updates tool conversion handler to use invocation.arguments and construct ToolResult using snake_case fields; adjusts imports. |
You can also share your feedback on Copilot code review. Take the survey.
Add tests to lock in the new ToolInvocation-based calling convention: - test_tool_handler_rejects_raw_dict_invocation: verifies passing a raw dict (old calling convention) raises TypeError/AttributeError - test_tool_handler_with_empty_arguments: verifies ToolInvocation with empty arguments works correctly for no-arg tools Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The repo CI runs with Python 3.10 (uv sync --all-packages) and all other packages require >=3.10. Raising this package to >=3.11 would break the shared install flow. The SDK dependency version constraint (>=0.1.32) will enforce any Python version requirement from the SDK itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-copilot-sdk>=0.1.32 requires Python>=3.11, which conflicts with the package's declared >=3.10 minimum, breaking uv sync.
Motivation and Context
The upgrade of github-copilot-sdk to >=0.1.32 introduced breaking changes:
Fixes #4549
Description
Production code changes (_agent.py):
Dependency & Python version changes:
CI/CD changes:
Sample fixes (github_copilot samples):
Test changes (test_github_copilot_agent.py):
Contribution Checklist