Add ProcessStartOptions class with platform-aware path resolution #124271

Copilot · 2026-02-11T13:33:48Z

Description

Implements ProcessStartOptions class providing path resolution and process configuration with explicit platform semantics.

Changes

API Surface

Added ProcessStartOptions with constructor that resolves executable paths
Properties: FileName (resolved), Arguments, Environment, WorkingDirectory, InheritedHandles, KillOnParentExit, CreateNewProcessGroup
Arguments, Environment, and InheritedHandles use lazy allocation
Environment reuses DictionaryWrapper implementation from ProcessStartInfo
Added null validation for Arguments and InheritedHandles setters

Path Resolution

ProcessStartOptions.ResolvePath is the primary path resolution implementation for both Windows and Unix
Windows: appends .exe, searches System32 (cached), Windows directory (not cached), uses ; PATH separator
Unix: validates executable permissions by calling Process.IsExecutable, uses : PATH separator
Process.Unix.cs now calls ProcessStartOptions.ResolvePath instead of having its own implementation
Made Process.IsExecutable internal for code reuse between ProcessStartOptions and Process
Removed Process.ResolvePath to eliminate duplication - all callers now use ProcessStartOptions.ResolvePath
Throws FileNotFoundException when path cannot be resolved

Infrastructure

Added Interop.GetWindowsDirectoryW for Windows directory lookup
Introduced WINDOWS compilation constant for platform-specific code
Moved StringParser to common section (required by both platforms)

Tests

25 tests organized as partial classes across 3 files (base, Windows, Unix)
Tests use target-typed new() syntax for conciseness
ResolvePath_FindsCommonUtilities uses [Theory] with [InlineData] for parameterized testing of multiple utilities ("ls", "cat", "echo", "sh")
Platform-specific constructor tests in respective partial class files
Windows-specific tests validate .exe appending, system directory search, semicolon PATH separator
Unix-specific tests validate executable permission checks, colon PATH separator, directory rejection
Test assertions use EndsWith and Path.GetFullPath() for precision and symlink handling
All try-finally blocks simplified to single-level nesting
Test naming follows repository conventions (no "Test" prefix, e.g., Constructor_NullFileName_Throws)
Removed tests that don't properly validate behavior (e.g., incorrect lazy initialization tests)

Example

// Resolves path during construction
var options = new ProcessStartOptions("cmd");
Console.WriteLine(options.FileName); // C:\Windows\System32\cmd.exe

// Lazy-allocated collections
options.Arguments.Add("/c");
options.Arguments.Add("echo test");

// Environment inherits current process environment
options.Environment["MY_VAR"] = "value";

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

dotnet-policy-service · 2026-02-11T13:55:10Z

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot please address my feedback

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot address my feedback

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik · 2026-02-11T18:34:15Z

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs

+                return filename;
+            }
+
+#if WINDOWS


To other reviewers: In contrary to other classes, where we have dedicated .Windows.cs and .Unix.cs files, I would prefer this method to use #if defs and have everything in once place to make it easier to ensure they do the same thing on every OS, and when they don't it's also clear.

adamsitnik

@copilot please address my feedback

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot please address my feedback

adamsitnik · 2026-02-12T07:17:49Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs

+
+namespace System.Diagnostics.Tests
+{
+    public class ProcessStartOptionsTests : ProcessTestBase


Do we need to inherit from ProcessTestBase?

Suggested change

public class ProcessStartOptionsTests : ProcessTestBase

public class ProcessStartOptionsTests

Fixed in c3517ee - removed inheritance from ProcessTestBase.

adamsitnik · 2026-02-12T07:18:26Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs

+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))]
+        public void TestConstructor_ResolvesCmdOnWindows()
+        {
+            ProcessStartOptions options = new ProcessStartOptions("cmd");


Prefer simpler syntax for creating new instances of ProcessStartOptions type in all the new tests.

Suggested change

ProcessStartOptions options = new ProcessStartOptions("cmd");

ProcessStartOptions options = new("cmd");

Fixed in c3517ee - using new() syntax throughout all tests.

adamsitnik · 2026-02-12T07:21:00Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

+        public void TestResolvePath_FindsCommonUtilities()
+        {
+            // Test common Unix utilities
+            string[] commonUtils = { "ls", "cat", "echo", "sh" };


Turn this test into Theory that accepts just a string utilName

Fixed in c3517ee - converted to Theory with InlineData for "ls", "cat", "echo", "sh".

adamsitnik · 2026-02-12T07:23:16Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs

+        [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)]
+        [Fact]
+        public void TestConstructor_ResolvesShOnUnix()


Move this test to ProcessStartOptionsTests.Unix.cs‎

Fixed in c3517ee - moved to ProcessStartOptionsTests.Unix.cs as Constructor_ResolvesShOnUnix.

adamsitnik · 2026-02-12T07:23:50Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs

+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))]
+        public void TestConstructor_ResolvesCmdOnWindows()


Move this test to ProcessStartOptionsTests.Windows.cs‎

Fixed in c3517ee - moved to ProcessStartOptionsTests.Windows.cs as Constructor_ResolvesCmdOnWindows.

adamsitnik · 2026-02-12T07:30:27Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

+            // sh should be findable in PATH on all Unix systems
+            ProcessStartOptions options = new ProcessStartOptions("sh");
+            Assert.True(File.Exists(options.FileName));
+            Assert.EndsWith("sh", options.FileName);


Is it safe to assume it's always going to be equal "/bin/sh"?

Suggested change

Assert.EndsWith("sh", options.FileName);

Assert.Equals("/bin/sh", options.FileName);

Fixed in c3517ee - using Assert.Equal("/bin/sh", options.FileName).

adamsitnik · 2026-02-12T07:31:27Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs

+
+namespace System.Diagnostics.Tests
+{
+    public class ProcessStartOptionsTests_Windows


Suggested change

public class ProcessStartOptionsTests_Windows

public partial class ProcessStartOptionsTests

Fixed in c3517ee - made it a partial class.

adamsitnik · 2026-02-12T07:34:11Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs

+{
+    public class ProcessStartOptionsTests_Windows
+    {
+        [Fact]


This test fails on Nano because notepad is not available on nano.

Suggested change

[Fact]

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))]

Fixed in c3517ee - added ConditionalFact back for IsNotWindowsNanoServer.

adamsitnik · 2026-02-12T07:36:17Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs

+                {
+                    Directory.SetCurrentDirectory(tempDir);
+                    ProcessStartOptions options = new ProcessStartOptions(fileName);
+                    Assert.Equal(fullPath, options.FileName);


This test keeps failing on OSX, I think it's because it's not the actual full path:

Assert.Equal() Failure: Strings differ ↓ (pos 1) Expected: "/tmp/helix/working/BDA1097F/t/testscript."··· Actual: "/private/tmp/helix/working/BDA1097F/t/tes"·

Most likely just using Path.GetFullPath should fix it.

Suggested change

Assert.Equal(fullPath, options.FileName);

Assert.Equal(Path.GetFullPath(fullPath), options.FileName);

Fixed in c3517ee - using Path.GetFullPath(fullPath) for assertion.

adamsitnik · 2026-02-12T07:40:51Z

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

+                {
+                    Directory.SetCurrentDirectory(tempDir);
+                    // Should throw because file is not executable
+                    Assert.Throws<FileNotFoundException>(() => new ProcessStartOptions(fileName));


This test fails:

Assert.Throws() Failure: No exception was thrown Expected: typeof(System.IO.FileNotFoundException)

I think it's because currently we check the permissions only if file was found in PATH:

runtime/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

Lines 737 to 748 in 6fbc0b6

private static string? FindProgramInPath(string program)

{

string path;

string? pathEnvVar = Environment.GetEnvironmentVariable("PATH");

if (pathEnvVar != null)

{

var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true);

while (pathParser.MoveNext())

{

string subPath = pathParser.ExtractCurrent();

path = Path.Combine(subPath, program);

if (IsExecutable(path))

And we don't do that for CWD:

runtime/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

Lines 721 to 726 in 6fbc0b6

// Then check the current directory

path = Path.Combine(Directory.GetCurrentDirectory(), filename);

if (File.Exists(path))

{

return path;

}

So this test should be either fixed or removed.

Fixed in c3517ee - removed the ChecksExecutablePermissions test since ProcessStartOptions.ResolvePath doesn't check executable permissions for files in CWD (matching Process.ResolvePath behavior).

…, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot please address my feedback

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs

…h for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Initial plan

4f3f950

Copilot AI assigned Copilot and adamsitnik Feb 11, 2026

Copilot started work on behalf of adamsitnik February 11, 2026 13:34 View session

Add ProcessStartOptions class with path resolution and tests

92de299

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

github-actions bot added the area-System.Diagnostics.Process label Feb 11, 2026

Copilot AI and others added 3 commits February 11, 2026 14:07

Fix build errors and test failures in ProcessStartOptions

0c9d3e2

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Improve code comments based on review feedback

794a8a2

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix exception documentation and add caching comment

a1b3426

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add ProcessStartOptions class to System.Diagnostics namespace~~ Add ProcessStartOptions class with platform-aware path resolution Feb 11, 2026

Copilot AI requested a review from adamsitnik February 11, 2026 14:16

Copilot finished work on behalf of adamsitnik February 11, 2026 14:16

adamsitnik reviewed Feb 11, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik February 11, 2026 15:51 View session

Address PR feedback: use Process.ResolvePath, fix tests, add null checks

10ff95a

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested a review from adamsitnik February 11, 2026 16:10

Copilot finished work on behalf of adamsitnik February 11, 2026 16:10

adamsitnik reviewed Feb 11, 2026

View reviewed changes

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs Outdated Show resolved Hide resolved

Copilot started work on behalf of adamsitnik February 11, 2026 17:07 View session

Refactor: ProcessStartOptions.ResolvePath as main impl, Process calls it

fc0fd30

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested a review from adamsitnik February 11, 2026 17:18

Copilot finished work on behalf of adamsitnik February 11, 2026 17:18

address my own feedback

f687eb8

adamsitnik reviewed Feb 11, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik February 11, 2026 18:40 View session

adamsitnik requested a review from GrabYourPitchforks February 11, 2026 18:42

Simplify Windows tests: remove ConditionalFact, fix nested try-finally

5459ec9

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested a review from adamsitnik February 11, 2026 18:49

Copilot finished work on behalf of adamsitnik February 11, 2026 18:49

build-analysis bot mentioned this pull request Feb 12, 2026

System.Security.Cryptography.CryptographicException : m_safeCertContext is an invalid handle. #124279

Open

adamsitnik reviewed Feb 12, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik February 12, 2026 07:41 View session

Refactor tests: partial classes, remove Test prefix, use new() syntax…

c3517ee

…, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested a review from adamsitnik February 12, 2026 07:53

Copilot finished work on behalf of adamsitnik February 12, 2026 07:53

adamsitnik reviewed Feb 12, 2026

View reviewed changes

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs Outdated Show resolved Hide resolved

src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs Show resolved Hide resolved

Copilot started work on behalf of adamsitnik February 12, 2026 07:56 View session

Fix test conditions: remove redundant IsWindows check, use GetFullPat…

bbea91e

…h for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested a review from adamsitnik February 12, 2026 08:04

Copilot finished work on behalf of adamsitnik February 12, 2026 08:04

	public class ProcessStartOptionsTests : ProcessTestBase
	public class ProcessStartOptionsTests

	ProcessStartOptions options = new ProcessStartOptions("cmd");
	ProcessStartOptions options = new("cmd");

		[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))]
		public void TestConstructor_ResolvesCmdOnWindows()

	Assert.EndsWith("sh", options.FileName);
	Assert.Equals("/bin/sh", options.FileName);

	public class ProcessStartOptionsTests_Windows
	public partial class ProcessStartOptionsTests

	[Fact]
	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))]

	Assert.Equal(fullPath, options.FileName);
	Assert.Equal(Path.GetFullPath(fullPath), options.FileName);

	private static string? FindProgramInPath(string program)
	{
	string path;
	string? pathEnvVar = Environment.GetEnvironmentVariable("PATH");
	if (pathEnvVar != null)
	{
	var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true);
	while (pathParser.MoveNext())
	{
	string subPath = pathParser.ExtractCurrent();
	path = Path.Combine(subPath, program);
	if (IsExecutable(path))

	// Then check the current directory
	path = Path.Combine(Directory.GetCurrentDirectory(), filename);
	if (File.Exists(path))
	{
	return path;
	}

Add ProcessStartOptions class with platform-aware path resolution #124271

Are you sure you want to change the base?

Add ProcessStartOptions class with platform-aware path resolution #124271

Uh oh!

Conversation

Copilot AI commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Example

Uh oh!

dotnet-policy-service bot commented Feb 11, 2026

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Copilot AI commented Feb 11, 2026 •

edited

Loading