From 749df2d553b94ad637878211cd43215ece7c7dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 26 Aug 2026 18:25:11 +0200 Subject: [PATCH 01/25] Harden JSON-RPC server mode protocol 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 --- docs/RFCs/019-Code-Coverage-Messages.md | 5 + .../001-protocol-intro.md | 162 ++-- .../server-mode-1.0.schema.json | 703 ++++++++++++++++++ .../Client/IMtpServerClient.cs | 7 +- .../Client/MtpServerClient.cs | 19 +- .../Client/MtpServerClientOptions.cs | 16 +- .../SerializerUtilities.ClientSerializers.cs | 18 +- .../Hosts/ServerTestHost.MessageLoop.cs | 137 +++- .../Hosts/ServerTestHost.Messaging.cs | 12 +- .../Hosts/ServerTestHost.RequestExecution.cs | 46 +- .../Hosts/ServerTestHost.cs | 10 +- .../InternalAPI/InternalAPI.Unshipped.txt | 12 + .../ServerMode/JsonRpc/ErrorCodes.cs | 1 + .../JsonRpc/Json/Json.Deserializers.cs | 80 +- .../JsonRpc/Json/Json.Serializers.cs | 19 +- .../ServerMode/JsonRpc/Json/Json.cs | 6 +- .../ServerMode/JsonRpc/JsonRpcMethods.cs | 30 + .../ServerMode/JsonRpc/PassiveNode.cs | 5 +- .../ServerMode/JsonRpc/RpcMessages.cs | 14 +- .../SerializerUtilities.Deserializers.cs | 93 ++- ...rializerUtilities.RpcMessageSerializers.cs | 5 + .../ServerMode/JsonRpc/SerializerUtilities.cs | 3 +- .../FakeMtpServer.cs | 5 +- .../MtpServerClientTests.cs | 30 + .../ServerMode/FormatterUtilitiesTests.cs | 152 +++- .../ServerMode/ServerTests.cs | 265 ++++++- 26 files changed, 1663 insertions(+), 192 deletions(-) create mode 100644 docs/mstest-runner-protocol/server-mode-1.0.schema.json diff --git a/docs/RFCs/019-Code-Coverage-Messages.md b/docs/RFCs/019-Code-Coverage-Messages.md index 618b7a9a01..dd7cb460ae 100644 --- a/docs/RFCs/019-Code-Coverage-Messages.md +++ b/docs/RFCs/019-Code-Coverage-Messages.md @@ -737,6 +737,11 @@ public sealed class CoverageReportReference } ``` +`ITestCoverageCapabilities.SupportsTestCoverageMessages` describes the platform's in-process message +and consumer contract. It is distinct from the JSON-RPC server-mode capability with the same suffix: +`capabilities.testing.supportsTestCoverageMessages` remains `false` until those first-class messages +are forwarded over the server-mode wire. + None of these public properties use `init`, per platform guidelines. ## How consumers use it — two tiers diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 13a2db3c8f..63a32b8ae8 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -12,6 +12,9 @@ This document describes the JSON-RPC server-mode protocol. The separate, binary [`dotnet test` named-pipe protocol](./004-protocol-dotnet-test-pipe.md) (`--server dotnettestcli`) is specified in its own document. +The machine-readable JSON Schema for the base protocol is +[`server-mode-1.0.schema.json`](./server-mode-1.0.schema.json). + ## API overview Here's the current list of APIs that supported by the client. @@ -24,8 +27,6 @@ Here's the current list of APIs that supported by the client. - [testing/testUpdates/tests](#discovery-of-tests) - Notifies client about test updates (test cases and test results) - [testing/testUpdates/attachments](#execution-of-tests) - Notifies client about additional attachments (trx/coverage) - Client notifications updates - - [client/launchDebugger](#launch-debugger) - Requests a client to launch a process with debugger attached to it - - [client/attachDebugger](#attach-debugger) - Requests a client to attach a debugger to a process by pid - [client/log](#logging-of-messages) - Notifies a client to logs a message to the output window - Miscellaneous requests - [telemetry/update](#telemetry) - Sends telemetry data to the client @@ -148,6 +149,9 @@ namespace ErrorCodes { export const testingPlatformErrorRangeStart = -31700; + // The initialize request did not contain a protocol version supported by the server. + export const ProtocolVersionNotSupported: integer = -31699; + // If a top level assertion has failed when running tests. // TODO: Decide if we can forward all assertion failures and attach them to test nodes. export const AssertionFailed: integer = -31001; @@ -181,6 +185,11 @@ what the client supports and limit functionality based on unsupported features. > Since the capabilities are fetched by sending an RPC request to a started executable, these can only be queried by the client after > the project was successfully built. +The `initialize` request MUST be the first request sent on a connection. Before initialization completes, +the server rejects requests with `ServerNotInitialized` (`-32002`) and ignores notifications other than +`exit` and `$/cancelRequest`. A second `initialize` request on an initialized connection is rejected with `InvalidRequest` +(`-32600`). If initialization fails, the client may correct the request and try again. + ### Determine capabilities during test runner initialization > [!NOTE] @@ -204,47 +213,33 @@ interface InitializeParams { // The name of the client. name: string, - // This is the version of the client's protocol and should follow - // semver. - // The client and the server should be able to communicate - // only if the major version stays the same. - // The client can attempt to fallback on the older protocol - // version if it has such a fallback and send another INITIALIZE - // request. + // Client compatibility version. Existing server integrations use this + // to gate client-specific behavior, so it remains separate from the + // independently negotiated wire-protocol version below. version: string, }, + // OPTIONAL for compatibility with clients predating protocol negotiation. + // The server selects its most preferred mutually supported version. + // If omitted or empty, the server uses the legacy base protocol (1.0.0). + protocolVersions?: string[], + capabilities: { // Note: Since the initialize message is compatible with the LSP protocol, // we should make sure that the capability paths for the testing features are unique. // As such, we put all of them under a single testing namespace. // This reduces collisions with other LSP capabilities. testing: { - // If true, the client supports the client/attachDebugger and client/launchDebugger requests. + // Reserved for future debugger callbacks. Protocol 1.0 accepts this field + // for compatibility but does not send debugger requests. debuggerProvider: true, - // If true, the client can receive a batch of log messages under client/log request. - batchLoggingSupport: true, - - // If true, the client supports the testing/testUpdates/attachments request. - attachmentsSupport: true, - // If true, the client is stateful: it persists an addressable set of test nodes for the // whole session and keeps each node in its last-known state until it is explicitly updated // (for example, an IDE test explorer). If false or missing, the client is stateless: it // consumes test updates as a stream and does not retain node state after the run // (for example, `dotnet test`). Defaults to false. isStateful: true, - - // If true, the client support a port to which child processes - // can connect to. - // Note: The test runner is expected to ensure the synchronization of messages - // for instance if additional processes are sending test updates - // or attachment updates, these must complete before the - // test runner sends the completion notification. - callbackProvider: { - port: integer - } }, } } @@ -256,15 +251,25 @@ Response: ```typescript interface InitializeResponse { + // Process ID of the server process, when one exists. + processId?: PID, + serverInfo: { // The name of the server. name: string, - // The server's protocol version. + // Product/build version of the server. This is not the wire-protocol version. version: string }, + + // Independently negotiated wire-protocol version. + protocolVersion?: string, + capabilities: { testing: { + // If true, the server accepts testing/discoverTests. + supportsDiscovery: boolean; + // Experimental: The client currently uses this variable to determine if the test runner process can // handle multiple discover/run requests. If true, then the client can keep the process alive. // This has a potential performance benefit, where startup time and time to load test assemblies/sources @@ -275,12 +280,21 @@ interface InitializeResponse { // of test updates during test runs. // The client will then wait on both to complete, // before it marks a test run as completed. - attachmentsProvider?: boolean; + attachmentsSupport: boolean; + + // If true, test nodes include VSTest compatibility properties described + // by 003-protocol-ide-integration-extensions.md. + vstestProvider: boolean; + + // If true, additional passive test-host processes can connect to the client. + multipleConnectionProvider: boolean; // If true, the server understands the first-class test-coverage message contract. // This advertises protocol support only; it does not imply that an enabled extension // will produce coverage during the run. - supportsTestCoverageMessages?: boolean; + // This MUST only be true when first-class coverage messages are actually + // forwarded over this JSON-RPC connection. + supportsTestCoverageMessages: boolean; }, } } @@ -291,6 +305,11 @@ interface InitializeResponse { Any behavior changes should be announced via capabilities, where the client/server should announce what kinds of additional RPC requests and responses they can handle. +The independently negotiated `protocolVersion` versions the base wire contract. It is intentionally +separate from `clientInfo.version` and `serverInfo.version`, which identify the client compatibility +surface and server product build. Protocol `1.0.0` is the legacy base. An unsupported set is rejected +with `ProtocolVersionNotSupported` (`-31699`). + For instance, let's say the server would like to be make the file/line location lazy. The client should announce that is supports lazy locations. If the capability is present the server can then send lazy location data. @@ -302,14 +321,13 @@ lazy locations and send the full location. > should be supported by all clients/servers. > As such, they're not expressed via capabilities. -## Callback provider +## Additional passive connections -In some cases the test runner might want to start additional child processes. If client has the `testing.callbackProvider` capability, the client -will provide a port for multiple connections. - -This allows for instance for the test runner to start multiple child processes that it distributes test run over, while each of these child processes can directly send callbacks to the client with test node updates, rather than have to relay all information via the main test runner node. - -Another use case is the collection of hang dumps, crash dumps, in which case the test runner node might crash, while the hang dump watcher process can still send back the hang dump/crash dump attachment, even after the crash. +A server advertising `multipleConnectionProvider` can use additional passive connections to push +artifacts from child or watcher processes. The endpoint is configured by the client out of band when +the process is launched; protocol 1.0 does not define a `testing.callbackProvider` initialize capability. +This is used, for example, to preserve hang-dump or crash-dump attachments if the primary test-host +process terminates. ## Discovery and run requests @@ -480,7 +498,7 @@ type ExecutionState = | 'failed' | 'timed-out' | 'error' - | "cancelled" + | "canceled" interface Trait { key: string; @@ -548,7 +566,10 @@ Notifications: processing all node update notifications. - Implementation detail: if a server sends callback notifications and returns a JSON-RPC response, vs-streamjsonrpc may resolve them in any order. As such, awaiting of the response is insufficient to guarantee that all updates were processed by the callback handler. - method: `testing/testUpdates/tests` - - params: `TestUpdateNotificationParams` where `params.children == null`. + - params: `TestUpdateNotificationParams` where `params.changes == null`. + - The server sends this terminal notification before the final success or error response for requests + that reached execution. Requests rejected by lifecycle checks and requests whose params could not be + deserialized receive only the error response. Response: @@ -596,7 +617,7 @@ Notifications: As soon as the client processes this notification it is guaranteed to be done processing all node update notifications. - method: `testing/testUpdates/tests` - - params: `TestUpdateNotificationParams` where `params.testCases == null`. + - params: `TestUpdateNotificationParams` where `params.changes == null`. - Attachment updates - method: `testing/testUpdates/attachments` - params: `AttachmentUpdatesParams` defined as follows: @@ -604,14 +625,9 @@ Notifications: ```typescript interface AttachmentUpdatesParams { attachments?: Attachment[], - runId: GUID } ``` -- Attachment updates completes - method: `testing/testUpdates/attachments` - params: `AttachmentUpdatesParams` where `params.attachments == null` - Response: - result: `RunTestsResponse` defined as follows: @@ -646,9 +662,9 @@ interface Attachment { // How the attachment can be displayed as by the client. // Example: "display-name": "Code Coverage" - displayName: string; + 'display-name': string; - description: string; + description?: string; } ``` @@ -720,6 +736,9 @@ interface CancelParams { > Message direction: Server -> Client +> [!NOTE] +> Reserved protocol shape. Microsoft.Testing.Platform 1.0 does not currently send this request. + Requests a client to attach a debugger to the spawned child process. Request: @@ -751,6 +770,9 @@ interface LaunchDebuggerParams { > Message direction: Server -> Client +> [!NOTE] +> Reserved protocol shape. Microsoft.Testing.Platform 1.0 does not currently send this request. + Requests a client to attach a debugger to the spawned child process. Request: @@ -787,50 +809,20 @@ Messages are logged to the output window. Notification: - method: `client/log` -- params: `LogMessageParams | BatchLogMessageParams` defined as follows: -- capability: If `testing.batchLoggingSupport` is true, the server can send BatchLogMessageParams instead. - The client will check the existence of `messages` property to determine if a batch was sent instead of a single - message. If `testing.batchLoggingSupport` is false, the server cannot send `BatchLogMessageParams` messages to the client. +- params: `LogMessageParams` defined as follows: ```typescript interface LogMessageParams { level: TestingPlatformLogLevel; message: string; } -``` - -```typescript -interface BatchLogMessageParams { - // If specified the message batch should be attributed to a specific run. - // Specifically, combined with the nodeUid, property the client should attribute - // the messages to a specific TestNode, rather than render them globally. - runId?: GUID; - - // List of messages to log. - messages: LogMessage[]; -} - -interface LogMessage { - // If a log message should be attributed to a single node, rather than be global. - nodeUid?: GUID; - - // The level of a single log message. - // Messages can have different log levels within a batch. - level: TestingPlatformLogLevel; - - message: string; -} -``` -```typescript -enum TestingPlatformLogLevel -{ - Trace = 0, - Debug = 1, - Information = 2, - Warning = 3, - Error = 4, - Critical = 5, - None = 6, -} +type TestingPlatformLogLevel = + 'Trace' + | 'Debug' + | 'Information' + | 'Warning' + | 'Error' + | 'Critical' + | 'None'; ``` diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json new file mode 100644 index 0000000000..c251bd642b --- /dev/null +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -0,0 +1,703 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://raw.githubusercontent.com/microsoft/testfx/main/docs/mstest-runner-protocol/server-mode-1.0.schema.json", + "title": "Microsoft.Testing.Platform JSON-RPC server-mode protocol 1.0", + "description": "Machine-readable envelope and payload contract for the MTP JSON-RPC server-mode protocol.", + "oneOf": [ + { + "$ref": "#/$defs/initializeRequest" + }, + { + "$ref": "#/$defs/discoverRequest" + }, + { + "$ref": "#/$defs/runRequest" + }, + { + "$ref": "#/$defs/cancelNotification" + }, + { + "$ref": "#/$defs/exitNotification" + }, + { + "$ref": "#/$defs/testUpdateNotification" + }, + { + "$ref": "#/$defs/attachmentNotification" + }, + { + "$ref": "#/$defs/logNotification" + }, + { + "$ref": "#/$defs/telemetryNotification" + }, + { + "$ref": "#/$defs/successResponse" + }, + { + "$ref": "#/$defs/errorResponse" + } + ], + "$defs": { + "rpcId": { + "description": "Protocol 1.0 normalizes signed 32-bit integer IDs and their canonical decimal string representation.", + "oneOf": [ + { + "type": "integer", + "minimum": -2147483648, + "maximum": 2147483647 + }, + { + "type": "string", + "oneOf": [ + { + "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])$" + } + ] + } + ] + }, + "requestEnvelope": { + "type": "object", + "required": [ + "jsonrpc", + "id", + "method" + ], + "properties": { + "jsonrpc": { + "const": "2.0" + }, + "id": { + "$ref": "#/$defs/rpcId" + }, + "method": { + "type": "string" + }, + "params": { + "type": [ + "object", + "null" + ] + } + }, + "additionalProperties": true + }, + "notificationEnvelope": { + "type": "object", + "required": [ + "jsonrpc", + "method" + ], + "properties": { + "jsonrpc": { + "const": "2.0" + }, + "method": { + "type": "string" + }, + "params": { + "type": [ + "object", + "null" + ] + } + }, + "additionalProperties": true + }, + "clientCapabilities": { + "type": "object", + "required": [ + "testing" + ], + "properties": { + "testing": { + "type": "object", + "required": [ + "debuggerProvider" + ], + "properties": { + "debuggerProvider": { + "type": "boolean" + }, + "isStateful": { + "type": "boolean", + "default": false + } + }, + "additionalProperties": true + } + }, + "additionalProperties": true + }, + "serverCapabilities": { + "type": "object", + "required": [ + "testing" + ], + "properties": { + "testing": { + "type": "object", + "required": [ + "supportsDiscovery", + "experimental_multiRequestSupport", + "vstestProvider", + "attachmentsSupport", + "multipleConnectionProvider", + "supportsTestCoverageMessages" + ], + "properties": { + "supportsDiscovery": { + "type": "boolean" + }, + "experimental_multiRequestSupport": { + "type": "boolean" + }, + "vstestProvider": { + "type": "boolean" + }, + "attachmentsSupport": { + "type": "boolean" + }, + "multipleConnectionProvider": { + "type": "boolean" + }, + "supportsTestCoverageMessages": { + "type": "boolean" + } + }, + "additionalProperties": true + } + }, + "additionalProperties": true + }, + "initializeRequest": { + "allOf": [ + { + "$ref": "#/$defs/requestEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "initialize" + }, + "params": { + "type": "object", + "required": [ + "processId", + "clientInfo", + "capabilities" + ], + "properties": { + "processId": { + "type": "integer" + }, + "clientInfo": { + "type": "object", + "required": [ + "name", + "version" + ], + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "additionalProperties": true + }, + "capabilities": { + "$ref": "#/$defs/clientCapabilities" + }, + "protocolVersions": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + }, + "uniqueItems": true + } + }, + "additionalProperties": true + } + } + } + ] + }, + "testSelectionParams": { + "type": "object", + "required": [ + "runId" + ], + "properties": { + "runId": { + "type": "string", + "format": "uuid" + }, + "tests": { + "type": [ + "array", + "null" + ], + "items": { + "$ref": "#/$defs/testNode" + } + }, + "filter": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": true + }, + "discoverRequest": { + "allOf": [ + { + "$ref": "#/$defs/requestEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "testing/discoverTests" + }, + "params": { + "$ref": "#/$defs/testSelectionParams" + } + } + } + ] + }, + "runRequest": { + "allOf": [ + { + "$ref": "#/$defs/requestEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "testing/runTests" + }, + "params": { + "$ref": "#/$defs/testSelectionParams" + } + } + } + ] + }, + "cancelNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "$/cancelRequest" + }, + "params": { + "type": "object", + "required": [ + "id" + ], + "properties": { + "id": { + "$ref": "#/$defs/rpcId" + } + }, + "additionalProperties": true + } + } + } + ] + }, + "exitNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "properties": { + "method": { + "const": "exit" + }, + "params": { + "type": [ + "object", + "null" + ], + "maxProperties": 0 + } + } + } + ] + }, + "testNode": { + "type": "object", + "required": [ + "uid", + "display-name" + ], + "properties": { + "uid": { + "type": "string", + "minLength": 1 + }, + "display-name": { + "type": "string" + }, + "node-type": { + "enum": [ + "action", + "group" + ] + }, + "execution-state": { + "enum": [ + "discovered", + "in-progress", + "passed", + "skipped", + "failed", + "timed-out", + "error", + "canceled" + ] + }, + "location.file": { + "type": "string" + }, + "location.line-start": { + "type": "integer", + "minimum": 0 + }, + "location.line-end": { + "type": "integer", + "minimum": 0 + }, + "time.duration-ms": { + "type": "number", + "minimum": 0 + }, + "retry.attempt": { + "type": "integer", + "minimum": 1 + }, + "retry.is-superseded": { + "type": "boolean" + } + }, + "additionalProperties": true + }, + "testUpdateNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "testing/testUpdates/tests" + }, + "params": { + "type": "object", + "required": [ + "runId", + "changes" + ], + "properties": { + "runId": { + "type": "string", + "format": "uuid" + }, + "changes": { + "oneOf": [ + { + "type": "null" + }, + { + "type": "array", + "items": { + "type": "object", + "required": [ + "node" + ], + "properties": { + "parent": { + "type": [ + "string", + "null" + ] + }, + "node": { + "$ref": "#/$defs/testNode" + } + }, + "additionalProperties": true + } + } + ] + } + }, + "additionalProperties": true + } + } + } + ] + }, + "artifact": { + "type": "object", + "required": [ + "uri", + "producer", + "type", + "display-name" + ], + "properties": { + "uri": { + "type": [ + "string", + "null" + ] + }, + "producer": { + "type": [ + "string", + "null" + ] + }, + "type": { + "type": [ + "string", + "null" + ] + }, + "display-name": { + "type": [ + "string", + "null" + ] + }, + "description": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": true + }, + "attachmentNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "testing/testUpdates/attachments" + }, + "params": { + "type": "object", + "required": [ + "attachments" + ], + "properties": { + "attachments": { + "type": "array", + "items": { + "$ref": "#/$defs/artifact" + } + } + }, + "additionalProperties": true + } + } + } + ] + }, + "logNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "client/log" + }, + "params": { + "type": "object", + "required": [ + "level", + "message" + ], + "properties": { + "level": { + "type": "string" + }, + "message": { + "type": "string" + } + }, + "additionalProperties": true + } + } + } + ] + }, + "telemetryNotification": { + "allOf": [ + { + "$ref": "#/$defs/notificationEnvelope" + }, + { + "required": [ + "params" + ], + "properties": { + "method": { + "const": "telemetry/update" + }, + "params": { + "type": "object", + "required": [ + "eventName", + "metrics" + ], + "properties": { + "eventName": { + "type": "string" + }, + "metrics": { + "type": "object" + } + }, + "additionalProperties": true + } + } + } + ] + }, + "successResponse": { + "type": "object", + "required": [ + "jsonrpc", + "id", + "result" + ], + "properties": { + "jsonrpc": { + "const": "2.0" + }, + "id": { + "$ref": "#/$defs/rpcId" + }, + "result": { + "type": [ + "object", + "null" + ], + "properties": { + "processId": { + "type": "integer" + }, + "serverInfo": { + "type": "object", + "required": [ + "name", + "version" + ], + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "additionalProperties": true + }, + "protocolVersion": { + "type": "string" + }, + "capabilities": { + "$ref": "#/$defs/serverCapabilities" + }, + "attachments": { + "type": "array", + "items": { + "$ref": "#/$defs/artifact" + } + } + }, + "additionalProperties": true + } + }, + "additionalProperties": true + }, + "errorResponse": { + "type": "object", + "required": [ + "jsonrpc", + "id", + "error" + ], + "properties": { + "jsonrpc": { + "const": "2.0" + }, + "id": { + "$ref": "#/$defs/rpcId" + }, + "error": { + "type": "object", + "required": [ + "code", + "message" + ], + "properties": { + "code": { + "type": "integer" + }, + "message": { + "type": "string" + }, + "data": true + }, + "additionalProperties": true + } + }, + "additionalProperties": true + } + } +} diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/IMtpServerClient.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/IMtpServerClient.cs index f5584ed07e..9ff871ea6d 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/IMtpServerClient.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/IMtpServerClient.cs @@ -159,7 +159,8 @@ public MtpServerCapabilities( bool multiRequestSupport, bool vstestProviderSupport, bool supportsAttachments, - bool multiConnectionProvider) + bool multiConnectionProvider, + string? protocolVersion = null) { ServerProcessId = serverProcessId; ServerName = serverName; @@ -169,6 +170,7 @@ public MtpServerCapabilities( VSTestProviderSupport = vstestProviderSupport; SupportsAttachments = supportsAttachments; MultiConnectionProvider = multiConnectionProvider; + ProtocolVersion = protocolVersion; } /// Gets the process id reported by the server. @@ -180,6 +182,9 @@ public MtpServerCapabilities( /// Gets the server version. public string? ServerVersion { get; } + /// Gets the independently negotiated server-mode protocol version. + public string? ProtocolVersion { get; } + /// Gets a value indicating whether the server supports discovery. public bool SupportsDiscovery { get; } diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs index bab20eb106..f47e53a70a 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs @@ -121,10 +121,21 @@ public async Task InitializeAsync(CancellationToken cance var args = new InitializeRequestArgs( GetCurrentProcessId(), new ClientInfo(_options.ClientName, _options.ClientVersion), - new ClientCapabilities(_options.DebuggerProvider, _options.IsStateful)); + new ClientCapabilities(_options.DebuggerProvider, _options.IsStateful)) + { + ProtocolVersions = _options.SupportedProtocolVersions.ToArray(), + }; ResponseMessage response = await _connection.SendRequestAsync(JsonRpcMethods.Initialize, args, cancellationToken).ConfigureAwait(false); MtpServerCapabilities capabilities = DecodeCapabilities(AsResultDictionary(response.Result)); + if (capabilities.ProtocolVersion is { } negotiatedProtocolVersion + && !_options.SupportedProtocolVersions.Contains(negotiatedProtocolVersion, StringComparer.Ordinal)) + { + throw new MtpServerClientException( + $"The server negotiated unsupported protocol version '{negotiatedProtocolVersion}'. " + + $"Supported versions: {string.Join(", ", _options.SupportedProtocolVersions)}."); + } + Capabilities = capabilities; return capabilities; } @@ -210,6 +221,9 @@ private static MtpServerCapabilities DecodeCapabilities(IDictionary capabilities && capabilities.TryGetValue(JsonRpcStrings.Testing, out object? testingObj) @@ -230,7 +244,8 @@ private static MtpServerCapabilities DecodeCapabilities(IDictionary - /// Gets or sets the client protocol/tool version reported to the server (clientInfo.version). + /// Gets or sets the client compatibility version reported to the server (clientInfo.version). + /// This is separate from . /// public string ClientVersion { get; set; } = "1.0.0"; /// - /// Gets or sets a value indicating whether the client advertises that it can provide a debugger - /// (capabilities.testing.debuggerProvider). When the server may send - /// client/attachDebugger / client/launchDebugger requests, which the caller must answer via - /// a debugger callback. Defaults to . + /// Gets or sets the server-mode protocol versions supported by the client. The server selects its most + /// preferred mutually supported version. + /// + public IReadOnlyCollection SupportedProtocolVersions { get; set; } = JsonRpcProtocolVersions.Supported; + + /// + /// Gets or sets a value indicating whether the client advertises the reserved debugger-provider + /// capability (capabilities.testing.debuggerProvider). Microsoft.Testing.Platform protocol 1.0 + /// accepts this field for compatibility but does not currently send debugger requests. /// public bool DebuggerProvider { get; set; } diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs index 74ad11be65..4f860df969 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs @@ -70,11 +70,21 @@ private static void RegisterClientSerializersCore() }, }); - Serializers[typeof(InitializeRequestArgs)] = new ObjectSerializer(args => new Dictionary + Serializers[typeof(InitializeRequestArgs)] = new ObjectSerializer(args => { - [JsonRpcStrings.ProcessId] = args.ProcessId, - [JsonRpcStrings.ClientInfo] = Serialize(args.ClientInfo), - [JsonRpcStrings.Capabilities] = Serialize(args.Capabilities), + Dictionary properties = new() + { + [JsonRpcStrings.ProcessId] = args.ProcessId, + [JsonRpcStrings.ClientInfo] = Serialize(args.ClientInfo), + [JsonRpcStrings.Capabilities] = Serialize(args.Capabilities), + }; + + if (args.ProtocolVersions is not null) + { + properties[JsonRpcStrings.ProtocolVersions] = args.ProtocolVersions; + } + + return properties; }); Serializers[typeof(DiscoverRequestArgs)] = new ObjectSerializer(SerializeRequestArgs); diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index ec4eb91d17..25ffc179f9 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -118,6 +118,13 @@ private async Task HandleNotificationAsync(NotificationMessage message, Cancella } } + if (Volatile.Read(ref _initializeState) != Initialized + && message.Method != JsonRpcMethods.CancelRequest) + { + _requestCounter.Signal(); + return; + } + // Note: Yield, so that the main message reading loop can continue. await Task.Yield(); @@ -169,6 +176,47 @@ private async Task HandleRequestAsync(RequestMessage request, CancellationToken } else { + bool isInitializeRequest = request.Method == JsonRpcMethods.Initialize; + if (isInitializeRequest) + { + if (Interlocked.CompareExchange(ref _initializeState, Initializing, NotInitialized) != NotInitialized) + { + try + { + await SendErrorAsync( + reqId: request.Id, + errorCode: ErrorCodes.InvalidRequest, + message: "The server has already received an initialize request.", + data: null, + cancellationToken).ConfigureAwait(false); + } + finally + { + _requestCounter.Signal(); + } + + return; + } + } + else if (Volatile.Read(ref _initializeState) != Initialized) + { + try + { + await SendErrorAsync( + reqId: request.Id, + errorCode: ErrorCodes.ServerNotInitialized, + message: "The server must be initialized before this request can be processed.", + data: null, + cancellationToken).ConfigureAwait(false); + } + finally + { + _requestCounter.Signal(); + } + + return; + } + // We enqueue the request before to "unlink" the current thread so we're sure that we // correctly handle the completion also after the "exit" RpcInvocationState rpcState = new(); @@ -177,35 +225,104 @@ private async Task HandleRequestAsync(RequestMessage request, CancellationToken // Note: Yield, so that the main message reading loop can continue. await Task.Yield(); + bool testUpdateCompletionSent = false; try { object response = await HandleRequestCoreAsync(request, rpcState, cancellationToken).ConfigureAwait(false); + testUpdateCompletionSent = await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken).ConfigureAwait(false); + if (isInitializeRequest) + { + Volatile.Write(ref _initializeState, Initialized); + } + await SendResponseAsync(reqId: request.Id, result: response, cancellationToken).ConfigureAwait(false); CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetResult(response)); } catch (OperationCanceledException e) { - // We don't return the stack of the exception if we're canceling the single request because it's expected and it's not an exception. - (string errorMessage, int errorCode) = rpcState.CancellationToken.IsCancellationRequested - ? (string.Empty, ErrorCodes.RequestCanceled) - : (e.ToString(), ErrorCodes.RequestCanceled); + if (isInitializeRequest) + { + Volatile.Write(ref _initializeState, NotInitialized); + } - await SendErrorAsync(reqId: request.Id, errorCode: errorCode, message: errorMessage, data: null, cancellationToken).ConfigureAwait(false); - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetCanceled()); + try + { + if (!testUpdateCompletionSent) + { + await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken, bestEffort: true).ConfigureAwait(false); + } + + // We don't return the stack of the exception if we're canceling the single request because it's expected and it's not an exception. + (string errorMessage, int errorCode) = rpcState.CancellationToken.IsCancellationRequested + ? (string.Empty, ErrorCodes.RequestCanceled) + : (e.ToString(), ErrorCodes.RequestCanceled); + + await SendErrorAsync(reqId: request.Id, errorCode: errorCode, message: errorMessage, data: null, cancellationToken).ConfigureAwait(false); + } + finally + { + CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetCanceled()); + } } catch (JsonRpcException e) { - await SendErrorAsync(reqId: request.Id, errorCode: e.ErrorCode, message: e.Message, data: null, cancellationToken).ConfigureAwait(false); - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetException(e)); + if (isInitializeRequest) + { + Volatile.Write(ref _initializeState, NotInitialized); + } + + try + { + if (!testUpdateCompletionSent) + { + await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken, bestEffort: true).ConfigureAwait(false); + } + + await SendErrorAsync(reqId: request.Id, errorCode: e.ErrorCode, message: e.Message, data: null, cancellationToken).ConfigureAwait(false); + } + finally + { + CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetException(e)); + } } catch (Exception e) { - await SendErrorAsync(reqId: request.Id, errorCode: 0, message: e.ToString(), data: null, cancellationToken).ConfigureAwait(false); - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.SetException(e)); + if (isInitializeRequest) + { + Volatile.Write(ref _initializeState, NotInitialized); + } + + try + { + if (!testUpdateCompletionSent) + { + await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken, bestEffort: true).ConfigureAwait(false); + } + + await SendErrorAsync(reqId: request.Id, errorCode: ErrorCodes.InternalError, message: e.ToString(), data: null, cancellationToken).ConfigureAwait(false); + } + finally + { + CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetException(e)); + } } } } + private async Task SendTestUpdateCompleteIfNeededAsync( + RequestMessage request, + CancellationToken cancellationToken, + bool bestEffort = false) + { + if (request.Params is not RequestArgsBase args) + { + return false; + } + + await SendTestUpdateCompleteAsync(args.RunId, cancellationToken, bestEffort).ConfigureAwait(false); + return true; + } + private void CompleteRequest( ref ConcurrentDictionary rpcStates, int reqId, diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs index a986671cbb..dfd9437a0a 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs @@ -91,13 +91,21 @@ private void QueueLog(LogLevel logLevel, string message) => _ = Task.Run(() => TryLogAsync(logLevel, message)); internal Task SendTestUpdateCompleteAsync(Guid runId, CancellationToken cancellationToken) - => SendTestUpdateAsync(new TestNodeStateChangedEventArgs(runId, Changes: null), cancellationToken); + => SendTestUpdateCompleteAsync(runId, cancellationToken, bestEffort: false); + + private Task SendTestUpdateCompleteAsync(Guid runId, CancellationToken cancellationToken, bool bestEffort) + => SendTestUpdateAsync(new TestNodeStateChangedEventArgs(runId, Changes: null), cancellationToken, bestEffort); public Task SendTestUpdateAsync(TestNodeStateChangedEventArgs update, CancellationToken cancellationToken) + => SendTestUpdateAsync(update, cancellationToken, bestEffort: false); + + private Task SendTestUpdateAsync(TestNodeStateChangedEventArgs update, CancellationToken cancellationToken, bool bestEffort) => SendMessageAsync( method: JsonRpcMethods.TestingTestUpdatesTests, @params: update, - cancellationToken); + cancellationToken, + checkServerExit: bestEffort, + rethrowException: !bestEffort); public Task SendTelemetryEventUpdateAsync(TelemetryEventArgs args, CancellationToken cancellationToken) => SendMessageAsync( diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.RequestExecution.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.RequestExecution.cs index 93c06c1336..a6949c129d 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.RequestExecution.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.RequestExecution.cs @@ -18,15 +18,6 @@ internal sealed partial class ServerTestHost { private async Task HandleRequestCoreAsync(RequestMessage message, RpcInvocationState rpcInvocationState, CancellationToken cancellationToken) { - var perRequestServiceProvider = (ServiceProvider)ServiceProvider.Clone(); - - // Add custom linked ITestApplicationCooperativeLifetimeService cancellation token source - perRequestServiceProvider.AddService(new PerRequestTestSessionContext( - rpcInvocationState.CancellationToken, - cancellationToken)); - - perRequestServiceProvider.AddService(new TestHostTestFrameworkInvoker(perRequestServiceProvider)); - AssertInitialized(); await _logger.LogDebugAsync($"Received {message.Method} request").ConfigureAwait(false); @@ -37,14 +28,20 @@ private async Task HandleRequestCoreAsync(RequestMessage message, RpcInv throw new JsonRpcException(invalidParams.ErrorCode, invalidParams.ErrorMessage); case (JsonRpcMethods.Initialize, InitializeRequestArgs args): + string negotiatedProtocolVersion = JsonRpcProtocolVersions.Negotiate(args.ProtocolVersions) + ?? throw new JsonRpcException( + ErrorCodes.ProtocolVersionNotSupported, + $"None of the client's protocol versions are supported. Server versions: {string.Join(", ", JsonRpcProtocolVersions.Supported)}."); + _client = new(args.ClientInfo.Name, args.ClientInfo.Version); _clientInfoService = new ClientInfoService(args.ClientInfo.Name, args.ClientInfo.Version, new ClientCapabilitiesService(args.Capabilities.IsStateful)); - await _logger.LogDebugAsync($"Connection established with '{_client.Id}', protocol version {_client.Version}").ConfigureAwait(false); + await _logger.LogDebugAsync( + $"Connection established with '{_client.Id}' version '{_client.Version}', protocol version '{negotiatedProtocolVersion}'").ConfigureAwait(false); INamedFeatureCapability? namedFeatureCapability = ServiceProvider.GetTestFrameworkCapabilities().GetCapability(); return new InitializeResponseArgs( ProcessId: ServiceProvider.GetEnvironment().ProcessId, - ServerInfo: new ServerInfo("test-anywhere", Version: ProtocolVersion), + ServerInfo: new ServerInfo("test-anywhere", Version: PlatformVersion.Version), Capabilities: new ServerCapabilities( new ServerTestingCapabilities( SupportsDiscovery: true, @@ -52,21 +49,37 @@ private async Task HandleRequestCoreAsync(RequestMessage message, RpcInv MultiRequestSupport: false, VSTestProviderSupport: namedFeatureCapability?.IsSupported(JsonRpcStrings.VSTestProviderSupport) == true, SupportsAttachments: true, - MultiConnectionProvider: false))); + MultiConnectionProvider: false))) + { + ProtocolVersion = negotiatedProtocolVersion, + }; case (JsonRpcMethods.TestingDiscoverTests, DiscoverRequestArgs args): - return await ExecuteRequestAsync(args, JsonRpcMethods.TestingDiscoverTests, perRequestServiceProvider, cancellationToken).ConfigureAwait(false); + return await ExecuteRequestAsync(args, JsonRpcMethods.TestingDiscoverTests, rpcInvocationState, cancellationToken).ConfigureAwait(false); case (JsonRpcMethods.TestingRunTests, RunRequestArgs args): - return await ExecuteRequestAsync(args, JsonRpcMethods.TestingRunTests, perRequestServiceProvider, cancellationToken).ConfigureAwait(false); + return await ExecuteRequestAsync(args, JsonRpcMethods.TestingRunTests, rpcInvocationState, cancellationToken).ConfigureAwait(false); default: - throw new NotImplementedException(); + throw new JsonRpcException(ErrorCodes.MethodNotFound, $"The method '{message.Method}' is not supported."); } } - private async Task ExecuteRequestAsync(RequestArgsBase args, string method, ServiceProvider perRequestServiceProvider, CancellationToken cancellationToken) + private async Task ExecuteRequestAsync( + RequestArgsBase args, + string method, + RpcInvocationState rpcInvocationState, + CancellationToken cancellationToken) { + var perRequestServiceProvider = (ServiceProvider)ServiceProvider.Clone(); + + // Add custom linked ITestApplicationCooperativeLifetimeService cancellation token source + perRequestServiceProvider.AddService(new PerRequestTestSessionContext( + rpcInvocationState.CancellationToken, + cancellationToken)); + + perRequestServiceProvider.AddService(new TestHostTestFrameworkInvoker(perRequestServiceProvider)); + DateTimeOffset requestStart = _clock.UtcNow; ITestSessionContext perRequestTestSessionContext = perRequestServiceProvider.GetTestSessionContext(); @@ -159,7 +172,6 @@ await ExecuteRequestAsync( // catch and propagated as correct json rpc error perRequestTestSessionContext.CancellationToken.ThrowIfCancellationRequested(); - await SendTestUpdateCompleteAsync(args.RunId, cancellationToken).ConfigureAwait(false); requestExecuteStop = _clock.UtcNow; } finally diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs index 81279d32c7..e5c349e6fa 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs @@ -20,11 +20,10 @@ namespace Microsoft.Testing.Platform.Hosts; [StackTraceHidden] internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDisposable, IOutputDeviceDataProducer { - // The value is the build-time version stamp (e.g. "2.4.0-dev" locally vs "2.4.0-ci" on CI), so it is an - // implementation detail rather than a stable API surface and must not be tracked by the internal API analyzers. -#pragma warning disable RS0051 // Add internal types and members to the declared API - public const string ProtocolVersion = PlatformVersion.Version; -#pragma warning restore RS0051 // Add internal types and members to the declared API + private const int NotInitialized = 0; + private const int Initializing = 1; + private const int Initialized = 2; + private readonly Func> _buildTestFrameworkAsync; private readonly IMessageHandlerFactory _messageHandlerFactory; @@ -53,6 +52,7 @@ internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDis private IMessageHandler? _messageHandler; private TestHost.ClientInfo? _client; private IClientInfo? _clientInfoService; + private int _initializeState; public ServerTestHost( ServiceProvider serviceProvider, diff --git a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt index 3171e40e0b..4169a575be 100644 --- a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt @@ -218,6 +218,18 @@ Microsoft.Testing.Platform.Services.ClientCapabilitiesService.Deconstruct(out bo Microsoft.Testing.Platform.Services.ClientCapabilitiesService.Equals(Microsoft.Testing.Platform.Services.ClientCapabilitiesService? other) -> bool Microsoft.Testing.Platform.Services.ClientCapabilitiesService.IsStateful.get -> bool Microsoft.Testing.Platform.Services.ClientCapabilitiesService.IsStateful.init -> void +Microsoft.Testing.Platform.ServerMode.InitializeRequestArgs.ProtocolVersions.get -> string![]? +Microsoft.Testing.Platform.ServerMode.InitializeRequestArgs.ProtocolVersions.init -> void +Microsoft.Testing.Platform.ServerMode.InitializeResponseArgs.ProtocolVersion.get -> string? +Microsoft.Testing.Platform.ServerMode.InitializeResponseArgs.ProtocolVersion.init -> void +Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions +const Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions.V1 = "1.0.0" -> string! +static Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions.Current.get -> string! +static Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions.Negotiate(System.Collections.Generic.IReadOnlyCollection? clientSupportedVersions) -> string? +static Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions.Supported.get -> System.Collections.Generic.IReadOnlyList! +const Microsoft.Testing.Platform.ServerMode.JsonRpcStrings.ProtocolVersion = "protocolVersion" -> string! +const Microsoft.Testing.Platform.ServerMode.JsonRpcStrings.ProtocolVersions = "protocolVersions" -> string! +static readonly Microsoft.Testing.Platform.ServerMode.ErrorCodes.ProtocolVersionNotSupported -> int Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.get -> Microsoft.Testing.Platform.Services.IClientCapabilities! Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.init -> void Microsoft.Testing.Platform.Services.ClientInfoService.ClientInfoService(string! Id, string! Version, Microsoft.Testing.Platform.Services.IClientCapabilities! Capabilities) -> void diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ErrorCodes.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ErrorCodes.cs index d001fa1d8c..002cb396ea 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ErrorCodes.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ErrorCodes.cs @@ -26,6 +26,7 @@ internal sealed class ErrorCodes #region Testing Platform error codes public static readonly int TestingPlatformErrorRangeStart = -31700; + public static readonly int ProtocolVersionNotSupported = -31699; public static readonly int TestingPlatformErrorRangeEnd = -31000; #endregion } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 87ec35c5b0..332248e0e8 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -90,7 +90,7 @@ private static void RegisterDefaultDeserializers(Dictionary(jsonElement, JsonRpcStrings.Id); + int id = BindRpcId(jsonElement); IDictionary? result = element.ValueKind == JsonValueKind.Null ? null : json.Bind>(jsonElement, JsonRpcStrings.Result); @@ -153,10 +160,17 @@ private static void RegisterDefaultDeserializers(Dictionary((json, jsonElement) => new InitializeRequestArgs( + deserializers[typeof(InitializeRequestArgs)] = new JsonElementDeserializer((json, jsonElement) => + { + json.TryArrayBind(jsonElement, out string[]? protocolVersions, JsonRpcStrings.ProtocolVersions); + return new InitializeRequestArgs( ProcessId: json.Bind(jsonElement, JsonRpcStrings.ProcessId), ClientInfo: json.Bind(jsonElement, JsonRpcStrings.ClientInfo), - Capabilities: json.Bind(jsonElement, JsonRpcStrings.Capabilities))); + Capabilities: json.Bind(jsonElement, JsonRpcStrings.Capabilities)) + { + ProtocolVersions = protocolVersions, + }; + }); deserializers[typeof(ClientInfo)] = new JsonElementDeserializer((json, jsonElement) => new ClientInfo( Name: json.Bind(jsonElement, JsonRpcStrings.Name), @@ -176,10 +190,17 @@ private static void RegisterDefaultDeserializers(Dictionary( - (json, jsonElement) => new InitializeResponseArgs( + (json, jsonElement) => + { + json.TryBind(jsonElement, out string? protocolVersion, JsonRpcStrings.ProtocolVersion); + return new InitializeResponseArgs( ProcessId: json.Bind(jsonElement, JsonRpcStrings.ProcessId), ServerInfo: json.Bind(jsonElement, JsonRpcStrings.ServerInfo), - Capabilities: json.Bind(jsonElement, JsonRpcStrings.Capabilities))); + Capabilities: json.Bind(jsonElement, JsonRpcStrings.Capabilities)) + { + ProtocolVersion = protocolVersion, + }; + }); deserializers[typeof(ServerInfo)] = new JsonElementDeserializer( (json, jsonElement) => new ServerInfo( @@ -192,11 +213,11 @@ private static void RegisterDefaultDeserializers(Dictionary( (json, jsonElement) => new ServerTestingCapabilities( - SupportsDiscovery: json.Bind(jsonElement, JsonRpcStrings.SupportsDiscovery), - MultiRequestSupport: json.Bind(jsonElement, JsonRpcStrings.MultiRequestSupport), - VSTestProviderSupport: json.Bind(jsonElement, JsonRpcStrings.VSTestProviderSupport), - SupportsAttachments: json.Bind(jsonElement, JsonRpcStrings.AttachmentsSupport), - MultiConnectionProvider: json.Bind(jsonElement, JsonRpcStrings.MultiConnectionProvider))); + SupportsDiscovery: json.Bind(jsonElement, JsonRpcStrings.SupportsDiscovery), + MultiRequestSupport: json.Bind(jsonElement, JsonRpcStrings.MultiRequestSupport), + VSTestProviderSupport: json.Bind(jsonElement, JsonRpcStrings.VSTestProviderSupport), + SupportsAttachments: json.Bind(jsonElement, JsonRpcStrings.AttachmentsSupport), + MultiConnectionProvider: json.Bind(jsonElement, JsonRpcStrings.MultiConnectionProvider))); deserializers[typeof(DiscoverRequestArgs)] = new JsonElementDeserializer((json, jsonElement) => { @@ -259,7 +280,9 @@ private static void RegisterDefaultDeserializers(Dictionary( - (json, jsonElement) => json.TryBind(jsonElement, out int id, JsonRpcStrings.Id) ? new CancelRequestArgs(id) : throw new MessageFormatException("id field should be an int")); + (json, jsonElement) => TryGetRpcId(jsonElement, out int id) + ? new CancelRequestArgs(id) + : throw new MessageFormatException("id field is missing")); deserializers[typeof(ExitRequestArgs)] = new JsonElementDeserializer( (json, jsonElement) => new ExitRequestArgs()); @@ -269,7 +292,7 @@ private static void RegisterDefaultDeserializers(Dictionary(jsonElement, JsonRpcStrings.Id); + int id = BindRpcId(jsonElement); JsonElement error = jsonElement.GetProperty(JsonRpcStrings.Error); int code = json.Bind(error, JsonRpcStrings.Code); @@ -335,4 +358,35 @@ private static object ReadNumber(JsonElement element) return element.GetDouble(); } #pragma warning restore IDE0046 // Convert to conditional expression + + private static bool TryGetRpcId(JsonElement jsonElement, out int id) + { + if (!jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) + || idElement.ValueKind == JsonValueKind.Null) + { + id = default; + return false; + } + + id = ReadRpcId(idElement); + return true; + } + + private static int BindRpcId(JsonElement jsonElement) + => jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) + ? ReadRpcId(idElement) + : throw new MessageFormatException($"'{JsonRpcStrings.Id}' field is missing"); + + private static int ReadRpcId(JsonElement idElement) + => idElement.ValueKind switch + { + JsonValueKind.Number when idElement.TryGetInt32(out int numericId) => numericId, + JsonValueKind.String when int.TryParse( + idElement.GetString(), + NumberStyles.Integer, + CultureInfo.InvariantCulture, + out int stringId) + && idElement.GetString() == stringId.ToString(CultureInfo.InvariantCulture) => stringId, + _ => throw new MessageFormatException($"'{JsonRpcStrings.Id}' field should be an int or a numeric string"), + }; } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs index 684f432863..7b46527b4d 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs @@ -52,11 +52,20 @@ private static void RegisterDefaultSerializers(Dictionary }); serializers[typeof(InitializeResponseArgs)] = new JsonObjectSerializer(response => - [ - (JsonRpcStrings.ProcessId, response.ProcessId), - (JsonRpcStrings.ServerInfo, response.ServerInfo), - (JsonRpcStrings.Capabilities, response.Capabilities) - ]); + response.ProtocolVersion is null + ? + [ + (JsonRpcStrings.ProcessId, response.ProcessId), + (JsonRpcStrings.ServerInfo, response.ServerInfo), + (JsonRpcStrings.Capabilities, response.Capabilities) + ] + : + [ + (JsonRpcStrings.ProcessId, response.ProcessId), + (JsonRpcStrings.ServerInfo, response.ServerInfo), + (JsonRpcStrings.Capabilities, response.Capabilities), + (JsonRpcStrings.ProtocolVersion, response.ProtocolVersion) + ]); serializers[typeof(ServerInfo)] = new JsonObjectSerializer(info => [ diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs index ea0fb7b061..4372dee7a7 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs @@ -94,7 +94,8 @@ internal T Bind(JsonElement element, string? property = null) internal bool TryBind(JsonElement element, out T? value, string? property = null) { - if (property is not null && !element.TryGetProperty(property, out element)) + if (property is not null + && (!element.TryGetProperty(property, out element) || element.ValueKind == JsonValueKind.Null)) { value = default; return false; @@ -106,7 +107,8 @@ internal bool TryBind(JsonElement element, out T? value, string? property = n internal bool TryArrayBind(JsonElement element, out T[]? value, string? property = null) { - if (property is not null && !element.TryGetProperty(property, out element)) + if (property is not null + && (!element.TryGetProperty(property, out element) || element.ValueKind == JsonValueKind.Null)) { value = default; return false; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs index 20ada74dff..514d5aafe8 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs @@ -17,6 +17,34 @@ internal static class JsonRpcMethods public const string TestingTestUpdatesAttachments = "testing/testUpdates/attachments"; } +internal static class JsonRpcProtocolVersions +{ + public const string V1 = "1.0.0"; + + public static string Current => V1; + + public static IReadOnlyList Supported { get; } = Array.AsReadOnly([V1]); + + public static string? Negotiate(IReadOnlyCollection? clientSupportedVersions) + { + if (clientSupportedVersions is null || clientSupportedVersions.Count == 0) + { + return Current; + } + + IReadOnlyList serverSupportedVersions = Supported; + for (int i = serverSupportedVersions.Count - 1; i >= 0; i--) + { + if (clientSupportedVersions.Contains(serverSupportedVersions[i])) + { + return serverSupportedVersions[i]; + } + } + + return null; + } +} + internal static class JsonRpcStrings { // Common @@ -35,6 +63,8 @@ internal static class JsonRpcStrings public const string ServerInfo = "serverInfo"; public const string Name = "name"; public const string Version = "version"; + public const string ProtocolVersions = "protocolVersions"; + public const string ProtocolVersion = "protocolVersion"; // Capabilities public const string Capabilities = "capabilities"; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs index ea6237ce3f..adbcd4e853 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs @@ -71,7 +71,10 @@ public async Task ConnectAsync() // This means we push attachments SupportsAttachments: true, // This means we're a push node - MultiConnectionProvider: true))); + MultiConnectionProvider: true))) + { + ProtocolVersion = JsonRpcProtocolVersions.Current, + }; await SendResponseAsync(requestMessage.Id, responseObject, _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); return true; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs index f9efe9faa8..a8a717bce4 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs @@ -35,9 +35,15 @@ internal sealed record ErrorMessage(int Id, int ErrorCode, string Message, objec /// internal sealed record ResponseMessage(int Id, object? Result) : RpcMessage; -internal sealed record InitializeRequestArgs(int ProcessId, ClientInfo ClientInfo, ClientCapabilities Capabilities); +internal sealed record InitializeRequestArgs(int ProcessId, ClientInfo ClientInfo, ClientCapabilities Capabilities) +{ + public string[]? ProtocolVersions { get; init; } +} -internal sealed record InitializeResponseArgs(int? ProcessId, ServerInfo ServerInfo, ServerCapabilities Capabilities); +internal sealed record InitializeResponseArgs(int? ProcessId, ServerInfo ServerInfo, ServerCapabilities Capabilities) +{ + public string? ProtocolVersion { get; init; } +} internal record RequestArgsBase(Guid RunId, ICollection? TestNodes, string? GraphFilter); @@ -103,7 +109,9 @@ internal sealed record ServerTestingCapabilities( bool SupportsAttachments, bool MultiConnectionProvider) { - public static bool SupportsTestCoverageMessages => true; + // This capability describes JSON-RPC wire forwarding, not in-process coverage-message consumption. + // Keep it false until server mode forwards the first-class messages defined by RFC 019. + public static bool SupportsTestCoverageMessages => false; } internal sealed record TestNodeStateChangedEventArgs(Guid RunId, TestNodeUpdateMessage[]? Changes) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index 3bcfb8d72a..f46b448125 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -26,40 +26,52 @@ private static void RegisterDeserializers() object? idObj = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Id); - IDictionary paramsObj = method != JsonRpcMethods.Exit - ? GetRequiredPropertyFromJson>(properties, JsonRpcStrings.Params) - : new Dictionary(); - int? id = idObj is null ? null : GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); object? @params; - try + object? rawParams = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Params); + bool paramsRequired = method is JsonRpcMethods.Initialize + or JsonRpcMethods.TestingDiscoverTests + or JsonRpcMethods.TestingRunTests + or JsonRpcMethods.CancelRequest; + if (paramsRequired && rawParams is not IDictionary) { - // Parse the specific methods - @params = method switch - { - JsonRpcMethods.Initialize => Deserialize(paramsObj), - JsonRpcMethods.TestingDiscoverTests => Deserialize(paramsObj), - JsonRpcMethods.TestingRunTests => Deserialize(paramsObj), - JsonRpcMethods.CancelRequest => Deserialize(paramsObj), - JsonRpcMethods.Exit => Deserialize(paramsObj), - - // Note: Let the server report unknown RPC request back to the client. - _ => null, - }; + @params = new InvalidRequestParamsArgs( + ErrorCodes.InvalidParams, + rawParams is null ? "'params' field is missing" : "'params' field has wrong type (expected Object)"); } - catch (Exception ex) when (ex is MessageFormatException or InvalidCastException) + else { - // If params can't be deserialized for a request, capture the failure so - // we can later send back a properly coded JSON-RPC error using the request id. - // For notifications there's no one to respond to, but we still avoid - // crashing the message-handling loop by swallowing into the sentinel. - // We catch the broader set of deserialization-related exceptions because the - // request payload is untrusted client input and the lower-level helpers can - // throw types other than MessageFormatException. - @params = new InvalidRequestParamsArgs(ErrorCodes.InvalidParams, ex.Message); + IDictionary paramsObj = rawParams as IDictionary ?? new Dictionary(); + try + { + // Parse the specific methods + @params = method switch + { + JsonRpcMethods.Initialize => Deserialize(paramsObj), + JsonRpcMethods.TestingDiscoverTests => Deserialize(paramsObj), + JsonRpcMethods.TestingRunTests => Deserialize(paramsObj), + JsonRpcMethods.CancelRequest => Deserialize(paramsObj), + JsonRpcMethods.Exit => Deserialize(paramsObj), + + // 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, + }; + } + catch (Exception ex) when (ex is MessageFormatException or InvalidCastException) + { + // If params can't be deserialized for a request, capture the failure so + // we can later send back a properly coded JSON-RPC error using the request id. + // For notifications there's no one to respond to, but we still avoid + // crashing the message-handling loop by swallowing into the sentinel. + // We catch the broader set of deserialization-related exceptions because the + // request payload is untrusted client input and the lower-level helpers can + // throw types other than MessageFormatException. + @params = new InvalidRequestParamsArgs(ErrorCodes.InvalidParams, ex.Message); + } } return id.HasValue @@ -92,8 +104,28 @@ private static void RegisterDeserializers() int processId = GetRequiredPropertyFromJson(properties, JsonRpcStrings.ProcessId); ClientInfo clientInfo = Deserialize(properties); ClientCapabilities capabilities = Deserialize(properties); + object? protocolVersionsValue = GetOptionalPropertyFromJson(properties, JsonRpcStrings.ProtocolVersions); + string[]? protocolVersions = null; + if (protocolVersionsValue is not null) + { + if (protocolVersionsValue is not ICollection protocolVersionsJson) + { + throw new MessageFormatException($"'{JsonRpcStrings.ProtocolVersions}' field has wrong type (expected Array)"); + } + + protocolVersions = new string[protocolVersionsJson.Count]; + int index = 0; + foreach (object? protocolVersion in protocolVersionsJson) + { + protocolVersions[index++] = protocolVersion as string + ?? throw new MessageFormatException($"'{JsonRpcStrings.ProtocolVersions}' entries must be strings"); + } + } - return new InitializeRequestArgs(processId, clientInfo, capabilities); + return new InitializeRequestArgs(processId, clientInfo, capabilities) + { + ProtocolVersions = protocolVersions, + }; }); Deserializers[typeof(ClientInfo)] = new ObjectDeserializer(properties => @@ -120,8 +152,12 @@ private static void RegisterDeserializers() int processId = GetRequiredPropertyFromJson(properties, JsonRpcStrings.ProcessId); ServerInfo serverInfo = Deserialize(GetRequiredPropertyFromJson>(properties, JsonRpcStrings.ServerInfo)); ServerCapabilities capabilities = Deserialize(GetRequiredPropertyFromJson>(properties, JsonRpcStrings.Capabilities)); + string? protocolVersion = GetOptionalPropertyFromJson(properties, JsonRpcStrings.ProtocolVersion) as string; - return new InitializeResponseArgs(processId, serverInfo, capabilities); + return new InitializeResponseArgs(processId, serverInfo, capabilities) + { + ProtocolVersion = protocolVersion, + }; }); Deserializers[typeof(ServerInfo)] = new ObjectDeserializer(properties => @@ -140,7 +176,6 @@ private static void RegisterDeserializers() bool vstestProviderSupport = GetRequiredPropertyFromJson(testingCapabilities, JsonRpcStrings.VSTestProviderSupport); bool attachmentsSupport = GetRequiredPropertyFromJson(testingCapabilities, JsonRpcStrings.AttachmentsSupport); bool multiConnectionProvider = GetRequiredPropertyFromJson(testingCapabilities, JsonRpcStrings.MultiConnectionProvider); - return new ServerCapabilities(new ServerTestingCapabilities( SupportsDiscovery: supportsDiscovery, MultiRequestSupport: multiRequestSupport, diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs index c1c835c7eb..d3b8dbf5ee 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs @@ -77,6 +77,11 @@ private static void RegisterRpcMessageSerializers() [JsonRpcStrings.Capabilities] = Serialize(res.Capabilities), }; + if (res.ProtocolVersion is not null) + { + values[JsonRpcStrings.ProtocolVersion] = res.ProtocolVersion; + } + return values; }); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs index 6c6b66e31a..95fff88b46 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs @@ -80,7 +80,8 @@ private static T GetRequiredPropertyFromJson(IDictionary pro => idObj switch { int idInt => idInt, - string idStr => int.TryParse(idStr, out int id) + string idStr => int.TryParse(idStr, NumberStyles.Integer, CultureInfo.InvariantCulture, out int id) + && idStr == id.ToString(CultureInfo.InvariantCulture) ? id : null, _ => null, diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs index e476fd556d..aeeac99029 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs @@ -69,7 +69,10 @@ public FakeMtpServer() MultiRequestSupport: true, VSTestProviderSupport: false, SupportsAttachments: true, - MultiConnectionProvider: false))); + MultiConnectionProvider: false))) + { + ProtocolVersion = JsonRpcProtocolVersions.Current, + }; _ = Task.Run(AcceptAndServeAsync); } diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs index c44c9c1e2f..5449de38aa 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs @@ -35,12 +35,42 @@ public async Task InitializeAsync_DecodesServerCapabilities() Assert.AreEqual(4242, capabilities.ServerProcessId); Assert.AreEqual("FakeMtpServer", capabilities.ServerName); Assert.AreEqual("1.2.3", capabilities.ServerVersion); + Assert.AreEqual(JsonRpcProtocolVersions.Current, capabilities.ProtocolVersion); Assert.IsTrue(capabilities.SupportsDiscovery); Assert.IsTrue(capabilities.MultiRequestSupport); Assert.IsFalse(capabilities.VSTestProviderSupport); Assert.IsTrue(capabilities.SupportsAttachments); Assert.IsFalse(capabilities.MultiConnectionProvider); Assert.AreSame(capabilities, client.Capabilities); + + InitializeRequestArgs initializeArgs = GetSingleRequestParams(server, JsonRpcMethods.Initialize); + Assert.AreSequenceEqual(JsonRpcProtocolVersions.Supported, initializeArgs.ProtocolVersions); + } + + [TestMethod] + public async Task InitializeAsync_LegacyServerWithoutProtocolVersion_Succeeds() + { + using FakeMtpServer server = new(); + server.InitializeResponse = server.InitializeResponse with { ProtocolVersion = null }; + using MtpServerClient client = server.ConnectClient(); + + MtpServerCapabilities capabilities = await WithTimeoutAsync(client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); + + Assert.IsNull(capabilities.ProtocolVersion); + } + + [TestMethod] + public async Task InitializeAsync_UnsupportedNegotiatedProtocolVersion_Throws() + { + using FakeMtpServer server = new(); + server.InitializeResponse = server.InitializeResponse with { ProtocolVersion = "2.0.0" }; + using MtpServerClient client = server.ConnectClient(); + + MtpServerClientException exception = await AssertThrowsAsync( + () => client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); + + Assert.Contains("2.0.0", exception.Message); + Assert.IsNull(client.Capabilities); } [TestMethod] diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index 7fc5165fb5..6fd11c6d2d 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -53,6 +53,132 @@ public void CanDeserializeTaskResponse() Assert.IsNull(response.Result); } + [TestMethod] + public void CanDeserializeNumericStringRequestId() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "id": "42", + "method": "testing/unknown" + } + """); + + Assert.AreEqual(42, Assert.IsInstanceOfType(message).Id); + } + + [TestMethod] + public void CanDeserializeNumericStringCancellationId() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": "42" + } + } + """); + + NotificationMessage notification = Assert.IsInstanceOfType(message); + Assert.AreEqual(42, Assert.IsInstanceOfType(notification.Params).CancelRequestId); + } + + [TestMethod] + public void NullRequestId_IsTreatedAsNotification() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "id": null, + "method": "testing/unknown" + } + """); + + Assert.IsInstanceOfType(message); + } + + [TestMethod] + public void DeserializeInitializeRequest_NullProtocolVersions_UsesLegacyNegotiation() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "test-client", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + }, + "protocolVersions": null + } + } + """); + + InitializeRequestArgs request = Assert.IsInstanceOfType(message).Params + as InitializeRequestArgs + ?? throw new InvalidOperationException("Expected typed initialize request arguments."); + Assert.IsNull(request.ProtocolVersions); + } + + [DataRow("\"1.0.0\"")] + [DataRow("[1]")] + [TestMethod] + public void DeserializeInitializeRequest_InvalidProtocolVersions_CapturesInvalidParams(string protocolVersions) + { + RpcMessage message = Deserialize( + $$""" + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "test-client", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + }, + "protocolVersions": {{protocolVersions}} + } + } + """); + + RequestMessage request = Assert.IsInstanceOfType(message); + InvalidRequestParamsArgs invalidParams = Assert.IsInstanceOfType(request.Params); + Assert.AreEqual(ErrorCodes.InvalidParams, invalidParams.ErrorCode); + } + + [DataRow(JsonRpcMethods.Initialize)] + [DataRow(JsonRpcMethods.TestingDiscoverTests)] + [DataRow(JsonRpcMethods.TestingRunTests)] + [DataRow(JsonRpcMethods.CancelRequest)] + [TestMethod] + public void DeserializeKnownRequest_MissingParams_CapturesInvalidParams(string method) + { + RpcMessage message = Deserialize( + $$""" + { + "jsonrpc": "2.0", + "id": 1, + "method": "{{method}}" + } + """); + + RequestMessage request = Assert.IsInstanceOfType(message); + InvalidRequestParamsArgs invalidParams = Assert.IsInstanceOfType(request.Params); + Assert.AreEqual(ErrorCodes.InvalidParams, invalidParams.ErrorCode); + } + [TestMethod] public async Task Serialize_TestNodeWithRetryAttempt_EmitsRetryProperties() { @@ -92,6 +218,18 @@ public async Task Serialize_TestNodeWithSupersededRetryAttempt_EmitsIsSuperseded Assert.Contains("\"retry.is-superseded\":true", serialized, serialized); } + [TestMethod] + public async Task Serialize_TestsAttachmentsWithoutRunId_PreservesLegacyShape() + { + TestsAttachments attachments = new([new RunTestAttachment("uri", "producer", "type", "name", null)]); + + string serialized = (await _formatter.SerializeAsync(attachments)).Replace(" ", string.Empty); + + Assert.AreEqual( + """{"attachments":[{"uri":"uri","producer":"producer","type":"type","display-name":"name","description":null}]}""", + serialized); + } + [DynamicData(nameof(SerializerTypesForDynamicData), DynamicDataDisplayName = nameof(FormatSerializerTypes))] [TestMethod] public async Task SerializeDeserialize_Succeed(Type type) @@ -409,13 +547,13 @@ private static void AssertSerialize(Type type, string instanceSerialized) if (type == typeof(ServerTestingCapabilities)) { - Assert.AreEqual("""{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":true}""".Replace(" ", string.Empty), instanceSerialized, because); + Assert.AreEqual("""{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":false}""".Replace(" ", string.Empty), instanceSerialized, because); return; } if (type == typeof(ServerCapabilities)) { - Assert.AreEqual("""{"testing":{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":true}}""".Replace(" ", string.Empty), instanceSerialized, because); + Assert.AreEqual("""{"testing":{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":false}}""".Replace(" ", string.Empty), instanceSerialized, because); return; } @@ -427,7 +565,7 @@ private static void AssertSerialize(Type type, string instanceSerialized) if (type == typeof(InitializeResponseArgs)) { - Assert.AreEqual("""{"processId":1,"serverInfo":{"name":"ServerInfoName","version":"Version"},"capabilities":{"testing":{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":true}}}""".Replace(" ", string.Empty), instanceSerialized, because); + Assert.AreEqual("""{"processId":1,"serverInfo":{"name":"ServerInfoName","version":"Version"},"capabilities":{"testing":{"supportsDiscovery":true,"experimental_multiRequestSupport":true,"vstestProvider":true,"attachmentsSupport":true,"multipleConnectionProvider":true,"supportsTestCoverageMessages":false}},"protocolVersion":"1.0.0"}""".Replace(" ", string.Empty), instanceSerialized, because); return; } @@ -587,7 +725,13 @@ private static object CreateInstance(Type type) if (type == typeof(InitializeResponseArgs)) { - return new InitializeResponseArgs(1, new ServerInfo("ServerInfoName", "Version"), new ServerCapabilities(new ServerTestingCapabilities(true, true, true, true, true))); + return new InitializeResponseArgs( + 1, + new ServerInfo("ServerInfoName", "Version"), + new ServerCapabilities(new ServerTestingCapabilities(true, true, true, true, true))) + { + ProtocolVersion = JsonRpcProtocolVersions.Current, + }; } if (type == typeof(ErrorMessage)) diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 6cd647aee2..6edb4242d9 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -117,10 +117,15 @@ await WriteMessageAsync( InitializeResponseArgs expectedResponse = new( 1, new ServerInfo("test-anywhere", "this is dynamic"), - new ServerCapabilities(new ServerTestingCapabilities(SupportsDiscovery: true, MultiRequestSupport: false, VSTestProviderSupport: false, SupportsAttachments: true, MultiConnectionProvider: false))); + new ServerCapabilities(new ServerTestingCapabilities(SupportsDiscovery: true, MultiRequestSupport: false, VSTestProviderSupport: false, SupportsAttachments: true, MultiConnectionProvider: false))) + { + ProtocolVersion = JsonRpcProtocolVersions.Current, + }; Assert.AreEqual(expectedResponse.Capabilities, resultJson.Capabilities); Assert.AreEqual(expectedResponse.ServerInfo.Name, resultJson.ServerInfo.Name); + Assert.AreEqual(JsonRpcProtocolVersions.Current, resultJson.ProtocolVersion); + Assert.IsNotEmpty(resultJson.ServerInfo.Version); await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); @@ -128,6 +133,174 @@ await WriteMessageAsync( Assert.AreEqual(0, result); } + [TestMethod] + public async Task ServerEnforcesLifecycleAndNegotiatesProtocolVersion() + { + using var server = TcpServer.Create(); + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = context => + { + context.Complete(); + return Task.CompletedTask; + }, + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000001" + } + } + """); + + var beforeInitializeError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 1 }, + "Wait server-not-initialized error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.ServerNotInitialized, beforeInitializeError.ErrorCode); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "42.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + }, + "protocolVersions": [ "99.0.0" ] + } + } + """); + + var incompatibleVersionError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 2 }, + "Wait incompatible protocol error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, incompatibleVersionError.ErrorCode); + + const string initializeMessage = """ + { + "jsonrpc": "2.0", + "id": 3, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "42.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + }, + "protocolVersions": [ "1.0.0" ] + } + } + """; + await WriteMessageAsync(writer, initializeMessage); + + var initializeResponse = (ResponseMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 3 }, + "Wait initialize response", + timeout.Token))!; + InitializeResponseArgs initializeResult = SerializerUtilities.Deserialize( + (IDictionary)initializeResponse.Result!); + Assert.AreEqual(JsonRpcProtocolVersions.Current, initializeResult.ProtocolVersion); + Assert.IsNotEmpty(initializeResult.ServerInfo.Version); + var responseResult = (IDictionary)initializeResponse.Result!; + var responseCapabilities = (IDictionary)responseResult[JsonRpcStrings.Capabilities]!; + var testingCapabilities = (IDictionary)responseCapabilities[JsonRpcStrings.Testing]!; + Assert.IsFalse((bool)testingCapabilities[JsonRpcStrings.SupportsTestCoverageMessages]!); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 4, + "method": "testing/unknown", + "params": {} + } + """); + + var methodNotFoundError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 4 }, + "Wait method-not-found error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.MethodNotFound, methodNotFoundError.ErrorCode); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 5, + "method": "testing/runTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000005", + "filter": "/A(&B)" + } + } + """); + + RpcMessage? failedRunCompletion = await WaitForMessage( + messageHandler, + IsTestUpdateCompletion, + "Wait failed run completion", + timeout.Token); + Assert.IsNotNull(failedRunCompletion); + + var internalError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 5 }, + "Wait failed run error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.InternalError, internalError.ErrorCode); + + await WriteMessageAsync(writer, initializeMessage.Replace("\"id\": 3", "\"id\": 6")); + var duplicateInitializeError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 6 }, + "Wait duplicate initialize error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.InvalidRequest, duplicateInitializeError.ErrorCode); + + await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); + + Assert.AreEqual(0, await serverTask); + } + [TestMethod] public async Task RunRequestWithEmptyTests_PreservesEmptyUidSelection() { @@ -288,7 +461,14 @@ public async Task DiscoveryRequestCanBeCanceled() await WriteMessageAsync(writer, cancelRequestMessage); using CancellationTokenSource cancellationTokenSource2 = new(TimeoutHelper.DefaultHangTimeSpanTimeout); - msg = await WaitForMessage(messageHandler, rpcMessage => rpcMessage is ErrorMessage, "Wait cancelRequest", cancellationTokenSource.Token); + RpcMessage? completion = await WaitForMessage( + messageHandler, + IsTestUpdateCompletion, + "Wait cancellation completion", + cancellationTokenSource2.Token); + Assert.IsNotNull(completion); + + msg = await WaitForMessage(messageHandler, rpcMessage => rpcMessage is ErrorMessage, "Wait cancelRequest", cancellationTokenSource2.Token); var error = (ErrorMessage)msg!; Assert.AreEqual(ErrorCodes.RequestCanceled, error.ErrorCode); @@ -299,6 +479,80 @@ public async Task DiscoveryRequestCanBeCanceled() Assert.AreEqual(0, result); } + [TestMethod] + public async Task GlobalCancellationDuringDiscovery_DoesNotHangShutdown() + { + using var server = TcpServer.Create(); + TaskCompletionSource discoveryStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = async context => + { + discoveryStarted.TrySetResult(true); + await Task.Delay(Timeout.Infinite, context.CancellationToken); + }, + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + _ = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 1 }, + "Wait initialize", + timeout.Token); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002" + } + } + """); + await discoveryStarted.Task.TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, timeout.Token); + + testApplication.ServiceProvider.GetTestApplicationCancellationTokenSource().Cancel(); + + await serverTask.TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, timeout.Token); + Assert.AreEqual((int)ExitCode.TestSessionAborted, await serverTask); + } + [DataRow(JsonRpcMethods.TestingDiscoverTests)] [DataRow(JsonRpcMethods.TestingRunTests)] [TestMethod] @@ -390,6 +644,13 @@ public async Task RequestWithInvalidRunId_ReturnsInvalidParamsError(string metho } } + private static bool IsTestUpdateCompletion(RpcMessage? rpcMessage) + => rpcMessage is NotificationMessage notification + && notification.Method == JsonRpcMethods.TestingTestUpdatesTests + && notification.Params is IDictionary completionParams + && completionParams.TryGetValue(JsonRpcStrings.Changes, out object? changes) + && changes is null; + private static async Task WriteMessageAsync(StreamWriter writer, string message) { await writer.WriteLineAsync($"Content-Length: {message.Length}"); From 8fdd33b27a6b7ec9eeb06248152e612733c0ec22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 11:08:26 +0200 Subject: [PATCH 02/25] Address JSON-RPC hardening review Align passive-node negotiation, legacy empty-version handling, serializer type validation, RPC ID schema validation, and Markdown formatting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../001-protocol-intro.md | 2 - .../server-mode-1.0.schema.json | 2 +- .../Client/MtpServerClient.cs | 22 ++++- .../JsonRpc/Json/Json.Deserializers.cs | 11 ++- .../ServerMode/JsonRpc/PassiveNode.cs | 34 ++++++- .../SerializerUtilities.Deserializers.cs | 11 ++- .../FakeMtpServer.cs | 5 +- .../MtpServerClientTests.cs | 49 ++++++++++ .../ServerMode/FormatterUtilitiesTests.cs | 41 ++++++++ .../ServerMode/PassiveNodeTests.cs | 97 +++++++++++++++++++ 10 files changed, 262 insertions(+), 12 deletions(-) create mode 100644 test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index dad3661c63..732f70d400 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -734,7 +734,6 @@ interface CancelParams { ### Launch debugger > Message direction: Server -> Client - > [!NOTE] > Reserved protocol shape. Microsoft.Testing.Platform 1.0 does not currently send this request. @@ -768,7 +767,6 @@ interface LaunchDebuggerParams { ### Attach debugger > Message direction: Server -> Client - > [!NOTE] > Reserved protocol shape. Microsoft.Testing.Platform 1.0 does not currently send this request. diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index c251bd642b..f509b6c7c4 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -54,7 +54,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-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])$" + "pattern": "^-(?:[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])$" } ] } diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs index f47e53a70a..e55175c021 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs @@ -129,7 +129,7 @@ public async Task InitializeAsync(CancellationToken cance ResponseMessage response = await _connection.SendRequestAsync(JsonRpcMethods.Initialize, args, cancellationToken).ConfigureAwait(false); MtpServerCapabilities capabilities = DecodeCapabilities(AsResultDictionary(response.Result)); if (capabilities.ProtocolVersion is { } negotiatedProtocolVersion - && !_options.SupportedProtocolVersions.Contains(negotiatedProtocolVersion, StringComparer.Ordinal)) + && !IsSupportedProtocolVersion(negotiatedProtocolVersion)) { throw new MtpServerClientException( $"The server negotiated unsupported protocol version '{negotiatedProtocolVersion}'. " @@ -221,9 +221,18 @@ private static MtpServerCapabilities DecodeCapabilities(IDictionary null, + string value => value, + _ => throw new MtpServerClientException( + $"Expected '{JsonRpcStrings.ProtocolVersion}' to be a string but it was '{protocolVersionObj.GetType()}'."), + }; + } + if (result.TryGetValue(JsonRpcStrings.Capabilities, out object? capabilitiesObj) && capabilitiesObj is IDictionary capabilities && capabilities.TryGetValue(JsonRpcStrings.Testing, out object? testingObj) @@ -248,6 +257,11 @@ private static MtpServerCapabilities DecodeCapabilities(IDictionary _options.SupportedProtocolVersions.Count == 0 + ? negotiatedProtocolVersion == JsonRpcProtocolVersions.Current + : _options.SupportedProtocolVersions.Contains(negotiatedProtocolVersion, StringComparer.Ordinal); + private static int? AsInt(object? value) => value switch { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 332248e0e8..846372fd2b 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -192,7 +192,16 @@ or JsonRpcMethods.TestingRunTests deserializers[typeof(InitializeResponseArgs)] = new JsonElementDeserializer( (json, jsonElement) => { - json.TryBind(jsonElement, out string? protocolVersion, JsonRpcStrings.ProtocolVersion); + string? protocolVersion = null; + if (jsonElement.TryGetProperty(JsonRpcStrings.ProtocolVersion, out JsonElement protocolVersionElement) + && protocolVersionElement.ValueKind != JsonValueKind.Null) + { + protocolVersion = protocolVersionElement.ValueKind == JsonValueKind.String + ? protocolVersionElement.GetString() + : throw new MessageFormatException( + $"'{JsonRpcStrings.ProtocolVersion}' field has wrong type (expected String)"); + } + return new InitializeResponseArgs( ProcessId: json.Bind(jsonElement, JsonRpcStrings.ProcessId), ServerInfo: json.Bind(jsonElement, JsonRpcStrings.ServerInfo), diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs index adbcd4e853..a7c3054f6b 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs @@ -60,6 +60,27 @@ public async Task ConnectAsync() } var requestMessage = (RequestMessage)message; + if (requestMessage.Params is not InitializeRequestArgs initializeRequest) + { + await SendErrorAsync( + requestMessage.Id, + ErrorCodes.InvalidParams, + "The initialize request params are invalid.", + _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); + return false; + } + + string? negotiatedProtocolVersion = JsonRpcProtocolVersions.Negotiate(initializeRequest.ProtocolVersions); + if (negotiatedProtocolVersion is null) + { + await SendErrorAsync( + requestMessage.Id, + ErrorCodes.ProtocolVersionNotSupported, + $"None of the client's protocol versions are supported. Server versions: {string.Join(", ", JsonRpcProtocolVersions.Supported)}.", + _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); + return false; + } + var responseObject = new InitializeResponseArgs( ProcessId: _environment.ProcessId, ServerInfo: new ServerInfo("test-anywhere", Version: PlatformVersion.Version), @@ -73,13 +94,24 @@ public async Task ConnectAsync() // This means we're a push node MultiConnectionProvider: true))) { - ProtocolVersion = JsonRpcProtocolVersions.Current, + ProtocolVersion = negotiatedProtocolVersion, }; await SendResponseAsync(requestMessage.Id, responseObject, _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); return true; } + private async Task SendErrorAsync(int reqId, int errorCode, string message, CancellationToken cancellationToken) + { + AssertInitialized(); + + ErrorMessage error = new(reqId, errorCode, message, Data: null); + using (await _messageMonitor.LockAsync(cancellationToken).ConfigureAwait(false)) + { + await _messageHandler.WriteRequestAsync(error, cancellationToken).ConfigureAwait(false); + } + } + private async Task SendResponseAsync(int reqId, object result, CancellationToken cancellationToken) { AssertInitialized(); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index f46b448125..6bdd4bf212 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -58,7 +58,7 @@ or JsonRpcMethods.TestingRunTests // 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, + _ => rawParams is IDictionary ? paramsObj : null, }; } catch (Exception ex) when (ex is MessageFormatException or InvalidCastException) @@ -152,7 +152,14 @@ or JsonRpcMethods.TestingRunTests int processId = GetRequiredPropertyFromJson(properties, JsonRpcStrings.ProcessId); ServerInfo serverInfo = Deserialize(GetRequiredPropertyFromJson>(properties, JsonRpcStrings.ServerInfo)); ServerCapabilities capabilities = Deserialize(GetRequiredPropertyFromJson>(properties, JsonRpcStrings.Capabilities)); - string? protocolVersion = GetOptionalPropertyFromJson(properties, JsonRpcStrings.ProtocolVersion) as string; + object? protocolVersionValue = GetOptionalPropertyFromJson(properties, JsonRpcStrings.ProtocolVersion); + string? protocolVersion = protocolVersionValue switch + { + null => null, + string value => value, + _ => throw new MessageFormatException( + $"'{JsonRpcStrings.ProtocolVersion}' field has wrong type (expected String)"), + }; return new InitializeResponseArgs(processId, serverInfo, capabilities) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs index aeeac99029..a1c1cef765 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs @@ -83,6 +83,9 @@ public FakeMtpServer() /// Gets or sets the response returned for an initialize request. public InitializeResponseArgs InitializeResponse { get; set; } + /// Gets or sets a raw response override for malformed-response tests. + public object? InitializeResponseOverride { get; set; } + /// Gets or sets the response returned for a testing/runTests request. public RunResponseArgs RunResponse { get; set; } = new RunResponseArgs([]); @@ -427,7 +430,7 @@ private async Task HandleClientRequestAsync(RequestMessage request) object? result; if (request.Method == JsonRpcMethods.Initialize) { - result = InitializeResponse; + result = InitializeResponseOverride ?? InitializeResponse; } else if (request.Method == JsonRpcMethods.TestingDiscoverTests) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs index 5449de38aa..2fed7ab69c 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs @@ -73,6 +73,55 @@ public async Task InitializeAsync_UnsupportedNegotiatedProtocolVersion_Throws() Assert.IsNull(client.Capabilities); } + [TestMethod] + public async Task InitializeAsync_EmptySupportedVersions_AcceptsLegacyVersion() + { + using FakeMtpServer server = new(); + using MtpServerClient client = server.ConnectClient(new MtpServerClientOptions + { + SupportedProtocolVersions = [], + }); + + MtpServerCapabilities capabilities = await WithTimeoutAsync( + client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); + + Assert.AreEqual(JsonRpcProtocolVersions.Current, capabilities.ProtocolVersion); + } + + [TestMethod] + public async Task InitializeAsync_NonStringProtocolVersion_Throws() + { + using FakeMtpServer server = new(); + server.InitializeResponseOverride = new Dictionary + { + [JsonRpcStrings.ProcessId] = 4242, + [JsonRpcStrings.ServerInfo] = new Dictionary + { + [JsonRpcStrings.Name] = "FakeMtpServer", + [JsonRpcStrings.Version] = "1.2.3", + }, + [JsonRpcStrings.Capabilities] = new Dictionary + { + [JsonRpcStrings.Testing] = new Dictionary + { + [JsonRpcStrings.SupportsDiscovery] = true, + [JsonRpcStrings.MultiRequestSupport] = true, + [JsonRpcStrings.VSTestProviderSupport] = false, + [JsonRpcStrings.AttachmentsSupport] = true, + [JsonRpcStrings.MultiConnectionProvider] = false, + }, + }, + [JsonRpcStrings.ProtocolVersion] = 1, + }; + using MtpServerClient client = server.ConnectClient(); + + MtpServerClientException exception = await AssertThrowsAsync( + () => client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); + + Assert.Contains(JsonRpcStrings.ProtocolVersion, exception.Message); + Assert.IsNull(client.Capabilities); + } + [TestMethod] public async Task DiscoverTestsAsync_All_SendsDiscoverRequest() { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index ed53971f98..2e61e83a27 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -230,6 +230,47 @@ public void DeserializeKnownRequest_MissingParams_CapturesInvalidParams(string m Assert.AreEqual(ErrorCodes.InvalidParams, invalidParams.ErrorCode); } + [TestMethod] + public void DeserializeUnknownNotification_NonObjectParams_DropsParams() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "method": "testing/unknown", + "params": "not-an-object" + } + """); + + Assert.IsNull(Assert.IsInstanceOfType(message).Params); + } + + [TestMethod] + public void DeserializeInitializeResponse_NonStringProtocolVersion_Throws() + { + const string Json = """ + { + "processId": 1, + "serverInfo": { + "name": "server", + "version": "1.2.3" + }, + "capabilities": { + "testing": { + "supportsDiscovery": true, + "experimental_multiRequestSupport": false, + "vstestProvider": false, + "attachmentsSupport": true, + "multipleConnectionProvider": false + } + }, + "protocolVersion": 1 + } + """; + + Assert.Throws(() => Deserialize(Json)); + } + [TestMethod] public async Task Serialize_TestNodeWithRetryAttempt_EmitsRetryProperties() { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs new file mode 100644 index 0000000000..ec8a9271e1 --- /dev/null +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Testing.Platform.Helpers; +using Microsoft.Testing.Platform.Logging; +using Microsoft.Testing.Platform.ServerMode; +using Microsoft.Testing.Platform.Services; + +using Moq; + +namespace Microsoft.Testing.Platform.UnitTests; + +[TestClass] +public sealed class PassiveNodeTests +{ + [TestMethod] + public async Task ConnectAsync_NegotiatesSupportedProtocolVersion() + { + TestMessageHandler handler = new(CreateInitializeRequest([JsonRpcProtocolVersions.Current])); + using PassiveNode node = CreatePassiveNode(handler); + + Assert.IsTrue(await node.ConnectAsync()); + + ResponseMessage response = Assert.IsInstanceOfType(handler.WrittenMessage); + InitializeResponseArgs result = Assert.IsInstanceOfType(response.Result); + Assert.AreEqual(JsonRpcProtocolVersions.Current, result.ProtocolVersion); + } + + [TestMethod] + public async Task ConnectAsync_RejectsUnsupportedProtocolVersion() + { + TestMessageHandler handler = new(CreateInitializeRequest(["2.0.0"])); + using PassiveNode node = CreatePassiveNode(handler); + + Assert.IsFalse(await node.ConnectAsync()); + + ErrorMessage error = Assert.IsInstanceOfType(handler.WrittenMessage); + Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, error.ErrorCode); + } + + private static PassiveNode CreatePassiveNode(TestMessageHandler handler) + { + var cancellationTokenSource = new Mock(); + cancellationTokenSource.SetupGet(source => source.CancellationToken).Returns(CancellationToken.None); + + var environment = new Mock(); + environment.SetupGet(value => value.ProcessId).Returns(42); + + var logger = new Mock>(); + logger.Setup(value => value.IsEnabled(It.IsAny())).Returns(false); + + return new PassiveNode( + new TestMessageHandlerFactory(handler), + cancellationTokenSource.Object, + environment.Object, + new SystemMonitorAsyncFactory(), + logger.Object); + } + + private static RequestMessage CreateInitializeRequest(string[] protocolVersions) + => new( + 1, + JsonRpcMethods.Initialize, + new InitializeRequestArgs( + 123, + new ClientInfo("test-client", "1.0.0"), + new ClientCapabilities(DebuggerProvider: false, IsStateful: false)) + { + ProtocolVersions = protocolVersions, + }); + + private sealed class TestMessageHandlerFactory(IMessageHandler messageHandler) : IMessageHandlerFactory + { + public Task CreateMessageHandlerAsync(CancellationToken cancellationToken) + => Task.FromResult(messageHandler); + } + + private sealed class TestMessageHandler(RpcMessage message) : IMessageHandler + { + private RpcMessage? _message = message; + + public RpcMessage? WrittenMessage { get; private set; } + + public Task ReadAsync(CancellationToken cancellationToken) + { + RpcMessage? message = _message; + _message = null; + return Task.FromResult(message); + } + + public Task WriteRequestAsync(RpcMessage message, CancellationToken cancellationToken) + { + WrittenMessage = message; + return Task.CompletedTask; + } + } +} From c31b67c702571b15112b9fdb7d752800fe161053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 11:59:36 +0200 Subject: [PATCH 03/25] Clarify legacy protocol fallback Normalize omitted versions to V1, pin the 1.0 schema response version, and reject top-level IDs on notifications. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../server-mode-1.0.schema.json | 7 ++++++- .../Client/MtpServerClient.cs | 8 ++++---- .../ServerMode/JsonRpc/JsonRpcMethods.cs | 2 +- .../MtpServerClientTests.cs | 19 ++++++++++++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index f509b6c7c4..82b8ac297c 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -106,6 +106,11 @@ ] } }, + "not": { + "required": [ + "id" + ] + }, "additionalProperties": true }, "clientCapabilities": { @@ -648,7 +653,7 @@ "additionalProperties": true }, "protocolVersion": { - "type": "string" + "const": "1.0.0" }, "capabilities": { "$ref": "#/$defs/serverCapabilities" diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs index e55175c021..0fe0eb3ffa 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpServerClient.cs @@ -128,11 +128,11 @@ public async Task InitializeAsync(CancellationToken cance ResponseMessage response = await _connection.SendRequestAsync(JsonRpcMethods.Initialize, args, cancellationToken).ConfigureAwait(false); MtpServerCapabilities capabilities = DecodeCapabilities(AsResultDictionary(response.Result)); - if (capabilities.ProtocolVersion is { } negotiatedProtocolVersion - && !IsSupportedProtocolVersion(negotiatedProtocolVersion)) + string effectiveProtocolVersion = capabilities.ProtocolVersion ?? JsonRpcProtocolVersions.V1; + if (!IsSupportedProtocolVersion(effectiveProtocolVersion)) { throw new MtpServerClientException( - $"The server negotiated unsupported protocol version '{negotiatedProtocolVersion}'. " + $"The server negotiated unsupported protocol version '{effectiveProtocolVersion}'. " + $"Supported versions: {string.Join(", ", _options.SupportedProtocolVersions)}."); } @@ -259,7 +259,7 @@ private static MtpServerCapabilities DecodeCapabilities(IDictionary _options.SupportedProtocolVersions.Count == 0 - ? negotiatedProtocolVersion == JsonRpcProtocolVersions.Current + ? negotiatedProtocolVersion == JsonRpcProtocolVersions.V1 : _options.SupportedProtocolVersions.Contains(negotiatedProtocolVersion, StringComparer.Ordinal); private static int? AsInt(object? value) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs index 514d5aafe8..be7e8f7828 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/JsonRpcMethods.cs @@ -29,7 +29,7 @@ internal static class JsonRpcProtocolVersions { if (clientSupportedVersions is null || clientSupportedVersions.Count == 0) { - return Current; + return V1; } IReadOnlyList serverSupportedVersions = Supported; diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs index 2fed7ab69c..f5e4f741a0 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs @@ -59,6 +59,23 @@ public async Task InitializeAsync_LegacyServerWithoutProtocolVersion_Succeeds() Assert.IsNull(capabilities.ProtocolVersion); } + [TestMethod] + public async Task InitializeAsync_LegacyServerWithoutSupportedVersion_Throws() + { + using FakeMtpServer server = new(); + server.InitializeResponse = server.InitializeResponse with { ProtocolVersion = null }; + using MtpServerClient client = server.ConnectClient(new MtpServerClientOptions + { + SupportedProtocolVersions = ["2.0.0"], + }); + + MtpServerClientException exception = await AssertThrowsAsync( + () => client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); + + Assert.Contains(JsonRpcProtocolVersions.V1, exception.Message); + Assert.IsNull(client.Capabilities); + } + [TestMethod] public async Task InitializeAsync_UnsupportedNegotiatedProtocolVersion_Throws() { @@ -85,7 +102,7 @@ public async Task InitializeAsync_EmptySupportedVersions_AcceptsLegacyVersion() MtpServerCapabilities capabilities = await WithTimeoutAsync( client.InitializeAsync(TestContext.CancellationToken)).ConfigureAwait(false); - Assert.AreEqual(JsonRpcProtocolVersions.Current, capabilities.ProtocolVersion); + Assert.AreEqual(JsonRpcProtocolVersions.V1, capabilities.ProtocolVersion); } [TestMethod] From 351c4dd5125dc554ff38443a8a55a457d8793e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 12:25:31 +0200 Subject: [PATCH 04/25] Preserve JSON-RPC string request IDs Retain numeric-string ID representation in responses and publish initialized state only after the initialize response write succeeds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../SerializerUtilities.ClientSerializers.cs | 4 +- .../Hosts/ServerTestHost.MessageLoop.cs | 44 ++++++++++++++++--- .../Hosts/ServerTestHost.Messaging.cs | 18 ++++++-- .../InternalAPI/InternalAPI.Unshipped.txt | 6 +++ .../JsonRpc/Json/Json.Deserializers.cs | 31 ++++++++----- .../JsonRpc/Json/Json.Serializers.cs | 6 +-- .../ServerMode/JsonRpc/PassiveNode.cs | 29 +++++++++--- .../ServerMode/JsonRpc/RpcMessages.cs | 15 +++++-- .../SerializerUtilities.Deserializers.cs | 10 +++-- ...rializerUtilities.RpcMessageSerializers.cs | 6 +-- .../ServerMode/FormatterUtilitiesTests.cs | 17 ++++++- .../ServerMode/PassiveNodeTests.cs | 4 +- .../ServerMode/ServerTests.cs | 3 +- 13 files changed, 148 insertions(+), 45 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs index 4f860df969..b8d3a7fed2 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs @@ -146,7 +146,7 @@ private static void RegisterClientSerializersCore() : GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); return id.HasValue - ? new RequestMessage(id.Value, method, @params) + ? new RequestMessage(id.Value, method, @params) { StringId = idObj as string } : new NotificationMessage(method, @params); } else if (properties.TryGetValue(JsonRpcStrings.Error, out _)) @@ -159,7 +159,7 @@ private static void RegisterClientSerializersCore() var result = resultObj as IDictionary; int id = GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); - return new ResponseMessage(id, result); + return new ResponseMessage(id, result) { StringId = idObj as string }; } throw new MessageFormatException(); diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index 25ffc179f9..23d9a9d8ea 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -166,7 +166,13 @@ private async Task HandleRequestAsync(RequestMessage request, CancellationToken { try { - await SendErrorAsync(reqId: request.Id, errorCode: ErrorCodes.InvalidRequest, message: "Server is closing", data: null, cancellationToken).ConfigureAwait(false); + await SendErrorAsync( + reqId: request.Id, + errorCode: ErrorCodes.InvalidRequest, + message: "Server is closing", + data: null, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { @@ -188,7 +194,8 @@ await SendErrorAsync( errorCode: ErrorCodes.InvalidRequest, message: "The server has already received an initialize request.", data: null, - cancellationToken).ConfigureAwait(false); + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { @@ -207,7 +214,8 @@ await SendErrorAsync( errorCode: ErrorCodes.ServerNotInitialized, message: "The server must be initialized before this request can be processed.", data: null, - cancellationToken).ConfigureAwait(false); + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { @@ -230,12 +238,16 @@ await SendErrorAsync( { object response = await HandleRequestCoreAsync(request, rpcState, cancellationToken).ConfigureAwait(false); testUpdateCompletionSent = await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken).ConfigureAwait(false); + await SendResponseAsync( + reqId: request.Id, + result: response, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); if (isInitializeRequest) { Volatile.Write(ref _initializeState, Initialized); } - await SendResponseAsync(reqId: request.Id, result: response, cancellationToken).ConfigureAwait(false); CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetResult(response)); } catch (OperationCanceledException e) @@ -257,7 +269,13 @@ await SendErrorAsync( ? (string.Empty, ErrorCodes.RequestCanceled) : (e.ToString(), ErrorCodes.RequestCanceled); - await SendErrorAsync(reqId: request.Id, errorCode: errorCode, message: errorMessage, data: null, cancellationToken).ConfigureAwait(false); + await SendErrorAsync( + reqId: request.Id, + errorCode: errorCode, + message: errorMessage, + data: null, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { @@ -278,7 +296,13 @@ await SendErrorAsync( await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken, bestEffort: true).ConfigureAwait(false); } - await SendErrorAsync(reqId: request.Id, errorCode: e.ErrorCode, message: e.Message, data: null, cancellationToken).ConfigureAwait(false); + await SendErrorAsync( + reqId: request.Id, + errorCode: e.ErrorCode, + message: e.Message, + data: null, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { @@ -299,7 +323,13 @@ await SendErrorAsync( await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken, bestEffort: true).ConfigureAwait(false); } - await SendErrorAsync(reqId: request.Id, errorCode: ErrorCodes.InternalError, message: e.ToString(), data: null, cancellationToken).ConfigureAwait(false); + await SendErrorAsync( + reqId: request.Id, + errorCode: ErrorCodes.InternalError, + message: e.ToString(), + data: null, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); } finally { diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs index dfd9437a0a..5409b42abc 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.Messaging.cs @@ -9,10 +9,16 @@ namespace Microsoft.Testing.Platform.Hosts; internal sealed partial class ServerTestHost { - private async Task SendErrorAsync(int reqId, int errorCode, string message, object? data, CancellationToken cancellationToken) + private async Task SendErrorAsync( + int reqId, + int errorCode, + string message, + object? data, + CancellationToken cancellationToken, + string? stringId = null) { AssertInitialized(); - ErrorMessage error = new(reqId, errorCode, message, data); + ErrorMessage error = new(reqId, errorCode, message, data) { StringId = stringId }; using (await _messageMonitor.LockAsync(cancellationToken).ConfigureAwait(false)) { @@ -20,10 +26,14 @@ private async Task SendErrorAsync(int reqId, int errorCode, string message, obje } } - private async Task SendResponseAsync(int reqId, object result, CancellationToken cancellationToken) + private async Task SendResponseAsync( + int reqId, + object result, + CancellationToken cancellationToken, + string? stringId = null) { AssertInitialized(); - ResponseMessage response = new(reqId, result); + ResponseMessage response = new(reqId, result) { StringId = stringId }; using (await _messageMonitor.LockAsync(cancellationToken).ConfigureAwait(false)) { diff --git a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt index dd23eef8cd..925bf385a1 100644 --- a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt @@ -273,6 +273,12 @@ static Microsoft.Testing.Platform.ServerMode.JsonRpcProtocolVersions.Supported.g const Microsoft.Testing.Platform.ServerMode.JsonRpcStrings.ProtocolVersion = "protocolVersion" -> string! const Microsoft.Testing.Platform.ServerMode.JsonRpcStrings.ProtocolVersions = "protocolVersions" -> string! static readonly Microsoft.Testing.Platform.ServerMode.ErrorCodes.ProtocolVersionNotSupported -> int +Microsoft.Testing.Platform.ServerMode.RequestMessage.StringId.get -> string? +Microsoft.Testing.Platform.ServerMode.RequestMessage.StringId.init -> void +Microsoft.Testing.Platform.ServerMode.ResponseMessage.StringId.get -> string? +Microsoft.Testing.Platform.ServerMode.ResponseMessage.StringId.init -> void +Microsoft.Testing.Platform.ServerMode.ErrorMessage.StringId.get -> string? +Microsoft.Testing.Platform.ServerMode.ErrorMessage.StringId.init -> void Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.get -> Microsoft.Testing.Platform.Services.IClientCapabilities! Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.init -> void Microsoft.Testing.Platform.Services.ClientInfoService.ClientInfoService(string! Id, string! Version, Microsoft.Testing.Platform.Services.IClientCapabilities! Capabilities) -> void diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 846372fd2b..c32df692f9 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -90,7 +90,7 @@ private static void RegisterDefaultDeserializers(Dictionary? result = element.ValueKind == JsonValueKind.Null ? null : json.Bind>(jsonElement, JsonRpcStrings.Result); - return new ResponseMessage(id, result); + return new ResponseMessage(id, result) { StringId = stringId }; } return json.TryBind(jsonElement, out ErrorMessage? errorMessage) ? errorMessage! : throw new MessageFormatException(); @@ -289,7 +289,7 @@ or JsonRpcMethods.TestingRunTests }); deserializers[typeof(CancelRequestArgs)] = new JsonElementDeserializer( - (json, jsonElement) => TryGetRpcId(jsonElement, out int id) + (json, jsonElement) => TryGetRpcId(jsonElement, out int id, out _) ? new CancelRequestArgs(id) : throw new MessageFormatException("id field is missing")); @@ -301,7 +301,7 @@ or JsonRpcMethods.TestingRunTests { ValidateJsonRpcHeader(json, jsonElement); - int id = BindRpcId(jsonElement); + int id = BindRpcId(jsonElement, out string? stringId); JsonElement error = jsonElement.GetProperty(JsonRpcStrings.Error); int code = json.Bind(error, JsonRpcStrings.Code); @@ -316,7 +316,10 @@ or JsonRpcMethods.TestingRunTests Id: id, ErrorCode: code, Message: message ?? string.Empty, - Data: data); + Data: data) + { + StringId = stringId, + }; }); } @@ -368,24 +371,32 @@ private static object ReadNumber(JsonElement element) } #pragma warning restore IDE0046 // Convert to conditional expression - private static bool TryGetRpcId(JsonElement jsonElement, out int id) + private static bool TryGetRpcId(JsonElement jsonElement, out int id, out string? stringId) { if (!jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) || idElement.ValueKind == JsonValueKind.Null) { id = default; + stringId = null; return false; } + stringId = idElement.ValueKind == JsonValueKind.String ? idElement.GetString() : null; id = ReadRpcId(idElement); return true; } - private static int BindRpcId(JsonElement jsonElement) + private static int BindRpcId(JsonElement jsonElement, out string? stringId) => jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) - ? ReadRpcId(idElement) + ? ReadRpcIdAndCaptureString(idElement, out stringId) : throw new MessageFormatException($"'{JsonRpcStrings.Id}' field is missing"); + private static int ReadRpcIdAndCaptureString(JsonElement idElement, out string? stringId) + { + stringId = idElement.ValueKind == JsonValueKind.String ? idElement.GetString() : null; + return ReadRpcId(idElement); + } + private static int ReadRpcId(JsonElement idElement) => idElement.ValueKind switch { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs index 7b46527b4d..541b2cb009 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs @@ -15,7 +15,7 @@ private static void RegisterDefaultSerializers(Dictionary serializers[typeof(RequestMessage)] = new JsonObjectSerializer(request => [ (JsonRpcStrings.JsonRpc, "2.0"), - (JsonRpcStrings.Id, request.Id), + (JsonRpcStrings.Id, request.StringId ?? (object)request.Id), (JsonRpcStrings.Method, request.Method), (JsonRpcStrings.Params, request.Params) ]); @@ -23,7 +23,7 @@ private static void RegisterDefaultSerializers(Dictionary serializers[typeof(ResponseMessage)] = new JsonObjectSerializer(response => [ (JsonRpcStrings.JsonRpc, "2.0"), - (JsonRpcStrings.Id, response.Id), + (JsonRpcStrings.Id, response.StringId ?? (object)response.Id), (JsonRpcStrings.Result, response.Result) ]); @@ -46,7 +46,7 @@ private static void RegisterDefaultSerializers(Dictionary return [ (JsonRpcStrings.JsonRpc, "2.0"), - (JsonRpcStrings.Id, error.Id), + (JsonRpcStrings.Id, error.StringId ?? (object)error.Id), (JsonRpcStrings.Error, errorMsg) ]; }); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs index a7c3054f6b..a721566b42 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs @@ -66,7 +66,8 @@ await SendErrorAsync( requestMessage.Id, ErrorCodes.InvalidParams, "The initialize request params are invalid.", - _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); + _testApplicationCancellationTokenSource.CancellationToken, + requestMessage.StringId).ConfigureAwait(false); return false; } @@ -77,7 +78,8 @@ await SendErrorAsync( requestMessage.Id, ErrorCodes.ProtocolVersionNotSupported, $"None of the client's protocol versions are supported. Server versions: {string.Join(", ", JsonRpcProtocolVersions.Supported)}.", - _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); + _testApplicationCancellationTokenSource.CancellationToken, + requestMessage.StringId).ConfigureAwait(false); return false; } @@ -97,26 +99,39 @@ await SendErrorAsync( ProtocolVersion = negotiatedProtocolVersion, }; - await SendResponseAsync(requestMessage.Id, responseObject, _testApplicationCancellationTokenSource.CancellationToken).ConfigureAwait(false); + await SendResponseAsync( + requestMessage.Id, + responseObject, + _testApplicationCancellationTokenSource.CancellationToken, + requestMessage.StringId).ConfigureAwait(false); return true; } - private async Task SendErrorAsync(int reqId, int errorCode, string message, CancellationToken cancellationToken) + private async Task SendErrorAsync( + int reqId, + int errorCode, + string message, + CancellationToken cancellationToken, + string? stringId) { AssertInitialized(); - ErrorMessage error = new(reqId, errorCode, message, Data: null); + ErrorMessage error = new(reqId, errorCode, message, Data: null) { StringId = stringId }; using (await _messageMonitor.LockAsync(cancellationToken).ConfigureAwait(false)) { await _messageHandler.WriteRequestAsync(error, cancellationToken).ConfigureAwait(false); } } - private async Task SendResponseAsync(int reqId, object result, CancellationToken cancellationToken) + private async Task SendResponseAsync( + int reqId, + object result, + CancellationToken cancellationToken, + string? stringId) { AssertInitialized(); - ResponseMessage response = new(reqId, result); + ResponseMessage response = new(reqId, result) { StringId = stringId }; using (await _messageMonitor.LockAsync(cancellationToken).ConfigureAwait(false)) { await _messageHandler.WriteRequestAsync(response, cancellationToken).ConfigureAwait(false); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs index a8a717bce4..f01c7117e2 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs @@ -12,7 +12,10 @@ internal abstract record RpcMessage; /// A request is a message for which the server should return a corresponding /// or . /// -internal sealed record RequestMessage(int Id, string Method, object? Params) : RpcMessage; +internal sealed record RequestMessage(int Id, string Method, object? Params) : RpcMessage +{ + public string? StringId { get; init; } +} /// /// A notification message is a message that notifies the server of an event. @@ -24,7 +27,10 @@ internal sealed record NotificationMessage(string Method, object? Params) : RpcM /// /// An error message is sent if some exception was thrown when processing the request. /// -internal sealed record ErrorMessage(int Id, int ErrorCode, string Message, object? Data) : RpcMessage; +internal sealed record ErrorMessage(int Id, int ErrorCode, string Message, object? Data) : RpcMessage +{ + public string? StringId { get; init; } +} /// /// An response message is sent if a request is handled successfully. @@ -33,7 +39,10 @@ internal sealed record ErrorMessage(int Id, int ErrorCode, string Message, objec /// If the RPC handler returns a the /// will be returned as null. /// -internal sealed record ResponseMessage(int Id, object? Result) : RpcMessage; +internal sealed record ResponseMessage(int Id, object? Result) : RpcMessage +{ + public string? StringId { get; init; } +} internal sealed record InitializeRequestArgs(int ProcessId, ClientInfo ClientInfo, ClientCapabilities Capabilities) { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index 6bdd4bf212..b7e97d059f 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -29,6 +29,7 @@ private static void RegisterDeserializers() int? id = idObj is null ? null : GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); + string? stringId = idObj as string; object? @params; object? rawParams = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Params); @@ -75,7 +76,7 @@ or JsonRpcMethods.TestingRunTests } return id.HasValue - ? new RequestMessage(id.Value, method, @params) + ? new RequestMessage(id.Value, method, @params) { StringId = stringId } : new NotificationMessage(method, @params); } else if (properties.TryGetValue(JsonRpcStrings.Error, out object? errorObj)) @@ -92,7 +93,7 @@ or JsonRpcMethods.TestingRunTests int id = GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); - return new ResponseMessage(id, paramsObj); + return new ResponseMessage(id, paramsObj) { StringId = idObj as string }; } throw new MessageFormatException(); @@ -314,7 +315,10 @@ or JsonRpcMethods.TestingRunTests Id: id, ErrorCode: code, Message: errorMessage, - Data: data); + Data: data) + { + StringId = idObj as string, + }; }); } } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs index d3b8dbf5ee..52710fdcaf 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs @@ -19,7 +19,7 @@ private static void RegisterRpcMessageSerializers() Dictionary values = new() { [JsonRpcStrings.JsonRpc] = "2.0", - [JsonRpcStrings.Id] = req.Id, + [JsonRpcStrings.Id] = req.StringId ?? (object)req.Id, [JsonRpcStrings.Method] = req.Method, [JsonRpcStrings.Params] = req.Params is null ? null : SerializeObject(req.Params), }; @@ -32,7 +32,7 @@ private static void RegisterRpcMessageSerializers() Dictionary values = new() { [JsonRpcStrings.JsonRpc] = "2.0", - [JsonRpcStrings.Id] = res.Id, + [JsonRpcStrings.Id] = res.StringId ?? (object)res.Id, [JsonRpcStrings.Result] = res.Result is null ? null : SerializeObject(res.Result), }; @@ -56,7 +56,7 @@ private static void RegisterRpcMessageSerializers() Dictionary values = new() { [JsonRpcStrings.JsonRpc] = "2.0", - [JsonRpcStrings.Id] = error.Id, + [JsonRpcStrings.Id] = error.StringId ?? (object)error.Id, [JsonRpcStrings.Error] = new Dictionary { [JsonRpcStrings.Code] = error.ErrorCode, diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index 2e61e83a27..7d5f92ebfe 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -116,7 +116,22 @@ public void CanDeserializeNumericStringRequestId() } """); - Assert.AreEqual(42, Assert.IsInstanceOfType(message).Id); + RequestMessage request = Assert.IsInstanceOfType(message); + Assert.AreEqual(42, request.Id); + Assert.AreEqual("42", request.StringId); + } + + [TestMethod] + public async Task NumericStringId_IsPreservedInResponsesAndErrors() + { + ResponseMessage response = new(42, Result: null) { StringId = "42" }; + ErrorMessage error = new(42, ErrorCodes.InvalidRequest, "invalid", Data: null) { StringId = "42" }; + + string serializedResponse = (await _formatter.SerializeAsync(response)).Replace(" ", string.Empty); + string serializedError = (await _formatter.SerializeAsync(error)).Replace(" ", string.Empty); + + Assert.Contains("\"id\":\"42\"", serializedResponse); + Assert.Contains("\"id\":\"42\"", serializedError); } [TestMethod] diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs index ec8a9271e1..75593aaca4 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs @@ -16,13 +16,15 @@ public sealed class PassiveNodeTests [TestMethod] public async Task ConnectAsync_NegotiatesSupportedProtocolVersion() { - TestMessageHandler handler = new(CreateInitializeRequest([JsonRpcProtocolVersions.Current])); + RequestMessage request = CreateInitializeRequest([JsonRpcProtocolVersions.Current]) with { StringId = "1" }; + TestMessageHandler handler = new(request); using PassiveNode node = CreatePassiveNode(handler); Assert.IsTrue(await node.ConnectAsync()); ResponseMessage response = Assert.IsInstanceOfType(handler.WrittenMessage); InitializeResponseArgs result = Assert.IsInstanceOfType(response.Result); + Assert.AreEqual("1", response.StringId); Assert.AreEqual(JsonRpcProtocolVersions.Current, result.ProtocolVersion); } diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 712dc002b9..b9ae83a7a8 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -247,7 +247,7 @@ await WriteMessageAsync( """ { "jsonrpc": "2.0", - "id": 4, + "id": "4", "method": "testing/unknown", "params": {} } @@ -259,6 +259,7 @@ await WriteMessageAsync( "Wait method-not-found error", timeout.Token))!; Assert.AreEqual(ErrorCodes.MethodNotFound, methodNotFoundError.ErrorCode); + Assert.AreEqual("4", methodNotFoundError.StringId); await WriteMessageAsync( writer, From d6258ae47d82ed687074c3afdd4b6cfdf344535d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 12:27:57 +0200 Subject: [PATCH 05/25] Restore serializer source BOMs Re-encode the edited serializer files as UTF-8 with BOM per repository policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs | 2 +- .../JsonRpc/SerializerUtilities.RpcMessageSerializers.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index b7e97d059f..90ce4c28e2 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. // Note: System.Text.Json is only available in .NET 6.0 and above. diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs index 52710fdcaf..8ccf21ad01 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. // Note: System.Text.Json is only available in .NET 6.0 and above. From 40fa63510aac95af5e38aea1d1d9d49c41e3e5a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 12:48:05 +0200 Subject: [PATCH 06/25] Close remaining JSON-RPC parity gaps Preserve string IDs on client replies, queue requests during initialization, validate Jsonite optional fields, and support primitive error data. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../Client/MtpJsonRpcConnection.cs | 4 +- .../Hosts/ServerTestHost.MessageLoop.cs | 77 ++++++++++++++----- .../Hosts/ServerTestHost.cs | 2 + .../JsonRpc/Json/Json.Deserializers.cs | 18 ++++- .../SerializerUtilities.Deserializers.cs | 48 +++++++++--- .../FakeMtpServer.cs | 7 +- .../MtpServerClientTests.cs | 12 +++ .../ServerMode/FormatterUtilitiesTests.cs | 43 +++++++++++ .../ServerMode/ServerTests.cs | 76 ++++++++++++++++++ 9 files changed, 256 insertions(+), 31 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs index f60c5206b4..b96ba849e4 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs @@ -258,7 +258,9 @@ private async Task HandleServerRequestAsync(RequestMessage request, Cancellation // Always answer so the server is never left waiting. try { - await WriteMessageAsync(new ResponseMessage(request.Id, result), cancellationToken).ConfigureAwait(false); + await WriteMessageAsync( + new ResponseMessage(request.Id, result) { StringId = request.StringId }, + cancellationToken).ConfigureAwait(false); } catch (Exception ex) { diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index 23d9a9d8ea..ce3d05d173 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -183,29 +183,57 @@ await SendErrorAsync( else { bool isInitializeRequest = request.Method == JsonRpcMethods.Initialize; - if (isInitializeRequest) + bool rejectRequest; + Task? initializationTask = null; + lock (_initializeStateLock) { - if (Interlocked.CompareExchange(ref _initializeState, Initializing, NotInitialized) != NotInitialized) + if (isInitializeRequest) { - try + rejectRequest = _initializeState != NotInitialized; + if (!rejectRequest) { - await SendErrorAsync( - reqId: request.Id, - errorCode: ErrorCodes.InvalidRequest, - message: "The server has already received an initialize request.", - data: null, - cancellationToken, - stringId: request.StringId).ConfigureAwait(false); + _initializeState = Initializing; + _initializationCompletionSource = new(TaskCreationOptions.RunContinuationsAsynchronously); } - finally + } + else + { + rejectRequest = _initializeState == NotInitialized; + if (_initializeState == Initializing) { - _requestCounter.Signal(); + RoslynDebug.Assert(_initializationCompletionSource is not null); + initializationTask = _initializationCompletionSource.Task; } + } + } - return; + if (isInitializeRequest && rejectRequest) + { + try + { + await SendErrorAsync( + reqId: request.Id, + errorCode: ErrorCodes.InvalidRequest, + message: "The server has already received an initialize request.", + data: null, + cancellationToken, + stringId: request.StringId).ConfigureAwait(false); + } + finally + { + _requestCounter.Signal(); } + + return; } - else if (Volatile.Read(ref _initializeState) != Initialized) + + if (initializationTask is not null) + { + bool initialized = await initializationTask.ConfigureAwait(false); + rejectRequest = !initialized; + } + + if (!isInitializeRequest && rejectRequest) { try { @@ -245,7 +273,7 @@ await SendResponseAsync( stringId: request.StringId).ConfigureAwait(false); if (isInitializeRequest) { - Volatile.Write(ref _initializeState, Initialized); + CompleteInitialization(success: true); } CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetResult(response)); @@ -254,7 +282,7 @@ await SendResponseAsync( { if (isInitializeRequest) { - Volatile.Write(ref _initializeState, NotInitialized); + CompleteInitialization(success: false); } try @@ -286,7 +314,7 @@ await SendErrorAsync( { if (isInitializeRequest) { - Volatile.Write(ref _initializeState, NotInitialized); + CompleteInitialization(success: false); } try @@ -313,7 +341,7 @@ await SendErrorAsync( { if (isInitializeRequest) { - Volatile.Write(ref _initializeState, NotInitialized); + CompleteInitialization(success: false); } try @@ -339,6 +367,19 @@ await SendErrorAsync( } } + private void CompleteInitialization(bool success) + { + TaskCompletionSource? completionSource; + lock (_initializeStateLock) + { + _initializeState = success ? Initialized : NotInitialized; + completionSource = _initializationCompletionSource; + _initializationCompletionSource = null; + } + + completionSource?.TrySetResult(success); + } + private async Task SendTestUpdateCompleteIfNeededAsync( RequestMessage request, CancellationToken cancellationToken, diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs index e5c349e6fa..598eb0038b 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs @@ -39,6 +39,7 @@ internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDis // We start by one so we can wait all other requests private readonly CountdownEvent _requestCounter = new(1); private readonly IClock _clock; + private readonly object _initializeStateLock = new(); // In-flight requests from the client to the server. // The client can cancel these requests at any time. @@ -52,6 +53,7 @@ internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDis private IMessageHandler? _messageHandler; private TestHost.ClientInfo? _client; private IClientInfo? _clientInfoService; + private TaskCompletionSource? _initializationCompletionSource; private int _initializeState; public ServerTestHost( diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index c32df692f9..1a4a098ef9 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -307,7 +307,10 @@ or JsonRpcMethods.TestingRunTests int code = json.Bind(error, JsonRpcStrings.Code); string message = json.Bind(error, JsonRpcStrings.Message); - if (json.TryBind(error, out IDictionary? data, JsonRpcStrings.Data) && data?.Count == 0) + object? data = error.TryGetProperty(JsonRpcStrings.Data, out JsonElement dataElement) + ? ReadUntypedValue(json, dataElement) + : null; + if (data is IDictionary { Count: 0 }) { data = null; } @@ -371,6 +374,19 @@ private static object ReadNumber(JsonElement element) } #pragma warning restore IDE0046 // Convert to conditional expression + private static object? ReadUntypedValue(Json json, JsonElement element) + => element.ValueKind switch + { + JsonValueKind.String => element.GetString(), + JsonValueKind.Number => ReadNumber(element), + JsonValueKind.True => true, + JsonValueKind.False => false, + JsonValueKind.Object => json.Bind>(element), + JsonValueKind.Array => json.Bind(element), + JsonValueKind.Null => null, + _ => throw new MessageFormatException($"Unsupported JSON value kind '{element.ValueKind}'"), + }; + private static bool TryGetRpcId(JsonElement jsonElement, out int id, out string? stringId) { if (!jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index 90ce4c28e2..a61b835b97 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -200,11 +200,8 @@ or JsonRpcMethods.TestingRunTests throw new MessageFormatException(JsonRpcStrings.InvalidRunIdErrorMessage); } - var testsJson = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Tests) as ICollection; - - ICollection? tests = testsJson?.OfType>()?.Select(obj => Deserialize(obj)).ToList(); - - string? filter = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Filter) as string; + ICollection? tests = DeserializeOptionalTestNodes(properties); + string? filter = GetOptionalTypedProperty(properties, JsonRpcStrings.Filter); return new DiscoverRequestArgs(runId, tests, filter); }); @@ -217,10 +214,8 @@ or JsonRpcMethods.TestingRunTests throw new MessageFormatException(JsonRpcStrings.InvalidRunIdErrorMessage); } - var testsJson = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Tests) as ICollection; - - ICollection? tests = testsJson?.OfType>().Select(obj => Deserialize(obj)).ToList(); - string? filter = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Filter) as string; + ICollection? tests = DeserializeOptionalTestNodes(properties); + string? filter = GetOptionalTypedProperty(properties, JsonRpcStrings.Filter); return new RunRequestArgs(runId, tests, filter); }); @@ -321,4 +316,39 @@ or JsonRpcMethods.TestingRunTests }; }); } + + private static ICollection? DeserializeOptionalTestNodes(IDictionary properties) + { + ICollection? testsJson = GetOptionalTypedProperty>(properties, JsonRpcStrings.Tests); + if (testsJson is null) + { + return null; + } + + List tests = []; + foreach (object? testJson in testsJson) + { + if (testJson is not IDictionary testProperties) + { + throw new MessageFormatException($"'{JsonRpcStrings.Tests}' entries must be objects"); + } + + tests.Add(Deserialize(testProperties)); + } + + return tests; + } + + private static T? GetOptionalTypedProperty(IDictionary properties, string propertyName) + where T : class + { + object? value = GetOptionalPropertyFromJson(properties, propertyName); + return value switch + { + null => null, + T typed => typed, + _ => throw new MessageFormatException( + $"'{propertyName}' field has wrong type (expected {typeof(T).Name})"), + }; + } } diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs index a1c1cef765..f9afc20050 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs @@ -226,7 +226,7 @@ public Task SendAttachmentAsync(string uri, string producer, string type, string /// answers. Params are null because the client keeps request params as a raw dictionary and the tests only /// assert on the method name and the returned result. /// - public Task SendServerRequestAsync(string method) + public Task SendServerRequestAsync(string method, bool useStringId = false) { int id = Interlocked.Increment(ref _nextServerRequestId); var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -235,7 +235,10 @@ public Task SendServerRequestAsync(string method) _pendingServerRequests[id] = tcs; } - _ = WriteAsync(new RequestMessage(id, method, null)); + _ = WriteAsync(new RequestMessage(id, method, null) + { + StringId = useStringId ? id.ToString(CultureInfo.InvariantCulture) : null, + }); return tcs.Task; } diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs index f5e4f741a0..d2ba4b8a82 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs @@ -553,6 +553,18 @@ public async Task ServerInitiatedRequest_NoHandler_RespondsWithNull() Assert.IsNull(response.Result); } + [TestMethod] + public async Task ServerInitiatedRequest_NumericStringId_PreservesResponseIdRepresentation() + { + using FakeMtpServer server = new(); + using MtpServerClient client = await ConnectAndInitializeAsync(server).ConfigureAwait(false); + + ResponseMessage response = await WithTimeoutAsync( + server.SendServerRequestAsync(ClientAttachDebuggerMethod, useStringId: true)).ConfigureAwait(false); + + Assert.AreEqual(response.Id.ToString(CultureInfo.InvariantCulture), response.StringId); + } + [TestMethod] public async Task ServerInitiatedRequest_WithHandler_InvokesHandler() { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index 7d5f92ebfe..3f3da2c153 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -260,6 +260,49 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() Assert.IsNull(Assert.IsInstanceOfType(message).Params); } + [DataRow("\"filter\": 42")] + [DataRow("\"tests\": \"not-an-array\"")] + [DataRow("\"tests\": [42]")] + [TestMethod] + public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) + { + RpcMessage message = Deserialize( + $$""" + { + "jsonrpc": "2.0", + "id": 1, + "method": "testing/runTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000001", + {{property}} + } + } + """); + + RequestMessage request = Assert.IsInstanceOfType(message); + InvalidRequestParamsArgs invalidParams = Assert.IsInstanceOfType(request.Params); + Assert.AreEqual(ErrorCodes.InvalidParams, invalidParams.ErrorCode); + } + + [TestMethod] + public void DeserializeError_PrimitiveData_PreservesValue() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "id": 1, + "error": { + "code": -32600, + "message": "invalid", + "data": "detail" + } + } + """); + + Assert.AreEqual("detail", Assert.IsInstanceOfType(message).Data); + } + [TestMethod] public void DeserializeInitializeResponse_NonStringProtocolVersion_Throws() { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index b9ae83a7a8..4325e24b03 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -302,6 +302,82 @@ await WriteMessageAsync( Assert.AreEqual(0, await serverTask); } + [TestMethod] + public async Task PipelinedRequestWaitsForInitializeResponse() + { + using var server = TcpServer.Create(); + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = context => + { + context.Complete(); + return Task.CompletedTask; + }, + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002" + } + } + """); + + _ = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 1 }, + "Wait initialize response", + timeout.Token); + RpcMessage? discoveryResponse = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 2 } or ErrorMessage { Id: 2 }, + "Wait pipelined discovery response", + timeout.Token); + Assert.IsInstanceOfType(discoveryResponse); + + await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); + Assert.AreEqual(0, await serverTask); + } + [TestMethod] public async Task RunRequestWithEmptyTests_PreservesEmptyUidSelection() { From d4697d643eae088deeb2b3b723bc015c1c980ef1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 13:02:53 +0200 Subject: [PATCH 07/25] Align initialize process ID documentation Document the required integer process ID used by the protocol 1.0 schema and implementation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- docs/mstest-runner-protocol/001-protocol-intro.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 732f70d400..163237e6e5 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -203,10 +203,8 @@ Request: ```typescript interface InitializeParams { - // The process Id of the parent process that started the server. Is null if - // the process has not been started by another process. If the parent - // process is not alive then the server should exit (see exit notification) - // its process. + // The process ID of the client process that started the server. + // Protocol 1.0 requires this value to be an integer. processId: PID, clientInfo: { From c4d5e075eee66b977fd30071e23b1ea981c1e868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 13:08:55 +0200 Subject: [PATCH 08/25] Harden queued request validation Register pipelined requests before initialization waits and align null version entries and test-node validation across serializers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../Hosts/ServerTestHost.MessageLoop.cs | 26 +++++- .../JsonRpc/Json/Json.Deserializers.cs | 9 ++ .../SerializerUtilities.Deserializers.cs | 19 +--- .../ServerMode/FormatterUtilitiesTests.cs | 4 + .../ServerMode/ServerTests.cs | 88 +++++++++++++++++++ 5 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index ce3d05d173..fcaef72b30 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -227,8 +227,12 @@ await SendErrorAsync( return; } + RpcInvocationState? rpcState = null; + bool requestRegistered = false; if (initializationTask is not null) { + rpcState = new RpcInvocationState(); + requestRegistered = _clientToServerRequests.TryAdd(request.Id, rpcState); bool initialized = await initializationTask.ConfigureAwait(false); rejectRequest = !initialized; } @@ -247,7 +251,20 @@ await SendErrorAsync( } finally { - _requestCounter.Signal(); + if (requestRegistered) + { + var exception = new JsonRpcException( + ErrorCodes.ServerNotInitialized, + "The server must be initialized before this request can be processed."); + CompleteRequest( + ref _clientToServerRequests, + request.Id, + completion => completion.TrySetException(exception)); + } + else + { + _requestCounter.Signal(); + } } return; @@ -255,8 +272,11 @@ await SendErrorAsync( // We enqueue the request before to "unlink" the current thread so we're sure that we // correctly handle the completion also after the "exit" - RpcInvocationState rpcState = new(); - _clientToServerRequests.TryAdd(request.Id, rpcState); + rpcState ??= new RpcInvocationState(); + if (!requestRegistered) + { + _clientToServerRequests.TryAdd(request.Id, rpcState); + } // Note: Yield, so that the main message reading loop can continue. await Task.Yield(); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 1a4a098ef9..45de3bfd4f 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -163,6 +163,15 @@ or JsonRpcMethods.TestingRunTests deserializers[typeof(InitializeRequestArgs)] = new JsonElementDeserializer((json, jsonElement) => { json.TryArrayBind(jsonElement, out string[]? protocolVersions, JsonRpcStrings.ProtocolVersions); + if (protocolVersions is not null) + { + for (int i = 0; i < protocolVersions.Length; i++) + { + protocolVersions[i] = protocolVersions[i] + ?? throw new MessageFormatException($"'{JsonRpcStrings.ProtocolVersions}' entries must be strings"); + } + } + return new InitializeRequestArgs( ProcessId: json.Bind(jsonElement, JsonRpcStrings.ProcessId), ClientInfo: json.Bind(jsonElement, JsonRpcStrings.ClientInfo), diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index a61b835b97..7bc094f11b 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -223,25 +223,10 @@ or JsonRpcMethods.TestingRunTests Deserializers[typeof(TestNode)] = new ObjectDeserializer( properties => { - string uid = string.Empty; - string displayName = string.Empty; + string uid = GetRequiredPropertyFromJson(properties, JsonRpcStrings.Uid); + string displayName = GetRequiredPropertyFromJson(properties, JsonRpcStrings.DisplayName); PropertyBag propertyBag = new(); - foreach (KeyValuePair kvp in properties) - { - if (kvp.Key == JsonRpcStrings.Uid) - { - uid = kvp.Value as string ?? string.Empty; - continue; - } - - if (kvp.Key == JsonRpcStrings.DisplayName) - { - displayName = kvp.Value as string ?? string.Empty; - continue; - } - } - if (properties.TryGetValue("location.file", out object? location_file)) { ApplicationStateGuard.Ensure(location_file is not null); diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index 3f3da2c153..bedec9f0cf 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -197,6 +197,7 @@ as InitializeRequestArgs [DataRow("\"1.0.0\"")] [DataRow("[1]")] + [DataRow("[null]")] [TestMethod] public void DeserializeInitializeRequest_InvalidProtocolVersions_CapturesInvalidParams(string protocolVersions) { @@ -263,6 +264,9 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() [DataRow("\"filter\": 42")] [DataRow("\"tests\": \"not-an-array\"")] [DataRow("\"tests\": [42]")] + [DataRow("\"tests\": [{}]")] + [DataRow("\"tests\": [{\"uid\": 42, \"display-name\": \"Test\"}]")] + [DataRow("\"tests\": [{\"uid\": \"test\"}]")] [TestMethod] public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 4325e24b03..f7178ed52e 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -378,6 +378,94 @@ await WriteMessageAsync( Assert.AreEqual(0, await serverTask); } + [TestMethod] + public async Task PipelinedRequestCanBeCanceledWhileInitializationCompletes() + { + using var server = TcpServer.Create(); + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = context => Task.Delay(Timeout.Infinite, context.CancellationToken), + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002" + } + } + """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": 2 + } + } + """); + + _ = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 1 }, + "Wait initialize response", + timeout.Token); + _ = await WaitForMessage( + messageHandler, + IsTestUpdateCompletion, + "Wait canceled discovery completion", + timeout.Token); + var cancellationError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 2 }, + "Wait canceled discovery error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.RequestCanceled, cancellationError.ErrorCode); + + await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); + Assert.AreEqual(0, await serverTask); + } + [TestMethod] public async Task RunRequestWithEmptyTests_PreservesEmptyUidSelection() { From 2816344b1e893d23a8a5b6d1ac802664b01ddd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 13:15:56 +0200 Subject: [PATCH 09/25] Reject null test-node identity fields Align System.Text.Json with Jsonite and the protocol schema for required test-node strings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../ServerMode/JsonRpc/Json/Json.Deserializers.cs | 6 ++++-- .../ServerMode/FormatterUtilitiesTests.cs | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 45de3bfd4f..06f4a30f4c 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -275,8 +275,10 @@ or JsonRpcMethods.TestingRunTests (json, properties) => { PropertyBag propertyBag = new(); - string uid = json.Bind(properties, JsonRpcStrings.Uid) ?? string.Empty; - string displayName = json.Bind(properties, JsonRpcStrings.DisplayName); + string uid = json.Bind(properties, JsonRpcStrings.Uid) + ?? throw new MessageFormatException($"'{JsonRpcStrings.Uid}' field cannot be null"); + string displayName = json.Bind(properties, JsonRpcStrings.DisplayName) + ?? throw new MessageFormatException($"'{JsonRpcStrings.DisplayName}' field cannot be null"); if (json.TryBind(properties, out string? locationFile, "location.file")) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index bedec9f0cf..092107d196 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -267,6 +267,8 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() [DataRow("\"tests\": [{}]")] [DataRow("\"tests\": [{\"uid\": 42, \"display-name\": \"Test\"}]")] [DataRow("\"tests\": [{\"uid\": \"test\"}]")] + [DataRow("\"tests\": [{\"uid\": null, \"display-name\": \"Test\"}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": null}]")] [TestMethod] public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) { From dae2cb2c57768fed622ece53483d8bedd46f60d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 13:30:05 +0200 Subject: [PATCH 10/25] Align test-node identity validation Reject blank UIDs consistently and document protocol lifecycle behavior across initialization states. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- docs/mstest-runner-protocol/001-protocol-intro.md | 12 ++++++++---- .../ServerMode/JsonRpc/Json/Json.Deserializers.cs | 4 ++++ .../JsonRpc/SerializerUtilities.Deserializers.cs | 5 +++++ .../ServerMode/FormatterUtilitiesTests.cs | 2 ++ 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 163237e6e5..a6fc0d75fe 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -185,10 +185,14 @@ what the client supports and limit functionality based on unsupported features. > Since the capabilities are fetched by sending an RPC request to a started executable, these can only be queried by the client after > the project was successfully built. -The `initialize` request MUST be the first request sent on a connection. Before initialization completes, -the server rejects requests with `ServerNotInitialized` (`-32002`) and ignores notifications other than -`exit` and `$/cancelRequest`. A second `initialize` request on an initialized connection is rejected with `InvalidRequest` -(`-32600`). If initialization fails, the client may correct the request and try again. +The `initialize` request MUST be the first request sent on a connection: + +- Before `initialize` is received, requests are rejected with `ServerNotInitialized` (`-32002`) and + notifications other than `exit` and `$/cancelRequest` are ignored. +- While initialization is in progress, non-initialize requests wait for it to complete. A second + `initialize` request is rejected with `InvalidRequest` (`-32600`). +- After initialization succeeds, normal requests are processed and another `initialize` request is rejected. + If initialization fails, the client may correct the request and try again. ### Determine capabilities during test runner initialization diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 06f4a30f4c..3110e85934 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -279,6 +279,10 @@ or JsonRpcMethods.TestingRunTests ?? throw new MessageFormatException($"'{JsonRpcStrings.Uid}' field cannot be null"); string displayName = json.Bind(properties, JsonRpcStrings.DisplayName) ?? throw new MessageFormatException($"'{JsonRpcStrings.DisplayName}' field cannot be null"); + if (RoslynString.IsNullOrWhiteSpace(uid)) + { + throw new MessageFormatException($"'{JsonRpcStrings.Uid}' field cannot be empty or whitespace"); + } if (json.TryBind(properties, out string? locationFile, "location.file")) { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index 7bc094f11b..d2aafd03df 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -225,6 +225,11 @@ or JsonRpcMethods.TestingRunTests { string uid = GetRequiredPropertyFromJson(properties, JsonRpcStrings.Uid); string displayName = GetRequiredPropertyFromJson(properties, JsonRpcStrings.DisplayName); + if (RoslynString.IsNullOrWhiteSpace(uid)) + { + throw new MessageFormatException($"'{JsonRpcStrings.Uid}' field cannot be empty or whitespace"); + } + PropertyBag propertyBag = new(); if (properties.TryGetValue("location.file", out object? location_file)) diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index 092107d196..ff437c4887 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -269,6 +269,8 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() [DataRow("\"tests\": [{\"uid\": \"test\"}]")] [DataRow("\"tests\": [{\"uid\": null, \"display-name\": \"Test\"}]")] [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": null}]")] + [DataRow("\"tests\": [{\"uid\": \"\", \"display-name\": \"Test\"}]")] + [DataRow("\"tests\": [{\"uid\": \" \", \"display-name\": \"Test\"}]")] [TestMethod] public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) { From e4ecd46a98e71759ab2917ed48c6f8ccf39688c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 13:52:37 +0200 Subject: [PATCH 11/25] Separate JSON-RPC ID correlation types Keep numeric and string IDs distinct, order failed initialization responses, and align final schema constraints with the runtime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../server-mode-1.0.schema.json | 20 ++- .../Hosts/ServerTestHost.MessageLoop.cs | 81 ++++++---- .../Hosts/ServerTestHost.cs | 4 +- .../InternalAPI/InternalAPI.Unshipped.txt | 2 + .../JsonRpc/Json/Json.Deserializers.cs | 4 +- .../JsonRpc/Json/Json.Serializers.cs | 2 +- .../ServerMode/JsonRpc/RpcMessages.cs | 5 +- .../SerializerUtilities.Deserializers.cs | 2 +- ...rializerUtilities.RpcMessageSerializers.cs | 2 +- .../ServerMode/FormatterUtilitiesTests.cs | 4 +- .../ServerMode/ServerTests.cs | 153 +++++++++++++++++- 11 files changed, 234 insertions(+), 45 deletions(-) diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index 82b8ac297c..c46683720a 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -201,7 +201,9 @@ ], "properties": { "processId": { - "type": "integer" + "type": "integer", + "minimum": -2147483648, + "maximum": 2147483647 }, "clientInfo": { "type": "object", @@ -366,7 +368,8 @@ "properties": { "uid": { "type": "string", - "minLength": 1 + "minLength": 1, + "pattern": "\\S" }, "display-name": { "type": "string" @@ -634,7 +637,9 @@ ], "properties": { "processId": { - "type": "integer" + "type": "integer", + "minimum": -2147483648, + "maximum": 2147483647 }, "serverInfo": { "type": "object", @@ -653,7 +658,14 @@ "additionalProperties": true }, "protocolVersion": { - "const": "1.0.0" + "oneOf": [ + { + "const": "1.0.0" + }, + { + "type": "null" + } + ] }, "capabilities": { "$ref": "#/$defs/serverCapabilities" diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index fcaef72b30..abc5f11552 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -78,12 +78,18 @@ private async Task HandleMessagesAsync(CancellationToken cancellationToken) _ = HandleNotificationAsync(notification, _serverClosingTokenSource.Token); break; case ResponseMessage response: - CompleteRequest(ref _serverToClientRequests, response.Id, completion => completion.TrySetResult(response)); + CompleteRequest( + ref _serverToClientRequests, + GetRequestKey(response.Id, response.StringId), + completion => completion.TrySetResult(response)); break; case ErrorMessage error: RemoteInvocationException exception = new(error.ErrorCode, error.Message, error.Data); - CompleteRequest(ref _serverToClientRequests, error.Id, completion => completion.TrySetException(exception)); + CompleteRequest( + ref _serverToClientRequests, + GetRequestKey(error.Id, error.StringId), + completion => completion.TrySetException(exception)); break; } } @@ -133,7 +139,9 @@ private async Task HandleNotificationAsync(NotificationMessage message, Cancella switch (message.Method, message.Params) { case (JsonRpcMethods.CancelRequest, CancelRequestArgs args): - if (_clientToServerRequests.TryGetValue(args.CancelRequestId, out RpcInvocationState? rpcState)) + if (_clientToServerRequests.TryGetValue( + GetRequestKey(args.CancelRequestId, args.StringId), + out RpcInvocationState? rpcState)) { Exception? cancellationException = rpcState.CancelRequest(); if (cancellationException is not null) @@ -232,7 +240,9 @@ await SendErrorAsync( if (initializationTask is not null) { rpcState = new RpcInvocationState(); - requestRegistered = _clientToServerRequests.TryAdd(request.Id, rpcState); + requestRegistered = _clientToServerRequests.TryAdd( + GetRequestKey(request.Id, request.StringId), + rpcState); bool initialized = await initializationTask.ConfigureAwait(false); rejectRequest = !initialized; } @@ -258,7 +268,7 @@ await SendErrorAsync( "The server must be initialized before this request can be processed."); CompleteRequest( ref _clientToServerRequests, - request.Id, + GetRequestKey(request.Id, request.StringId), completion => completion.TrySetException(exception)); } else @@ -275,7 +285,7 @@ await SendErrorAsync( rpcState ??= new RpcInvocationState(); if (!requestRegistered) { - _clientToServerRequests.TryAdd(request.Id, rpcState); + _clientToServerRequests.TryAdd(GetRequestKey(request.Id, request.StringId), rpcState); } // Note: Yield, so that the main message reading loop can continue. @@ -296,15 +306,13 @@ await SendResponseAsync( CompleteInitialization(success: true); } - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetResult(response)); + CompleteRequest( + ref _clientToServerRequests, + GetRequestKey(request.Id, request.StringId), + completion => completion.TrySetResult(response)); } catch (OperationCanceledException e) { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } - try { if (!testUpdateCompletionSent) @@ -327,16 +335,19 @@ await SendErrorAsync( } finally { - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetCanceled()); + if (isInitializeRequest) + { + CompleteInitialization(success: false); + } + + CompleteRequest( + ref _clientToServerRequests, + GetRequestKey(request.Id, request.StringId), + completion => completion.TrySetCanceled()); } } catch (JsonRpcException e) { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } - try { if (!testUpdateCompletionSent) @@ -354,16 +365,19 @@ await SendErrorAsync( } finally { - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetException(e)); + if (isInitializeRequest) + { + CompleteInitialization(success: false); + } + + CompleteRequest( + ref _clientToServerRequests, + GetRequestKey(request.Id, request.StringId), + completion => completion.TrySetException(e)); } } catch (Exception e) { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } - try { if (!testUpdateCompletionSent) @@ -381,7 +395,15 @@ await SendErrorAsync( } finally { - CompleteRequest(ref _clientToServerRequests, request.Id, completion => completion.TrySetException(e)); + if (isInitializeRequest) + { + CompleteInitialization(success: false); + } + + CompleteRequest( + ref _clientToServerRequests, + GetRequestKey(request.Id, request.StringId), + completion => completion.TrySetException(e)); } } } @@ -415,13 +437,13 @@ private async Task SendTestUpdateCompleteIfNeededAsync( } private void CompleteRequest( - ref ConcurrentDictionary rpcStates, - int reqId, + ref ConcurrentDictionary<(int Id, bool IsString), RpcInvocationState> rpcStates, + (int Id, bool IsString) requestKey, Action> completion) { try { - if (rpcStates.TryRemove(reqId, out RpcInvocationState? completedInvocation)) + if (rpcStates.TryRemove(requestKey, out RpcInvocationState? completedInvocation)) { completion(completedInvocation.CompletionSource); completedInvocation.Dispose(); @@ -441,6 +463,9 @@ private void CompleteRequest( } } + private static (int Id, bool IsString) GetRequestKey(int id, string? stringId) + => (id, stringId is not null); + private sealed class RpcInvocationState : IDisposable { #if NET9_0_OR_GREATER diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs index 598eb0038b..9300322306 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs @@ -44,12 +44,12 @@ internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDis // In-flight requests from the client to the server. // The client can cancel these requests at any time. // When the server completes the handler it will complete the backing RpcRequest. - private ConcurrentDictionary _clientToServerRequests; + private ConcurrentDictionary<(int Id, bool IsString), RpcInvocationState> _clientToServerRequests; // In-flight requests from the server to the client. // Whenever a client responds with a result or an error, the requests // get completed. - private ConcurrentDictionary _serverToClientRequests; + private ConcurrentDictionary<(int Id, bool IsString), RpcInvocationState> _serverToClientRequests; private IMessageHandler? _messageHandler; private TestHost.ClientInfo? _client; private IClientInfo? _clientInfoService; diff --git a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt index 925bf385a1..d002cd9d01 100644 --- a/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt +++ b/src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt @@ -279,6 +279,8 @@ Microsoft.Testing.Platform.ServerMode.ResponseMessage.StringId.get -> string? Microsoft.Testing.Platform.ServerMode.ResponseMessage.StringId.init -> void Microsoft.Testing.Platform.ServerMode.ErrorMessage.StringId.get -> string? Microsoft.Testing.Platform.ServerMode.ErrorMessage.StringId.init -> void +Microsoft.Testing.Platform.ServerMode.CancelRequestArgs.StringId.get -> string? +Microsoft.Testing.Platform.ServerMode.CancelRequestArgs.StringId.init -> void Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.get -> Microsoft.Testing.Platform.Services.IClientCapabilities! Microsoft.Testing.Platform.Services.ClientInfoService.Capabilities.init -> void Microsoft.Testing.Platform.Services.ClientInfoService.ClientInfoService(string! Id, string! Version, Microsoft.Testing.Platform.Services.IClientCapabilities! Capabilities) -> void diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 3110e85934..07a06a0aaa 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -304,8 +304,8 @@ or JsonRpcMethods.TestingRunTests }); deserializers[typeof(CancelRequestArgs)] = new JsonElementDeserializer( - (json, jsonElement) => TryGetRpcId(jsonElement, out int id, out _) - ? new CancelRequestArgs(id) + (json, jsonElement) => TryGetRpcId(jsonElement, out int id, out string? stringId) + ? new CancelRequestArgs(id) { StringId = stringId } : throw new MessageFormatException("id field is missing")); deserializers[typeof(ExitRequestArgs)] = new JsonElementDeserializer( diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs index 541b2cb009..03dd9495a9 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs @@ -126,7 +126,7 @@ response.ProtocolVersion is null serializers[typeof(CancelRequestArgs)] = new JsonObjectSerializer(request => [ - (JsonRpcStrings.Id, request.CancelRequestId) + (JsonRpcStrings.Id, request.StringId ?? (object)request.CancelRequestId) ]); serializers[typeof(TelemetryEventArgs)] = new JsonObjectSerializer(ev => diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs index f01c7117e2..557f56e1d8 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcMessages.cs @@ -92,7 +92,10 @@ public override string ToString() internal sealed record Artifact(string Uri, string Producer, string Type, string DisplayName, string? Description = null); -internal sealed record CancelRequestArgs(int CancelRequestId); +internal sealed record CancelRequestArgs(int CancelRequestId) +{ + public string? StringId { get; init; } +} internal sealed record ExitRequestArgs; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index d2aafd03df..bc8dbf4fd8 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -259,7 +259,7 @@ or JsonRpcMethods.TestingRunTests object? idObj = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Id); int id = GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); - return new CancelRequestArgs(id); + return new CancelRequestArgs(id) { StringId = idObj as string }; }); Deserializers[typeof(ExitRequestArgs)] = new ObjectDeserializer(_ => new ExitRequestArgs()); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs index 8ccf21ad01..318009163c 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs @@ -126,7 +126,7 @@ private static void RegisterRpcMessageSerializers() { Dictionary values = new() { - [JsonRpcStrings.Id] = ev.CancelRequestId, + [JsonRpcStrings.Id] = ev.StringId ?? (object)ev.CancelRequestId, }; return values; diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index ff437c4887..d3b568948d 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -149,7 +149,9 @@ public void CanDeserializeNumericStringCancellationId() """); NotificationMessage notification = Assert.IsInstanceOfType(message); - Assert.AreEqual(42, Assert.IsInstanceOfType(notification.Params).CancelRequestId); + CancelRequestArgs args = Assert.IsInstanceOfType(notification.Params); + Assert.AreEqual(42, args.CancelRequestId); + Assert.AreEqual("42", args.StringId); } [TestMethod] diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index f7178ed52e..fe2ec45c33 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -201,13 +201,30 @@ await WriteMessageAsync( } } """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 20, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000020" + } + } + """); + + ErrorMessage incompatibleVersionError = Assert.IsInstanceOfType( + await messageHandler.ReadAsync(timeout.Token)); + Assert.AreEqual(2, incompatibleVersionError.Id); + Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, incompatibleVersionError.ErrorCode); - var incompatibleVersionError = (ErrorMessage)(await WaitForMessage( + var queuedRequestError = (ErrorMessage)(await WaitForMessage( messageHandler, - rpcMessage => rpcMessage is ErrorMessage { Id: 2 }, - "Wait incompatible protocol error", + rpcMessage => rpcMessage is ErrorMessage { Id: 20 }, + "Wait queued request error", timeout.Token))!; - Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, incompatibleVersionError.ErrorCode); + Assert.AreEqual(ErrorCodes.ServerNotInitialized, queuedRequestError.ErrorCode); const string initializeMessage = """ { @@ -466,6 +483,134 @@ await WriteMessageAsync( Assert.AreEqual(0, await serverTask); } + [TestMethod] + public async Task NumericAndStringRequestIdsHaveIndependentCancellation() + { + using var server = TcpServer.Create(); + TaskCompletionSource bothRequestsStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource releaseNumericRequest = new(TaskCreationOptions.RunContinuationsAsynchronously); + int startedRequestCount = 0; + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = async context => + { + TreeNodeFilter filter = Assert.IsInstanceOfType( + Assert.IsInstanceOfType(context.Request).Filter); + if (Interlocked.Increment(ref startedRequestCount) == 2) + { + bothRequestsStarted.TrySetResult(true); + } + + if (filter.Filter == "/string") + { + await Task.Delay(Timeout.Infinite, context.CancellationToken); + } + else + { + await releaseNumericRequest.Task; + context.Complete(); + } + }, + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + _ = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 1 }, + "Wait initialize response", + timeout.Token); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002", + "filter": "/numeric" + } + } + """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": "2", + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000003", + "filter": "/string" + } + } + """); + await bothRequestsStarted.Task.TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout, timeout.Token); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": "2" + } + } + """); + + var stringRequestError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 2, StringId: "2" }, + "Wait string request cancellation", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.RequestCanceled, stringRequestError.ErrorCode); + + releaseNumericRequest.TrySetResult(true); + var numericResponse = (ResponseMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 2, StringId: null }, + "Wait numeric request response", + timeout.Token))!; + Assert.IsNull(numericResponse.StringId); + + await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); + Assert.AreEqual(0, await serverTask); + } + [TestMethod] public async Task RunRequestWithEmptyTests_PreservesEmptyUidSelection() { From 1a08b8d334250a8f2490ed23aff27e47b88f3ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 14:10:13 +0200 Subject: [PATCH 12/25] Restore strict scalar null validation Limit null-as-absent behavior to arrays and validate nullable location payloads consistently across serializers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../Hosts/ServerTestHost.cs | 4 +++ .../JsonRpc/Json/Json.Deserializers.cs | 7 ++++- .../ServerMode/JsonRpc/Json/Json.cs | 3 +- .../SerializerUtilities.Deserializers.cs | 28 +++++++++++++------ .../ServerMode/FormatterUtilitiesTests.cs | 2 ++ 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs index 9300322306..31eed8a1b4 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs @@ -39,7 +39,11 @@ internal sealed partial class ServerTestHost : CommonHost, IServerTestHost, IDis // We start by one so we can wait all other requests private readonly CountdownEvent _requestCounter = new(1); private readonly IClock _clock; +#if NET9_0_OR_GREATER + private readonly Lock _initializeStateLock = new(); +#else private readonly object _initializeStateLock = new(); +#endif // In-flight requests from the client to the server. // The client can cancel these requests at any time. diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 07a06a0aaa..85b6c97a10 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -286,11 +286,16 @@ or JsonRpcMethods.TestingRunTests if (json.TryBind(properties, out string? locationFile, "location.file")) { + if (locationFile is null) + { + throw new MessageFormatException("'location.file' field cannot be null"); + } + json.TryBind(properties, out int locationLineStart, "location.line-start"); json.TryBind(properties, out int locationLineEnd, "location.line-end"); TestFileLocationProperty testFileLocationProperty = new( - locationFile!, + locationFile, new LinePositionSpan(new LinePosition(locationLineStart, 0), new LinePosition(locationLineEnd, 0))); propertyBag.Add(testFileLocationProperty); } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs index 4372dee7a7..637c69d6e4 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs @@ -94,8 +94,7 @@ internal T Bind(JsonElement element, string? property = null) internal bool TryBind(JsonElement element, out T? value, string? property = null) { - if (property is not null - && (!element.TryGetProperty(property, out element) || element.ValueKind == JsonValueKind.Null)) + if (property is not null && !element.TryGetProperty(property, out element)) { value = default; return false; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index bc8dbf4fd8..2544a44d95 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -7,7 +7,6 @@ using Jsonite; #endif using Microsoft.Testing.Platform.Extensions.Messages; -using Microsoft.Testing.Platform.Helpers; namespace Microsoft.Testing.Platform.ServerMode; @@ -232,16 +231,29 @@ or JsonRpcMethods.TestingRunTests PropertyBag propertyBag = new(); - if (properties.TryGetValue("location.file", out object? location_file)) + if (properties.TryGetValue("location.file", out object? locationFileValue)) { - ApplicationStateGuard.Ensure(location_file is not null); - if (properties.TryGetValue("location.line-start", out object? location_lineStart) && properties.TryGetValue("location.line-end", out object? location_lineEnd)) + if (locationFileValue is not string locationFile) { - ApplicationStateGuard.Ensure(location_lineStart is not null); - ApplicationStateGuard.Ensure(location_lineEnd is not null); + throw new MessageFormatException("'location.file' field has wrong type (expected String)"); + } + + bool hasLineStart = properties.TryGetValue("location.line-start", out object? locationLineStartValue); + bool hasLineEnd = properties.TryGetValue("location.line-end", out object? locationLineEndValue); + if (hasLineStart || hasLineEnd) + { + if (!hasLineStart || locationLineStartValue is not int locationLineStart + || !hasLineEnd || locationLineEndValue is not int locationLineEnd) + { + throw new MessageFormatException( + "'location.line-start' and 'location.line-end' fields must both be integers"); + } + TestFileLocationProperty testFileLocationProperty = new( - (string)location_file, - new LinePositionSpan(new LinePosition((int)location_lineStart, 0), new LinePosition((int)location_lineEnd, 0))); + locationFile, + new LinePositionSpan( + new LinePosition(locationLineStart, 0), + new LinePosition(locationLineEnd, 0))); propertyBag.Add(testFileLocationProperty); } } diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index d3b568948d..ce01225638 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -273,6 +273,8 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": null}]")] [DataRow("\"tests\": [{\"uid\": \"\", \"display-name\": \"Test\"}]")] [DataRow("\"tests\": [{\"uid\": \" \", \"display-name\": \"Test\"}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": null}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": \"file\", \"location.line-start\": null, \"location.line-end\": 2}]")] [TestMethod] public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) { From cb9c84329f65a1497a58830ac7caf5b81527f46c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 14:29:25 +0200 Subject: [PATCH 13/25] Define complete test-node locations Require file and both line endpoints together across serializers and the protocol schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../server-mode-1.0.schema.json | 14 +++++++++ .../JsonRpc/Json/Json.Deserializers.cs | 15 ++++++++-- .../SerializerUtilities.Deserializers.cs | 30 ++++++++++--------- .../ServerMode/FormatterUtilitiesTests.cs | 3 ++ 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index c46683720a..b21d6c2e91 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -415,6 +415,20 @@ "type": "boolean" } }, + "dependentRequired": { + "location.file": [ + "location.line-start", + "location.line-end" + ], + "location.line-start": [ + "location.file", + "location.line-end" + ], + "location.line-end": [ + "location.file", + "location.line-start" + ] + }, "additionalProperties": true }, "testUpdateNotification": { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 85b6c97a10..6459541356 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -291,14 +291,25 @@ or JsonRpcMethods.TestingRunTests throw new MessageFormatException("'location.file' field cannot be null"); } - json.TryBind(properties, out int locationLineStart, "location.line-start"); - json.TryBind(properties, out int locationLineEnd, "location.line-end"); + bool hasLineStart = json.TryBind(properties, out int locationLineStart, "location.line-start"); + bool hasLineEnd = json.TryBind(properties, out int locationLineEnd, "location.line-end"); + if (!hasLineStart || !hasLineEnd) + { + throw new MessageFormatException( + "'location.file', 'location.line-start', and 'location.line-end' fields must be specified together"); + } TestFileLocationProperty testFileLocationProperty = new( locationFile, new LinePositionSpan(new LinePosition(locationLineStart, 0), new LinePosition(locationLineEnd, 0))); propertyBag.Add(testFileLocationProperty); } + else if (properties.TryGetProperty("location.line-start", out _) + || properties.TryGetProperty("location.line-end", out _)) + { + throw new MessageFormatException( + "'location.file', 'location.line-start', and 'location.line-end' fields must be specified together"); + } return new TestNode { diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index 2544a44d95..d4fe09bdff 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -240,22 +240,24 @@ or JsonRpcMethods.TestingRunTests bool hasLineStart = properties.TryGetValue("location.line-start", out object? locationLineStartValue); bool hasLineEnd = properties.TryGetValue("location.line-end", out object? locationLineEndValue); - if (hasLineStart || hasLineEnd) + if (!hasLineStart || locationLineStartValue is not int locationLineStart + || !hasLineEnd || locationLineEndValue is not int locationLineEnd) { - if (!hasLineStart || locationLineStartValue is not int locationLineStart - || !hasLineEnd || locationLineEndValue is not int locationLineEnd) - { - throw new MessageFormatException( - "'location.line-start' and 'location.line-end' fields must both be integers"); - } - - TestFileLocationProperty testFileLocationProperty = new( - locationFile, - new LinePositionSpan( - new LinePosition(locationLineStart, 0), - new LinePosition(locationLineEnd, 0))); - propertyBag.Add(testFileLocationProperty); + throw new MessageFormatException( + "'location.file', 'location.line-start', and 'location.line-end' fields must be specified together as strings and integers"); } + + TestFileLocationProperty testFileLocationProperty = new( + locationFile, + new LinePositionSpan( + new LinePosition(locationLineStart, 0), + new LinePosition(locationLineEnd, 0))); + propertyBag.Add(testFileLocationProperty); + } + else if (properties.ContainsKey("location.line-start") || properties.ContainsKey("location.line-end")) + { + throw new MessageFormatException( + "'location.file', 'location.line-start', and 'location.line-end' fields must be specified together"); } return new TestNode diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index ce01225638..d46823c341 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -275,6 +275,9 @@ public void DeserializeUnknownNotification_NonObjectParams_DropsParams() [DataRow("\"tests\": [{\"uid\": \" \", \"display-name\": \"Test\"}]")] [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": null}]")] [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": \"file\", \"location.line-start\": null, \"location.line-end\": 2}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": \"file\"}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.file\": \"file\", \"location.line-start\": 1}]")] + [DataRow("\"tests\": [{\"uid\": \"test\", \"display-name\": \"Test\", \"location.line-start\": 1, \"location.line-end\": 2}]")] [TestMethod] public void DeserializeRunRequest_InvalidOptionalPropertyType_CapturesInvalidParams(string property) { From 14f50f595213a6852d90bdc423ebc3becaa55272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 14:45:07 +0200 Subject: [PATCH 14/25] Align protocol location line bounds Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- docs/mstest-runner-protocol/server-mode-1.0.schema.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index b21d6c2e91..cfc3757c06 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -397,11 +397,13 @@ }, "location.line-start": { "type": "integer", - "minimum": 0 + "minimum": -2147483648, + "maximum": 2147483647 }, "location.line-end": { "type": "integer", - "minimum": 0 + "minimum": -2147483648, + "maximum": 2147483647 }, "time.duration-ms": { "type": "number", From dc74cfa591d852ecc99e7980f0fc0a403ba789b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 14:52:40 +0200 Subject: [PATCH 15/25] Close remaining protocol contract gaps Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../001-protocol-intro.md | 6 +- .../server-mode-1.0.schema.json | 4 +- .../ServerMode/ServerTests.cs | 113 ++++++++++++------ 3 files changed, 82 insertions(+), 41 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index a6fc0d75fe..a1d820ef02 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -223,8 +223,8 @@ interface InitializeParams { // OPTIONAL for compatibility with clients predating protocol negotiation. // The server selects its most preferred mutually supported version. - // If omitted or empty, the server uses the legacy base protocol (1.0.0). - protocolVersions?: string[], + // If omitted, null, or empty, the server uses the legacy base protocol (1.0.0). + protocolVersions?: string[] | null, capabilities: { // Note: Since the initialize message is compatible with the LSP protocol, @@ -543,7 +543,7 @@ Notifications: // These should be processed in order and also should be complete, // i.e. a server will send an update for the test, if it already sent updates // for all of the parent nodes. - changes: TestUpdateChange[] + changes: TestUpdateChange[] | null // Run id for which the notification is sent. It should match the id sent during the discovery request. runId: GUID diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index cfc3757c06..5b3ea5084b 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -720,7 +720,9 @@ ], "properties": { "code": { - "type": "integer" + "type": "integer", + "minimum": -2147483648, + "maximum": 2147483647 }, "message": { "type": "string" diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index fe2ec45c33..93994f40ad 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -399,10 +399,11 @@ await WriteMessageAsync( public async Task PipelinedRequestCanBeCanceledWhileInitializationCompletes() { using var server = TcpServer.Create(); + using var testFrameworkCapabilities = new BlockingTestFrameworkCapabilities(); string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); - builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + builder.RegisterTestFramework(_ => testFrameworkCapabilities, (_, __) => new MockTestAdapter { DiscoveryAction = context => Task.Delay(Timeout.Infinite, context.CancellationToken), }); @@ -420,47 +421,56 @@ public async Task PipelinedRequestCanBeCanceledWhileInitializationCompletes() serverToClientStream: client.GetStream(), FormatterUtilities.CreateFormatter()); - await WriteMessageAsync( - writer, - """ - { - "jsonrpc": "2.0", - "id": 1, - "method": "initialize", - "params": { - "processId": 32, - "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, - "capabilities": { - "testing": { - "debuggerProvider": false + testFrameworkCapabilities.BlockNextAccess(); + try + { + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } } } } - } - """); - await WriteMessageAsync( - writer, - """ - { - "jsonrpc": "2.0", - "id": 2, - "method": "testing/discoverTests", - "params": { - "runId": "00000000-0000-0000-0000-000000000002" + """); + await testFrameworkCapabilities.WaitUntilBlockedAsync().WaitAsync(timeout.Token); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002" + } } - } - """); - await WriteMessageAsync( - writer, - """ - { - "jsonrpc": "2.0", - "method": "$/cancelRequest", - "params": { - "id": 2 + """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": 2 + } } - } - """); + """); + } + finally + { + testFrameworkCapabilities.Release(); + } _ = await WaitForMessage( messageHandler, @@ -1139,6 +1149,35 @@ private sealed class MockTestAdapter : ITestFramework public Task ExecuteRequestAsync(ExecuteRequestContext context) => DiscoveryAction is not null ? DiscoveryAction(context) : Task.CompletedTask; } + private sealed class BlockingTestFrameworkCapabilities : ITestFrameworkCapabilities, IDisposable + { + private readonly TaskCompletionSource _blocked = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly ManualResetEventSlim _release = new(initialState: false); + private int _blockNextAccess; + + public IReadOnlyCollection Capabilities + { + get + { + if (Interlocked.Exchange(ref _blockNextAccess, 0) == 1) + { + _blocked.TrySetResult(true); + _release.Wait(); + } + + return []; + } + } + + public void BlockNextAccess() => Volatile.Write(ref _blockNextAccess, 1); + + public Task WaitUntilBlockedAsync() => _blocked.Task; + + public void Release() => _release.Set(); + + public void Dispose() => _release.Dispose(); + } + private sealed record ServerRequestState( IStopPoliciesService StopPoliciesService, ITestApplicationProcessExitCode TestApplicationResult, From 94c2ff531b6a1de856497e079e81a64b67b555d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 15:01:50 +0200 Subject: [PATCH 16/25] Prevent canceled queued request setup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../mstest-runner-protocol/001-protocol-intro.md | 5 ++++- .../Hosts/ServerTestHost.MessageLoop.cs | 1 + .../ServerMode/JsonRpc/PassiveNode.cs | 11 +++++++++++ .../ServerMode/PassiveNodeTests.cs | 16 ++++++++++++++++ .../ServerMode/ServerTests.cs | 8 +++++++- 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index a1d820ef02..2a70addfd7 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -726,8 +726,11 @@ Notification: - params: `CancelParams` defined as follows: ```typescript +type NumericStringId = `${bigint}`; + interface CancelParams { - id: number; + // String IDs use the canonical decimal integer representation and signed 32-bit range. + id: number | NumericStringId; } ``` diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index abc5f11552..acad26439c 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -294,6 +294,7 @@ await SendErrorAsync( bool testUpdateCompletionSent = false; try { + rpcState.CancellationToken.ThrowIfCancellationRequested(); object response = await HandleRequestCoreAsync(request, rpcState, cancellationToken).ConfigureAwait(false); testUpdateCompletionSent = await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken).ConfigureAwait(false); await SendResponseAsync( diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs index a721566b42..a5987ce8f9 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs @@ -60,6 +60,17 @@ public async Task ConnectAsync() } var requestMessage = (RequestMessage)message; + if (requestMessage.Method != JsonRpcMethods.Initialize) + { + await SendErrorAsync( + requestMessage.Id, + ErrorCodes.ServerNotInitialized, + "The server must be initialized before this request can be processed.", + _testApplicationCancellationTokenSource.CancellationToken, + requestMessage.StringId).ConfigureAwait(false); + return false; + } + if (requestMessage.Params is not InitializeRequestArgs initializeRequest) { await SendErrorAsync( diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs index 75593aaca4..852ec543f2 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs @@ -40,6 +40,22 @@ public async Task ConnectAsync_RejectsUnsupportedProtocolVersion() Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, error.ErrorCode); } + [TestMethod] + public async Task ConnectAsync_RejectsNonInitializeRequestBeforeInitialization() + { + RequestMessage request = new( + 1, + JsonRpcMethods.TestingDiscoverTests, + new DiscoverRequestArgs(Guid.NewGuid(), TestNodes: null, GraphFilter: null)); + TestMessageHandler handler = new(request); + using PassiveNode node = CreatePassiveNode(handler); + + Assert.IsFalse(await node.ConnectAsync()); + + ErrorMessage error = Assert.IsInstanceOfType(handler.WrittenMessage); + Assert.AreEqual(ErrorCodes.ServerNotInitialized, error.ErrorCode); + } + private static PassiveNode CreatePassiveNode(TestMessageHandler handler) { var cancellationTokenSource = new Mock(); diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 93994f40ad..1562646c30 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -400,12 +400,17 @@ public async Task PipelinedRequestCanBeCanceledWhileInitializationCompletes() { using var server = TcpServer.Create(); using var testFrameworkCapabilities = new BlockingTestFrameworkCapabilities(); + int discoveryInvocationCount = 0; string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); builder.RegisterTestFramework(_ => testFrameworkCapabilities, (_, __) => new MockTestAdapter { - DiscoveryAction = context => Task.Delay(Timeout.Infinite, context.CancellationToken), + DiscoveryAction = context => + { + Interlocked.Increment(ref discoveryInvocationCount); + return Task.Delay(Timeout.Infinite, context.CancellationToken); + }, }); var testApplication = (TestApplication)await builder.BuildAsync(); testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); @@ -488,6 +493,7 @@ await WriteMessageAsync( "Wait canceled discovery error", timeout.Token))!; Assert.AreEqual(ErrorCodes.RequestCanceled, cancellationError.ErrorCode); + Assert.AreEqual(0, Volatile.Read(ref discoveryInvocationCount)); await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); Assert.AreEqual(0, await serverTask); From c6871a461ad6502732ea597b51b070afd16c43de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 15:17:13 +0200 Subject: [PATCH 17/25] Separate response IDs and initialization release Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../Client/MtpJsonRpcConnection.cs | 20 +++++---- .../Hosts/ServerTestHost.MessageLoop.cs | 45 ++++++++++++------- .../FakeMtpServer.cs | 30 +++++++++++++ .../MtpServerClientTests.cs | 19 ++++++++ .../ServerMode/ServerTests.cs | 29 +++++++----- 5 files changed, 107 insertions(+), 36 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs index b96ba849e4..e4c23dd811 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs @@ -19,7 +19,7 @@ internal sealed class MtpJsonRpcConnection : IDisposable { private readonly IMessageHandler _handler; private readonly IMtpClientLogger _logger; - private readonly ConcurrentDictionary _pendingRequests = new(); + private readonly ConcurrentDictionary<(int Id, bool IsString), PendingRequest> _pendingRequests = new(); private readonly SemaphoreSlim _writeLock = new(1, 1); private readonly CancellationTokenSource _readLoopCancellation = new(); private readonly object _startLock = new(); @@ -113,15 +113,16 @@ public async Task SendRequestAsync(string method, object? @para } int id = Interlocked.Increment(ref _nextRequestId); + (int Id, bool IsString) requestKey = GetRequestKey(id, stringId: null); var pending = new PendingRequest(method); - _pendingRequests[id] = pending; + _pendingRequests[requestKey] = pending; // Re-check after registering: the read loop may have latched a terminal reason and run // FailAllPending between the check above and this insert, missing this entry. Observing the reason // here guarantees the request is completed rather than left waiting. if (Volatile.Read(ref _closedReason) is { } closedAfter) { - _pendingRequests.TryRemove(id, out _); + _pendingRequests.TryRemove(requestKey, out _); throw closedAfter; } @@ -135,7 +136,7 @@ public async Task SendRequestAsync(string method, object? @para } finally { - _pendingRequests.TryRemove(id, out _); + _pendingRequests.TryRemove(requestKey, out _); } } @@ -202,7 +203,7 @@ private void Dispatch(RpcMessage message, CancellationToken cancellationToken) switch (message) { case ResponseMessage response: - if (_pendingRequests.TryGetValue(response.Id, out PendingRequest? successful)) + if (_pendingRequests.TryGetValue(GetRequestKey(response.Id, response.StringId), out PendingRequest? successful)) { successful.Completion.TrySetResult(response); } @@ -210,7 +211,7 @@ private void Dispatch(RpcMessage message, CancellationToken cancellationToken) break; case ErrorMessage error: - if (_pendingRequests.TryGetValue(error.Id, out PendingRequest? failed)) + if (_pendingRequests.TryGetValue(GetRequestKey(error.Id, error.StringId), out PendingRequest? failed)) { failed.Completion.TrySetException(new MtpServerErrorException(error.ErrorCode, error.Message)); } @@ -270,7 +271,7 @@ await WriteMessageAsync( private void CancelPendingRequest(int id, CancellationToken cancellationToken) { - if (!_pendingRequests.TryGetValue(id, out PendingRequest? pending)) + if (!_pendingRequests.TryGetValue(GetRequestKey(id, stringId: null), out PendingRequest? pending)) { return; } @@ -307,7 +308,7 @@ private void Close(Exception reason) private void FailAllPending(Exception exception) { - foreach (KeyValuePair entry in _pendingRequests) + foreach (KeyValuePair<(int Id, bool IsString), PendingRequest> entry in _pendingRequests) { if (_pendingRequests.TryRemove(entry.Key, out PendingRequest? pending)) { @@ -316,6 +317,9 @@ private void FailAllPending(Exception exception) } } + private static (int Id, bool IsString) GetRequestKey(int id, string? stringId) + => (id, stringId is not null); + public void Dispose() { Task? readLoop; diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index acad26439c..b9a906575f 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -304,7 +304,7 @@ await SendResponseAsync( stringId: request.StringId).ConfigureAwait(false); if (isInitializeRequest) { - CompleteInitialization(success: true); + CompleteInitialization(); } CompleteRequest( @@ -314,6 +314,9 @@ await SendResponseAsync( } catch (OperationCanceledException e) { + TaskCompletionSource? failedInitialization = isInitializeRequest + ? MakeInitializationRetryable() + : null; try { if (!testUpdateCompletionSent) @@ -336,10 +339,7 @@ await SendErrorAsync( } finally { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } + failedInitialization?.TrySetResult(false); CompleteRequest( ref _clientToServerRequests, @@ -349,6 +349,9 @@ await SendErrorAsync( } catch (JsonRpcException e) { + TaskCompletionSource? failedInitialization = isInitializeRequest + ? MakeInitializationRetryable() + : null; try { if (!testUpdateCompletionSent) @@ -366,10 +369,7 @@ await SendErrorAsync( } finally { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } + failedInitialization?.TrySetResult(false); CompleteRequest( ref _clientToServerRequests, @@ -379,6 +379,9 @@ await SendErrorAsync( } catch (Exception e) { + TaskCompletionSource? failedInitialization = isInitializeRequest + ? MakeInitializationRetryable() + : null; try { if (!testUpdateCompletionSent) @@ -396,10 +399,7 @@ await SendErrorAsync( } finally { - if (isInitializeRequest) - { - CompleteInitialization(success: false); - } + failedInitialization?.TrySetResult(false); CompleteRequest( ref _clientToServerRequests, @@ -410,17 +410,30 @@ await SendErrorAsync( } } - private void CompleteInitialization(bool success) + private void CompleteInitialization() { TaskCompletionSource? completionSource; lock (_initializeStateLock) { - _initializeState = success ? Initialized : NotInitialized; + _initializeState = Initialized; completionSource = _initializationCompletionSource; _initializationCompletionSource = null; } - completionSource?.TrySetResult(success); + completionSource?.TrySetResult(true); + } + + private TaskCompletionSource? MakeInitializationRetryable() + { + lock (_initializeStateLock) + { + RoslynDebug.Assert(_initializeState == Initializing); + RoslynDebug.Assert(_initializationCompletionSource is not null); + _initializeState = NotInitialized; + TaskCompletionSource? completionSource = _initializationCompletionSource; + _initializationCompletionSource = null; + return completionSource; + } } private async Task SendTestUpdateCompleteIfNeededAsync( diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs index f9afc20050..d4fec7b565 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/FakeMtpServer.cs @@ -242,6 +242,36 @@ public Task SendServerRequestAsync(string method, bool useStrin return tcs.Task; } + /// Waits for and returns a client request with the given method. + public async Task WaitForRequestAsync(string method, TimeSpan timeout) + { + DateTime deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + lock (_receivedRequestsLock) + { + foreach (RequestMessage request in _receivedRequests) + { + if (request.Method == method) + { + return request; + } + } + } + + await Task.Delay(15).ConfigureAwait(false); + } + + throw new TimeoutException($"Timed out waiting for a '{method}' request from the client."); + } + + /// Sends the configured run response using the request's numeric or numeric-string ID form. + public Task SendRunResponseAsync(RequestMessage request, bool useStringId) + => WriteAsync(new ResponseMessage(request.Id, RunResponse) + { + StringId = useStringId ? request.Id.ToString(CultureInfo.InvariantCulture) : null, + }); + /// /// Writes a raw, pre-framed body to the client so a test can inject a malformed message. The /// Content-Length header is computed from the UTF-8 body so the client reads exactly this body. diff --git a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs index d2ba4b8a82..257cfa89af 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests/MtpServerClientTests.cs @@ -510,6 +510,25 @@ public async Task RunTestsAsync_Cancellation_SendsCancelRequestAndThrows() await server.WaitForNotificationAsync(JsonRpcMethods.CancelRequest, DefaultTimeout).ConfigureAwait(false); } + [TestMethod] + public async Task RunTestsAsync_NumericStringResponseId_DoesNotCompleteNumericRequest() + { + using FakeMtpServer server = new() { WithholdRunResponse = true }; + using MtpServerClient client = await ConnectAndInitializeAsync(server).ConfigureAwait(false); + + Task runTask = client.RunTestsAsync(TestContext.CancellationToken); + RequestMessage request = await server.WaitForRequestAsync(JsonRpcMethods.TestingRunTests, DefaultTimeout).ConfigureAwait(false); + Task responseProcessed = WaitForEventAsync(handler => client.LogReceived += handler); + + await server.SendRunResponseAsync(request, useStringId: true).ConfigureAwait(false); + await server.SendLogAsync("response barrier").ConfigureAwait(false); + _ = await WithTimeoutAsync(responseProcessed).ConfigureAwait(false); + Assert.IsFalse(runTask.IsCompleted); + + await server.SendRunResponseAsync(request, useStringId: false).ConfigureAwait(false); + _ = await WithTimeoutAsync(runTask).ConfigureAwait(false); + } + [TestMethod] public async Task ReadLoop_MalformedFrame_FailsPendingRequestWithClientException() { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 1562646c30..59195ff82d 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -219,13 +219,6 @@ await WriteMessageAsync( Assert.AreEqual(2, incompatibleVersionError.Id); Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, incompatibleVersionError.ErrorCode); - var queuedRequestError = (ErrorMessage)(await WaitForMessage( - messageHandler, - rpcMessage => rpcMessage is ErrorMessage { Id: 20 }, - "Wait queued request error", - timeout.Token))!; - Assert.AreEqual(ErrorCodes.ServerNotInitialized, queuedRequestError.ErrorCode); - const string initializeMessage = """ { "jsonrpc": "2.0", @@ -245,11 +238,23 @@ await WriteMessageAsync( """; await WriteMessageAsync(writer, initializeMessage); - var initializeResponse = (ResponseMessage)(await WaitForMessage( - messageHandler, - rpcMessage => rpcMessage is ResponseMessage { Id: 3 }, - "Wait initialize response", - timeout.Token))!; + ErrorMessage? queuedRequestError = null; + ResponseMessage? initializeResponse = null; + while (queuedRequestError is null || initializeResponse is null) + { + RpcMessage? message = await messageHandler.ReadAsync(timeout.Token); + if (queuedRequestError is null && message is ErrorMessage { Id: 20 } error) + { + queuedRequestError = error; + } + + if (initializeResponse is null && message is ResponseMessage { Id: 3 } response) + { + initializeResponse = response; + } + } + + Assert.AreEqual(ErrorCodes.ServerNotInitialized, queuedRequestError.ErrorCode); InitializeResponseArgs initializeResult = SerializerUtilities.Deserialize( (IDictionary)initializeResponse.Result!); Assert.AreEqual(JsonRpcProtocolVersions.Current, initializeResult.ProtocolVersion); From 8dc90f9661932a0318a1c110f7359327eb9a4709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 15:41:25 +0200 Subject: [PATCH 18/25] Accept integral JSON-RPC number IDs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../001-protocol-intro.md | 3 ++- .../JsonRpc/Json/Json.Deserializers.cs | 22 ++++++++++++++++++- .../ServerMode/JsonRpc/SerializerUtilities.cs | 6 +++++ .../ServerMode/FormatterUtilitiesTests.cs | 20 +++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 2a70addfd7..5c8b888083 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -569,7 +569,8 @@ Notifications: - method: `testing/testUpdates/tests` - params: `TestUpdateNotificationParams` where `params.changes == null`. - The server sends this terminal notification before the final success or error response for requests - that reached execution. Requests rejected by lifecycle checks and requests whose params could not be + whose discovery/run params were successfully deserialized, including requests canceled while queued + during initialization. Requests rejected by lifecycle checks and requests whose params could not be deserialized receive only the error response. Response: diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 6459541356..0442217a07 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -447,7 +447,7 @@ private static int ReadRpcIdAndCaptureString(JsonElement idElement, out string? private static int ReadRpcId(JsonElement idElement) => idElement.ValueKind switch { - JsonValueKind.Number when idElement.TryGetInt32(out int numericId) => numericId, + JsonValueKind.Number when TryReadIntegralInt32(idElement, out int numericId) => numericId, JsonValueKind.String when int.TryParse( idElement.GetString(), NumberStyles.Integer, @@ -456,4 +456,24 @@ JsonValueKind.String when int.TryParse( && idElement.GetString() == stringId.ToString(CultureInfo.InvariantCulture) => stringId, _ => throw new MessageFormatException($"'{JsonRpcStrings.Id}' field should be an int or a numeric string"), }; + + private static bool TryReadIntegralInt32(JsonElement element, out int value) + { + if (element.TryGetInt32(out value)) + { + return true; + } + + if (element.TryGetDecimal(out decimal decimalValue) + && decimalValue == decimal.Truncate(decimalValue) + && decimalValue >= int.MinValue + && decimalValue <= int.MaxValue) + { + value = decimal.ToInt32(decimalValue); + return true; + } + + value = default; + return false; + } } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs index 95fff88b46..3478279fda 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs @@ -80,6 +80,12 @@ private static T GetRequiredPropertyFromJson(IDictionary pro => idObj switch { int idInt => idInt, + double idDouble when idDouble == Math.Truncate(idDouble) + && idDouble >= int.MinValue + && idDouble <= int.MaxValue => (int)idDouble, + decimal idDecimal when idDecimal == decimal.Truncate(idDecimal) + && idDecimal >= int.MinValue + && idDecimal <= int.MaxValue => decimal.ToInt32(idDecimal), string idStr => int.TryParse(idStr, NumberStyles.Integer, CultureInfo.InvariantCulture, out int id) && idStr == id.ToString(CultureInfo.InvariantCulture) ? id diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index d46823c341..aa3198b98b 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -121,6 +121,26 @@ public void CanDeserializeNumericStringRequestId() Assert.AreEqual("42", request.StringId); } + [DataRow("1.0", 1)] + [DataRow("1e0", 1)] + [DataRow("-2.000", -2)] + [TestMethod] + public void CanDeserializeIntegralNumericRequestId(string serializedId, int expectedId) + { + RpcMessage message = Deserialize( + $$""" + { + "jsonrpc": "2.0", + "id": {{serializedId}}, + "method": "testing/unknown" + } + """); + + RequestMessage request = Assert.IsInstanceOfType(message); + Assert.AreEqual(expectedId, request.Id); + Assert.IsNull(request.StringId); + } + [TestMethod] public async Task NumericStringId_IsPreservedInResponsesAndErrors() { From c543bed5536337dacc335829f8ad276eb62e1845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 15:55:39 +0200 Subject: [PATCH 19/25] Align attachment and log contracts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../001-protocol-intro.md | 22 +++++++++---------- .../server-mode-1.0.schema.json | 13 +++++++++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 5c8b888083..72f4e9cae4 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -626,7 +626,7 @@ Notifications: ```typescript interface AttachmentUpdatesParams { - attachments?: Attachment[], + attachments: Attachment[], } ``` @@ -640,33 +640,33 @@ interface RunTestsResponse { // rather than to a single test result. // Note: If multiple data collectors are generating attachments, they should send separate // attachment events. - attachments?: Attachment[] + attachments: Attachment[] } interface Attachment { - // OPTIONAL: If the attachment is based on a file (and the client can show a hyperlink to it) - // the file's location should be specified by this property. + // If the attachment is based on a file (and the client can show a hyperlink to it), + // the file's location should be specified; otherwise this value is null. // Example: "uri": "file://some/coverage.trx" - uri: string, + uri: string | null, // The name of the extension that generated the attachment. // Note: For the time being the client does not special case attachments // based on their producer. // Example: "producer": "TrxReportGeneratorProcessLifetimeHandler" - producer: string, + producer: string | null, - // OPTIONAL: The file extension can be used to resolve the attachment type - // if that isn't ambiguous. + // The file extension can be used to resolve the attachment type if that isn't ambiguous; + // otherwise this value is null. // Note: For the time being the client does not special case attachments // based on their type. // Example: "type": "file" - type: string, + type: string | null, // How the attachment can be displayed as by the client. // Example: "display-name": "Code Coverage" - 'display-name': string; + 'display-name': string | null; - description?: string; + description: string | null; } ``` diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index 5b3ea5084b..f8d8aee025 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -498,7 +498,8 @@ "uri", "producer", "type", - "display-name" + "display-name", + "description" ], "properties": { "uri": { @@ -587,7 +588,15 @@ ], "properties": { "level": { - "type": "string" + "enum": [ + "Trace", + "Debug", + "Information", + "Warning", + "Error", + "Critical", + "None" + ] }, "message": { "type": "string" From be01ba955d705ac0d2f8bc58af5567bdcfe0ec51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 16:48:52 +0200 Subject: [PATCH 20/25] Process cancellation notifications in order Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Hosts/ServerTestHost.MessageLoop.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index b9a906575f..02c9e1e031 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -75,6 +75,7 @@ private async Task HandleMessagesAsync(CancellationToken cancellationToken) case NotificationMessage notification: // This task is recorded inside the _clientToServerRequests + // Cancellation is applied synchronously so queued requests observe it before they resume. _ = HandleNotificationAsync(notification, _serverClosingTokenSource.Token); break; case ResponseMessage response: @@ -131,9 +132,6 @@ private async Task HandleNotificationAsync(NotificationMessage message, Cancella return; } - // Note: Yield, so that the main message reading loop can continue. - await Task.Yield(); - try { switch (message.Method, message.Params) From 2790b14617a1179d7f4e8184fcc8a5b9c192b2f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 17:13:59 +0200 Subject: [PATCH 21/25] Reject invalid passive handshake frames Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../ServerMode/JsonRpc/PassiveNode.cs | 6 +++++- .../ServerMode/PassiveNodeTests.cs | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs index a5987ce8f9..1df705f357 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PassiveNode.cs @@ -59,7 +59,11 @@ public async Task ConnectAsync() await _logger.LogTraceAsync(message.ToString()).ConfigureAwait(false); } - var requestMessage = (RequestMessage)message; + if (message is not RequestMessage requestMessage) + { + return false; + } + if (requestMessage.Method != JsonRpcMethods.Initialize) { await SendErrorAsync( diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs index 852ec543f2..3e3017adf8 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/PassiveNodeTests.cs @@ -56,6 +56,21 @@ public async Task ConnectAsync_RejectsNonInitializeRequestBeforeInitialization() Assert.AreEqual(ErrorCodes.ServerNotInitialized, error.ErrorCode); } + [DataRow(true)] + [DataRow(false)] + [TestMethod] + public async Task ConnectAsync_RejectsNonRequestInitialMessage(bool isNotification) + { + RpcMessage message = isNotification + ? new NotificationMessage(JsonRpcMethods.Exit, Params: null) + : new ResponseMessage(1, Result: null); + TestMessageHandler handler = new(message); + using PassiveNode node = CreatePassiveNode(handler); + + Assert.IsFalse(await node.ConnectAsync()); + Assert.IsNull(handler.WrittenMessage); + } + private static PassiveNode CreatePassiveNode(TestMessageHandler handler) { var cancellationTokenSource = new Mock(); From 26edfa58c1aeebd4bd36bc8e48f3cf8f71f3dfce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 17:41:44 +0200 Subject: [PATCH 22/25] Preserve exact JSON-RPC numeric IDs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../001-protocol-intro.md | 11 +- .../server-mode-1.0.schema.json | 3 +- .../ServerMode/JsonRpc/FormatterUtilities.cs | 132 +++++++++++++++++- .../ServerMode/JsonRpc/SerializerUtilities.cs | 3 - .../ServerMode/FormatterUtilitiesTests.cs | 34 +++++ 5 files changed, 176 insertions(+), 7 deletions(-) diff --git a/docs/mstest-runner-protocol/001-protocol-intro.md b/docs/mstest-runner-protocol/001-protocol-intro.md index 72f4e9cae4..6e369bdd16 100644 --- a/docs/mstest-runner-protocol/001-protocol-intro.md +++ b/docs/mstest-runner-protocol/001-protocol-intro.md @@ -265,7 +265,7 @@ interface InitializeResponse { }, // Independently negotiated wire-protocol version. - protocolVersion?: string, + protocolVersion?: string | null, capabilities: { testing: { @@ -524,6 +524,12 @@ Request: ```typescript interface DiscoverTestsParams { + // Optional explicit test selection. If omitted, all tests matching the filter are discovered. + tests?: TestNode[], + + // Optional graph filter that narrows discovery. + filter?: string, + runId: GUID } ``` @@ -603,6 +609,9 @@ interface RunTestsParams { // If not specified all tests will run. tests?: TestNode[], + // Optional graph filter that narrows execution. + filter?: string, + // Token which should be specified for all update notifications. // This way the client can under which the update notifications should be reported. runId: GUID diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index f8d8aee025..6e3e92108d 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -231,8 +231,7 @@ ], "items": { "type": "string" - }, - "uniqueItems": true + } } }, "additionalProperties": true diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs index 5f58ec1139..9428a8fb77 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs @@ -22,13 +22,143 @@ internal static IMessageFormatter CreateFormatter() internal sealed class MessageFormatter : IMessageFormatter { + private static readonly Jsonite.JsonSettings NumericTextSettings = new() + { + ParseValuesAsStrings = true, + }; + public string Id => "Jsonite"; public T Deserialize(string serializedUtf8Content) - => SerializerUtilities.Deserialize((Jsonite.JsonObject)Jsonite.Json.Deserialize(serializedUtf8Content)); + { + var properties = (Jsonite.JsonObject)Jsonite.Json.Deserialize(serializedUtf8Content); + PreserveExactRpcIds(properties, serializedUtf8Content); + return SerializerUtilities.Deserialize(properties); + } public Task SerializeAsync(object obj) => Task.FromResult(Jsonite.Json.Serialize(SerializerUtilities.Serialize(obj.GetType(), obj))); + + private static void PreserveExactRpcIds(Jsonite.JsonObject properties, string serializedContent) + { + bool hasFloatingPointMessageId = properties.TryGetValue(JsonRpcStrings.Id, out object? messageId) + && messageId is double; + Jsonite.JsonObject? paramsObject = properties.TryGetValue(JsonRpcStrings.Params, out object? paramsValue) + ? paramsValue as Jsonite.JsonObject + : null; + bool hasFloatingPointCancellationId = properties.TryGetValue(JsonRpcStrings.Method, out object? method) + && method is JsonRpcMethods.CancelRequest + && paramsObject?.TryGetValue(JsonRpcStrings.Id, out object? cancellationId) == true + && cancellationId is double; + if (!hasFloatingPointMessageId && !hasFloatingPointCancellationId) + { + return; + } + + var rawProperties = (Jsonite.JsonObject)Jsonite.Json.Deserialize(serializedContent, NumericTextSettings); + if (hasFloatingPointMessageId) + { + PreserveExactRpcId(properties, rawProperties); + } + + if (hasFloatingPointCancellationId + && rawProperties[JsonRpcStrings.Params] is Jsonite.JsonObject rawParams) + { + PreserveExactRpcId(paramsObject!, rawParams); + } + } + + private static void PreserveExactRpcId(Jsonite.JsonObject properties, Jsonite.JsonObject rawProperties) + { + string rawId = (string)rawProperties[JsonRpcStrings.Id]!; + properties[JsonRpcStrings.Id] = TryParseNumericRpcId(rawId, out int id) + ? id + : rawId; + } + + private static bool TryParseNumericRpcId(string value, out int result) + { + int start = value[0] == '-' ? 1 : 0; + bool isNegative = start == 1; + int exponentIndex = value.IndexOf('e'); + if (exponentIndex < 0) + { + exponentIndex = value.IndexOf('E'); + } + + string mantissa = exponentIndex < 0 ? value.Substring(start) : value.Substring(start, exponentIndex - start); + int decimalPointIndex = mantissa.IndexOf('.'); + int fractionalDigits = decimalPointIndex < 0 ? 0 : mantissa.Length - decimalPointIndex - 1; + string digits = decimalPointIndex < 0 ? mantissa : mantissa.Remove(decimalPointIndex, 1); + if (digits.All(c => c == '0')) + { + result = 0; + return true; + } + + int exponent = 0; + if (exponentIndex >= 0 + && !int.TryParse( + value.Substring(exponentIndex + 1), + NumberStyles.AllowLeadingSign, + CultureInfo.InvariantCulture, + out exponent)) + { + result = default; + return false; + } + + long scale = (long)fractionalDigits - exponent; + if (scale > 0) + { + if (scale > digits.Length) + { + result = default; + return false; + } + + int firstFractionalIndex = digits.Length - (int)scale; + for (int i = firstFractionalIndex; i < digits.Length; i++) + { + if (digits[i] != '0') + { + result = default; + return false; + } + } + + digits = digits.Substring(0, firstFractionalIndex); + } + else if (scale < 0) + { + long trailingZeroCount = -scale; + int significantDigitCount = digits.TrimStart('0').Length; + if (trailingZeroCount > 10 || significantDigitCount + trailingZeroCount > 10) + { + result = default; + return false; + } + + digits += new string('0', (int)trailingZeroCount); + } + + digits = digits.TrimStart('0'); + if (!long.TryParse(digits, NumberStyles.None, CultureInfo.InvariantCulture, out long magnitude)) + { + result = default; + return false; + } + + long signedValue = isNegative ? -magnitude : magnitude; + if (signedValue is < int.MinValue or > int.MaxValue) + { + result = default; + return false; + } + + result = (int)signedValue; + return true; + } } #else internal static IMessageFormatter CreateFormatter() => new MessageFormatter(); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs index 3478279fda..ea1461d3cc 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs @@ -80,9 +80,6 @@ private static T GetRequiredPropertyFromJson(IDictionary pro => idObj switch { int idInt => idInt, - double idDouble when idDouble == Math.Truncate(idDouble) - && idDouble >= int.MinValue - && idDouble <= int.MaxValue => (int)idDouble, decimal idDecimal when idDecimal == decimal.Truncate(idDecimal) && idDecimal >= int.MinValue && idDecimal <= int.MaxValue => decimal.ToInt32(idDecimal), diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index aa3198b98b..f3a1a1e58a 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -141,6 +141,20 @@ public void CanDeserializeIntegralNumericRequestId(string serializedId, int expe Assert.IsNull(request.StringId); } + [DataRow("1.00000000000000001")] + [DataRow("1e-1")] + [DataRow("2147483648.0")] + [TestMethod] + public void RejectsNonIntegralOrOutOfRangeNumericRequestId(string serializedId) + => Assert.ThrowsExactly(() => Deserialize( + $$""" + { + "jsonrpc": "2.0", + "id": {{serializedId}}, + "method": "testing/unknown" + } + """)); + [TestMethod] public async Task NumericStringId_IsPreservedInResponsesAndErrors() { @@ -174,6 +188,26 @@ public void CanDeserializeNumericStringCancellationId() Assert.AreEqual("42", args.StringId); } + [TestMethod] + public void CanDeserializeIntegralNumericCancellationId() + { + RpcMessage message = Deserialize( + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": 42.0 + } + } + """); + + NotificationMessage notification = Assert.IsInstanceOfType(message); + CancelRequestArgs args = Assert.IsInstanceOfType(notification.Params); + Assert.AreEqual(42, args.CancelRequestId); + Assert.IsNull(args.StringId); + } + [TestMethod] public void NullRequestId_IsTreatedAsNotification() { From 4fb6c0a4a8124dac716e7c48b9f129b3feb99f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 19:28:59 +0200 Subject: [PATCH 23/25] Harden cancellation and exact RPC IDs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- ....Platform.ServerMode.Client.Sources.csproj | 1 + .../Hosts/ServerTestHost.MessageLoop.cs | 28 ++- .../ServerMode/JsonRpc/FormatterUtilities.cs | 86 +-------- .../JsonRpc/Json/Json.Deserializers.cs | 22 +-- .../ServerMode/JsonRpc/RpcIdParser.cs | 94 ++++++++++ .../ServerMode/FormatterUtilitiesTests.cs | 1 + .../ServerMode/ServerTests.cs | 163 +++++++++++++++++- 7 files changed, 283 insertions(+), 112 deletions(-) create mode 100644 src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcIdParser.cs diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Microsoft.Testing.Platform.ServerMode.Client.Sources.csproj b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Microsoft.Testing.Platform.ServerMode.Client.Sources.csproj index 62c0d94e22..421c614dc6 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Microsoft.Testing.Platform.ServerMode.Client.Sources.csproj +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Microsoft.Testing.Platform.ServerMode.Client.Sources.csproj @@ -107,6 +107,7 @@ + diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index 02c9e1e031..d97c9e42e9 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -141,7 +141,14 @@ private async Task HandleNotificationAsync(NotificationMessage message, Cancella GetRequestKey(args.CancelRequestId, args.StringId), out RpcInvocationState? rpcState)) { - Exception? cancellationException = rpcState.CancelRequest(); + if (!rpcState.TryRequestCancellation()) + { + break; + } + + // Record cancellation synchronously so a queued request cannot resume into execution. + // Run token callbacks asynchronously so extension code cannot block the message reader. + Exception? cancellationException = await Task.Run(rpcState.CancelRequest).ConfigureAwait(false); if (cancellationException is not null) { // This is intentionally not using PlatformResources.ExceptionDuringCancellationWarningMessage @@ -292,7 +299,7 @@ await SendErrorAsync( bool testUpdateCompletionSent = false; try { - rpcState.CancellationToken.ThrowIfCancellationRequested(); + rpcState.ThrowIfCancellationRequested(); object response = await HandleRequestCoreAsync(request, rpcState, cancellationToken).ConfigureAwait(false); testUpdateCompletionSent = await SendTestUpdateCompleteIfNeededAsync(request, cancellationToken).ConfigureAwait(false); await SendResponseAsync( @@ -323,7 +330,7 @@ await SendResponseAsync( } // We don't return the stack of the exception if we're canceling the single request because it's expected and it's not an exception. - (string errorMessage, int errorCode) = rpcState.CancellationToken.IsCancellationRequested + (string errorMessage, int errorCode) = rpcState.IsCancellationRequested ? (string.Empty, ErrorCodes.RequestCanceled) : (e.ToString(), ErrorCodes.RequestCanceled); @@ -487,6 +494,7 @@ private sealed class RpcInvocationState : IDisposable #endif private readonly CancellationTokenSource _cancellationTokenSource = new(); private volatile bool _isDisposed; + private int _cancellationRequested; /// /// For outbound requests, this is populated with the response from the client. @@ -498,6 +506,20 @@ private sealed class RpcInvocationState : IDisposable // We don't expose directly the source because we need to synchronize the complete/cancel public CancellationToken CancellationToken => _cancellationTokenSource.Token; + public bool IsCancellationRequested + => Volatile.Read(ref _cancellationRequested) != 0 || _cancellationTokenSource.IsCancellationRequested; + + public bool TryRequestCancellation() + => Interlocked.Exchange(ref _cancellationRequested, 1) == 0; + + public void ThrowIfCancellationRequested() + { + if (IsCancellationRequested) + { + throw new OperationCanceledException(CancellationToken); + } + } + public AggregateException? CancelRequest() { if (!_isDisposed) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs index 9428a8fb77..7ed3be2cb6 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/FormatterUtilities.cs @@ -71,94 +71,10 @@ private static void PreserveExactRpcIds(Jsonite.JsonObject properties, string se private static void PreserveExactRpcId(Jsonite.JsonObject properties, Jsonite.JsonObject rawProperties) { string rawId = (string)rawProperties[JsonRpcStrings.Id]!; - properties[JsonRpcStrings.Id] = TryParseNumericRpcId(rawId, out int id) + properties[JsonRpcStrings.Id] = RpcIdParser.TryParseNumericId(rawId, out int id) ? id : rawId; } - - private static bool TryParseNumericRpcId(string value, out int result) - { - int start = value[0] == '-' ? 1 : 0; - bool isNegative = start == 1; - int exponentIndex = value.IndexOf('e'); - if (exponentIndex < 0) - { - exponentIndex = value.IndexOf('E'); - } - - string mantissa = exponentIndex < 0 ? value.Substring(start) : value.Substring(start, exponentIndex - start); - int decimalPointIndex = mantissa.IndexOf('.'); - int fractionalDigits = decimalPointIndex < 0 ? 0 : mantissa.Length - decimalPointIndex - 1; - string digits = decimalPointIndex < 0 ? mantissa : mantissa.Remove(decimalPointIndex, 1); - if (digits.All(c => c == '0')) - { - result = 0; - return true; - } - - int exponent = 0; - if (exponentIndex >= 0 - && !int.TryParse( - value.Substring(exponentIndex + 1), - NumberStyles.AllowLeadingSign, - CultureInfo.InvariantCulture, - out exponent)) - { - result = default; - return false; - } - - long scale = (long)fractionalDigits - exponent; - if (scale > 0) - { - if (scale > digits.Length) - { - result = default; - return false; - } - - int firstFractionalIndex = digits.Length - (int)scale; - for (int i = firstFractionalIndex; i < digits.Length; i++) - { - if (digits[i] != '0') - { - result = default; - return false; - } - } - - digits = digits.Substring(0, firstFractionalIndex); - } - else if (scale < 0) - { - long trailingZeroCount = -scale; - int significantDigitCount = digits.TrimStart('0').Length; - if (trailingZeroCount > 10 || significantDigitCount + trailingZeroCount > 10) - { - result = default; - return false; - } - - digits += new string('0', (int)trailingZeroCount); - } - - digits = digits.TrimStart('0'); - if (!long.TryParse(digits, NumberStyles.None, CultureInfo.InvariantCulture, out long magnitude)) - { - result = default; - return false; - } - - long signedValue = isNegative ? -magnitude : magnitude; - if (signedValue is < int.MinValue or > int.MaxValue) - { - result = default; - return false; - } - - result = (int)signedValue; - return true; - } } #else internal static IMessageFormatter CreateFormatter() => new MessageFormatter(); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 0442217a07..23e84346a5 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -447,7 +447,7 @@ private static int ReadRpcIdAndCaptureString(JsonElement idElement, out string? private static int ReadRpcId(JsonElement idElement) => idElement.ValueKind switch { - JsonValueKind.Number when TryReadIntegralInt32(idElement, out int numericId) => numericId, + JsonValueKind.Number when RpcIdParser.TryParseNumericId(idElement.GetRawText(), out int numericId) => numericId, JsonValueKind.String when int.TryParse( idElement.GetString(), NumberStyles.Integer, @@ -456,24 +456,4 @@ JsonValueKind.String when int.TryParse( && idElement.GetString() == stringId.ToString(CultureInfo.InvariantCulture) => stringId, _ => throw new MessageFormatException($"'{JsonRpcStrings.Id}' field should be an int or a numeric string"), }; - - private static bool TryReadIntegralInt32(JsonElement element, out int value) - { - if (element.TryGetInt32(out value)) - { - return true; - } - - if (element.TryGetDecimal(out decimal decimalValue) - && decimalValue == decimal.Truncate(decimalValue) - && decimalValue >= int.MinValue - && decimalValue <= int.MaxValue) - { - value = decimal.ToInt32(decimalValue); - return true; - } - - value = default; - return false; - } } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcIdParser.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcIdParser.cs new file mode 100644 index 0000000000..ab2279fe1c --- /dev/null +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/RpcIdParser.cs @@ -0,0 +1,94 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.Testing.Platform.ServerMode; + +[Embedded] +internal static class RpcIdParser +{ + public static bool TryParseNumericId(string value, out int result) + { + int start = value[0] == '-' ? 1 : 0; + bool isNegative = start == 1; + int exponentIndex = value.IndexOf('e'); + if (exponentIndex < 0) + { + exponentIndex = value.IndexOf('E'); + } + + string mantissa = exponentIndex < 0 ? value.Substring(start) : value.Substring(start, exponentIndex - start); + int decimalPointIndex = mantissa.IndexOf('.'); + int fractionalDigits = decimalPointIndex < 0 ? 0 : mantissa.Length - decimalPointIndex - 1; + string digits = decimalPointIndex < 0 ? mantissa : mantissa.Remove(decimalPointIndex, 1); + if (digits.All(c => c == '0')) + { + result = 0; + return true; + } + + int exponent = 0; + if (exponentIndex >= 0 + && !int.TryParse( + value.Substring(exponentIndex + 1), + NumberStyles.AllowLeadingSign, + CultureInfo.InvariantCulture, + out exponent)) + { + result = default; + return false; + } + + long scale = (long)fractionalDigits - exponent; + if (scale > 0) + { + if (scale > digits.Length) + { + result = default; + return false; + } + + int firstFractionalIndex = digits.Length - (int)scale; + for (int i = firstFractionalIndex; i < digits.Length; i++) + { + if (digits[i] != '0') + { + result = default; + return false; + } + } + + digits = digits.Substring(0, firstFractionalIndex); + } + else if (scale < 0) + { + long trailingZeroCount = -scale; + int significantDigitCount = digits.TrimStart('0').Length; + if (trailingZeroCount > 10 || significantDigitCount + trailingZeroCount > 10) + { + result = default; + return false; + } + + digits += new string('0', (int)trailingZeroCount); + } + + digits = digits.TrimStart('0'); + if (!long.TryParse(digits, NumberStyles.None, CultureInfo.InvariantCulture, out long magnitude)) + { + result = default; + return false; + } + + long signedValue = isNegative ? -magnitude : magnitude; + if (signedValue is < int.MinValue or > int.MaxValue) + { + result = default; + return false; + } + + result = (int)signedValue; + return true; + } +} diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index f3a1a1e58a..f33633cf6d 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -142,6 +142,7 @@ public void CanDeserializeIntegralNumericRequestId(string serializedId, int expe } [DataRow("1.00000000000000001")] + [DataRow("1.0000000000000000000000000000001")] [DataRow("1e-1")] [DataRow("2147483648.0")] [TestMethod] diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index 59195ff82d..e05df147a5 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -214,9 +214,11 @@ await WriteMessageAsync( } """); - ErrorMessage incompatibleVersionError = Assert.IsInstanceOfType( - await messageHandler.ReadAsync(timeout.Token)); - Assert.AreEqual(2, incompatibleVersionError.Id); + var incompatibleVersionError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 2 }, + "Wait incompatible protocol version error", + timeout.Token))!; Assert.AreEqual(ErrorCodes.ProtocolVersionNotSupported, incompatibleVersionError.ErrorCode); const string initializeMessage = """ @@ -476,6 +478,31 @@ await WriteMessageAsync( } } """); + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 3, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + + var duplicateInitializeError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 3 }, + "Wait duplicate initialize error after cancellation", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.InvalidRequest, duplicateInitializeError.ErrorCode); } finally { @@ -504,6 +531,136 @@ await WriteMessageAsync( Assert.AreEqual(0, await serverTask); } + [TestMethod] + public async Task SlowCancellationCallbackDoesNotBlockMessageReader() + { + using var server = TcpServer.Create(); + TaskCompletionSource discoveryStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + TaskCompletionSource cancellationCallbackStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); + using var releaseCancellationCallback = new ManualResetEventSlim(initialState: false); + using CancellationTokenSource cancellationCallbackTimeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + + string[] args = ["--no-banner", "--server", "--client-port", $"{server.Port}", "--internal-testingplatform-skipbuildercheck"]; + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new MockTestAdapter + { + DiscoveryAction = async context => + { + var cancellationDelay = Task.Delay(Timeout.Infinite, context.CancellationToken); + using CancellationTokenRegistration registration = context.CancellationToken.Register(() => + { + cancellationCallbackStarted.TrySetResult(true); + releaseCancellationCallback.Wait(cancellationCallbackTimeout.Token); + }); + discoveryStarted.TrySetResult(true); + await cancellationDelay; + }, + }); + var testApplication = (TestApplication)await builder.BuildAsync(); + testApplication.ServiceProvider.GetRequiredService().SuppressOutput(); + Task serverTask = Task.Run(testApplication.RunAsync); + + using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout); + using TcpClient client = await server.WaitForConnectionAsync(timeout.Token); + using NetworkStream stream = client.GetStream(); + using StreamWriter writer = new(stream, Encoding.UTF8); + TcpMessageHandler messageHandler = new( + client, + clientToServerStream: client.GetStream(), + serverToClientStream: client.GetStream(), + FormatterUtilities.CreateFormatter()); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": 32, + "clientInfo": { "name": "testingplatform-unittests", "version": "1.0.0" }, + "capabilities": { + "testing": { + "debuggerProvider": false + } + } + } + } + """); + _ = await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ResponseMessage { Id: 1 }, + "Wait initialize response", + timeout.Token); + + try + { + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 2, + "method": "testing/discoverTests", + "params": { + "runId": "00000000-0000-0000-0000-000000000002" + } + } + """); + await discoveryStarted.Task.WaitAsync(timeout.Token); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "method": "$/cancelRequest", + "params": { + "id": 2 + } + } + """); + await cancellationCallbackStarted.Task.WaitAsync(timeout.Token); + + await WriteMessageAsync( + writer, + """ + { + "jsonrpc": "2.0", + "id": 3, + "method": "testing/unknown", + "params": {} + } + """); + var methodNotFoundError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 3 }, + "Wait method-not-found response while cancellation callback is blocked", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.MethodNotFound, methodNotFoundError.ErrorCode); + } + finally + { + releaseCancellationCallback.Set(); + } + + _ = await WaitForMessage( + messageHandler, + IsTestUpdateCompletion, + "Wait canceled discovery completion", + timeout.Token); + var cancellationError = (ErrorMessage)(await WaitForMessage( + messageHandler, + rpcMessage => rpcMessage is ErrorMessage { Id: 2 }, + "Wait canceled discovery error", + timeout.Token))!; + Assert.AreEqual(ErrorCodes.RequestCanceled, cancellationError.ErrorCode); + + await WriteMessageAsync(writer, """{ "jsonrpc": "2.0", "method": "exit", "params": { } }"""); + Assert.AreEqual(0, await serverTask); + } + [TestMethod] public async Task NumericAndStringRequestIdsHaveIndependentCancellation() { From 1aeba4fe6a23460947596d797bf9134af97c5446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 19:48:31 +0200 Subject: [PATCH 24/25] Handle immediate initialize ID reuse Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../server-mode-1.0.schema.json | 10 +++ .../Hosts/ServerTestHost.MessageLoop.cs | 61 ++++++++++++++++--- .../ServerMode/ServerTests.cs | 6 +- 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/docs/mstest-runner-protocol/server-mode-1.0.schema.json b/docs/mstest-runner-protocol/server-mode-1.0.schema.json index 6e3e92108d..221863bcf7 100644 --- a/docs/mstest-runner-protocol/server-mode-1.0.schema.json +++ b/docs/mstest-runner-protocol/server-mode-1.0.schema.json @@ -704,6 +704,11 @@ "additionalProperties": true } }, + "not": { + "required": [ + "method" + ] + }, "additionalProperties": true }, "errorResponse": { @@ -740,6 +745,11 @@ "additionalProperties": true } }, + "not": { + "required": [ + "method" + ] + }, "additionalProperties": true } } diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs index d97c9e42e9..478c787245 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.MessageLoop.cs @@ -320,7 +320,9 @@ await SendResponseAsync( catch (OperationCanceledException e) { TaskCompletionSource? failedInitialization = isInitializeRequest - ? MakeInitializationRetryable() + ? MakeInitializationRetryable( + GetRequestKey(request.Id, request.StringId), + rpcState) : null; try { @@ -346,16 +348,19 @@ await SendErrorAsync( { failedInitialization?.TrySetResult(false); - CompleteRequest( - ref _clientToServerRequests, + CompleteFailedRequest( + isInitializeRequest, GetRequestKey(request.Id, request.StringId), + rpcState, completion => completion.TrySetCanceled()); } } catch (JsonRpcException e) { TaskCompletionSource? failedInitialization = isInitializeRequest - ? MakeInitializationRetryable() + ? MakeInitializationRetryable( + GetRequestKey(request.Id, request.StringId), + rpcState) : null; try { @@ -376,16 +381,19 @@ await SendErrorAsync( { failedInitialization?.TrySetResult(false); - CompleteRequest( - ref _clientToServerRequests, + CompleteFailedRequest( + isInitializeRequest, GetRequestKey(request.Id, request.StringId), + rpcState, completion => completion.TrySetException(e)); } } catch (Exception e) { TaskCompletionSource? failedInitialization = isInitializeRequest - ? MakeInitializationRetryable() + ? MakeInitializationRetryable( + GetRequestKey(request.Id, request.StringId), + rpcState) : null; try { @@ -406,9 +414,10 @@ await SendErrorAsync( { failedInitialization?.TrySetResult(false); - CompleteRequest( - ref _clientToServerRequests, + CompleteFailedRequest( + isInitializeRequest, GetRequestKey(request.Id, request.StringId), + rpcState, completion => completion.TrySetException(e)); } } @@ -428,12 +437,17 @@ private void CompleteInitialization() completionSource?.TrySetResult(true); } - private TaskCompletionSource? MakeInitializationRetryable() + private TaskCompletionSource? MakeInitializationRetryable( + (int Id, bool IsString) requestKey, + RpcInvocationState rpcState) { lock (_initializeStateLock) { RoslynDebug.Assert(_initializeState == Initializing); RoslynDebug.Assert(_initializationCompletionSource is not null); + bool requestDetached = ((ICollection>)_clientToServerRequests) + .Remove(new(requestKey, rpcState)); + RoslynDebug.Assert(requestDetached); _initializeState = NotInitialized; TaskCompletionSource? completionSource = _initializationCompletionSource; _initializationCompletionSource = null; @@ -441,6 +455,33 @@ private void CompleteInitialization() } } + private void CompleteFailedRequest( + bool requestWasDetached, + (int Id, bool IsString) requestKey, + RpcInvocationState rpcState, + Action> completion) + { + if (!requestWasDetached) + { + CompleteRequest(ref _clientToServerRequests, requestKey, completion); + return; + } + + try + { + completion(rpcState.CompletionSource); + rpcState.Dispose(); + if (_clientToServerRequests.IsEmpty && _serverClosingTokenSource.IsCancellationRequested) + { + _stopMessageHandler.Cancel(); + } + } + finally + { + _requestCounter.Signal(); + } + } + private async Task SendTestUpdateCompleteIfNeededAsync( RequestMessage request, CancellationToken cancellationToken, diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs index e05df147a5..233f8ff17f 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/ServerTests.cs @@ -224,7 +224,7 @@ await WriteMessageAsync( const string initializeMessage = """ { "jsonrpc": "2.0", - "id": 3, + "id": 2, "method": "initialize", "params": { "processId": 32, @@ -250,7 +250,7 @@ await WriteMessageAsync( queuedRequestError = error; } - if (initializeResponse is null && message is ResponseMessage { Id: 3 } response) + if (initializeResponse is null && message is ResponseMessage { Id: 2 } response) { initializeResponse = response; } @@ -313,7 +313,7 @@ await WriteMessageAsync( timeout.Token))!; Assert.AreEqual(ErrorCodes.InternalError, internalError.ErrorCode); - await WriteMessageAsync(writer, initializeMessage.Replace("\"id\": 3", "\"id\": 6")); + await WriteMessageAsync(writer, initializeMessage.Replace("\"id\": 2", "\"id\": 6")); var duplicateInitializeError = (ErrorMessage)(await WaitForMessage( messageHandler, rpcMessage => rpcMessage is ErrorMessage { Id: 6 }, From 906495b1505489f0c1f41e64d354b5e1835a824d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 27 Aug 2026 20:05:02 +0200 Subject: [PATCH 25/25] Reject explicit null JSON-RPC IDs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1399eff-1328-4e8f-92d4-d730891fbefd --- .../Client/SerializerUtilities.ClientSerializers.cs | 8 ++++++-- .../ServerMode/JsonRpc/Json/Json.Deserializers.cs | 8 ++++++-- .../JsonRpc/SerializerUtilities.Deserializers.cs | 8 ++++++-- .../ServerMode/FormatterUtilitiesTests.cs | 10 +++------- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs index b8d3a7fed2..e04c5d0b9e 100644 --- a/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/SerializerUtilities.ClientSerializers.cs @@ -136,12 +136,16 @@ private static void RegisterClientSerializersCore() if (properties.TryGetValue(JsonRpcStrings.Method, out object? methodObj) && methodObj is not null) { string method = (string)methodObj; - object? idObj = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Id); + bool hasId = properties.TryGetValue(JsonRpcStrings.Id, out object? idObj); + if (hasId && idObj is null) + { + throw new MessageFormatException($"'{JsonRpcStrings.Id}' field cannot be null"); + } // Keep the params as the raw dictionary; the client decodes it based on the method name. object? @params = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Params); - int? id = idObj is null + int? id = !hasId ? null : GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs index 23e84346a5..fab3b33dc0 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs @@ -420,14 +420,18 @@ private static object ReadNumber(JsonElement element) private static bool TryGetRpcId(JsonElement jsonElement, out int id, out string? stringId) { - if (!jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement) - || idElement.ValueKind == JsonValueKind.Null) + if (!jsonElement.TryGetProperty(JsonRpcStrings.Id, out JsonElement idElement)) { id = default; stringId = null; return false; } + if (idElement.ValueKind == JsonValueKind.Null) + { + throw new MessageFormatException($"'{JsonRpcStrings.Id}' field cannot be null"); + } + stringId = idElement.ValueKind == JsonValueKind.String ? idElement.GetString() : null; id = ReadRpcId(idElement); return true; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs index d4fe09bdff..f470cab938 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.Deserializers.cs @@ -23,9 +23,13 @@ private static void RegisterDeserializers() { string method = (string)methodObj; - object? idObj = GetOptionalPropertyFromJson(properties, JsonRpcStrings.Id); + bool hasId = properties.TryGetValue(JsonRpcStrings.Id, out object? idObj); + if (hasId && idObj is null) + { + throw new MessageFormatException($"'{JsonRpcStrings.Id}' field cannot be null"); + } - int? id = idObj is null + int? id = !hasId ? null : GetIdFromJson(idObj) ?? throw new MessageFormatException("id field should be a string or an int"); string? stringId = idObj as string; diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs index f33633cf6d..507c5a61bf 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs @@ -210,19 +210,15 @@ public void CanDeserializeIntegralNumericCancellationId() } [TestMethod] - public void NullRequestId_IsTreatedAsNotification() - { - RpcMessage message = Deserialize( + public void NullRequestId_IsRejected() + => Assert.ThrowsExactly(() => Deserialize( """ { "jsonrpc": "2.0", "id": null, "method": "testing/unknown" } - """); - - Assert.IsInstanceOfType(message); - } + """)); [TestMethod] public void DeserializeInitializeRequest_NullProtocolVersions_UsesLegacyNegotiation()