Fix TaskHostTask to pass request-level global properties to TaskHost by JanProvaznik · Pull Request #13443 · dotnet/msbuild

JanProvaznik · 2026-03-24T09:18:47Z

Summary

Fixes #13153 — -mt mode breaks NuGet static graph restore for conditional ProjectReference items that depend on MSBuildRestoreSessionId.

Root Cause

When -mt mode routes tasks to an out-of-process TaskHost (via TaskRouter.NeedsTaskHostInMultiThreadedMode()), TaskHostTask.Execute() was sending BuildParameters.GlobalProperties (build-level) to the TaskHostConfiguration. These build-level properties do not include per-request properties like MSBuildRestoreSessionId that are added by ExecuteRestore() in XMake.cs.

This caused NuGet's RestoreTaskEx (which lacks MSBuildMultiThreadableTaskAttribute and thus gets routed to TaskHost in -mt mode) to receive an empty MSBuildRestoreSessionId via IBuildEngine6.GetGlobalProperties(). The NuGet static graph restore then excluded conditional ProjectReference items, breaking restore for projects like �xternals.csproj in dotnet/runtime.

Without -mt: TaskHost.GetGlobalProperties() returns _requestEntry.RequestConfiguration.GlobalProperties → ✅ includes MSBuildRestoreSessionId

With -mt: OutOfProcTaskHostNode.GetGlobalProperties() returns _currentConfiguration.GlobalProperties (from TaskHostConfiguration) → ❌ only has build-level properties from BuildParameters.GlobalProperties

Fix

New method GetGlobalPropertiesForTaskHost() in TaskHostTask that:

Uses BuildEngine.GetGlobalProperties() via IBuildEngine6 (returns correct request-level properties from RequestConfiguration)
Falls back to BuildParameters.GlobalProperties if IBuildEngine6 is not available
Works on both net10.0 and net472 targets

Validation

Built MSBuild (Release, net10.0 + net472) — ✅ compiles clean
Built dotnet/runtime with -mt -m -restore using fixed MSBuild
- externals.csproj correctly restored (was excluded before fix)
- Binlog confirms: -mt in command line, MSBuildRestoreSessionId set with GUID, RestoreUseStaticGraphEvaluation=true, RestoreTaskEx executed

When -mt mode routes tasks to an out-of-process TaskHost, the TaskHostTask was sending BuildParameters.GlobalProperties (build-level) to the TaskHostConfiguration. These build-level properties do not include per-request properties like MSBuildRestoreSessionId that are added by ExecuteRestore(). This caused NuGet's RestoreTaskEx (which lacks MSBuildMultiThreadableTaskAttribute and thus gets routed to TaskHost in -mt mode) to receive empty MSBuildRestoreSessionId via IBuildEngine6.GetGlobalProperties(). The NuGet static graph restore then excluded conditional ProjectReference items that depend on MSBuildRestoreSessionId, breaking restore for projects like externals.csproj in dotnet/runtime. The fix uses BuildEngine.GetGlobalProperties() (which returns the correct request-level properties from RequestConfiguration) instead of BuildParameters.GlobalProperties, with a fallback to the old behavior if IBuildEngine6 is not available. Fixes dotnet#13153 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

JanProvaznik

24-Dimension MSBuild Code Review

Verdict: COMMENT — approve after addressing test coverage; comparer fix is nice-to-have.

The fix is well-targeted and correct. TaskHostTask.Execute() was sending build-level BuildParameters.GlobalProperties instead of request-level properties from RequestConfiguration, causing MSBuildRestoreSessionId to be missing in -mt mode.

Findings

Severity	Finding
🔴 MAJOR	No regression test for MT + request-level global properties scenario
🟠 MODERATE	Fallback path (line 437) uses case-sensitive comparer instead of `OrdinalIgnoreCase`
🟡 NIT	Silent fallback should log a diagnostic message

24-Dimension Checklist

All 24 dimensions pass except:

Test Coverage (🔴) — No test for the fixed scenario
String Comparison (🟠) — Fallback comparer inconsistency
Logging (🟡) — Silent fallback

See inline comments for details.

src/Build/Instance/TaskFactories/TaskHostTask.cs

- Gate the fix behind ChangeWave 18.6 so users can opt out via MSBUILDDISABLEFEATURESFROMVERSION=18.6 to get the old behavior (build-level BuildParameters.GlobalProperties) exactly as before - Remove dead IBuildEngine6 fallback path — BuildEngine is always a TaskHost implementing IBuildEngine6 in this context - Add GetGlobalPropertiesTask helper and two regression tests: - GlobalProperties_ForwardedToAutoEjectedTaskInMultiThreadedMode verifies request-level properties reach auto-ejected tasks (dotnet#13153) - GlobalProperties_UseBuildLevelWhenChangeWaveDisabled verifies the old behavior is preserved when the wave is opted out - Document the change in ChangeWaves.md under 18.6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

JanProvaznik · 2026-03-24T12:06:09Z

validation on runtime running https://dev.azure.com/dnceng-public/public/_build?definitionId=334
runtime passed (in first run, though both failed overall on a race condition that's fixed in main but this branch started too early)

With the TaskHostTask fix, runtime builds successfully with -mt mode. Remove it from MSBuildMTExcludedReposOverride so VMR validation exercises runtime with -mt enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

AR-May

LGTM, but I’d like to double‑check with @rainersigwald in case there are any hidden issues with fixing this behavior.

src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs

rainersigwald

One thing I'd like to understand: how did this not break the MSBuild repo? we use static graph restore

documentation/wiki/ChangeWaves.md

JanProvaznik · 2026-03-26T17:14:01Z

It didn't break other stuff such as msbuild because it does not use this pattern:

https://github.com/dotnet/runtime/blob/7602c30b43e12e3ced08087cc1503f017ac4206e/src/libraries/pretest.proj#L18

…otnet#13443) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

JanProvaznik commented Mar 24, 2026

View reviewed changes

src/Build/Instance/TaskFactories/TaskHostTask.cs Show resolved Hide resolved

JanProvaznik force-pushed the fix/taskhosttask-global-properties branch from 16a88e8 to 36a130e Compare March 24, 2026 11:25

JanProvaznik force-pushed the fix/taskhosttask-global-properties branch from 36a130e to 20c08c1 Compare March 24, 2026 11:52

JanProvaznik and others added 2 commits March 24, 2026 12:55

Merge branch 'main' into fix/taskhosttask-global-properties

6c7b6c3

JanProvaznik marked this pull request as ready for review March 24, 2026 16:52

Copilot AI review requested due to automatic review settings March 24, 2026 16:52

Copilot started reviewing on behalf of JanProvaznik March 24, 2026 16:53 View session

JanProvaznik force-pushed the fix/taskhosttask-global-properties branch from 120070c to cccae85 Compare March 24, 2026 16:55

JanProvaznik force-pushed the fix/taskhosttask-global-properties branch from cccae85 to e3ab1b4 Compare March 24, 2026 17:10

Copilot AI reviewed Mar 25, 2026

View reviewed changes

JanProvaznik self-assigned this Mar 25, 2026

AR-May approved these changes Mar 25, 2026

View reviewed changes

src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs Outdated Show resolved Hide resolved

JanProvaznik commented Mar 25, 2026

View reviewed changes

src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs Outdated Show resolved Hide resolved

Apply suggestion from @JanProvaznik

1de0a85

rainersigwald approved these changes Mar 25, 2026

View reviewed changes

documentation/wiki/ChangeWaves.md Show resolved Hide resolved

JanProvaznik merged commit 5065de5 into dotnet:main Mar 27, 2026
10 checks passed

This was referenced Mar 27, 2026

[release/10.0.3xx] Source code updates from dotnet/msbuild dotnet/dotnet#5740

Open

[release/11.0.1xx-preview3] Source code updates from dotnet/msbuild dotnet/dotnet#5752

Merged

[main] Source code updates from dotnet/msbuild dotnet/dotnet#5736

Merged

jimpark mentioned this pull request Mar 29, 2026

Fix unresolved P2P references losing Configuration in solution builds #13458

Open

dfederm pushed a commit to dfederm/msbuild that referenced this pull request Apr 9, 2026

Fix TaskHostTask to pass request-level global properties to TaskHost (d…

7abf333

…otnet#13443) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TaskHostTask to pass request-level global properties to TaskHost#13443

Fix TaskHostTask to pass request-level global properties to TaskHost#13443
JanProvaznik merged 6 commits intodotnet:mainfrom
JanProvaznik:fix/taskhosttask-global-properties

JanProvaznik commented Mar 24, 2026

Uh oh!

JanProvaznik left a comment

Uh oh!

Uh oh!

JanProvaznik commented Mar 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

AR-May left a comment

Uh oh!

Uh oh!

Uh oh!

rainersigwald left a comment

Uh oh!

Uh oh!

JanProvaznik commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JanProvaznik commented Mar 24, 2026

Summary

Root Cause

Fix

Validation

Uh oh!

JanProvaznik left a comment

Choose a reason for hiding this comment

24-Dimension MSBuild Code Review

Findings

24-Dimension Checklist

Uh oh!

Uh oh!

JanProvaznik commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

AR-May left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JanProvaznik commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JanProvaznik commented Mar 24, 2026 •

edited

Loading