Fail fast when the MSBuildCache graph build hits real build errors

[Evangelink]

The symptom

In build 1529362 the Build solution graph with MSBuildCache step is a green check after 4m 10s — and then the full Arcade Build step runs anyway for another 7m 18s. Same shape on 1529356, 1529364 and 1529373.

Why the skip "never works"

The step was not actually green. It ended with:

Build FAILED.
...XliffTasks.targets(84,5): error : 'xlf\Resources.fr.xlf' is out-of-date with 'Resources.resx' [MSTest.Analyzers.csproj]
    1 Error(s)
##[warning]MSBuildCache canary failed with exit code 1; continuing with the regular Arcade build.

The wrapper swallowed the exit code with exit 0, and ##vso[task.logissue type=warning] does not downgrade a task result. The AzDO timeline for that step literally records result=succeeded, errorCount=0, warningCount=1 — a plain green check on a build that failed. So the skip logic worked exactly as designed; the step just lied about its outcome, and the same error was then reported a second time 7 minutes later.

The mechanism itself is fine — on healthy runs it does skip. E.g. 1529231: graph build 9.7m + prepare/pack 2.9m, Build → skipped.

What this changes

The graph build step now reports its real outcome, and stops re-running a build that cannot produce a different result:

Outcome	Step result	Build step
Cache build succeeds	✅ succeeded	skipped (unchanged)
Fails with build errors not attributable to MSBuildCache	❌ failed, errors surfaced on the step	skipped — job fails at ~4 min
Fails for something the fallback may fix	✅ succeeded + warning	runs (unchanged)

The middle row is the new behaviour. A cache hit only materializes previously stored outputs — it never runs the compiler — so every error XXnnnn: in that log came from a project that genuinely executed, exactly as the fallback would execute it. Running the whole solution through Arcade again just reproduces the same errors several minutes later.

Three failure classes deliberately stay on the fallback path, because the non-cached build can legitimately behave differently:

• anything MSBuildCache itself reports (plugin / cache / file-access problems),
• restore failures (NU*, often transient),
• any failure that never reached MSBuild's error summary (crash, OOM, msbuild.ps1 throwing).

Those keep the step green on purpose: a SucceededWithIssues step makes the job PartiallySucceeded, which Azure Repos build-validation policies treat as a failure and would block merges on runs that ultimately build fine — the same reasoning already documented for _MacOSNonBlockingTrailer.

Both cache steps now also log explicitly whether Build is going to be skipped, so the decision is readable without reverse-engineering step conditions.

Verification

Replayed the captured log of build 1529362 through the new script, plus synthetic logs for each failure class:

Scenario	Step exit	Build step
Real 1529362 log (xlf break)	1	skipped — error surfaced, ~7m18s saved
Synthetic error CS1002	1	skipped
MSBuildCache file-access violation	0	runs (fallback)
NuGet NU1301	0	runs (fallback)
Crash, no MSBuild summary	0	runs (fallback)
Successful cache build	0	skipped (cache used)

Also confirmed: YAML parses, the inline PowerShell parses clean, $LASTEXITCODE survives the Tee-Object pipe, and no trailing whitespace. stderr is deliberately left unredirected so the step's native-command error handling is byte-for-byte the previous behaviour.

Not fixed here — main is currently broken

Worth flagging separately: every PR is red right now because of a genuine break on main, not because of this pipeline.

866ed183f (OneLocBuild check-in #10268) wrote CRLF line endings inside the <source> elements of src/Analyzers/MSTest.Analyzers/xlf/*.xlf, while Resources.resx uses LF. All 211 keys are present and the text is identical — only the line endings differ — so XliffTasks reports 12 entries as out-of-date and fails the build. That needs its own fix (/t:UpdateXlf).

[Copilot]

Pull request overview

Updates the Windows CI cache canary to fail fast on classified build errors while retaining Arcade fallback for cache, restore, or infrastructure failures.

Changes:

• Captures and classifies graph-build output.
• Skips redundant fallback builds after genuine errors.
• Clarifies cache-step outcomes in pipeline logs.

Show a summary per file

File	Description
azure-pipelines.yml	Adds MSBuildCache failure classification and fallback control.

Review details

Comments suppressed due to low confidence (1)

azure-pipelines.yml:445

• This only makes a nonzero child-process exit non-blocking. Now that continueOnError is gone, any terminating PowerShell error before or after the invocation fails the step outright; because the fallback Build requires succeeded(), it will not run. That contradicts the stated contract that any preparation failure switches back to the regular build. Add top-level error handling that resets the variable, logs the warning, and exits 0 for unexpected preparation-script failures.

            if ($exitCode -ne 0) {
              # Same reasoning as the graph build above: staying green keeps a run that the fallback repairs from
              # being reported as PartiallySucceeded and blocking the merge.
              Write-Host "##vso[task.logissue type=warning]Preparing cached outputs failed with exit code $exitCode; continuing with the regular Arcade build."
              exit 0

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

azure-pipelines.yml:386

• The developer guide still says that every cache-build failure is cleaned up and sent through the regular Arcade fallback (docs/dev-guide.md:89). This branch now deliberately fails the job and skips that fallback for classified build errors, so the documented operational behavior becomes incorrect. Please update the MSBuildCache section in the same change to describe the three outcomes and the new fail-fast case.

            if ($reportedErrorCount -gt 0 -and $errorLines.Count -gt 0 -and $mayHelpLines.Count -eq 0) {
              foreach ($line in @($errorLines | Select-Object -First 20)) {
                Write-Host "##vso[task.logissue type=error]$line"
              }

              Write-Host "The solution does not build. The Arcade 'Build' step would report the same $reportedErrorCount error(s), so the build is failed here instead of repeating it."
              exit 1

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

azure-pipelines.yml:391

• This negative classifier still hard-fails MSBuild errors that the Arcade fallback can legitimately recover from. For example, MSB4166 is a child-node crash (already recognized at test/Utilities/Microsoft.Testing.TestInfrastructure/DotnetCli.cs:182-185), and MSB4260 is a static-graph-only reference failure documented at .agents/skills/build-perf-baseline/SKILL.md:306-314. Both can appear with a normal error summary, match $errorLines, and then satisfy the fail-fast condition even though rerunning without /graph may succeed. Route these infrastructure/graph failures to the fallback rather than treating every non-cache/non-NU MSBuild code as a reproducible source error.

            $fallbackMayHelp = '(?i)MSBuildCache|ProjectCache|project cache|cache plugin|CacheClient|file access|\bNU\d{4}\b'

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

azure-pipelines.yml:391

• This does not catch every error emitted by MSBuildCache itself. The pinned plugin reports duplicate outputs as Node ... produced output ... which was already produced by another node ...; MSBuild's project-cache logger adds no plugin/category prefix, so that line matches $errorLines but none of these tokens. With a normal error summary, the step therefore exits 1 and suppresses the fallback for a cache-plugin failure—the opposite of the intended classification. Include the plugin's duplicate-output diagnostic in the fallback pattern.

            $fallbackMayHelp = '(?i)MSBuildCache|ProjectCache|project cache|cache plugin|CacheClient|file access|\bNU\d{4}\b'

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Evangelink]

Picking up the three items that landed in review summaries as "suppressed due to low confidence" rather than as inline threads, since they have no thread to resolve. All three were correct and all three are now implemented.

This only makes a nonzero child-process exit non-blocking. Now that continueOnError is gone, any terminating PowerShell error before or after the invocation fails the step outright

Correct, and it applied to both cache steps. AzDO injects $ErrorActionPreference = 'stop', so a terminating error anywhere in either wrapper failed the task, made succeeded() false, and skipped the fallback. Both steps now open with a trap that sets MSBuildCacheBuildSucceeded=false, warns, and exits 0 (d62c61e). exit is flow control rather than a terminating error, so the deliberate exit 1 for a classified source break still propagates - verified explicitly, including from inside a nested try/finally.

MSB4260 is a static-graph-only reference failure documented at .agents/skills/build-perf-baseline/SKILL.md:306-314

This is the better of the two examples, and I had missed it. MSB4260 is a limitation of the /graph switch that only the cache step passes; the Arcade fallback does not build the graph statically, so it can resolve the same reference and succeed. It reaches the error summary and matches no cache or restore term, so it was satisfying the fail-fast condition. Added in e20a443, and I renamed the pattern to $fallbackCanDiffer because a graph-mode limitation is not "infrastructure". (MSB4166 was already handled in 1f091bd from the inline thread.)

The developer guide still says that every cache-build failure is cleaned up and sent through the regular Arcade fallback (docs/dev-guide.md:89)

Also correct - this PR made that sentence untrue. Replaced it with the three actual outcomes: cache hit, fail-fast on reproducible build errors, and fallback for everything else (cache/plugin problems, NU*, engine crashes, MSB4260, wrapper failures, no-summary failures), including why the cache step deliberately stays green when it falls back.

Classifier verification is now 14 scenarios, 0 mismatches, run through the real step script with AzDO's ErrorActionPreference simulated. It includes the captured log of the build that motivated this PR, plus the composition cases that matter: MSB4260 alongside a genuine CS1002, and MSB4166 alongside a genuine CS1002, both correctly taking the fallback - if a node crashed or the graph could not be resolved, the surviving projects' results are not trustworthy enough to fail on.

[Copilot]

Review details

Comments suppressed due to low confidence (1)

azure-pipelines.yml:405

• The fallback classifier still hard-fails standard transient file-lock errors. For example, MSB3021/MSB3027 (“process cannot access the file because it is being used by another process” / “exceeded retry count”) produce a normal error line and summary, match none of these markers, and therefore suppress the Arcade retry even though a differently scheduled/retried build can succeed. Add these copy/locking markers (including continuation-line text) to the fallback-eligible set rather than treating them as source breaks.

            $fallbackCanDiffer = '(?i)\bMSB4166\b|\bMSB0001\b|\bMSB1025\b|\bMSB4017\b|\bMSB4260\b|exited prematurely|OutOfMemoryException'
            $fallbackCanDifferLines = @($log | Where-Object { $_ -match $fallbackCanDiffer })

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Evangelink]

Two more suppressed-confidence items from the latest review passes. Both were right, and I verified each against the pinned plugin binary (Microsoft.MSBuildCache.AzurePipelines 0.1.328-preview) rather than taking the wording on trust. Fixed in 6ae40bd.

The pinned plugin reports duplicate outputs as Node ... produced output ... which was already produced by another node ...; MSBuild's project-cache logger adds no plugin/category prefix

Confirmed verbatim in the assembly:

Node {0} produced output {1} with hash {2} which was already produced by another node {3} with a different hash {4}.

No plugin or cache token anywhere in it, so it read as an ordinary build error and hard-failed - the exact opposite of the contract that everything the plugin reports goes to the fallback. It is reachable too: MSBuildCacheIdenticalDuplicateOutputPatterns is set to \**, which tolerates duplicates only when the content is identical, so a genuine differing-content duplicate still surfaces. Matching on already produced by another node now.

MSB3021/MSB3027 ... produce a normal error line and summary, match none of these markers

Also confirmed (The process cannot access the file because it is being used by another process is in the same assembly). Worth adding beyond the general transient argument: copying is heavier in this step than in a default build, because the cache runs with copy rather than hardlink semantics, so copy contention is more likely here than the codes' usual base rate would suggest.

Both markers went into the whole-log $fallbackCanDiffer set rather than the error-line set, so the continuation-line text you flagged is covered. The dev guide lists them alongside the other fallback-eligible failures.

Verification is now 16 scenarios, 0 mismatches, using the plugin's exact message wording. That includes the duplicate-output diagnostic appearing alongside a genuine CS1002, which correctly takes the fallback - the same composition rule as the crash cases, since a duplicate-output or node failure means the surviving results are not trustworthy enough to fail on. The real captured log from the build that motivated this PR still fails fast.

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Medium