fix(nodejs): Map suppressResumeEvent to disableResume on the wire#1529
Merged
MRayermannMSFT merged 3 commits intoJun 1, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Renames the property suppressResumeEvent to disableResume when passing the config value through in the Copilot client.
Changes:
- Updated property name from
suppressResumeEventtodisableResumein the config mapping.
Show a summary per file
| File | Description |
|---|---|
| nodejs/src/client.ts | Renames the outgoing config field to disableResume while keeping the source field config.suppressResumeEvent. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
The Node.js SDK's resumeSession() sends suppressResumeEvent as-is on the JSON-RPC wire, but the server recognizes the field as disableResume. This aligns with the Rust SDK which already maps suppress_resume_event to disableResume at the serialization boundary. No public API changes - ResumeSessionConfig.suppressResumeEvent remains the user-facing field name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The .NET SDK's JsonSerializerDefaults.Web camelCases SuppressResumeEvent to suppressResumeEvent, but the server expects disableResume. Add an explicit JsonPropertyName attribute to produce the correct wire name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The createTcpServer() helper was missing gitHubToken, causing the suspend-resume test to fail when COPILOT_HMAC_KEY is unavailable (e.g. fork PRs, local development). This matches the pattern used in pending_work_resume and other multi-client E2E tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
willglas
force-pushed
the
willglas/fix-suppress-resume-event-wire-mapping
branch
from
dc3204e to
395decd
Compare
MRayermannMSFT
approved these changes
Jun 1, 2026
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.
Summary
The Node.js and .NET SDKs send the wrong wire field name when mapping
suppressResumeEventto the JSON-RPCsession.resumepayload. The server expectsdisableResume, but:suppressResumeEventas-is (silently dropped by the server)JsonSerializerDefaults.WebcamelCasing, which producessuppressResumeEventinstead ofdisableResumeThe Rust, Go, and Java SDKs already handle this correctly with explicit rename annotations.
Changes
nodejs/src/client.ts— mapconfig.suppressResumeEventtodisableResumein thesession.resumeRPC payloaddotnet/src/Client.cs— add[property: JsonPropertyName("disableResume")]toSuppressResumeEvent(same pattern asConfigDirectory→configDir)nodejs/test/e2e/suspend.e2e.test.ts— addgitHubTokento the suspend test server so the test works withoutCOPILOT_HMAC_KEY(matching pattern in other multi-client E2E tests)No public API changes — the user-facing field names remain the same in both SDKs.