CtrlK
BlogDocsLog inGet started
Tessl Logo

multithreaded-task-migration

Guide for migrating MSBuild tasks to multithreaded mode support, including compatibility red-team review. Use this when converting tasks to thread-safe versions, implementing IMultiThreadableTask, adding TaskEnvironment support, or auditing migrations for behavioral compatibility.

74

Quality

92%

Does it follow best practices?

Run evals on this skill

Adds up to 20 points to the overall score

View guide

SecuritybySnyk

Passed

No findings from the security scan

SKILL.md
Quality
Evals
Security

Migrating MSBuild Tasks to Multithreaded API

MSBuild's multithreaded execution model requires tasks to avoid global process state (working directory, environment variables). Thread-safe tasks declare this capability via MSBuildMultiThreadableTask and use TaskEnvironment from IMultiThreadableTask for safe alternatives.

Migration Steps

Step 1: Update Task Class Declaration

a. Ensure the task implementing class is decorated with the MSBuildMultiThreadableTask attribute. b. Implement IMultiThreadableTask only if the task needs TaskEnvironment APIs (path absolutization, env vars, process start). If the task has no file/environment operations (e.g., a stub class), the attribute alone is sufficient.

[MSBuildMultiThreadableTask]
public class MyTask : Task, IMultiThreadableTask
{
    public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
    ...
}

Note: [MSBuildMultiThreadableTask] has Inherited = false — it must be on each concrete class, not just the base. The corollary is easy to miss: the base class still executes multithreaded, but is not analyzed. Audit the whole base chain — see Unsafe Code in an Unannotated Base Class.

Step 2: Absolutize Paths Before File Operations

All path strings must be absolutized with TaskEnvironment.GetAbsolutePath() before use in file system APIs. This resolves paths relative to the project directory, not the process working directory.

AbsolutePath absolutePath = TaskEnvironment.GetAbsolutePath(inputPath);
if (File.Exists(absolutePath))
{
    string content = File.ReadAllText(absolutePath);
}

The AbsolutePath struct:

  • Value — the absolute path string
  • OriginalValue — preserves the input path (use for error messages and [Output] properties)
  • Implicitly convertible to string for File/Directory API compatibility
  • GetCanonicalForm() — resolves .. segments and normalizes separators (see Sin 5)

CAUTION: GetAbsolutePath() throws ArgumentException for null/empty inputs. See Sin 3 and Sin 6 for compatibility implications.

Step 3: Replace Environment Variable APIs

BEFORE (UNSAFE)AFTER (SAFE)
Environment.GetEnvironmentVariable("VAR");TaskEnvironment.GetEnvironmentVariable("VAR");
Environment.SetEnvironmentVariable("VAR", "v");TaskEnvironment.SetEnvironmentVariable("VAR", "v");

Step 4: Replace Process Start APIs

BEFORE (UNSAFE - inherits process state)AFTER (SAFE - uses task's isolated environment)
var psi = new ProcessStartInfo("tool.exe");var psi = TaskEnvironment.GetProcessStartInfo();
psi.FileName = GetFullPathToTool(); // must be absolute

Updating Unit Tests

Built-in MSBuild tasks now initialize TaskEnvironment with a MultiProcessTaskEnvironmentDriver-backed default. Tests creating instances of built-in tasks no longer need manual TaskEnvironment setup. For custom or third-party tasks that implement IMultiThreadableTask without a default initializer, set TaskEnvironment = TaskEnvironment.Fallback (or use TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(path) to point at a specific project directory).

APIs to Avoid

CategoryAPIsAlternative
ForbiddenEnvironment.Exit, FailFast, Process.Kill, ThreadPool.SetMin/MaxThreads, Console.*Return false, throw, or use Log
Use TaskEnvironmentEnvironment.CurrentDirectory, Get/SetEnvironmentVariable, Path.GetFullPath, ProcessStartInfoSee Steps 2-4
Need absolute pathsFile.*, Directory.*, FileInfo, DirectoryInfo, FileStream, StreamReader/WriterAbsolutize first (File System APIs)
Need absolute paths (analyzer-invisible)AssemblyName.GetAssemblyName, XDocument/XElement/XmlDocument.Load(string), XmlReader/XmlWriter.Create(string), ZipFile.*, X509CertificateLoader, Image.FromFile, Assembly.LoadFromAbsolutize first — the analyzer does not flag these
Review requiredAssembly.Load*, Activator.CreateInstance*Check for version conflicts

Analyzer-Invisible Path Consumers

MSBuildTask0003 monitors a fixed list of types (File, Directory, FileInfo, DirectoryInfo, FileStream, StreamReader, StreamWriter, FileSystemWatcher). Any other API that accepts a path string and touches disk is equally unsafe but produces no diagnostic.

In a ~150-task migration across dotnet/arcade, dotnet/source-build-assets and the dotnet/dotnet VMR, AssemblyName.GetAssemblyName on a raw input was the single most-repeated defect. Do not treat a clean analyzer run as evidence that path handling is complete — see Verification.

Note the overload distinction: the string overloads are hazards; XDocument.Load(stream) and new StreamReader(stream) are fine, because the caller already resolved the path to open the stream.

Practical Notes

CRITICAL: Trace All Path String Usage

Trace every path string through all method calls and assignments to find all places it flows into file system operations — including helper methods that may internally use File System APIs.

  1. Find every path string (e.g., item.ItemSpec, function parameters)
  2. Trace downstream through all method calls
  3. Absolutize BEFORE any code path that touches the file system
  4. Use OriginalValue for user-facing output (logs, errors) — see Sin 2

Trace through abstractions, not just concrete calls. A path can reach the file system without a single System.IO type appearing in the task. In InstallDotNetTool (dotnet/arcade), the DestinationPath, DotnetPath and WorkingDirectory inputs flow raw through IFileSystem and ICommandFactory interfaces — the task body looks completely clean, and the analyzer reports nothing. When a task input is passed to an interface or delegate, resolve it at the task boundary rather than hoping the implementation does.

Resolve once, at the boundary. The recurring failure is not "forgot to absolutize" but "absolutized at three of four use sites". If a variable is going to be used as a path at all, convert it to AbsolutePath where it enters the task and keep it that way — see Sin 8.

Exception Handling in Batch Operations

In batch processing (iterating over files), GetAbsolutePath() throwing on one bad path aborts the entire batch. Match the original task's error semantics:

bool success = true;
foreach (ITaskItem item in SourceFiles)
{
    try
    {
        AbsolutePath path = TaskEnvironment.GetAbsolutePath(item.ItemSpec);
        ProcessFile(path);
    }
    catch (ArgumentException ex)
    {
        Log.LogError("Invalid path '{0}': {1}", item.ItemSpec, ex.Message);
        success = false;
    }
}
return success;

Prefer AbsolutePath Over String

Stay in the AbsolutePath world — it's implicitly convertible to string where needed. Avoid round-tripping through string and back.

TaskEnvironment is Not Thread-Safe

If your task spawns multiple threads internally, synchronize access to TaskEnvironment. Each task instance gets its own environment, so no synchronization between tasks is needed.

API Discipline When Plumbing Through Helpers

When the migration ripples into shared helpers:

  • Helper signatures: take AbsolutePath, not (string path, string pathForMessages). Two-string signatures drift apart; the caller passing one AbsolutePath keeps .Value and .OriginalValue in lockstep.
  • Repo-wide helpers belong in an extensions class (e.g., TaskEnvironmentExtensions), not on the task. If a per-task helper looks generic, extract it.
  • Don't condition behavior on MT-mode. TaskEnvironment.Fallback already gives single-process semantics; if (mtMode) { … } else { … } doubles the maintenance surface and skips test coverage of one branch.
  • Don't silently swallow ArgumentException from GetAbsolutePath in a helper — log a diagnostic so customers can debug bad inputs.

References


Compatibility Red-Team Playbook

After migration, review for behavioral compatibility. Every observable difference is a bug until proven otherwise.

Observable behavior = Execute() return value, [Output] property values, error/warning message content, exception types, files written, and which code path runs.

The 8 Deadly Compatibility Sins

Real bugs found during MSBuild task migrations. Every one shipped in initial "passing" code with green tests.

Edge-case discipline: For every migrated code path, verify behavior when inputs are null, empty string (""), or whitespace-only. GetAbsolutePath throws ArgumentException on null/empty — if the pre-migration code handled these differently (e.g., returned early, used a default, or threw a different exception type), the migration must preserve that behavior. Even if a scenario seems unlikely, treat it as a relevant finding if it is theoretically possible.

Sin 1: Output Property Contamination

Absolutized values leak into [Output] properties that users/other tasks consume.

// BROKEN: ManifestPath was "bin\Release\app.manifest", now "C:\repo\bin\Release\app.manifest"
AbsolutePath abs = TaskEnvironment.GetAbsolutePath(Path.Combine(OutputDirectory, name));
ManifestPath = abs; // implicit string conversion!

// CORRECT: separate original form from absolutized path
string originalPath = Path.Combine(OutputDirectory, name);
AbsolutePath outputPath = TaskEnvironment.GetAbsolutePath(originalPath);
ManifestPath = originalPath;          // [Output]: original form
document.Save((string)outputPath);    // file I/O: absolute path

Detect: For every [Output] property, trace backward — is it ever assigned from an AbsolutePath?

Sin 2: Error/Log Message Path Inflation

Error messages and exception messages show absolutized paths instead of the user's original input.

Direct leakage — passing an AbsolutePath to logging APIs:

// BROKEN: "Cannot find 'C:\repo\app.manifest'" instead of "Cannot find 'app.manifest'"
AbsolutePath abs = TaskEnvironment.GetAbsolutePath(path);
Log.LogError("Cannot find '{0}'", abs); // implicit conversion!

// CORRECT: use OriginalValue
Log.LogError("Cannot find '{0}'", abs.OriginalValue);

Indirect leakage — exception messages from helpers that received the absolutized path:

// BROKEN: ex.FileName / ex.Message embed the absolutized path
catch (FileNotFoundException ex) { Log.LogError("Not found: {0}", ex.FileName); }
catch (Exception ex)              { Log.LogError(ex.Message); }

// CORRECT: prefer the original input; if you must use the exception, sanitize
catch (FileNotFoundException ex) { Log.LogError("Not found: {0}", abs.OriginalValue); }
catch (Exception ex)              { Log.LogError(ex.Message.Replace(abs.Value, abs.OriginalValue)); }

Detect: Search every Log.LogError/LogWarning/LogMessage — is any argument an AbsolutePath? Also check every Log.LogError(ex.Message …) / ex.FileName downstream of a GetAbsolutePath — did the exception originate from a helper that received the absolutized path?

Sin 3: Null Coalescing That Changes Control Flow

Adding ?? "" silently swallows an exception the old code relied on for error handling.

// BEFORE: Path.GetDirectoryName("C:\") → null → Path.Combine(null, x) → ArgumentNullException
//   → task fails with an exception / error logged → Execute() returns false

// BROKEN: ?? "" added "for safety"
string dir = Path.GetDirectoryName(fileName) ?? string.Empty;
// Path.Combine("", x) succeeds silently → no error → Execute() returns TRUE!

Detect: For every ?? you added, ask: "What happened when this was null before?" If it threw and was caught → your ?? is a bug.

Sin 4: Try-Catch Scope Mismatch

GetAbsolutePath() inside a try block leaves the absolutized value out of scope in the catch block. Helper methods in the catch (like LockCheck) then use the original non-absolute path.

// CORRECT: hoist above try so catch can use it too
AbsolutePath abs = TaskEnvironment.GetAbsolutePath(OutputManifest.ItemSpec);
try {
    WriteFile(abs);
} catch (Exception ex) {
    string lockMsg = LockCheck.GetLockedFileMessage(abs);        // absolute → correct file
    Log.LogError("Failed: {0}", OutputManifest.ItemSpec, ...);   // original → user-friendly
}

Detect: For every GetAbsolutePath inside a try, check if the catch block needs the absolutized value.

Sin 5: Canonicalization Mismatch

GetAbsolutePath does NOT canonicalize. Path.GetFullPath does TWO things: absolutize AND canonicalize (.. resolution, separator normalization). If the old code used Path.GetFullPath for dictionary keys, comparisons, or display, you must add .GetCanonicalForm():

// GetAbsolutePath("foo/../bar")  → "C:\repo\foo/../bar"  (NOT canonical)
// Path.GetFullPath("foo/../bar") → "C:\repo\bar"         (canonical)

// BROKEN for dictionary keys — "C:\repo\foo\..\bar" ≠ "C:\repo\bar"
var map = items.ToDictionary(p => (string)TaskEnvironment.GetAbsolutePath(p.ItemSpec), ...);

// CORRECT
var map = items.ToDictionary(
    p => (string)TaskEnvironment.GetAbsolutePath(p.ItemSpec).GetCanonicalForm(),
    StringComparer.OrdinalIgnoreCase);

Detect: Find every Dictionary/HashSet/ToDictionary using path keys, and every place the old code called Path.GetFullPath. If canonicalization mattered, add .GetCanonicalForm().

Sin 6: Exception Type Change

Old code threw FileNotFoundException for missing files; new code throws ArgumentException from GetAbsolutePath("") before reaching the file check. Custom catch blocks filtering by exception type may be bypassed. (ExceptionHandling.IsIoRelatedException catches ArgumentException, but task-specific handlers might not.)

Detect: For every GetAbsolutePath, check what the old code threw for null/empty and whether the calling code has type-specific catch blocks.

Sin 7: Path.IsPathRooted as an Absolutize Gate

Path.IsPathRooted returns true for drive-relative (C:foo\bar) and root-relative (\foo\bar) Windows paths — both still depend on process current-directory / current-drive state. "Absolutize only if not rooted" silently leaves those paths process-dependent.

GetAbsolutePath correctly handles all path forms — including the Windows edge cases (C:foo, \foo) that Path.IsPathRooted considers "rooted" but that are still CWD/drive-dependent. Call it unconditionally; remove any IsPathRooted short-circuit.

Sin 8: Swallowed Exceptions Hiding an Unresolved Path

The usual mental model is "unresolved path → FileNotFoundException → loud failure". That model is wrong wherever a catch treats an exception as a semantic answer. There, an unresolved path does not fail the build — it silently returns the wrong answer.

Real example from the dotnet/dotnet VMR (CheckForPoison), where the same variable is resolved three lines later:

// BROKEN
try
{
    AssemblyName asm = AssemblyName.GetAssemblyName(fileToCheck);   // raw, relative-capable
    ...
    if (IsAssemblyFromSbrp(TaskEnvironment.GetAbsolutePath(fileToCheck))) { ... }   // resolved
    else if (IsAssemblyPoisoned(TaskEnvironment.GetAbsolutePath(fileToCheck))) { ... }
}
catch (BadImageFormatException) { /* not a managed assembly - fine */ }

The GetAssemblyName call is only an "is this a managed assembly?" probe, and the catch means "not an assembly". So an unresolved path throws, gets swallowed, and both poison checks are skipped — turning a leaked binary into a false negative in the leak-detection gate itself. The build stays green while the check it exists to perform quietly stops working.

This sin is especially dangerous because it defeats the usual verification strategy: the decoy-CWD test (Pattern A) still "passes" unless it asserts on the result, not just on Execute() returning true.

Detect: For every catch in a migrated task, ask what the handler means. If it means anything other than "fail" — "not an assembly", "not found, use default", "unsupported, skip" — then every path-consuming call inside the corresponding try must be absolutized, and the catch should be narrowed to the exception types that genuinely represent that semantic answer (BadImageFormatException here, not IOException/ArgumentException).


Call-Chain Hazards Beyond the Task

The 7 sins above are local to the task body. The other half of MT migration is auditing the transitive call chain — helpers and shared utility classes the task reaches into.

Helpers That Capture Process State

Helpers reached from Execute() can quietly depend on process state in any of these ways:

  • A relative path falls through to Directory.GetCurrentDirectory() (often as a "fallback") or to Path.GetFullPath(x) without a base.
  • An env var is read directly via Environment.GetEnvironmentVariable (not TaskEnvironment).
  • A static field is seeded from process state on first use (static string s_x = Directory.GetCurrentDirectory()), permanently capturing the first caller's environment for every later caller.
  • A BCL API echoes its input path back through ex.Message / ex.FileName (Sin 2).
  • The helper takes a string path and returns a string — losing the .OriginalValue distinction, so callers either lie in messages or absolutize twice.

Resolution patterns:

  1. Add an MT-aware overload that takes TaskEnvironment (or an AbsolutePath / explicit fallback path). Legacy overload delegates with TaskEnvironment.Fallback. Don't branch on "MT mode on/off".
  2. Promote the parameter type to AbsolutePath so .Value and .OriginalValue travel together.
  3. Replace process-state-seeded static caches with ConcurrentDictionary<TKey, TValue> keyed on the inputs that determine uniqueness — never on process state.
  4. For cross-repo helpers (foundation types used by many tasks), migrate the foundation in a dedicated PR before the tasks that consume it; the hydration step is usually where Path.GetFullPath is hiding.

Unsafe Code in an Unannotated Base Class

[MSBuildMultiThreadableTask] is Inherited = false, so it goes on each concrete task. The consequence people miss is the mirror image: the base class runs multithreaded too, but nothing marks it as such — and with msbuild_task_analyzer.scope = multithreadable_only (the recommended setting for incremental migration) the analyzer does not look inside it at all.

From dotnet/arcade: CreateAkaMSLinks and DeleteAkaMSLinks were both annotated, both analyzer-clean. Their shared base contained:

// AkaMSLinksBase - NOT annotated, so never analyzed
File.ReadAllText(ClientCertificate)   // ClientCertificate is a task input property

A monitored API (File) on a raw task input — exactly what the analyzer exists to catch — invisible purely because of where it lived.

Resolution: when you annotate a task, audit its full base chain up to Task/ToolTask. If the base holds shared input properties or path handling, have the base implement IMultiThreadableTask and do the resolution there, so every derived task inherits the fix:

public abstract class AkaMSLinksBase : Task, IMultiThreadableTask
{
    public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;
    // ... resolve ClientCertificate here, once, for all derived tasks
}

Note that implementing IMultiThreadableTask on the base is safe and does not change routing — routing keys off the attribute, which stays on the concrete classes.

Engine-Owned Shared State: RegisterTaskObject

The guidance about static fields has a less obvious sibling: IBuildEngine4.GetRegisteredTaskObject / RegisterTaskObject. The state lives in the engine, so no static field appears and nothing looks shared — but the read/write pair is not atomic, and under MT two instances of the same task in one node can both miss and both populate.

Whether that matters depends entirely on what is being cached:

Cached computationVerdict
Pure and deterministic for the key (e.g., "where is dotnet?")Benign — the loser overwrites an identical entry. Leave it, but say so in a comment.
Deduplication is the point (e.g., "log this error exactly once")Broken — both instances observe "not yet reported" and both act.
A cached failureSuspect — a failure computed under one task's environment gets served to a task with a different ProjectDirectory. Key the cache on the inputs that determine the result, or don't cache failures.

Real examples from dotnet/arcade: LocateDotNet is benign (pure computation). SingleError — whose entire purpose is emitting exactly one error per build — is broken by the race, and we removed the annotation rather than ship it. Same API shape, opposite conclusions.

Tasks Instantiated Directly by Other Tasks

[MSBuildMultiThreadableTask] is only honored when the TaskFactory creates the task (i.e., a target invokes it as <MyTask … />). A task created via new MyTask() inside another task's body bypasses the factory and never gets TaskEnvironment injected — it silently defaults to TaskEnvironment.Fallback (= process CWD).

Migrating a nested task in isolation is therefore meaningless. Either migrate the parent and have it explicitly propagate its TaskEnvironment to the nested instance before calling Execute(), or restructure so the nested task is a regular TaskFactory-created task.

Detect: grep for new …Task() followed by .Execute() — every direct instantiation is a hole.

ToolTask Override Hot Spots

ToolTask subclasses inherit virtual methods that run before or after Execute() and frequently touch the file system. Audit every override:

OverrideHazardMigration
GenerateFullPathToTool()Builds ProcessStartInfo.FileName — relative tool path → wrong tool launchedAbsolutize the tool path before returning
SkipTaskExecution()Up-to-date check on input/output timestamps using relative pathsAbsolutize both sides of the comparison
ValidateParameters()File.Exists on input parametersAbsolutize before probing
GenerateResponseFileCommands() / GenerateCommandLineCommands()Tempting to absolutize argsDon't — the child's WorkingDirectory is the project dir, so relative args resolve correctly; absolutizing inflates user-visible output and can leak into tool diagnostics or generated artifacts
GetWorkingDirectory()Default null → child inherits host CWDLeave alone if you use TaskEnvironment.GetProcessStartInfo(); it already sets WorkingDirectory

Key contract: ProcessStartInfo.FileName is resolved by the OS before WorkingDirectory takes effect, so the executable path must be absolute. Tool arguments are interpreted by the child process in its working directory, so they should stay relative.

Engine-Owned Environment Variables Are Immutable in MT Mode

Env vars consumed by the engine before tasks run (e.g., MSBUILD*-prefixed flags and the framework/SDK discovery variables) are snapshotted into engine caches at startup. Tasks that mutate them later have no observable effect on the engine — but the mutation appears to succeed locally, masking bugs.

If a migrated task previously mutated such a variable to influence a downstream tool, re-architect to pass the value via ProcessStartInfo.EnvironmentVariables on the child process instead of mutating the parent's environment.


Test Patterns for MT Migrations

A migration test must fail when the migration is undone. Tests that pass identically against pre- and post-migration code are theater. The two patterns below are the ones that reliably exercise MT-specific behavior.

Pattern A: Decoy-CWD Test (for tasks that touch the file system)

Set the process working directory to a decoy dir with no relevant files; set TaskEnvironment.ProjectDirectory to a different dir with the inputs/expected outputs. The task must read/write against the project directory, not the decoy.

using TestEnvironment env = TestEnvironment.Create();
TransientTestFolder projectDir = env.CreateFolder();
TransientTestFolder decoyCwd = env.CreateFolder();
File.WriteAllText(Path.Combine(projectDir.Path, "input.txt"), "expected");

env.SetCurrentDirectory(decoyCwd.Path); // auto-restored when env is disposed

var task = new MyTask
{
    TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path),
    Input = "input.txt", // relative
};
task.Execute().ShouldBeTrue();

CRITICAL: Directory.SetCurrentDirectory is process-global. xUnit parallelizes tests within a class by default — a CWD-mutating test will flake unless pinned to a non-parallel collection ([Collection] + [CollectionDefinition(DisableParallelization = true)]). The IDisposable restore protects sequential safety but not concurrent siblings.

Pattern B: Cross-Instance Independence (for tasks doing relative-path resolution)

Two task instances, two TaskEnvironment.ProjectDirectory values, same relative input. Assert each task's output is rooted to its own project directory — proving resolution is per-instance, not shared.

When NOT to Write a "Migration Test"

Attribute-only migrations (just [MSBuildMultiThreadableTask], no IMultiThreadableTask) on tasks whose Execute() body contains no file/env/process/static-state interactions have nothing to test — a decoy-CWD test would pass whether the attribute is present or not. Don't write the test. Document the call-chain audit conclusion in the PR description instead.

Test Hygiene

  • Use TestEnvironment / TransientTestFolder for temp dirs; never Path.GetTempPath() + Guid.NewGuid() with manual try/finally.
  • Hard-code expected assertion values; don't recompute them from Path.Combine(...) in the assertion — when the test fails you want to know what was actually wrong.

Verification: A Clean Analyzer Run Is Not a Migration

Microsoft.Build.TaskAuthoring.Analyzer is the right first line of defense, and it should be enabled. It is not sufficient on its own, and it is worth being concrete about why so you know what work it leaves you.

The data. In a migration of ~150 tasks across dotnet/arcade, dotnet/source-build-assets and the dotnet/dotnet VMR, the analyzer was enabled in all three repos and every build was 0 warnings, 0 errors. A manual audit afterwards found 9 real defects:

Why the analyzer missed itCountExample
The code was outside the analysis scope2unsafe call in an unannotated base class
The API is not on the monitored list3AssemblyName.GetAssemblyName
The failure class is not modeled at all4task-object race, memoized failure, nested task construction, path crossing a DI boundary

The last row is the important one: those are dataflow and lifetime problems, not banned-API problems. No amount of allowlist tuning reaches them, which is why the Red-Team Audit Protocol below is not optional.

Why "it worked when I tested it" is weak evidence. An unresolved relative path in MT mode does not reliably throw. It resolves against whatever the shared node's current directory happens to be, which depends on which project scheduled first. So the same defect passes locally single-threaded every time, passes in CI most of the time, and fails occasionally on one machine under load. And per Sin 8, some never throw at all.

Verification checklist. The Sign-Off Checklist at the end of this document is the actionable list. The items most likely to be skipped — because nothing warns you about them — are:

  1. The full base chain, not just the concrete class.
  2. Every catch, classified: does it mean "fail", or a semantic answer? (Sin 8)
  3. Every task input traced to every use site — the common defect is resolved at 3 of 4 sites, not 0 of 4.
  4. Caches and memoization: static fields, RegisterTaskObject, and anything storing a failure.
  5. A Pattern A or Pattern B test that fails when the migration is reverted — asserting on the result, not merely that Execute() returned true.

If your host supports agents, the mt-migration-reviewer agent automates most of this.

Migrating Is Not Always the Right Answer

Adding the attribute is a claim that the task is safe to run concurrently in a shared process. When that claim cannot be made honestly, leaving the task unannotated is a correct, supported outcome — it simply keeps routing to the TaskHost sidecar, which is exactly the behavior it has today. A slower task is better than a wrong one.

De-annotate (or never annotate) when:

  • The task's semantics depend on being a singleton within the build — SingleError above is the canonical case.
  • The task deliberately mutates process-global state for a downstream consumer and cannot be re-architected to pass it via ProcessStartInfo.
  • The correctness argument depends on a race being benign and you cannot convince yourself it is.

When you do this, leave a comment saying why, so the next person does not "fix" the missing attribute:

// Deliberately not [MSBuildMultiThreadableTask]: this task must report exactly
// one error per build, and the Get/RegisterTaskObject pair used to enforce that
// is not atomic across concurrent instances in a shared node.

Red-Team Audit Protocol

Phase 1: Trace Every Changed Line

For each modified line: What was the exact runtime value before? After? Where does it flow (outputs, logs, file paths, dictionary keys)? Does each destination produce identical observable behavior?

Phase 2: Null/Empty/Edge Input Analysis

InputGetAbsolutePathOld behaviorMatch?
nullArgumentExceptionVaries
""ArgumentExceptionVaries
"C:\" (root)ValidValid✅ usually
".""C:\repo\." (not canonical)"C:\repo" if GetFullPath❌ maybe
"foo\..\bar""C:\repo\foo\..\bar""C:\repo\bar" if GetFullPath❌ maybe
Already absolutePass-throughPass-through

Path.GetDirectoryNamePath.Combine chains:

InputGetDirectoryName returnsPath.Combine(result, x)
"C:\"nullThrows ArgumentNullException
"""" (.NET Fx) / null (.NET Core+)Works / Throws ⚠️
"file.resx" (no dir)""Works

Verify behavior on both .NET Framework and .NET TFMs.

Phase 3: Downstream Impact

  1. Output properties: What consumes this task's [Output]? Does it compare, display, or use as a path?
  2. Written files: Does file content change? (e.g., XML with embedded paths)
  3. Helper methods: Do LockCheck, ManifestWriter, etc. internally resolve relative paths?
  4. Error codes: MSBuild error codes (MSBxxxx) must be identical.

Phase 4: Concurrency

  1. Two tasks with different ProjectDirectory values don't interfere
  2. No writes to static fields (shared across threads)
  3. No non-atomic GetRegisteredTaskObject / RegisterTaskObject pair whose race is not provably benign
  4. No cached failures — a failure computed under one task's environment must not be served to another
  5. All file operations use absolutized paths

Compatibility Test Matrix

[Task] × [Input Type] × [Assertion]

Inputs: relative path, absolute path, null, empty, ".." segments, root "C:\",
        forward slashes, trailing separator, UNC path, 260+ char path

Assertions: Execute() return value, [Output] exact string, error message content,
            exception type, file location, file content

Sign-Off Checklist

  • [MSBuildMultiThreadableTask] on every concrete class (not just base — Inherited=false)
  • Base chain audited up to Task/ToolTask — unannotated bases still run multithreaded and are not analyzed
  • IMultiThreadableTask on classes that use TaskEnvironment APIs, with default initializer = TaskEnvironment.Fallback
  • Every [Output] property: exact string value matches pre-migration (Sin 1)
  • Every Log.LogError/LogWarning: path in message matches pre-migration (use OriginalValue) (Sin 2)
  • Every Log.LogError(ex.Message …) / ex.FileName: exception path sanitized to original input (Sin 2)
  • Every GetAbsolutePath call: null/empty exception behavior matches old code path (Sin 3, 6)
  • Every dictionary/set with path keys: canonicalization preserved (GetCanonicalForm()) (Sin 5)
  • Every try-catch: absolutized value available in catch block where needed (Sin 4)
  • Every ?? or ?. added: verified it doesn't swallow a previously-thrown exception (Sin 3)
  • No Path.IsPathRooted short-circuits around GetAbsolutePath — call unconditionally (Sin 7)
  • Every catch classified — any handler meaning a semantic answer rather than "fail" has all path calls in its try absolutized, and is narrowed to the types representing that answer (Sin 8)
  • Analyzer-invisible path consumers absolutized: AssemblyName.GetAssemblyName, XDocument/XmlDocument.Load(string), XmlReader.Create(string), ZipFile.*, X509CertificateLoader, Assembly.LoadFrom — none of these produce a diagnostic
  • Every task input resolved at every use site, not most of them
  • Task inputs crossing an interface / delegate / DI boundary (IFileSystem, ICommandFactory) resolved at the task boundary
  • No AbsolutePath leaks into user-visible strings unintentionally
  • No behavior conditioned on "MT mode on/off" — TaskEnvironment.Fallback handles single-process case
  • Call chain traced end-to-end for every helper invoked from Execute():
    • No Environment.CurrentDirectory / Directory.GetCurrentDirectory() / Path.GetFullPath(x) without a base anywhere in the transitive call graph
    • No direct Environment.Get/SetEnvironmentVariable (route through TaskEnvironment)
    • No static mutable fields seeded from process state; replace with ConcurrentDictionary keyed on inputs
    • No non-atomic GetRegisteredTaskObject/RegisterTaskObject pair unless the cached computation is pure and deduplication is not the point
    • No cached failures — a failure computed under one task's environment must not be served to another
    • No Console.*, Environment.Exit, Process.Kill, FailFast
  • ToolTask overrides audited (GenerateFullPathToTool, SkipTaskExecution, ValidateParameters); ProcessStartInfo.FileName is absolute, tool arguments stay relative
  • No nested tasks created via new …Task() without explicit TaskEnvironment propagation
  • No mutations of engine-owned env vars (e.g. MSBUILD* and the framework/SDK discovery vars) in MT mode
  • Tests for custom tasks set TaskEnvironment = TaskEnvironmentHelper.CreateForTest() (built-in tasks have a default)
  • Migration test follows Pattern A (decoy-CWD) or Pattern B (cross-instance independence), or PR explains why no test is meaningful
  • CWD-mutating tests pinned to a non-parallel xUnit collection
  • Cross-framework: tested on .NET Framework and .NET TFMs.
  • If the task was deliberately left unannotated, a comment records why (see Migrating Is Not Always the Right Answer)
Repository
dotnet/msbuild
Last updated
First committed

Is this your skill?

If you maintain this skill, you can claim it as your own. Once claimed, you can manage eval scenarios, bundle related skills, attach documentation or rules, and ensure cross-agent compatibility.