Replace StreamJsonRpc with a custom JSON-RPC implementation in the .NET SDK by stephentoub · Pull Request #1170 · github/copilot-sdk

stephentoub · 2026-04-29T22:26:10Z

Why

StreamJsonRpc was the only JSON-RPC client the .NET SDK used to talk to the Copilot CLI, but it dragged in 10 transitive runtime dependencies (Newtonsoft.Json, MessagePack, Nerdbank.MessagePack, Nerdbank.Streams, Microsoft.VisualStudio.Threading, etc.) that the SDK never exercised. It also routed every wire interaction through a serialization stack we did not control.

What

This PR drops StreamJsonRpc and replaces it with a focused internal JsonRpc class (~720 lines) that implements only what the SDK actually uses against the CLI:

LSP-style header-delimited framing (Content-Length: N\r\n\r\n + body) — the same wire format the Go and Python SDKs already implement against the same CLI.
JSON-RPC 2.0 requests, responses, notifications, error codes.
Reflection-based handler dispatch limited to the two shapes we actually register: singleObjectParam: true (whole params object deserialized as the first arg) and positional (JSON array). Anything else throws -32603.
AOT-friendly serialization via JsonTypeInfo everywhere; no UnconditionalSuppressMessage shims required.

The codegen script (scripts/codegen/csharp.ts) was updated to emit registration calls against the new JsonRpc class.

Notable points

Public API surface is unchanged for SDK consumers. StreamJsonRpc was already PrivateAssets="compile", so nothing leaks out either way.
Deployment footprint drops by ~3.24 MB / 10 assemblies in a published net8.0 app (10.14 MB -> 6.90 MB). Newtonsoft.Json, MessagePack(+Annotations), Nerdbank.MessagePack, Nerdbank.Streams, Microsoft.VisualStudio.Threading, Microsoft.VisualStudio.Validation, PolyType, and Microsoft.NET.StringTools all disappear from the dependency closure.

…ET SDK StreamJsonRpc was the only JSON-RPC client the .NET SDK used, but it dragged in 10 transitive runtime dependencies including Newtonsoft.Json, MessagePack, Nerdbank.Streams, and Microsoft.VisualStudio.Threading - none of which the SDK actually exercised. It also made every wire interaction go through a serialization stack we did not control, complicating AOT/trim work. This PR drops StreamJsonRpc and replaces it with a focused, internal JsonRpc class (~720 lines) that implements only what the SDK uses to talk to the Copilot CLI: LSP-style header-delimited framing (Content-Length: N\r\n\r\n + body), JSON-RPC 2.0 requests/responses/notifications, and reflection-based dispatch for the small set of methods the SDK registers. The wire format is the same one the Go and Python SDKs implement against the same CLI. Notable points: - Public API surface is unchanged for SDK consumers. - No StreamJsonRpc types leak through the public surface (they were already PrivateAssets=compile). - Codegen for the generated RPC handlers (scripts/codegen/csharp.ts) was updated to emit calls against the new JsonRpc class. - Cuts deployment footprint by ~3.24 MB / 10 assemblies in a published net8.0 app (10.14 MB -> 6.90 MB; Newtonsoft.Json, MessagePack, Nerdbank.MessagePack, Nerdbank.Streams, MessagePack.Annotations, Microsoft.VisualStudio.Threading, Microsoft.VisualStudio.Validation, PolyType, Microsoft.NET.StringTools all gone). - All existing unit tests pass; the read loop and framing parser were reviewed for correctness across header/body edge cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SerializeArgs was calling GetTypeInfo(typeof(object?[])), which is not provided by any of the source-generated JsonSerializerContexts and therefore threw NotSupportedException at runtime on macOS/Ubuntu (Windows happened to skip these code paths in earlier failures). Build the params JSON array manually, looking up TypeInfo by each argument's runtime type, which is what the merged source-gen resolver actually contains. Also insert the missing space before ':' in the PendingRequest primary-constructor base list to satisfy 'dotnet format --verify-no-changes'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR replaces the .NET SDK’s StreamJsonRpc dependency with a new internal, purpose-built JSON-RPC implementation that speaks the Copilot CLI’s LSP-style Content-Length framing, and updates the C# codegen + generated RPC layer accordingly.

Changes:

Introduces a new internal JsonRpc implementation and wires CopilotClient to use it instead of StreamJsonRpc.
Updates C# code generation and the generated RPC file to register handlers via SetLocalRpcMethod(...) and use ValueTask for incoming handlers.
Removes StreamJsonRpc package references and related tests/ignores; adjusts serialization tests and .gitignore placement for .vs/.

Show a summary per file

File	Description
scripts/codegen/csharp.ts	Updates codegen to emit `SetLocalRpcMethod(...)`, `ValueTask` handler delegates, and makes registration type internal.
dotnet/src/JsonRpc.cs	Adds the new custom JSON-RPC transport (framing, dispatch, invoke, cancellation, errors).
dotnet/src/Client.cs	Switches connection setup and handler registration from `StreamJsonRpc` to the new `JsonRpc`.
dotnet/src/Generated/Rpc.cs	Updates generated registration to `SetLocalRpcMethod(...)`, removes `StreamJsonRpc` import, and changes registration visibility.
dotnet/src/GitHub.Copilot.SDK.csproj	Removes `StreamJsonRpc` package reference from the SDK.
dotnet/src/Session.cs	Removes unused `StreamJsonRpc` import after transport swap.
dotnet/test/SerializationTests.cs	Drops StreamJsonRpc-specific `RequestId` round-trip tests and reframes scope to SDK serializer options.
dotnet/test/GitHub.Copilot.SDK.Test.csproj	Removes `StreamJsonRpc` test dependency.
dotnet/Directory.Packages.props	Removes `StreamJsonRpc` package version pin.
dotnet/.gitignore	Stops ignoring `.vs/` locally (moved to repo root).
.gitignore	Adds `.vs/` ignore at repo root.

Copilot's findings

Files reviewed: 9/11 changed files
Comments generated: 6

The Copilot CLI uses vscode-jsonrpc-style request handlers which expect `params` to be the request object directly. The other SDKs (Node/Python/Go) all send single-object params, but the .NET InvokeAsync was wrapping the single arg in a positional JSON array, so the CLI couldn't deserialize it and never responded — every round-trip RPC test timed out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

vscode-jsonrpc (used by the Copilot CLI) rejects responses that lack both `result` and `error` with 'The received response has neither a result nor an error property'. Our `JsonRpcResponse` inherited the context-level `DefaultIgnoreCondition = WhenWritingNull`, so void/ nullable-returning handlers (e.g. session.plan.update, every sessionFs handler that returns SessionFsError?) emitted `{jsonrpc, id}` with no `result` field, hanging the CLI and timing out tests. Override the policy on the Result property with [JsonInclude] + [JsonIgnore(Condition = Never)] so we always serialize `result: null`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

If AssistantMessageEvent and SessionIdleEvent both arrived between SendAsync returning and the subscription being installed, AND GetMessagesAsync returned only the assistant message before SessionIdleEvent had arrived, the helper would hang. The subscription would later receive SessionIdleEvent but skip it because the local finalAssistantMessage variable was still null (the AssistantMessageEvent had been delivered before subscription). Now CheckExistingMessages backfills finalAssistantMessage from already-delivered events under a lock, so the SessionIdleEvent handler can complete the wait. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SteveSandersonMS

Awesome! Looks great.

[JsonInclude] is for opting in non-public members; the property is already public with a public getter/setter. The override of the context-level WhenWritingNull policy is purely from [JsonIgnore(Condition = Never)]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Previous fix backfilled the assistant message but still missed cases where SessionIdleEvent arrived via the subscription before backfill completed (subscription saw finalAssistantMessage==null and skipped) and the GetMessagesAsync snapshot was taken before SessionIdleEvent arrived (existingIdle==false). Both paths now feed a shared (finalAssistantMessage, sawIdle) state and call a single TryComplete that fires when both have been observed regardless of which path saw which. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Single-test 60s timeouts have been intermittently failing CI on different tests across runs (Should_Create_Session_With_Custom_Config_Dir on macOS, Should_Create_A_Session_With_Appended_SystemMessage_Config on Windows). The race in GetFinalAssistantMessageAsync is fixed; remaining failures appear to be CI runner slowness on snapshot replay. 120s gives loaded runners more room without affecting healthy runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SendMessageAsync was passing the user's cancellation token to all three write/flush calls. If the token fired between writing the header and the body (or mid-body), the peer was left waiting for N body bytes that never arrived, desynchronizing the LSP-style stream for every subsequent message on the connection. Because our E2E test fixture shares a single CopilotClient (and underlying CLI process) across all tests in a class, one cancelled write could corrupt the wire and cause every subsequent test to hang in GetFinalAssistantMessageAsync. Cancellation now only applies to *waiting* for the write lock. Once we hold the lock and start writing a framed message we commit the whole thing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

+        catch (Exception ex)
+        {
+            _logger.LogDebug(ex, "JSON-RPC read loop ended");
+        }


+        catch (Exception ex)
+        {
+            // Belt-and-braces: this method is fire-and-forget from the read loop, so any
+            // exception escaping here would become an unobserved task exception. The most
+            // likely sources are IOException/ObjectDisposedException from sending the error
+            // response after the underlying transport is gone.
+            if (_logger.IsEnabled(LogLevel.Debug))
+            {
+                _logger.LogDebug(ex, "Unobserved error in JSON-RPC method dispatch for {Method}", methodName);
+            }
+        }


github-actions · 2026-04-30T12:46:46Z

Cross-SDK Consistency Review ✅

This PR modifies only the .NET SDK internals (and its codegen script). The public API surface is unchanged, so SDK consumers see no difference.

Cross-language alignment note: This change actually improves consistency at the wire-protocol level. The new custom JsonRpc class uses LSP-style Content-Length header-delimited framing — the same approach already used by the Go and Python SDKs when talking to the CLI. All four SDKs now share the same wire format.

No cross-SDK consistency issues found.

Generated by SDK Consistency Review Agent for issue #1170 · ● 131.2K · ◷

Copilot AI review requested due to automatic review settings April 29, 2026 22:26

stephentoub requested a review from a team as a code owner April 29, 2026 22:26

Copilot started reviewing on behalf of stephentoub April 29, 2026 22:26 View session

github-code-quality Bot found potential problems Apr 29, 2026

View reviewed changes

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs

Comment thread dotnet/src/JsonRpc.cs Fixed

This comment has been minimized.

Sign in to view

stephentoub and others added 2 commits April 29, 2026 18:31

Combine nested Content-Length validation if-statements

38d7b35

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread scripts/codegen/csharp.ts

Comment thread dotnet/src/JsonRpc.cs

Comment thread dotnet/src/JsonRpc.cs

Comment thread dotnet/src/JsonRpc.cs

Comment thread dotnet/src/Generated/Rpc.cs

Comment thread dotnet/src/Generated/Rpc.cs

github-code-quality Bot found potential problems Apr 29, 2026

View reviewed changes

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs Fixed

github-actions Bot mentioned this pull request Apr 29, 2026

[aw] No-Op Runs #1038

Open

github-code-quality Bot found potential problems Apr 30, 2026

View reviewed changes

Comment thread dotnet/src/JsonRpc.cs Fixed

Comment thread dotnet/src/JsonRpc.cs Fixed

Filter SendCancelNotificationAsync catch to expected lifecycle except…

a23bc39

…ions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This comment has been minimized.

Sign in to view

Retrigger CI after Windows-only PermissionTests timeout flake

6f77185

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This comment has been minimized.

Sign in to view

SteveSandersonMS approved these changes Apr 30, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

stephentoub and others added 2 commits April 30, 2026 08:19

github-code-quality Bot found potential problems Apr 30, 2026

View reviewed changes

stephentoub merged commit 155e887 into main Apr 30, 2026
27 checks passed

stephentoub deleted the stephentoub/custom-jsonrpc-dotnet branch April 30, 2026 13:09

Conversation

stephentoub commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Notable points

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SteveSandersonMS left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026

Cross-SDK Consistency Review ✅

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stephentoub commented Apr 29, 2026 •

edited

Loading