Remove non-conforming top-level 'code' from JSON-RPC error envelope

[Evangelink]

Follow-up to #9040 (comment).

Problem

Per JSON-RPC 2.0 §5.1, the code field belongs only inside the nested error object, never at the top level of the response envelope. Both serialization paths emitted a stray top-level code that duplicated the one inside error:

• src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs (System.Text.Json path, .NET Core)
• src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs (Jsonite, .NET Framework / netstandard2.0)

The deserializer side already reads code only from within the nested error object, so removing the stray top-level field is purely spec-cleanup with no impact on round-trip behavior.

Before / after

Before (non-conforming):

{"jsonrpc":"2.0","code":2,"id":1,"error":{"code":2,"data":{},"message":"This is error"}}

After (JSON-RPC 2.0 compliant):

{"jsonrpc":"2.0","id":1,"error":{"code":2,"data":{},"message":"This is error"}}

Tests

• Updated FormatterUtilitiesTests.AssertSerialize expectation for ErrorMessage to match the compliant shape (covers both Jsonite and System.Text.Json paths via the existing parameterized harness).
• Added two new tests in JsonTests:
  • Serialize_ErrorMessage_EmitsJsonRpc20CompliantShape — asserts no top-level code and code inside error.
  • Serialize_ErrorMessage_RoundTripsViaDeserializer — confirms serialize ↔ deserialize behavior is preserved.

Verified passing on both net8.0 (System.Text.Json) and net462 (Jsonite).

[Copilot]

Pull request overview

This PR updates the JSON-RPC error response serialization in Microsoft.Testing.Platform to comply with JSON-RPC 2.0 §5.1 by removing an invalid top-level code field from the response envelope (while preserving error.code inside the nested error object). It also updates and extends unit tests to validate the compliant shape and preserve round-trip behavior.

Changes:

• Remove the stray top-level code field from the ErrorMessage JSON-RPC envelope in both serialization implementations (System.Text.Json and Jsonite-based).
• Update the existing serialization expectation for ErrorMessage to match the compliant JSON shape.
• Add unit tests that assert the absence of top-level code and verify serialize→deserialize round-tripping.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/JsonTests.cs	Adds focused tests asserting JSON-RPC compliant ErrorMessage shape and round-trip behavior.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs	Updates the ErrorMessage serialization expectation to remove top-level code.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.RpcMessageSerializers.cs	Removes top-level code from the Jsonite-based error envelope serialization.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Serializers.cs	Removes top-level code from the System.Text.Json-based error envelope serialization.

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 0

[Evangelink]

🧪 Test quality grade — PR #9050

Δ	Test	Grade	Band	Notes
new	JsonTests. Serialize_ ErrorMessage_ EmitsJsonRpc20CompliantShape	B	80–89	Rich assertions include a key negative check (absent top-level code); magic literal 42 for Id could use a named constant.
new	JsonTests. Serialize_ ErrorMessage_ RoundTripsViaDeserializer	B	80–89	Three equality checks verify data survives the round-trip; minor magic literals (7, -32000) would benefit from named constants.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 230 AIC · ⌖ 13 AIC · [◷]( · ◷)

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Expert Review — PR #9050: Remove non-conforming top-level code from JSON-RPC error envelope

Overall verdict: ✅ LGTM — 1 NIT (see inline comment)

The fix is correct and complete. Both serialization paths were updated symmetrically, and both deserializers already read code exclusively from the nested error object — confirming zero round-trip behavioral impact. The test coverage is solid: the STJ-native path is exercised by the updated FormatterUtilitiesTests.AssertSerialize expectation (which runs under #if NETCOREAPP asserting "System.Text.Json" as the formatter id), and the Jsonite path is exercised by the two new JsonTests methods.

---

Dimension Verdicts (22/22 assessed)

#	Dimension	Verdict	Notes
1	Algorithmic Correctness	✅ LGTM	Both deserializers (Json.Deserializers.cs and SerializerUtilities.Deserializers.cs) already read code from errorObject[JsonRpcStrings.Code], never from the top-level envelope — no round-trip impact
2	Threading & Concurrency	N/A	Pure serialization/test code; no shared mutable state
3	Security & IPC Contract Safety	✅ LGTM	Spec-compliant change; no security regression
4	Public API & Binary Compatibility	✅ LGTM	ErrorMessage is internal sealed record; no public API surface touched
5	Performance & Allocations	✅ LGTM	Slight positive: removes one key-value pair per serialized error response
6	Cross-TFM Compatibility	✅ LGTM	Both the NETCOREAPP path (Json.Serializers.cs) and the !NETCOREAPP Jsonite path (SerializerUtilities.RpcMessageSerializers.cs) are fixed in lockstep
7	Resource & IDisposable Management	✅ LGTM	using var document = JsonDocument.Parse(actual) correctly disposes the document
8	Defensive Coding at Boundaries	N/A	No boundary-crossing code changed
9	Localization & Resources	N/A	No user-facing strings involved
10	Test Isolation	✅ LGTM	New tests construct local ErrorMessage instances; no shared state
11	Assertion Quality	✅ LGTM	Assert.IsFalse(root.TryGetProperty("code", out _), "<message>") is the idiomatic STJ pattern for asserting field absence, with a clear diagnostic message
12	Flakiness Patterns	N/A	New tests are fully deterministic
13	Test Completeness & Coverage	✅ LGTM	STJ-native serializer path covered by updated FormatterUtilitiesTests; Jsonite-via-Json path covered by both new JsonTests methods; round-trip deserialization covered by Serialize_ErrorMessage_RoundTripsViaDeserializer
14	Data-Driven Test Coverage	N/A	No data-driven tests in scope
15	Code Structure & Simplification	✅ LGTM	Surgical one-line removals in both production files; no structural changes needed
16	Naming & Conventions	✅ LGTM	Test names clearly describe the scenario and outcome; 1 NIT on type aliases (see inline comment)
17	Documentation Accuracy	✅ LGTM	Spec citation (JSON-RPC 2.0 §5.1) is accurate; the comment added to FormatterUtilitiesTests.cs provides useful context for future maintainers
18	Analyzer & Code Fix Quality	N/A	No analyzer changes
19	IPC Wire Compatibility	✅ LGTM	Removing a non-spec-compliant field is a safe wire change: the internal deserializer never read the top-level code; any external JSON-RPC 2.0–compliant client (VS, VS Code, Rider) reads code from error.code per spec and is unaffected
20	Build Infrastructure & Dependencies	N/A	No build infrastructure changes
21	Scope & PR Discipline	✅ LGTM	Single-concern fix with appropriate follow-up linkage to PR #9040
22	PowerShell Scripting Hygiene	N/A	No .ps1 changes

---

Highlights worth calling out

• Symmetry: the two production-code changes are perfect mirrors of each other — one in the STJ array-of-tuples path, one in the Jsonite dictionary-initializer path. This is exactly right.
• Test design of Serialize_ErrorMessage_EmitsJsonRpc20CompliantShape: parsing the output with System.Text.Json.JsonDocument and asserting structural properties (field presence/absence) rather than a fragile exact-string match is a good pattern for protocol-shape tests.
• FormatterUtilitiesTests comment: adding the spec reference comment alongside the assertion string is a low-cost, high-value documentation touch that will prevent future regressions from reintroducing the stray field.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 369.2 AIC · ⌖ 12.7 AIC · ◷