[JanProvaznik]

Part of the node-ownership work specced in #14508, but standalone: it depends on nothing else in that stack and is a correctness fix on its own.

The bug

IsAllowedBitnessMismatch exists so a parent that cannot describe the architecture of the SDK TaskHost it launches can still connect to it. A .NET Framework parent launching a Runtime="NET" TaskHost is the motivating case, and to make it work the parent deliberately suppresses its own architecture bit.

The tolerance was never scoped to architecture. It returned true whenever the parent sent no architecture bit and the child expected x64/arm64 — regardless of every other flag in the options word:

bool receivedIsX86 = !X64 && !Arm64;
return receivedIsX86 && (expectedIsX64 || expectedIsArm64);

So any parent that suppressed its architecture bit also matched a TaskHost that differed from it in identity flags.

That matters most for NodeReuse, because NodeReuse is the only thing separating a long-lived sidecar TaskHost from one that exits with the build:

• effectiveNodeReuse = EnableNodeReuse && _useSidecarTaskHost (TaskHostTask.cs)
• that flows into GetHandshakeOptions(taskHost: true, nodeReuse: effectiveNodeReuse, ...), which sets HandshakeOptions.NodeReuse
• the resulting word is both the in-process pool key and the wire handshake

Nothing else separates them for this connection: a sidecar is a .NET TaskHost, and .NET TaskHost handshakes deliberately neutralize the file-version components with the NetTaskHostHandshakeVersion = 99 sentinel, while the salt is the tools directory, which coincides whenever a parent resolves the same installed SDK.

Consequence: a parent that never asked for a sidecar could be admitted to one, take it over for its own build, and afterwards return it to the global reconnectable pool. LowPriority and Administrator were ignored the same way.

The fix

Require every non-architecture flag to match; allow divergence only in X64/Arm64.

The case the tolerance exists for is unaffected — those parents differ from the TaskHost only in architecture, which is exactly what stays tolerated.

Compatibility

The blast radius is flags that should already agree between a matched parent and child:

• NodeReuse and LowPriority are handed to the child as launch arguments by the same parent that computed them
• Administrator is derived on both sides from elevation the child inherits
• CLR2 / NET / TaskHost are structural

This is nonetheless the one change in the stack that can surface as MSB4216, so it is landing on its own, ahead of the lifetime work, and carries a cross-version check of a Runtime="NET" UsingTask in both directions across the 10.0.3xx boundary.

Tests

Six added to NodeEndpointOutOfProcBase_Tests. Four are load-bearing — verified failing against the unpatched method and passing after:

• NonSidecarParent_SidecarTaskHost_NotTolerated
• SidecarParent_NonSidecarTaskHost_NotTolerated
• DifferingLowPriority_NotTolerated
• DifferingAdministrator_NotTolerated

Two are regression guards that must keep passing either way:

• SidecarParent_SidecarTaskHost_ArchitectureStillTolerated — the motivating case still connects
• HandshakeVersionInUpperByte_IsIgnored — the version byte still does not affect comparison

All 12 tests in the class pass (net11.0).

Why this is separate from the lifetime fix

This does not fix #14313 on its own. That leak is a sidecar disconnecting after a build and relisting on its pipe, which only the ownership-lifetime change stops.

The two are complementary and this one is a precondition: once sidecars are owned, an owned sidecar could still be adopted in the window before its owner connects, or by a differently-versioned MSBuild during rollout, if this hole stayed open.

[JanProvaznik]

/review

[github-actions]

❌ Expert Code Review (command) failed. Please review the logs for details.

[Copilot]

Pull request overview

This PR fixes an over-broad .NET TaskHost handshake tolerance that previously allowed any mismatch in the handshake options word (when the parent suppressed its architecture bits) to be treated as acceptable, rather than limiting the tolerance strictly to architecture differences. This tightens correctness and prevents unintended cross-identity matching (notably around NodeReuse, LowPriority, and Administrator) while preserving the intended cross-arch connectivity scenario.

Changes:

• Scope IsAllowedBitnessMismatch so only X64/Arm64 bits may differ; all other HandshakeOptions flags must match.
• Add unit tests covering previously-permitted (but incorrect) mismatches for NodeReuse, LowPriority, and Administrator, plus regression guards for version-byte masking and the motivating arch-tolerance scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Shared/NodeEndpointOutOfProcBase.cs	Restricts the handshake leniency to architecture-only differences by enforcing equality of all non-architecture handshake flags.
src/Build.UnitTests/BackEnd/NodeEndpointOutOfProcBase_Tests.cs	Adds targeted tests to prevent future regressions and validate the narrowed tolerance behavior.

[JanProvaznik]

/review

[github-actions]

❌ Expert Code Review (command) failed. Please review the logs for details.

[AR-May]

Consulted a bit with AI and have a question about handshake version mismatches. Is ignoring the version byte here intentional and wanted? If so, the Windows handshake path might not actually tolerate that mismatch. The upper byte is the handshake version, but on Windows TryReadIntForHandshake validates that byte before this helper runs. If it differs, it writes two rejection markers, returns OldMSBuild, and HandshakeResult.Value is 0. The caller still reaches this method, which may treat that 0 as an allowed x86-to-x64/Arm64 mismatch, but the parent will already reject the connection after reading the queued markers.

[JanProvaznik]

i believe it is intentional specifically for the NET taskhost use case

[AR-May]

I guess as well, but then the bug is that windows path in TryReadIntForHandshake verifies the version and fails, leading to rejection of such connection.

[AR-May]

Here is related code snippets
in PacketPumpProc function during handshake check:

                            if (!_pipeServer.TryReadIntForHandshake(
                                byteToAccept: index == 0 ? (byte?)CommunicationsUtilities.handshakeVersion : null, /* this will disconnect a < 16.8 host; it expects leading 00 or F5 or 06. 0x00 is a wildcard */
#if NETCOREAPP2_1_OR_GREATER
                             ClientConnectTimeout, /* wait a long time for the handshake from this side */
#endif
                              out HandshakeResult result))
                            {
                                CommunicationsUtilities.Trace($"Handshake failed with error: {result.ErrorMessage}");
                            }

In CommunicationsUtilities.TryReadIntForHandshake

// Abort for connection attempts from ancient MSBuild.exes
if (byteToAccept != null && bytesRead > 0 && byteToAccept != bytes[0])
{
    stream.WriteIntForHandshake(0x0F0F0F0F);
    stream.WriteIntForHandshake(0x0F0F0F0F);
    result = HandshakeResult.Failure(HandshakeStatus.OldMSBuild, $"Client: rejected old host. Received byte {bytes[0]} instead of {byteToAccept}.");
    return false;
}