Expand built-in metadata references in the task host

[JanProvaznik]

Fixes #14763.

The problem

A task that runs in a task host reads item metadata differently than the same task in the MSBuild process.

A project defines metadata on an item definition:

<ItemDefinitionGroup>
  <PreprocessCompile>
    <OutputName>%(Filename)</OutputName>
  </PreprocessCompile>
</ItemDefinitionGroup>

A task in the MSBuild process reads OutputName as hello. The same task in a task host reads %(Filename). One build wrote a file path that ended with \%(Filename).cs.

The defect is old. It is easy to find now, because -mt sends almost all tasks to a task host.

Terms

This description uses these terms. Each term has one meaning.

Term	Meaning
task host	A separate process that runs a task.
engine item	ProjectItemInstance.TaskItem. The item type that the engine uses.
marshalled item	TaskParameterTaskItem. The item type that crosses a process boundary.
direct metadata	Metadata that the project sets on the item. A task can also write it.
definition metadata	Metadata that the item gets from an ItemDefinitionGroup.
to expand	To replace a reference such as %(Filename) with its value.
unexpanded	The value still contains %(...).
origin	Direct or definition. See concept 1.

Three concepts

Read these first. The rest of this description follows from them.

Concept 1: origin

An engine item keeps two groups of metadata. The origin of a value controls how the item reads that value.

Origin	How the item reads it	Example
Direct — the project sets it on the item, or a task writes it later	as stored. Direct metadata wins over definition metadata.	OutputFileExtension = .cs
Definition — the item gets it from an ItemDefinitionGroup	expanded, against this item, at the time of the read	OutputName = %(Filename), stored as text, read as hello

The item expands definition metadata at each read. The item does not expand it one time and keep the result. This is necessary. A task can change ItemSpec, and then %(Filename) must give a new value.

Concept 2: the boundary discards the origin

flowchart LR
    subgraph BEFORE["Engine item: two groups"]
        direction TB
        D1["Direct<br/>OutputFileExtension = .cs"]
        I1["Definition<br/>OutputName = %(Filename)"]
    end

    subgraph AFTER["Marshalled item: one dictionary"]
        direction TB
        M1["OutputFileExtension = .cs"]
        M2["OutputName = %(Filename)"]
    end

    BEFORE -->|"CloneCustomMetadataEscaped()"| AFTER

Loading

CloneCustomMetadataEscaped() copies both groups into one dictionary. The marshalled item receives that dictionary. Thus the marshalled item does not know the origin of a value.

But the value still shows its origin. Evaluation expands all other expression types. Only a built-in metadata reference stays unexpanded. Thus a value that still contains %(...) is definition metadata. One exception exists. See limit 1 below.

The marshalled item uses this fact to find the origin again. This is the key point of the change. The alternative is to send the origin across the boundary. That alternative changes the serialized data, and a serialization change cannot go into a servicing branch.

Concept 3: how the task host finds the origin

flowchart TB
    V["a stored value"] --> Q1{"did the task write it,<br/>after the task host<br/>received the item?"}
    Q1 -->|yes| P1["<b>Direct</b> — known<br/>recorded at the write"]
    Q1 -->|no| Q2{"is the value unexpanded?<br/>does it contain %(...)?"}
    Q2 -->|no| P2["<b>Direct</b> — known<br/>evaluation expanded it already"]
    Q2 -->|yes| P3["<b>Definition</b> — assumed"]
    P1 --> R1(["read as stored"])
    P2 --> R2(["read as stored"])
    P3 --> R3(["read expanded"])
    P3 -.->|"the one wrong result"| W["direct metadata that contains %(...)<br/>as stored text. It arrives here only<br/>after a task output made the item flat."]

Loading

Two questions give the origin. The first question has a known answer. The second question has an assumed answer.

The code uses these names:

Name	Purpose
_writtenByTask	The first question.
IsUnexpanded	The second question.
ExpandIfFromItemDefinition	The full decision.

Not all reads use this decision. The next table gives the rule.

Reads that give a value to a task	Reads that give the full collection
GetMetadata, GetMetadataValueEscaped, CopyMetadataTo, and the TaskItem copy constructor that calls it	CloneCustomMetadata, CloneCustomMetadataEscaped, EnumerateMetadata
expand	do not expand
The task gets a complete value. The destination of a copy cannot hold an unexpanded value.	These reads copy the collection of the engine item. The value goes back to the engine without a change.

If you add an accessor, put it on the side of the accessor that it resembles. The type-level comment in the code gives this rule.

The changes

Each row is a way for a task to detect that it runs in a task host. All rows were measured. The test ran the same task two times: in the MSBuild process, and with TaskHostFactory. The item is folder\hello.txt. The definition sets NameMeta to %(Filename).

#	Difference	In process	In task host, before	Change that corrects it
1	The task reads the metadata.	hello	%(Filename)	GetMetadataValueEscaped calls ExpandIfFromItemDefinition. This is concept 3.
2	The task changes ItemSpec, then reads again.	renamed	%(Filename)	The same change. The item expands at each read, thus the value follows ItemSpec.
3	The definition uses %(RecursiveDir), for example out\%(RecursiveDir)%(Filename)%(Extension).	out\sub1\sub2\hello.txt	out\%(RecursiveDir)hello.txt	The item reads RecursiveDir from its own metadata. RecursiveDir comes from the wildcard, not from ItemSpec.
4	The task copies its input with new TaskItem(input) or CopyMetadataTo.	hello	%(Filename)	CopyMetadataTo expands. This is what BulkImportMetadata does on the engine item.
5	The task reads %(FullPath), changes ItemSpec, then reads again.	the new path	the old path	Both ItemSpec setters call _cachedModifiers.Clear(). The engine item does this already.

Two notes on that table.

Row 4 is important. Many tasks create new TaskItem(input). Without this change, the defect stays for those tasks.

Row 5 is an old defect, not a new one. The released MSBuild gives the same old path for a direct %(FullPath) read after a change to ItemSpec. This change corrects it, because concept 3 would otherwise put the same defect into definition metadata.

These conditions do not change:

Condition	In process	In task host	Reason
The item sets the metadata directly.	explicit	explicit	Direct metadata wins in both processes.
The value is %25(Filename).	%(Filename)	%(Filename)	Escaped text is not an expression.
The task writes %(Filename), then reads it.	%(Filename)	%(Filename)	_writtenByTask. This is question 1 of concept 3.
The task writes %(Filename), then copies the item.	%(Filename)	%(Filename)	The same. The copy keeps the value.
The project reads the item after a task output.	%(Filename)	%(Filename)	Reads that give the full collection do not expand.

Two known limits

Limit 1. Direct metadata that contains %(...) as text, after a task output.

GatherTaskItemOutputs makes task outputs flat in the MSBuild process. After that, such a value looks the same as definition metadata. The task host then expands it.

To find this limit, a build must do all of these steps:

1. A task writes %(Filename) as text.
2. The task returns the item.
3. A second task reads the value.
4. The build compares the value against text that contains %(.

Steps 1 to 3 keep the correct value inside one task. Only step 4 can show the difference. No MSBuild code expands %(...) in metadata after evaluation. Thus the value is text that has no function. To remove this limit, the change must add data to the serialized item.

Limit 2. A reference with an item type, for example %(Thing.Filename).

The engine uses a metadata table that has no item type. Thus such a reference gives an empty value in the MSBuild process. The task host keeps the text.

An empty value here is not better. It would remove all text of that shape. This condition is the same as before the change. A test holds this decision.

Compatibility

The change does not modify the serialized data. Thus there is no version negotiation and no packet version increase. A task host from older sources runs older code and does not change its behavior.

TaskParameter is used only when items cross a process boundary. The four users are TaskHostConfiguration, TaskHostTaskComplete, TaskHostBuildResponse, and the RAR node. Thus the change does not affect a task in the MSBuild process.

The CLR2 task host has its own copy of TaskParameter. The change does not affect it.

No change wave. The change replaces two kinds of value: text that no build can use, and an old path. A project cannot select the task host under -mt. Thus no build selected the current behavior. A change wave also stops a clean backport, because the current wave does not exist in the servicing branches.

The backport was tested. Both commits go into vs18.9 and vs18.10 with no conflict. Both branches build and pass the tests. On a vs18.9 build, the example gives hello. The released 18.9.1 gives %(Filename).

Tests

TaskParameter_Tests covers the five differences above. It also covers escaped text, spaces in a reference, an incomplete reference, collection reads, and a second boundary crossing. Most tests compare the marshalled item against the engine item. They do not use a fixed value. Thus the two item types cannot become different without a test failure.

One test is a theory over ItemSpecModifiers.All. A new modifier gets a test automatically.

ItemDefinitionMetadataInTaskHost_Tests runs a task in a real task host. Each test asserts the process of the task, then compares the value against the value from the MSBuild process.

Each change was removed one time to confirm that its own test fails. The RecursiveDir code was also tested with a mutation.

[Copilot]

Pull request overview

Fixes a long-standing mismatch between in-proc and out-of-proc (TaskHost) task execution: item-definition metadata containing built-in metadata references (for example %(Filename)) is now expanded for the task when read, aligning TaskHost behavior with in-proc ProjectItemInstance.TaskItem behavior. This prevents tasks from receiving inert literal strings like %(Filename) (and producing incorrect paths such as ...\%(Filename).cs) when running under -mt/TaskHost.

Changes:

• Teach TaskParameterTaskItem to expand built-in metadata references on “task-facing” reads when the stored metadata appears to originate from an item definition (while preserving raw/bulk-read semantics and values written by the task).
• Add a minimal built-in metadata expander in Microsoft.Build.Framework to support expansion in TaskHost scenarios without pulling in the full evaluation expander.
• Add targeted unit tests and end-to-end TaskHost tests to validate parity with in-proc behavior (including RecursiveDir and cache invalidation on ItemSpec reassignment).

Reviewed changes

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

Show a summary per file

File	Description
src/Shared/TaskParameter.cs	Updates TaskHost-marshalled ITaskItem behavior to expand item-definition built-in metadata references on read; tracks task-written metadata and clears cached modifiers on ItemSpec changes.
src/Framework/BuiltInMetadataExpander.cs	Adds an internal helper to expand built-in metadata references (%(Filename), etc.) against an item spec (including RecursiveDir support).
src/Shared/UnitTests/TaskParameter_Tests.cs	Adds regression/unit coverage for TaskHost boundary behavior, cloning/copy semantics, RecursiveDir, and cache invalidation.
src/Build.UnitTests/BackEnd/MetadataObservationTask.cs	Adds a simple test task that reports observed metadata and process id to distinguish in-proc vs TaskHost execution.
src/Build.UnitTests/BackEnd/ItemDefinitionMetadataInTaskHost_Tests.cs	Adds end-to-end tests that run the task both in-proc and via TaskHostFactory and assert identical observed results.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[JanProvaznik]

I decided against having the protocol change due to concerns about backportability and risk to introduce this so late to NET 11 and cross-version taskhost compat.

The tradeoff is expander duplication here. This should probably be refactored so the needed unified expander part goes to Framework and can be used in both Shared and Build. But that would also be massive and hard to justify backporting.

[JanProvaznik]

waiting for permissions to see the actual offending task, but I am pretty confident in the parity being acheived.

[rainersigwald]

Suggested change

	/// Names of metadata the task wrote on this item. Their values are read as stored.
	/// Names of metadata the task wrote on this item. The values of these metadata are returned without expansion.

[rainersigwald]

I don't love doing a linear scan over the metadata to determine if we need to do a linear transform over the data. Could we collapse that?

[rainersigwald]

In other contexts @DustinCampbell has found that searching for % by itself followed by an i+1 check for ( is faster (because the single-character search can vectorize better). Unfortunately the optimized helper method ExpressionShredder.ContainsMetadataMarker doesn't exist in all of our target branches (it's in #14697). Let's do this and file a followup to adopt that in the main branch.

[DustinCampbell]

I hope it'll be there soon. I'm working on addressing CR feedback now.

[rainersigwald]

Yeah, the gotcha here is that the bug addressed by this PR is affecting internal CloudBuild users now, so we'd like to patch it, and I don't want to have to backport your change too.

[DustinCampbell]

Makes sense. Yeah, just make a follow-up on main once #14697 goes in.

(FYI, #14697 just needs reapproval now.)

[rainersigwald]

I am on board with this for the purpose of getting a fix out, but please file a follow-up to unify these.