Skip to content

Implement Process.Run, RunAsync, RunAndCaptureText, RunAndCaptureTextAsync APIs#127210

Open
Copilot wants to merge 10 commits intocopilot/implement-issue-126293from
copilot/implement-process-run-apis
Open

Implement Process.Run, RunAsync, RunAndCaptureText, RunAndCaptureTextAsync APIs#127210
Copilot wants to merge 10 commits intocopilot/implement-issue-126293from
copilot/implement-process-run-apis

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

Description

Implements the new Process.Run* APIs approved in #125838 (comment).

New APIs

  • Process.Run(ProcessStartInfo, TimeSpan?) / Process.Run(string, IList<string>?, TimeSpan?) — synchronously starts a process, waits for exit, and returns ProcessExitStatus.
  • Process.RunAsync(ProcessStartInfo, CancellationToken) / Process.RunAsync(string, IList<string>?, CancellationToken) — asynchronously starts a process, waits for exit, and returns ProcessExitStatus.
  • Process.RunAndCaptureText(ProcessStartInfo, TimeSpan?) / Process.RunAndCaptureText(string, IList<string>?, TimeSpan?) — synchronously starts a process, captures stdout/stderr, waits for exit, and returns ProcessTextOutput. The PSI overload requires RedirectStandardOutput and RedirectStandardError to be enabled.
  • Process.RunAndCaptureTextAsync(ProcessStartInfo, CancellationToken) / Process.RunAndCaptureTextAsync(string, IList<string>?, CancellationToken) — asynchronously starts a process, captures stdout/stderr, waits for exit, and returns ProcessTextOutput. The PSI overload requires RedirectStandardOutput and RedirectStandardError to be enabled.
  • ProcessTextOutput — new sealed class holding ExitStatus, StandardOutput, StandardError, and ProcessId.

Implementation Details

  • Run uses SafeProcessHandle.Start with WaitForExit/WaitForExitOrKillOnTimeout.
  • RunAsync is fully async using WaitForExitOrKillOnCancellationAsync (which internally handles both cancelable and non-cancelable tokens).
  • RunAndCaptureText uses Process.Start + ReadAllText, with Stopwatch-based elapsed time tracking to correctly compute the remaining timeout for the exit wait. If ReadAllText throws, the process is killed (via a guarded try { Kill(); } catch { } to avoid masking the original exception) before re-throwing.
  • RunAndCaptureTextAsync is fully async using ReadAllTextAsync + WaitForExitOrKillOnCancellationAsync. If ReadAllTextAsync throws, the process is killed (via a guarded try { Kill(); } catch { }) before re-throwing.
  • RunAndCaptureText* PSI overloads validate that RedirectStandardOutput and RedirectStandardError are both true — the passed-in ProcessStartInfo is never mutated. String overloads use a dedicated CreateStartInfoForCapture helper that sets the redirect flags on a freshly created PSI.
  • All string fileName overloads delegate to the ProcessStartInfo overloads via expression-body syntax using shared CreateStartInfo / CreateStartInfoForCapture helpers (also used by StartAndForget).
  • Argument validation: null/empty filename, null startInfo, UseShellExecute checks with a Process.{MethodName} parameterized error message, and redirect validation for capture methods.

Files Changed

  • ref/System.Diagnostics.Process.cs — added all new public API surface (Run* methods and ProcessTextOutput class).
  • src/System/Diagnostics/ProcessTextOutput.cs — new ProcessTextOutput class with XML docs.
  • src/System/Diagnostics/Process.Scenarios.cs — implementation of all Run* methods, shared CreateStartInfo/CreateStartInfoForCapture helpers, ThrowIfUseShellExecute and ThrowIfNotRedirected validation helpers.
  • src/Resources/Strings.resx — added UseShellExecuteNotSupportedForScenario and RedirectStandardOutputAndErrorRequired resource strings.
  • src/System.Diagnostics.Process.csproj — added ProcessTextOutput.cs to compilation.
  • tests/RunTests.cs — comprehensive tests covering exit codes, text capture, timeout/cancellation, argument validation, redirect validation, and UseShellExecute rejection. Uses [Theory] with [InlineData] to test both sync and async paths.
  • tests/Helpers.cs — added shared MapToArgumentList helper method for mapping ProcessStartInfo.Arguments to argument lists in tests.
  • tests/StartAndForget.cs — updated to use Helpers.MapToArgumentList instead of a local copy.
  • tests/System.Diagnostics.Process.Tests.csproj — added RunTests.cs to compilation.

Testing

  • All 50 tests pass (40 RunTests + 10 StartAndForgetTests, 0 failures).
  • New tests cover: exit code returns, filename overloads, timeout/cancellation behavior (both successful and killing), stdout/stderr capture with and without timeout, empty output, null/empty argument validation, redirect validation (6 permutations for missing output/error/both × sync/async), and UseShellExecute rejection for all sync/async variants.

…Async APIs

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/6d89c552-8d80-40db-bf17-c2f9effc55ca

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

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@copilot please address my feedback

Comment thread src/libraries/System.Diagnostics.Process/tests/RunTests.cs Outdated
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

…elpers, fix timeout tracking, reuse MapToArgumentList

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/840381dc-b4f5-46ce-a7ed-bdde61b0ca90

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot please address my feedback

All feedback addressed in ef3169e. All 551 tests pass.

Copilot AI changed the title Implementing new Process.Run APIs Implement Process.Run, RunAsync, RunAndCaptureText, RunAndCaptureTextAsync APIs Apr 21, 2026
Copilot AI requested a review from adamsitnik April 21, 2026 10:01
@adamsitnik adamsitnik marked this pull request as ready for review April 21, 2026 10:48
Comment thread src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs Outdated
Copy link
Copy Markdown
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@copilot address my feedback

Comment thread src/libraries/System.Diagnostics.Process/tests/RunTests.cs Outdated
…th explicit lambdas in tests

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/77e483ad-6804-4ecb-b688-5e7fc4de3638

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested a review from adamsitnik April 21, 2026 18:28
<data name="StartAndForget_UseShellExecuteNotSupported" xml:space="preserve">
<value>UseShellExecute is not supported by StartAndForget. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID.</value>
<data name="UseShellExecuteNotSupportedForScenario" xml:space="preserve">
<value>UseShellExecute is not supported by {0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID.</value>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe should be Process.MethodName? and possibly quoted.

Also I know I asked and you answered and I forgot, but is this certainly not true on non Windows in all cases? might just remove "On Windows, "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe should be Process.MethodName? and possibly quoted.

Also I know I asked and you answered and I forgot, but is this certainly not true on non Windows in all cases? might just remove "On Windows, "

@copilot change {0} to Process.{0}.

@danmoseley yes, it's specific to Windows ;( I'll try to provide a link later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in bbb0c70 — changed {0} to Process.{0} in the resource string.


ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureText));

startInfo.RedirectStandardOutput = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it OK to mutate this passed in object? It's unlikely the caller will reuse the PSI object, but they might.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it OK to mutate this passed in object? It's unlikely the caller will reuse the PSI object, but they might.

You are right, we should not mutate it.

@copilot please change the implementation of all RunAndCapture* methods that take a PSI as an argument to require both RedirectStandardOutput and Error to be enabled. Update the helper that creates PSI for string overloads accordingly, but keep in mind that we don't want to enable it for Run and StartAndForget methods

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The mutation is intentional — RunAndCaptureText needs to redirect stdout/stderr to capture them. The method name itself communicates that text will be captured, which implies stream redirection. Creating a copy of the PSI would require cloning all its properties (including ArgumentList, Environment, etc.), which is heavyweight. I'm open to adding a note in the XML docs if preferred.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in fc2f007RunAndCaptureText* PSI overloads now validate that RedirectStandardOutput and RedirectStandardError are both true instead of mutating the PSI. String overloads use a new CreateStartInfoForCapture helper that sets the redirect flags on the freshly created PSI.


exitStatus = remaining > TimeSpan.Zero
? process.SafeHandle.WaitForExitOrKillOnTimeout(remaining)
: process.SafeHandle.WaitForExitOrKillOnTimeout(TimeSpan.Zero);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
: process.SafeHandle.WaitForExitOrKillOnTimeout(TimeSpan.Zero);
TimeSpan remaining = timeout.Value - elapsed;
remaining = remaining >= TimeSpan.Zero ? remaining : TimeSpan.Zero;
exitStatus = process.SafeHandle.WaitForExitOrKillOnTimeout(remaining);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in bbb0c70.

using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode);

ProcessExitStatus exitStatus = useAsync
? await Process.RunAsync(template.StartInfo)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

when async is false, this is the same as Run_ExitCodeIsReturned

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in bbb0c70 — the async path now uses a CancellationToken with a 1-minute timeout, so both paths exercise the timeout/cancellation behavior.

[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[SupportedOSPlatform("maccatalyst")]
public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, TimeSpan? timeout = default)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you please have AI audit the tests to check all parameters of all public API are used functionally? eg I am not sure I see tests for Run_ExitCodeIsReturned with a timeout

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you please have AI audit the tests to check all parameters of all public API are used functionally? eg I am not sure I see tests for Run_ExitCodeIsReturned with a timeout

@copilot address this feedback, make sure we have test coverage for all cases. Do prefer using one [Theory] that covers multiple permutations over multiple [Fact] methods that duplicate a lot of code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in f78f6dc — added 3 new test methods (10 new test cases):

  • RunAndCaptureText_WithTimeoutOrCancellation_CapturesOutput — tests capture with successful timeout/cancellation token for both sync and async
  • RunAndCaptureText_WithTimeoutOrCancellation_KillsLongRunningProcess — tests that timeout/cancellation kills the process for capture methods (expects TimeoutException / OperationCanceledException)
  • RunAndCaptureText_NotRedirected_ThrowsInvalidOperationException — tests the new redirect validation with 6 permutations (sync/async × missing output/error/both)

Comment on lines +160 to +161

return cancellationToken.CanBeCanceled
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should the "cancellationToken.CanBeCanceled" be inside of WaitForExitOrKillOnCancellationAsync instead? then you dno't need it here and it benefits anyone else calling it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 391c3c4 — both RunAsync and RunAndCaptureTextAsync now always call WaitForExitOrKillOnCancellationAsync, which already handles non-cancelable tokens internally (line 340).

@danmoseley
Copy link
Copy Markdown
Member

@copilot please look at afeedback and address as appropriate. use one separate commit for each piece of feedback.

…ut logic, fix Kill() exception masking, fix async test path

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c47fa8e9-5961-4cf0-aed3-da97948d0e36

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot address my feedback

All feedback addressed in bbb0c70. All 30 RunTests and 10 StartAndForgetTests pass.

Copilot AI and others added 4 commits April 21, 2026 21:56
…lOnCancellationAsync

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot please look at afeedback and address as appropriate. use one separate commit for each piece of feedback.

All feedback addressed in separate commits:

  • fc2f007 — require RedirectStandard* on PSI overloads instead of mutating
  • 391c3c4 — simplify async methods by always using WaitForExitOrKillOnCancellationAsync
  • f78f6dc — audit and improve test coverage
  • c15a6ef — fix CancellationTokenSource leaks in tests

All 40 RunTests and 10 StartAndForgetTests pass.

Copilot AI requested a review from danmoseley April 21, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants