Skip to content

feat(dotnet11): add process-api-net11 skill for new process APIs - #866

Open
shaikhsharukh wants to merge 3 commits into
dotnet:mainfrom
shaikhsharukh:contrib/process-api-net11
Open

feat(dotnet11): add process-api-net11 skill for new process APIs#866
shaikhsharukh wants to merge 3 commits into
dotnet:mainfrom
shaikhsharukh:contrib/process-api-net11

Conversation

@shaikhsharukh

Copy link
Copy Markdown

Resolves issue #649 by adding a new skill process-api-net11 for the new System.Diagnostics.Process APIs introduced in .NET 11.

This skill guides coding agents on using the new convenience one-liners, reliable deadlock-free output reading, and parent-child process lifetime properties. Evals have been included and verified using the skill-validator.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Note

This PR is from a fork and modifies infrastructure files (eng/ or .github/).

Changes to infrastructure typically need to be submitted from a branch in dotnet/skills (not a fork) so that CI workflows run with the correct permissions and secrets.

Please consider recreating this PR from an upstream branch. If you don't have push access to dotnet/skills, ask a maintainer to push your branch for you.

@shaikhsharukh

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@shaikhsharukh
shaikhsharukh force-pushed the contrib/process-api-net11 branch from bb05fea to 29286e7 Compare July 7, 2026 12:14
@rickgonzalez

Copy link
Copy Markdown

A short narrated explainer of this PR (dev testing my tool of public repos) - I hope you find it helpful. https://www.lenzon.ai/viewer/cmrb1zapc000o14os7auramuh?voice=google-chirp3

@AbhitejJohn

Copy link
Copy Markdown
Collaborator

🤖 Posted by GitHub Copilot on behalf of the repo maintainers.

Thanks for this contribution, @shaikhsharukh! 🎉 I verified every API in this skill against dotnet/runtime main (which is the .NET 11 branch — eng/Versions.props shows ProductVersion 11.0.0; no release/11.0 branch exists yet), and the good news is these are all genuinely shipping .NET 11 APIsProcess.Run/RunAsync, RunAndCaptureText/Async, StartAndForget, the ReadAll* family, and the new ProcessStartInfo properties. Nice job picking a real, useful new surface. 👍

Before we run evals and move toward merge, though, the SKILL.md documents several of these APIs inaccurately versus the public ref assembly. Since a skill trains agents to emit code, these would lead to non-compiling output. Could you correct the following?

1. ReadAll* return types are wrong. They return tuples / streaming types, not plain strings/arrays:

Documented Actual
string ReadAllText() (string StandardOutput, string StandardError) ReadAllText(TimeSpan? timeout = default)
byte[] ReadAllBytes() (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(...)
string[] ReadAllLines() IEnumerable<ProcessOutputLine> ReadAllLines(...)
Task<string[]> ReadAllLinesAsync() IAsyncEnumerable<ProcessOutputLine> ReadAllLinesAsync(...)
Task<string> ReadAllTextAsync() Task<(string, string)> ReadAllTextAsync(...)

2. InheritedHandles — invented type. The doc uses ProcessHandleInheritance InheritedHandles, but that type doesn't exist. The real property is IList<SafeHandle>? InheritedHandles.

3. Static method signatures. Run, RunAndCaptureText, and StartAndForget don't take (string arguments = "", ProcessStartInfo? startInfo = null). Actual: arguments is IList<string>? with an optional TimeSpan? timeout (Run also has a bool silent parameter); StartAndForget has no startInfo trailing parameter.

4. Example 1 won't compile. Process.RunAndCaptureTextAsync("git", "--version") — a bare string can't bind to IList<string>?. Use ["--version"] (or pass a ProcessStartInfo).

5. Example 3 won't compile. string[] lines = await process.ReadAllLinesAsync(); foreach (var line in lines)ReadAllLinesAsync() returns IAsyncEnumerable<ProcessOutputLine>, so it needs await foreach, and each item is a ProcessOutputLine, not a string.

For reference, Example 2 (KillOnParentExit = true) and the ProcessTextOutput/ProcessExitStatus usage in Example 1 are correct as written. ✅

Once the signatures and examples are aligned with the ref assembly, we'll kick off evals and continue the review. Thanks again for contributing! 🙏

@shaikhsharukh
shaikhsharukh force-pushed the contrib/process-api-net11 branch from 8c1066b to 2ae8051 Compare July 11, 2026 00:26
@shaikhsharukh

Copy link
Copy Markdown
Author

I have aligned the skill definitions and examples with the dotnet/runtime ref assembly:

  1. ReadAll* Return Types: Corrected the signatures to match the exact tuple return types ((string StandardOutput, string StandardError) for ReadAllText/Async, (byte[] StandardOutput, byte[] StandardError) for ReadAllBytes/Async) and streaming types (IEnumerable<ProcessOutputLine> / IAsyncEnumerable<ProcessOutputLine> for ReadAllLines/Async).
  2. InheritedHandles Property: Changed the property type from the hypothetical ProcessHandleInheritance to IList<SafeHandle>?.
  3. Static Method Signatures: Aligned the static Run, RunAndCaptureText, and StartAndForget methods to accept IList<string>? arguments and appropriate timeout/silent parameters.
  4. Example 1 Compilation: Modified the call to pass ["status"] as a collection expression instead of a bare string parameter.
  5. Example 3 Compilation: Updated the ping example to use await foreach over IAsyncEnumerable<ProcessOutputLine> and access line.Content / line.StandardError.

@AbhitejJohn AbhitejJohn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Follow-up review — API accuracy re-verified + eval-hardening suggestions

Thanks for the fixes, @shaikhsharukh. I re-checked everything and ran a deeper evaluation pass. Summary below; concrete suggestions are inline on eval.yaml.

1. API accuracy — all confirmed ✅ (proven)
I re-verified every signature in SKILL.md against the current dotnet/runtime main reference assemblies. All five items from the earlier review are now correct: the tuple-returning ReadAllText/ReadAllTextAsync(string StandardOutput, string StandardError), streaming ReadAllLinesAsyncIAsyncEnumerable<ProcessOutputLine>, RunAndCaptureText(Async)ProcessTextOutput, and KillOnParentExit. Content looks accurate.

2. Does the skill actually help? — yes, in plugin/deployment mode (proven, scoped)
I hardened the eval to use intent-only prompts (never naming the API) and ran it with skill-validator (--runs 5, judge = claude-opus-4.6). In the whole-plugin configuration (how the skill actually ships), it produced large, low run-to-run variance (CV 1–6%) gains on its target APIs vs a no-skill baseline:

Scenario Baseline With skill
Run + capture (RunAndCaptureText) 1.8 / 5 4.2 / 5
Full read (ReadAllText tuple) 1.0 / 5 4.2 / 5
Stream lines (ReadAllLinesAsync) 1.4 / 5 4.2 / 5

The no-skill baseline reached for the pre-.NET-11 pattern (StandardOutput.ReadToEndAsync() + Task.WhenAll); the judge noted it "didn't even attempt to research what new APIs .NET 11 offers." So the skill is teaching genuinely non-obvious APIs — that's its value.

3. Honest caveat — the blended eval verdict is not green
Being transparent: the validator's official pass metric doesn't clear the 0.10 bar (this skill scored 0.065). The plugin-mode wins above are real, but the isolated single-skill arm shows only small lift and the skill roughly doubles output tokens — both of which the weighted score penalizes. So it's a strong deployment-mode signal with a failing blended score; flagging for maintainers to weigh rather than claiming a clean pass.

4. Suggested eval improvements (inline suggestions)

  • Strengthen the run+capture assertions to require both .StandardOutput and .StandardError and reject the old event model.
  • Add two scenarios that hit the harder, wrong-signature-prone surface — the tuple-returning ReadAllText and streaming ReadAllLinesAsync / await foreach. These are where the skill adds the most measurable value.
  • Add a subtle net10.0 non-activation negative (the new APIs are net11-only).
  • The existing KillOnParentExit scenario is fine as coverage, but note it shows ~0 marginal lift — the base model already emits it correctly — so it isn't what demonstrates the skill's value.

cc @lewing @SamMonoRT — would appreciate your read on point 3: the deployment-mode gains are real and stable, but the blended eval score doesn't clear the gate. Is the plugin-mode evidence enough to accept, or do you want the eval reworked to a clean green first?

Comment thread tests/dotnet11/process-api-net11/eval.yaml
Comment thread tests/dotnet11/process-api-net11/eval.yaml
Comment thread tests/dotnet11/process-api-net11/eval.yaml Outdated
Comment thread tests/dotnet11/process-api-net11/eval.yaml
@shaikhsharukh

Copy link
Copy Markdown
Author

I have applied all of the suggested evaluation improvements:

  1. Hardened run+capture assertions: Updated to verify that both .StandardOutput and .StandardError are accessed, and blocked usage of the legacy event-based BeginOutputReadLine / OutputDataReceived patterns.
  2. Added ReadAllText (tuple return) scenario: Created a new scenario verifying that agents properly destructure the (StandardOutput, StandardError) tuple when reading process output via instance methods.
  3. Added ReadAllLinesAsync scenario: Added a streaming scenario verifying that agents consume IAsyncEnumerable<ProcessOutputLine> using await foreach and read .Content / .StandardError properly.
  4. Broadened non-activation guards: Updated the .NET 8 negative scenario to block leakages of any of the new .NET 11 APIs (RunAndCaptureText, ReadAllTextAsync, ReadAllLinesAsync, KillOnParentExit).
  5. Added .NET 10 negative scenario: Added a subtle negative scenario verifying that agents correctly fallback to pre-.NET 11 redirect and stream-reader APIs when targeting net10.0.

@AbhitejJohn

Copy link
Copy Markdown
Collaborator

/evaluate

@github-actions

Copy link
Copy Markdown
Contributor

❌ Evaluation did not complete successfully (the evaluate job reported failure). Check the workflow run logs, then re-post /evaluate to try again.

@AbhitejJohn

Copy link
Copy Markdown
Collaborator

@eiriktsarpalis could you please review this PR? You did the deepest review on the closely analogous #535 (new dotnet11 plugin skill + evals), so your feedback here would be much appreciated.

@eiriktsarpalis

Copy link
Copy Markdown
Member

It might be best if @adamsitnik took a look.

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.

4 participants