.NET: Update to the latest version of GitHub.Copilot.SDK#4151
.NET: Update to the latest version of GitHub.Copilot.SDK#4151
Conversation
… copying Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs
Outdated
Show resolved
Hide resolved
…feedback Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the .NET GitHub Copilot agent integration to use newly introduced Clone() support in GitHub.Copilot.SDK for safer, forward-compatible session config copying.
Changes:
- Bump
GitHub.Copilot.SDKfrom0.1.23to0.1.25. - Replace manual
SessionConfigproperty copying withsource.Clone()+Streaming = true. - Adjust unit tests to reflect cloned (non-shared) collection instances.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/Directory.Packages.props | Updates the referenced GitHub.Copilot.SDK package version to 0.1.25. |
| dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs | Uses SDK Clone() for SessionConfig copying and forces streaming on. |
| dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs | Updates assertions to account for cloned collections in SessionConfig. |
Comments suppressed due to low confidence (1)
dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs:152
- Similarly for
McpServers, the test now only checks the dictionary instance is different and has one entry. Add assertions that the expected key exists and that the corresponding value was preserved (e.g., same config instance or expected equality) so the test fails ifClone()drops or alters MCP server entries.
Assert.NotSame(mcpServers, result.McpServers);
Assert.Single(result.McpServers!);
| /// Clones a <see cref="SessionConfig"/> and sets <see cref="SessionConfig.Streaming"/> to <c>true</c>. | ||
| /// </summary> | ||
| internal static SessionConfig CopySessionConfig(SessionConfig source) | ||
| { |
There was a problem hiding this comment.
CopySessionConfig now calls source.Clone() without validating source. Since this file consistently uses Throw.IfNull(...) for argument validation, consider adding an explicit null check so callers get an ArgumentNullException instead of a potential NullReferenceException if the method is ever reused elsewhere.
| { | |
| { | |
| Throw.IfNull(source); |
| Assert.NotSame(tools, result.Tools); | ||
| Assert.Single(result.Tools!); |
There was a problem hiding this comment.
The updated assertions for Tools only check that the cloned collection is a different instance and has a single item, but they no longer verify that the cloned config preserved the original tool entry. Add an assertion that the tool element in result.Tools matches the corresponding element from tools (e.g., same reference or expected equality) to keep the test validating copy correctness, not just count.
This issue also appears on line 151 of the same file.
Motivation and Context
The GitHub.Copilot.SDK now exposes
Clone()onSessionConfigandResumeSessionConfig. This replaces the brittle manual property-by-property copy logic inCopySessionConfig, which must be updated every time the SDK adds a new config property.Description
Bumps
GitHub.Copilot.SDKfrom0.1.23→0.1.25and uses the SDK's built-inClone()to replace custom config copying inCopySessionConfig.Directory.Packages.props: Version bump from 0.1.23 to 0.1.25CopySessionConfig: Replaced 17-property manual copy withsource.Clone()+ setStreaming = true, making it forward-compatible with new SDK propertiesCopySessionConfigtest assertions fromAssert.Same→Assert.NotSameforToolsandMcpServerssinceClone()creates independent collection instancesContribution Checklist
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.