Skip to content

Migrate Exec task to TaskEnvironment API#13171

Open
Copilot wants to merge 9 commits intomainfrom
copilot/migrate-exec-task-to-api
Open

Migrate Exec task to TaskEnvironment API#13171
Copilot wants to merge 9 commits intomainfrom
copilot/migrate-exec-task-to-api

Conversation

Copy link
Contributor

Copilot AI commented Feb 2, 2026

Context

The Exec task needs to support MSBuild's multithreaded execution model. Tasks that rely on global process state (working directory, environment variables) are not thread-safe and cannot run in parallel.

Changes Made

  • Add [MSBuildMultiThreadableTask] attribute and implement IMultiThreadableTask
  • Add TaskEnvironment property for thread-safe access to execution environment
  • Replace Directory.GetCurrentDirectory() with TaskEnvironment.ProjectDirectory
  • Use TaskEnvironment.GetAbsolutePath(WorkingDirectory) for relative path resolution
  • Change _workingDirectory field from string to AbsolutePath type for type safety
  • Use .OriginalValue for logging (user-facing messages) and .Value for file system operations
  • Update all test helpers to set TaskEnvironment = TaskEnvironmentHelper.CreateForTest()
  • Add TaskEnvironmentHelper.CreateMultithreadedForTest(projectDirectory) helper method for testing with the multithreaded driver
// Before - uses global process state
_workingDirectory = !string.IsNullOrEmpty(WorkingDirectory)
    ? WorkingDirectory
    : Directory.GetCurrentDirectory();

// After - uses TaskEnvironment with AbsolutePath for thread safety
_workingDirectory = !string.IsNullOrEmpty(WorkingDirectory)
    ? TaskEnvironment.GetAbsolutePath(WorkingDirectory)
    : TaskEnvironment.ProjectDirectory;

Testing

Added 4 comprehensive tests covering both TaskEnvironment drivers and multithreaded scenarios:

  • ExecResolvesRelativeWorkingDirectoryWithMultiProcessDriver - Tests relative WorkingDirectory resolution with MultiProcessDriver
  • ExecUsesProjectDirectoryAsDefaultWorkingDirectory - Tests default working directory when not specified
  • ExecHandlesAbsoluteWorkingDirectory - Tests absolute WorkingDirectory paths
  • ExecResolvesRelativeWorkingDirectoryRelativeToProjectDirectory - Uses MultiThreadedTaskEnvironmentDriver to test relative path resolution when process CWD differs from project directory (simulating multithreaded mode)

All 49 Exec tests pass (1 pre-existing Windows-only failure on Linux, 2 correctly skipped)

Notes

  • The static Environment.GetEnvironmentVariable("ComSpec") in CommandProcessorPath is unchanged—it's system-level initialization that occurs once before any task execution, and on Unix it returns just "sh" (not a rooted path).
  • UNC paths (e.g., \\server\share) work correctly with AbsolutePath since they are fully qualified paths and Path.IsPathFullyQualified() returns true for them.
Original prompt

This section details on the original issue you should resolve

<issue_title>Migrate Exec task to TaskEnvironment API</issue_title>
<issue_description></issue_description>

<agent_instructions>Make sure to preserve correctness, performance and that the tests pass</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…support

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Copilot AI changed the title [WIP] Migrate Exec task to TaskEnvironment API Migrate Exec task to TaskEnvironment API Feb 2, 2026
Copilot AI requested a review from JanProvaznik February 2, 2026 12:03
Copy link
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improve testing @copilot

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 2, 2026

improve testing @copilot

Improved testing in dd8d189 with 4 comprehensive tests covering:

  • MultiProcessDriver relative path resolution
  • Default working directory behavior (no WorkingDirectory specified)
  • Absolute WorkingDirectory handling
  • MultiThreadedDriver relative path resolution (simulating multithreaded mode where process CWD differs from project directory)

Also added TaskEnvironmentHelper.CreateMultithreadedForTest(projectDirectory) helper method for testing with the multithreaded driver.

Copy link
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…gging

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Copilot AI requested a review from JanProvaznik February 4, 2026 13:54
@JanProvaznik JanProvaznik marked this pull request as ready for review February 4, 2026 14:19
Copilot AI review requested due to automatic review settings February 4, 2026 14:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the Exec task toward MSBuild’s TaskEnvironment API so it can participate safely in the multithreaded (/mt) execution model by avoiding reliance on global process state.

Changes:

  • Mark Exec as [MSBuildMultiThreadableTask] and implement IMultiThreadableTask with a TaskEnvironment property.
  • Resolve/default WorkingDirectory via TaskEnvironment and use AbsolutePath for type-safe filesystem operations and user-facing logging.
  • Update/unit-test infrastructure: add a multithreaded TaskEnvironment test helper and update/add Exec tests to supply TaskEnvironment.

Reviewed changes

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

File Description
src/UnitTests.Shared/TaskEnvironmentHelper.cs Adds CreateMultithreadedForTest helper to construct a TaskEnvironment backed by the multithreaded driver for /mt-style tests.
src/Tasks/Exec.cs Converts Exec to an IMultiThreadableTask, switches working directory handling to TaskEnvironment, and uses AbsolutePath for safety/clarity.
src/Tasks.UnitTests/Exec_Tests.cs Ensures tests set TaskEnvironment for Exec instances and adds new tests around working directory resolution behavior.

- Fix ExecUsesProjectDirectoryAsDefaultWorkingDirectory test to use
  MultiThreadedTaskEnvironmentDriver with different process CWD
- Fix ExecResolvesRelativeWorkingDirectoryWithMultiProcessDriver test
  to verify correct base directory resolution
- Update stale comment about working directory default
- Override GetProcessStartInfo to inject TaskEnvironment's virtualized
  environment variables into spawned processes (critical for /mt mode)
On macOS, /var is a symlink to /private/var. Directory.GetCurrentDirectory()
resolves symlinks, so comparing it with the raw temp path fails. Read back
the CWD immediately after setting it to get the resolved path for comparison.
…ility

The base ToolTask.GetProcessStartInfo applies both the obsolete
EnvironmentOverride and EnvironmentVariables. After clearing the
environment to replace it with TaskEnvironment's, we must re-apply
both — not just EnvironmentVariables — for compatibility with any
class deriving from Exec that overrides EnvironmentOverride.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate Exec task to TaskEnvironment API

2 participants

Comments