Harden JSON-RPC server mode protocol - #10779
Open
Amaury Levé (Evangelink) wants to merge 1 commit into
Open
Conversation
Add explicit protocol negotiation, enforce lifecycle and error semantics, make completion handling resilient, and align both JSON serializers with a machine-readable protocol schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens MTP JSON-RPC server mode through protocol negotiation, lifecycle enforcement, serializer parity, and updated documentation.
Changes:
- Adds independent protocol-version negotiation and client validation.
- Standardizes request errors, cancellation, and terminal update ordering.
- Adds protocol schema, documentation, and cross-serializer tests.
Show a summary per file
| File | Description |
|---|---|
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs |
Tests lifecycle, negotiation, cancellation, and completion ordering. |
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs |
Tests IDs, params, capabilities, and serialization. |
test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs |
Tests client negotiation and legacy compatibility. |
test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs |
Adds negotiated version to the fake server. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs |
Serializes negotiated protocol versions. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs |
Handles params, versions, and notifications. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs |
Normalizes numeric-string IDs. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs |
Extends initialization models and corrects coverage capability. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs |
Advertises the server-mode protocol version. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs |
Defines protocol versions and wire keys. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs |
Adds protocol-version serialization. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs |
Aligns IDs, params, and version deserialization. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs |
Treats JSON null as an absent optional value. |
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ErrorCodes.cs |
Adds unsupported-protocol error code. |
src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt |
Tracks new internal APIs. |
src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.RequestExecution.cs |
Negotiates versions and standardizes method errors. |
src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs |
Supports best-effort terminal notifications. |
src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs |
Enforces lifecycle and response ordering. |
src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs |
Stores initialization state. |
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs |
Serializes client-supported versions. |
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClientOptions.cs |
Exposes protocol-version configuration. |
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs |
Negotiates and validates the server version. |
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/IMtpServerClient.cs |
Exposes the negotiated version. |
docs/RFCs/019-Code-Coverage-Messages.md |
Clarifies coverage capability semantics. |
docs/mstest-runner-protocol/server-mode-1.0.schema.json |
Defines the Draft 2020-12 wire schema. |
docs/mstest-runner-protocol/001-protocol-intro.md |
Updates the protocol specification. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (2)
src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs:226
- A present but non-string
protocolVersionis silently converted tonull, so the client treats a malformed negotiation response as a compatible legacy server and bypasses the new version check. Only an absent property should activate legacy compatibility; reject the property when it is present with the wrong type.
string? protocolVersion = result.TryGetValue(JsonRpcStrings.ProtocolVersion, out object? protocolVersionObj)
? protocolVersionObj as string
: null;
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs:155
- Jsonite treats every non-string
protocolVersionvalue as if the property were absent, while the System.Text.Json path rejects the same malformed value. This preserves a serializer-dependent legacy fallback; explicitly reject non-null, non-string values.
string? protocolVersion = GetOptionalPropertyFromJson(properties, JsonRpcStrings.ProtocolVersion) as string;
- Files reviewed: 26/26 changed files
- Comments generated: 4
- Review effort level: Balanced
Comment on lines
+131
to
+132
| if (capabilities.ProtocolVersion is { } negotiatedProtocolVersion | ||
| && !_options.SupportedProtocolVersions.Contains(negotiatedProtocolVersion, StringComparer.Ordinal)) |
| MultiConnectionProvider: true))); | ||
| MultiConnectionProvider: true))) | ||
| { | ||
| ProtocolVersion = JsonRpcProtocolVersions.Current, |
| "pattern": "^(?:0|[1-9][0-9]{0,8}|1[0-9]{9}|20[0-9]{8}|21[0-3][0-9]{7}|214[0-6][0-9]{6}|2147[0-3][0-9]{5}|21474[0-7][0-9]{4}|214748[0-2][0-9]{3}|2147483[0-5][0-9]{2}|21474836[0-3][0-9]|214748364[0-7])$" | ||
| }, | ||
| { | ||
| "pattern": "^-(?:0|[1-9][0-9]{0,8}|1[0-9]{9}|20[0-9]{8}|21[0-3][0-9]{7}|214[0-6][0-9]{6}|2147[0-3][0-9]{5}|21474[0-7][0-9]{4}|214748[0-2][0-9]{3}|2147483[0-5][0-9]{2}|21474836[0-3][0-9]|214748364[0-8])$" |
|
|
||
| // Preserve server-to-client notification params when this formatter is used by a | ||
| // client or protocol test, matching the System.Text.Json path. | ||
| _ => rawParams is null ? null : paramsObj, |
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
Validation
Microsoft.Testing.Platformfornet8.0,net9.0, andnetstandard2.0net8.0andnet462for both serializer implementationsserver-mode-1.0.schema.jsonagainst the Draft 2020-12 metaschema