Add netstandard and net10 targets to C# SDK#1320
Merged
Merged
Conversation
Multi-target the C# SDK for net8.0, net10.0, and netstandard2.0, with downlevel-only polyfills for APIs missing from netstandard2.0. Add Windows-only net472 test coverage so the netstandard2.0 asset is exercised, and keep net10.0 on the shared System.Text.Json surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds multi-targeting and downlevel support for the .NET SDK so it can ship net8.0, net10.0, and netstandard2.0 assets while preserving newer framework behavior where available.
Changes:
- Multi-targets the SDK and tests, adding Windows-only
net472test coverage for thenetstandard2.0asset. - Adds internal downlevel polyfills for missing BCL APIs/attributes.
- Updates JSON-RPC
ValueTask<T>unwrapping and downlevel regex/uninitialized-object handling.
Show a summary per file
| File | Description |
|---|---|
| dotnet/Directory.Build.props | Removes the shared default target framework. |
| dotnet/Directory.Packages.props | Adds package versions needed by downlevel targets. |
| dotnet/src/Client.cs | Adds downlevel regex fallback and RID import cleanup. |
| dotnet/src/GitHub.Copilot.SDK.csproj | Adds SDK multi-targeting and netstandard-specific dependencies/polyfills. |
| dotnet/src/JsonRpc.cs | Updates ValueTask<T> result extraction using declared return-type metadata. |
| dotnet/src/Types.cs | Adjusts nullability after string-token validation. |
| dotnet/src/Polyfills/ArrayBufferWriter.cs | Adds downlevel ArrayBufferWriter<T>. |
| dotnet/src/Polyfills/BclAttributes.cs | Adds compiler/BCL attribute polyfills. |
| dotnet/src/Polyfills/CodeAnalysisAttributes.cs | Adds code-analysis attribute polyfills. |
| dotnet/src/Polyfills/DataAnnotationsAttributes.cs | Adds Base64StringAttribute polyfill. |
| dotnet/src/Polyfills/DownlevelExtensions.cs | Adds extension-based polyfills for missing framework APIs. |
| dotnet/src/Polyfills/IsExternalInit.cs | Adds downlevel IsExternalInit and forwarding for newer TFMs. |
| dotnet/src/Polyfills/TaskCompletionSource.cs | Adds non-generic TaskCompletionSource polyfill. |
| dotnet/src/Polyfills/Utf8.cs | Adds downlevel Utf8.TryWrite polyfill. |
| dotnet/test/GitHub.Copilot.SDK.Test.csproj | Adds Windows-only net472 test target and conditional test settings. |
| dotnet/test/Unit/JsonRpcTests.cs | Adjusts in-memory stream test helper for downlevel stream APIs. |
| dotnet/test/Unit/SerializationTests.cs | Uses downlevel uninitialized-object API when needed. |
Copilot's findings
- Files reviewed: 17/17 changed files
- Comments generated: 5
Narrow socket connect exception handling and use a using declaration for cancellation registration disposal in the downlevel polyfills. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Add an explicit target framework for the sample project now that the shared default was removed, guard target-framework compatibility checks during outer builds, and fix formatting in the downlevel polyfill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Multi-targeted projects run an outer build with no TargetFramework. Skip the CLI download/copy targets there so clean CI builds do not require CopilotCliVersion before the generated props file is available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Fix downlevel Unix epoch and UTF-8 writing behavior, add best-effort non-Windows process tree cleanup, and avoid parallel version props generation across target frameworks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Ensure the generated Copilot CLI version property is populated before the CLI download target for every target framework, and fix remaining polyfill formatting caught by dotnet format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Ensure the Copilot CLI version is read before local SDK build targets need it, while keeping generated props creation limited to pack. Also simplifies the downlevel pgrep error handling to avoid the Linux formatting issue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Make the descendant process parsing filter explicit and replace empty best-effort cleanup catches with debug diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency Review ✅This PR modifies only the .NET SDK ( No cross-SDK consistency concerns. Changes are limited to:
No equivalent changes are needed in the Node.js, Python, or Go SDKs — this is .NET-ecosystem-specific infrastructure.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds downlevel and current-target coverage for the C# SDK so netstandard2.0 consumers can use the package while newer TFMs keep their modern framework-provided behavior.
Summary
net8.0,net10.0, andnetstandard2.0.dotnet/src/Polyfills, with extension members placed in the BCL namespaces they polyfill.System.Text.Jsonas a package dependency only for TFMs that are not compatible with thenet10.0surface area.Regexfallback instead of a fakeGeneratedRegexAttribute, and adds MCP-styleIsExternalInittype forwarding.ValueTask<T>unwrapping to use declared return-type metadata, avoiding trim/AOT suppressions.net472test coverage so thenetstandard2.0asset is exercised.Validation
dotnet build dotnet\src\GitHub.Copilot.SDK.csproj --no-restore --nologo --verbosity:minimal /tl:offdotnet test dotnet\test\GitHub.Copilot.SDK.Test.csproj --no-restore --nologo --verbosity:minimal /tl:off --filter "FullyQualifiedName!~E2E"