Backport additional 4.4.1 servicing fixes

[Amaury Levé (Evangelink)]

Summary

Backports the following merged fixes from main to rel/4.4 in dependency order:

• Make client statefulness capability nullable #11037
• Fix cross-assembly source generation accessibility #11014
• Relax MSTEST0032 for enum value assertions #11175
• Preserve inner exceptions in server mode #11221

The port preserves the existing rel/4.4 servicing changes, including #11258 and the branch-specific source-generation work.

Validation

• .\build.cmd -pack -bl — passed with 0 warnings and 0 errors.
• Targeted unit tests — 399 passed, 0 failed.
• Targeted source-generation, server-mode, and package acceptance tests — 31 passed, 0 failed, 1 skipped (Unix execute-bit coverage on Windows).

All MSBuild-based validation commands produced binary logs.

[github-actions]

🧵 Parallel-safety audit — PR #11292

Nothing audited here touches process-global state, shared filesystem paths, or [ResourceLock] / [DoNotParallelize] declarations. All six changed test files add or adjust purely self-contained assertions (Roslyn analyzer/source-generator snapshot tests, DTO record shape changes, and stateless helper renames) with no Environment.*, Directory.SetCurrentDirectory, Console.Set*, culture, or shared-path mutation, and no static mutable state introduced. No .runsettings, testconfig.json, .csproj/.props/.targets, or assembly-level [Parallelize]/[DoNotParallelize] files were touched by this PR, so no assembly's parallelization state changed either. Nothing to flag for parallel-safety.

Test assembly	Scope	Workers	Analyzer coverage
MSTest.Acceptance.IntegrationTests	MethodLevel	0 (CPU count)	coverable once the parallel-safety analyzers ship (attribute)
Microsoft.Testing.Platform.Acceptance.IntegrationTests	MethodLevel	0 (CPU count)	coverable once the parallel-safety analyzers ship (attribute)
MSTest.Analyzers.UnitTests	MethodLevel	0 (CPU count)	coverable once the parallel-safety analyzers ship (attribute)
MSTest.SourceGeneration.UnitTests	off (no [assembly: Parallelize] present)	n/a	n/a
MSTestAdapter.UnitTests	off (TestContainer-based bespoke engine, no MSTest parallel scheduler)	n/a	n/a
Microsoft.Testing.Extensions.VSTestBridge.UnitTests	MethodLevel	0 (CPU count)	coverable once the parallel-safety analyzers ship (attribute)

_{Re-run with /parallel-audit.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 192.7 AIC · ⌖ 2.88 AIC · ⊞ 23.9K · [◷]( · ◷)

[github-actions]

🧪 Expert test review — PR #11292

This PR's test changes cover two independent efforts: (1) making client "stateful" capability nullable/undeclared across the server-mode protocol, run-settings patching, and MTP client, and (2) hardening the reflection-based MSTest source generator (inheritance/hiding rules, AreGeneratedDescriptorsComplete, ResolveMethod overload disambiguation) plus a small analyzer improvement for enum-underlying-value assertions and exception-flattening in FormatterUtilities. Every reviewed test's assertions were cross-checked against the corresponding production code and matched the implemented behavior (branch-by-branch for the nullable-IsStateful tri-state, and case-by-case for the generator's member-hiding matrix). No incorrect, ineffective, or unreliable tests were found; the most common minor observation is long [TestMethod]s with several Should().Contain()/NotContain() checks in the source-generator suite, which is idiomatic for this file (multi-assertion generator-output verification) and not penalized here. No inline suggestions are posted this run — no high-confidence actionable defect was found in the changed tests.

Grade	Test	Mutation	Notes	How to improve
A (90–100)	mod ReviewAlwaysTrueAssertConditionAnalyzerTests. WhenAssertAreEqualPinsEnumUnderlyingValue_ NoDiagnostic	3/3 killed	Verifies enum-to-underlying-type contract assertions across all comparison orders don't diagnose.	—
A (90–100)	new ReviewAlwaysTrueAssertConditionAnalyzerTests. WhenAssertAreEqualPinsEnumsWithAllUnderlyingTypes_ NoDiagnostic	4/4 killed	Exercises every integral enum underlying type to confirm the analyzer's type check is complete.	—
A (90–100)	new ReviewAlwaysTrueAssertConditionAnalyzerTests. WhenAssertAreEqualUsesOtherConstantExpressions_ Diagnostic	4/4 killed	Confirms the new enum-pinning exception doesn't over-suppress unrelated constant-vs-constant diagnostics.	—
A (90–100)	new ReviewAlwaysTrueAssertConditionAnalyzerTests. WhenAssertAreEqualUsesOtherEnumConstantExpressions_ Diagnostic	4/4 killed	Covers enum-vs-enum, flags, and mismatched conversions that must still diagnose.	—
A (90–100)	mod MSTestRunSettingsTests. UndeclaredNonVisualStudioClientDoesNotSetDesignMode	2/2 killed	Verify(...)-equivalent Should().BeFalse() checks the undeclared/non-VS branch of the null-coalescing default.	—
A (90–100)	new MSTestRunSettingsTests. UndeclaredVisualStudioClientSetsDesignModeForBackwardCompatibility	2/2 killed	Confirms the VS backward-compatibility fallback fires only when statefulness is undeclared.	—
A (90–100)	new MSTestRunSettingsTests. StatelessVisualStudioClientDoesNotSetDesignMode	2/2 killed	Proves an explicit stateless declaration overrides the VS legacy fallback.	—
A (90–100)	new RunSettingsPatcherTests. Patch_UndeclaredNonVisualStudioClient_DoesNotSetDesignMode	2/2 killed	Mirrors the adapter-side undeclared/non-VS case for the VSTest bridge patcher.	—
A (90–100)	new RunSettingsPatcherTests. Patch_UndeclaredVisualStudioClient_SetsDesignModeForBackwardCompatibility	2/2 killed	Mirrors the adapter-side VS legacy-fallback case for the bridge patcher.	—
A (90–100)	new RunSettingsPatcherTests. Patch_StatelessVisualStudioClient_DoesNotSetDesignMode	2/2 killed	Confirms explicit stateless declaration on VS client suppresses the legacy default.	—
A (90–100)	new MtpServerClientTests. InitializeAsync_DefaultOptions_ LeaveStatefulnessUndeclared	1/1 killed	Confirms the default client options omit IsStateful (null), matching the new opt-in contract.	—
A (90–100)	new MtpServerClientTests. SerializeClientCapabilities_UndeclaredStatefulness_ OmitsProperty	1/1 killed	Verifies the wire payload omits isStateful entirely when null, not a false literal.	—
A (90–100)	new MtpServerClientTests. SerializeClientCapabilities_DeclaredStatefulness_ IncludesProperty	2/2 killed	Data-driven true/false cases confirm the declared value round-trips onto the wire.	—
A (90–100)	new JsonTests. Deserialize_InitializeRequest_WithIsStatefulFalse_StjPath_ SurfacesStatelessClient	1/1 killed	Renamed/refocused test now asserts an explicit false, distinct from the undeclared case.	—
A (90–100)	new JsonTests. Deserialize_InitializeRequest_WithoutIsStateful_StjPath_ LeavesCapabilityUndeclared	1/1 killed	Confirms the STJ path leaves IsStateful null (not false) when the field is absent.	—
A (90–100)	new JsonTests. Deserialize_InitializeRequest_WithInvalidIsStateful_StjPath_ Throws	1/1 killed	Confirms a non-boolean isStateful value throws MessageFormatException with the field name.	—
A (90–100)	new JsonTests. Deserialize_InitializeRequest_WithNullIsStateful_StjPath_ Throws	1/1 killed	Confirms a JSON null isStateful (present but no value) still throws rather than defaulting.	—
A (90–100)	new JsonTests. Deserialize_ClientCapabilities_WithIsStatefulFalse_ JsonitePath_SurfacesStatelessClient	1/1 killed	Mirrors the STJ false-case test for the legacy Jsonite deserialization path.	—
A (90–100)	new JsonTests. Deserialize_ClientCapabilities_WithoutIsStateful_ JsonitePath_LeavesCapabilityUndeclared	1/1 killed	Mirrors the STJ absent-field test for the Jsonite path.	—
A (90–100)	new JsonTests. Deserialize_ClientCapabilities_WithInvalidIsStateful_ JsonitePath_Throws	1/1 killed	Mirrors the STJ invalid-type test for the Jsonite path.	—
A (90–100)	new JsonTests. Deserialize_ClientCapabilities_WithNullIsStateful_ JsonitePath_Throws	1/1 killed	Mirrors the STJ null-value test for the Jsonite path.	—
A (90–100)	new ClientCapabilitiesExtensionsTests. IsStateful_UndeclaredCapability_ DefaultsToFalse	1/1 killed	Confirms IClientCapabilities.IsStateful still defaults false when construction leaves it undeclared.	—
A (90–100)	new ClientCapabilitiesExtensionsTests. GetIsStateful_ForwardsCustomImplementation	2/2 killed	Data-driven true/false verify the extension falls back to the interface's IsStateful for non-service implementers.	—
A (90–100)	new FormatterUtilitiesTests. Serialize_ExceptionStates_ IncludeInnerExceptionMessagesAndStackTraces	4/4 killed	Table-driven loop over Failed/Error/Timeout/Cancelled states confirms inner-exception text is spliced for each.	—
A (90–100)	new FormatterUtilitiesTests. FormatException_AggregateException_ IncludesEveryBranch	2/2 killed	Confirms both flattened inner exceptions of an AggregateException appear in message and stack trace.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_ExcludesInaccessibleMembersFromBaseTypeInAnotherAssembly	9/9 killed	Broad but focused cross-assembly matrix (hidden test/context members, static/property hiding) with one assertion per case.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_PropertyHidingTestMethod_ MarksDescriptorsIncomplete	2/2 killed	Confirms a hiding property both flags incompleteness and excludes the base test call.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_NonMethodMembersHideBaseMethods	4/4 killed	Field/event/nested-type hiding all correctly suppress the corresponding base test call with no compile errors.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_MethodHidingTestAttributedPropertyAccessor_ MarksDescriptorsIncomplete	2/2 killed	Confirms a method hiding a [TestMethod]-attributed property getter is treated as incomplete.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_HiddenUnsupportedInheritedMethods_ ReportDiagnostics	2/2 killed	Confirms AOTSG0004/0005 still fire for generic/by-ref base methods hidden by incompatible derived members.	—
A (90–100)	mod MSTestReflectionMetadataGeneratorTests. Generator_IncludesMethodsFromMultiLevelInheritance	1/1 killed	Added AreGeneratedDescriptorsComplete=true assertion confirms the override case stays flagged complete.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_PrivateSameSignatureMethod_ DoesNotReplaceInheritedTest	2/2 killed	Confirms a private same-signature member neither replaces nor marks descriptors complete.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_PublicNonTestSameSignatureMethod_ DoesNotReplaceInheritedTest	2/2 killed	Confirms a public non-test same-signature member replaces the call but still marks incomplete.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_InstanceTestMethod_ ReplacesStaticAncestor	2/2 killed	Confirms instance override of a static base test method both replaces the call and is complete.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_DynamicTestMethod_ ReplacesObjectAncestor	2/2 killed	Confirms dynamic/object parameter-type equivalence in the runtime signature comparison.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. RuntimeSignature_DynamicAndObjectReturnTypes_ AreEquivalent	1/1 killed	Focused unit check on the signature-equivalence helper itself, isolating the dynamic/object return case.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. RuntimeSignature_DifferentReturnTypes_ AreNotEquivalent	2/2 killed	Confirms a differing return type both fails the helper directly and marks generation incomplete end-to-end.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. RuntimeSignature_DifferentlyNamedMethodTypeParameters_ AreNotEquivalent	1/1 killed	Confirms generic type-parameter name sensitivity mirrors reflection's ToString()-based identity.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_NestedGenericContainingTypeSubstitutions_ HaveDistinctRuntimeSignatures	3/3 killed	Confirms nested generic containing-type substitutions (Outer(int) vs Outer(long)) are treated as distinct.	—
A (90–100)	mod MSTestReflectionMetadataGeneratorTests. Generator_DerivedMethodGroup_ HidesBaseOverloads	4/4 killed	Rewritten to assert overload hiding (was previously asserting both overloads survive) with no compile errors.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_DerivedIndexer_ DoesNotHideBaseItemMethod	2/2 killed	Confirms an indexer (this[int]) does not collide with a base method literally named Item.	—
A (90–100)	new MSTestReflectionMetadataGeneratorTests. Generator_DerivedPropertyAccessor_ DoesNotHideBaseAccessorNamedMethod	4/4 killed	Goes further than string checks: emits and loads the assembly, reflects the registered MethodInfo, and asserts its DeclaringType — directly validating the DeclaringType-based ResolveMethod fix.	—
A (90–100)	mod MSTestReflectionMetadataGeneratorTests. ExcludesInaccessibleMembersFromBaseTypeInAnotherAssembly (registration helper stub)	N/A	Test-double stub update (RegisteredTestMethods property) supports reflection assertions in the new accessor test.	—
A (90–100)	mod ObjectModelConvertersTests field init (ClientInfo capability rename)	N/A	Mechanical rename of IsStateful to DeclaredIsStateful constructor argument; no behavior change.	—
A (90–100)	mod SourceGenerationNonAotTests. SourceGenerationNonAot_BuildsAndRunsTests_ WithExitCodeZero	N/A (integration)	Updated StringAssert.Contains lines track the ResolveMethod(..., DeclaringType, ...) signature change.	—
A (90–100)	mod ClientCapabilities acceptance record (IsStateful nullable)	N/A (integration)	Protocol-version test asset updated to match the server's nullable, NullValueHandling.Ignore contract.	—

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 219.6 AIC · ⌖ 3.41 AIC · ⊞ 15.9K · [◷]( · ◷)

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

#	Dimension	Verdict
17	Documentation Accuracy	🟡 1 MODERATE

✅ 21/22 dimensions clean.

• GetIsStateful doc comment overstates the null-guarantee for third-party IClientCapabilities implementations (see inline comment)

Summary of what was reviewed:

This PR does three largely independent things:

1. Makes IClientCapabilities.IsStateful presence-aware end-to-end (wire format, ClientCapabilities record, ClientCapabilitiesService, new GetIsStateful() extension, RunSettingsPatcher/MSTestRunSettings VS-compat fallback) so clients that predate the capability can be distinguished from clients that explicitly declare themselves stateless. This aligns with the newly added .github/copilot-instructions.md guidance on modeling "unsupported or not declared" separately from an explicit value, and is thoroughly covered by new unit tests across both the STJ and Jsonite deserialization paths (including throwing MessageFormatException for a non-boolean or null isStateful value, which is a nice defensive touch for a wire-format field).
2. Preserves inner-exception messages and stack traces (via a shared SerializerUtilities.FormatException/ExceptionFlattener) when serializing failed/error/timeout/canceled TestNodeStateProperty values for server mode, instead of only surfacing the outermost exception. Also incidentally cleans up leftover, unresolved git merge-conflict markers (<<<<<<</=======/>>>>>>>) that had been committed into InternalAPI.Unshipped.txt on a prior merge -- good catch, and verified the PR head file is clean of these markers.
3. Fixes cross-assembly/inheritance correctness bugs in the MSTest source generator's reflection-metadata emission: method resolution at runtime now disambiguates by DeclaringType in addition to Name/ParameterTypes (fixing overload/hiding ambiguity across a type hierarchy), and TestClassModelBuilder's member-collection walk now correctly implements C#'s member-hiding semantics (a derived member of any kind hides a same-named base method, while overloads on the same type are preserved) instead of a signature-based dictionary key that couldn't distinguish "hidden" from "overloaded." This is exercised by an extensive, well-targeted new test matrix (fields/events/nested types hiding methods, new keyword shadowing, generic method hiding, dynamic vs object equivalence, static/instance hiding, nested closed-generic parameter types). A companion analyzer change relaxes MSTEST0032 (ReviewAlwaysTrueAssertConditionAnalyzer) so it no longer flags Assert.AreEqual(1, (int)SomeEnum.Value)-style checks that pin an enum's underlying numeric value against a literal -- a legitimate use case previously misclassified as an always-true assertion -- also with thorough positive/negative test coverage across all integral underlying enum types.

All three areas are cross-referenced by corresponding acceptance/unit test updates, and the public/internal API surface changes are correctly declared in PublicAPI.Unshipped.txt / InternalAPI.Unshipped.txt with no init-accessor additions to genuinely new public API (the existing init on ClientCapabilities.IsStateful predates this PR and is grandfathered per repo policy). No threading, resource-disposal, or cross-TFM issues were found in the changed files.

[github-actions]

[MODERATE] Documentation Accuracy / Public API Design

The XML doc for GetIsStateful states it returns null "when the client did not declare the capability," but that guarantee only holds for the internal ClientCapabilitiesService implementation. Any other IClientCapabilities implementation (including third-party or test-double implementations built against this [Experimental] public interface) falls through to capabilities.IsStateful, which is a non-nullable bool -- so GetIsStateful() can never return null for those implementations, even if the underlying client genuinely never declared the capability. The accompanying unit test (GetIsStateful_ForwardsCustomImplementation) confirms this by construction: a custom IClientCapabilities can only ever yield true/false, never the undeclared state.

Recommendation: Either (a) clarify the XML doc to state explicitly that only the platform's own internal capability implementation can represent the undeclared state, and that custom implementations of IClientCapabilities cannot signal it through this extension, or (b) consider exposing a nullable-aware member directly on the interface (e.g., an optional bool? DeclaredIsStateful alongside the existing non-nullable IsStateful) so the undeclared/false distinction is representable for any implementer, not just the internal record.

[Amaury Levé (Evangelink)]

Addressed in #11293 with b9d5553. Clarified that only the platform-provided capability service can return null for an undeclared capability, while custom implementations fall back to IsStateful.

[Amaury Levé (Evangelink)]

Correction to my previous reply: #11293 was already merged, so it does not contain this documentation clarification. The fix is now in canonical follow-up #11298 targeting main (commit 4965b90) and backport #11299 targeting rel/4.4 (commit b77da21, milestone 80).

Both clarify that only the platform-provided capability service can return null for an undeclared capability; custom implementations return their non-nullable IsStateful value. The accidentally recreated #11293 head branch has been deleted. This supersedes my earlier #11293/b9d5553f0 reference.