Commit 155e887
Replace StreamJsonRpc with a custom JSON-RPC implementation in the .NET SDK (#1170)
* Replace StreamJsonRpc with a custom JSON-RPC implementation in the .NET 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>
* Fix JsonRpc serialization of args and formatting
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>
* Combine nested Content-Length validation if-statements
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Send single-arg RPC params as object, not positional array
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>
* Filter SendCancelNotificationAsync catch to expected lifecycle exceptions
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Always emit `result` on JSON-RPC responses, even when null
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>
* Retrigger CI after Windows-only PermissionTests timeout flake
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix race in GetFinalAssistantMessageAsync that could miss early events
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>
* Remove redundant [JsonInclude] from JsonRpcResponse.Result
[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>
* Fix remaining race in GetFinalAssistantMessageAsync helper
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>
* Bump TestHelper default timeout from 60s to 120s
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>
* Don't honor user cancellation between header and body in JSON-RPC writes
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>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>1 parent fd16c3d commit 155e887
12 files changed
Lines changed: 955 additions & 324 deletions
File tree
- dotnet
- src
- Generated
- test
- Harness
- scripts/codegen
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
20 | 19 | | |
21 | 20 | | |
22 | 21 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
14 | 13 | | |
15 | 14 | | |
16 | 15 | | |
| |||
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
40 | | - | |
41 | 40 | | |
42 | 41 | | |
43 | 42 | | |
| |||
0 commit comments