Add refactoring-to-async skill by mrsharm · Pull Request #269 · dotnet/skills

mrsharm · 2026-03-06T17:23:42Z

Adds the refactoring-to-async skill for bottom-up async conversion patterns in .NET.

Note: Replaces #79 (migrated from skills-old repo to new plugins/ structure).

What the Skill Teaches

Bottom-up async conversion order — start from leaf I/O methods, propagate upward
Correct sync-to-async API mappings (stream.Read → ReadAsync, connection.Open → OpenAsync, etc.)
CancellationToken propagation through the entire call chain
Identifying sync-over-async anti-patterns (.Result, .Wait(), .GetAwaiter().GetResult())
ConfigureAwait(false) usage in library code vs ASP.NET Core apps
Recognizing when async does not apply (CPU-bound work)Adds the refactoring-to-async skill for bottom-up async conversion patterns in .NET.

mrsharm · 2026-03-06T17:26:27Z

Migration Note

This PR replaces #79 which was opened from mrsharm/skills-old. The skill and eval files have been migrated to the new plugins/ directory structure:

src/dotnet/skills/refactoring-to-async/ → plugins/dotnet/skills/refactoring-to-async/
src/dotnet/tests/refactoring-to-async/ → tests/dotnet/refactoring-to-async/

All prior review feedback from #79 still applies — please see that PR for the full discussion history.

Copilot

Pull request overview

Adds a new refactoring-to-async skill to the dotnet plugin, along with an evaluation scenario and fixture project intended to test bottom-up async conversion guidance.

Changes:

Added refactoring-to-async skill documentation under plugins/dotnet/skills/.
Added a new eval scenario plus C# fixture project under tests/dotnet/refactoring-to-async/.
Updated .github/CODEOWNERS to assign ownership for the new skill and test directory.

Reviewed changes

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

Show a summary per file

File	Description
plugins/dotnet/skills/refactoring-to-async/SKILL.md	New skill content describing async refactoring workflow and pitfalls.
tests/dotnet/refactoring-to-async/eval.yaml	New eval scenarios/assertions for async refactoring and CPU-bound non-async guidance.
tests/dotnet/refactoring-to-async/UserService.cs	Fixture code containing sync I/O and blocking patterns to be refactored by the model.
tests/dotnet/refactoring-to-async/SyncService.csproj	Fixture project enabling `dotnet build`-style validation.
.github/CODEOWNERS	Adds code owners for the new skill and tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

tests/dotnet/refactoring-to-async/UserService.cs

tests/dotnet/refactoring-to-async/eval.yaml

plugins/dotnet/skills/refactoring-to-async/SKILL.md

tests/dotnet/refactoring-to-async/UserService.cs

mrsharm · 2026-03-06T18:17:29Z

Feedback carried over from #79 (refactoring-to-async)

Code Review Comments (10 threads)

@copilot — eval.yaml L12

The scenario requires identifying sync-over-async patterns like .Result/.Wait() (rubric line 14), but the output_not_matches assertion fails the run if the agent output contains .Result or .Wait() anywhere (including when calling them out as anti-patterns). This creates false negatives and makes the rubric effectively incompatible with the assertions. Consider tightening the regex to ...

@copilot — SKILL.md L35

The grep pattern uses \b for a word-boundary, but grep's default regex syntax does not treat \b as a word boundary (itΓÇÖs typically interpreted as a backspace). Also, \.Read() only matches parameterless Read() calls and will miss the common Read(...) overloads. Consider switching to rg (ripgrep) or grep -P/grep -E with a corrected pattern (e.g., matching \.Read\( and `.W...

@copilot — SKILL.md L155

Same issue as Step 1: grep without -P won't treat \b as a word boundary, so this command may not reliably find .Result usages. Adjust the command to use a regex flavor that supports word boundaries (or use \</\>), so the "should return zero results" guidance is accurate.
grep -rnP '\.Result\b|\.Wait\(\)|\.GetAwaiter\(\)\.GetResult\(\)' --include="*.cs" .

@danmoseley — SKILL.md L8

move any part of use/not use into decription similar to
https://github.com/dotnet/runtime/pull/125005/changes#diff-ded9450821c1df27638def6250c00784c7f795e3a9c56ad13d09a34853a0d09bR3-R4
if it can avoid unnecessaryt reloading.

Possibly not all can go there. For example: user wants to parallelize work is something that can be known before loading the skill, and avoid load. Whereas possibly "code ...

@danmoseley — SKILL.md L167

this is missing from all the examples. maybe indicate those are app code?

It might be worth mentioning as instructions, not just in anti-patterns -- if code is library add .ConfigureAwait(false) to every await. It should be used consistently or not at all.

@danmoseley — SKILL.md L185

this may be a big high level/conceptual for this skill?

@danmoseley — SKILL.md L53

| `reader.Read()` (DbDataReader) | `await reader.ReadAsync()` |
| `command.ExecuteNonQuery()` (DbCommand) | `await command.ExecuteNonQueryAsync()` |

maybe

@danmoseley — eval.yaml L25

is this really something the user would write? seems like a gimme as written. more likely the user would say "make DoIt() async so it's faster" and the AI would have to figure out that it's CPU bound?

@danmoseley — eval.yaml L33

do you expect ConfigureAwait(false) in this UserService case? either way, should be a test for the opposite case (eg winforms) and verify in each case it's present or not as expected

@danmoseley — SKILL.md L148

| Error | Fix |
|---|---|
 Error | Fix |
   |---|---|
   | `CS4032`: `await` in non-async method | Add `async` to the method signature and return `Task` or `Task<T>` |
   | `CS0029`: Cannot convert `Task<T>` to `T` | Add `await` before the call |
   | `CS0127`: Method returns `Task` but body returns value | Change return type to `Task<T>` |
   | `CS1983`: Return type of async meth...

Discussion Comments (5)

@ViktorHofer:

Please share the results as a comment similar to #75 (comment)
Also make sure that you test at least 3 runs (--runs 3 for local validation): https://github.com/dotnet/skills/pull/82/changes

@mrsharm:

Skill Validation Results ΓÇö refactoring-to-async

Skill Test Baseline With Skill ╬ö Verdict

refactoring-to-async Refactor synchronous service to async 3.3/5 5.0/5 +1.7 Γ£à

refactoring-to-async Async refactoring should not apply to CPU-bound code 1.0/5 1.0/5 0.0 Γ£à

**Overall improvement: +18.8%...

@mrsharm:

Skill Validation Results ΓÇö refactoring-to-async

Skill Test Baseline With Skill ╬ö Verdict

refactoring-to-async Refactor synchronous service to async 3.3/5 5.0/5 +1.7 Γ£à

refactoring-to-async Async refactoring should not apply to CPU-bound code 1.0/5 1.0/5 0.0 Γ£à

**Overall improvement: +18.8%...

@mrsharm:

Please share the results as a comment similar to #75 (comment) Also make sure that you test at least 3 runs (--runs 3 for local validation): https://github.com/dotnet/skills/pull/82/changes

Done.

@danmoseley:

add codeowners entry, move files around to match new pattern in main.

.github/CODEOWNERS

ViktorHofer · 2026-03-06T18:52:28Z

/evaluate

github-actions · 2026-03-06T18:57:46Z

Skill Validation Results

Skill	Scenario	Baseline	With Skill	Δ	Skills Loaded	Overfit	Verdict
refactoring-to-async	Refactor synchronous service to async	4.0/5	5.0/5	+1.0	✅ refactoring-to-async; tools: skill, grep	✅ 0.10	✅
refactoring-to-async	Async refactoring should not apply to CPU-bound code	2.7/5	5.0/5	+2.3	ℹ️ not activated (expected)	✅ 0.10	✅
refactoring-to-async	Library code should use ConfigureAwait(false)	4.3/5	5.0/5	+0.7	✅ refactoring-to-async; tools: skill, bash	✅ 0.10	✅
refactoring-to-async	Partial async conversion in large codebase	3.3/5	4.0/5	+0.7	✅ refactoring-to-async; tools: skill	✅ 0.10	✅

Model: claude-opus-4.6 | Judge: claude-opus-4.6

Full results

mrsharm · 2026-03-06T19:00:57Z

/evaluate

github-actions · 2026-03-06T19:06:47Z

✅ Evaluation completed. View results | View workflow run

mrsharm · 2026-03-06T19:07:20Z

@danmoseley - could you please take another look? I believe I have addressed your feedback.

ViktorHofer · 2026-03-06T19:14:44Z

@mrsharm check the one scenario in which the skill doesn't get activated.

mrsharm · 2026-03-06T19:29:01Z

@mrsharm check the one scenario in which the skill doesn't get activated.

@ViktorHofer: Intentional - is a negative test and is the correct outcome. The eval suggests optimizing matrix multiplication (CPU-bound), not about converting I/O to async. The skill's description targets I/O-bound async refactoring, so the agent should recognize this isn't an async problem and not load the skill. That's exactly what happened in all 3 runs.

ViktorHofer · 2026-03-08T16:51:02Z

@mrsharm such tests must be annotated with expect_activation: false:

skills/tests/dotnet-diag/dump-collect/eval.yaml

Line 161 in 8e50fc8

expect_activation: false

Otherwise we have no way to distinguish between intentionally not activated and unintentionally not activated.

mrsharm · 2026-03-08T17:46:47Z

/evaluate

github-actions · 2026-03-08T17:52:24Z

✅ Evaluation completed. View results | View workflow run

mrsharm · 2026-03-08T17:58:59Z

@ViktorHofer - thanks! Been implemented for this and the other PRs.

ManishJayaswal · 2026-03-10T02:07:49Z

@mrsharm - the repo has undergone some restructuring to make everything more organized. Hence, we are asking all open PRs to update the branch. Sorry about this.
This skill is already under the correct plugin - dotnet. Please update the PR and submit again.

@jasonmalinowski @jaredpar - please review.

Teaches bottom-up async conversion patterns: identifying sync-over-async, CancellationToken propagation, ConfigureAwait usage, and ValueTask selection. Eval results: +46.9% improvement over baseline (threshold: 10%) Includes eval.yaml with positive scenario + negative test and fixture files.

The 'should not apply to CPU-bound code' scenario was dropping agents into an empty workspace, causing them to give up instead of engaging with the problem. Now provides MatrixMultiplier.cs and .csproj so the agent has actual code to optimize.

SKILL.md changes: - Move when-to-use/not-use into description for lazy-loading activation - Clarify ConfigureAwait(false) guidance: app code vs library code - Streamline decision tree section eval.yaml changes: - Fix CPU-bound negative scenario: inline MatrixMultiplier.cs fixture - Make negative test prompt realistic (user doesn't state 'CPU-bound') - Add ConfigureAwait(false) library code scenario - Add partial async conversion scenario Eval results (3 runs, 4 scenarios): Refactor sync to async: 3.0 -> 5.0 (+2.0) PASS CPU-bound negative: 2.0 -> 5.0 (+3.0) PASS ConfigureAwait library code: 4.0 -> 4.0 ( 0.0) FAIL Partial async conversion: 3.3 -> 3.7 (+0.4) PASS Overall: +33.8% (3/4 passed)

…/IAsyncEnumerable, fix overfitting - Shorten skill description to be more concise (jaredpar feedback) - Replace grep commands with method name lists (danmoseley feedback) - Add Step 5: IAsyncDisposable / await using guidance (danmoseley feedback) - Add IAsyncEnumerable/await foreach and ValueTask sections (danmoseley feedback) - Fix ConfigureAwait wording: Winforms/WPF/ASP.NET (danmoseley suggestion) - De-overfit controller scenario prompt (jaredpar feedback) - Prefer sequential awaits over Task.WhenAll in rubric (danmoseley feedback) - Add output_not_matches assertion for controller scenario (copilot feedback) - Add await using assertion and rubric item to scenario 1 (danmoseley feedback) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Move refactoring-to-async CODEOWNERS entries from dotnet-upgrade to dotnet section - Add .GetAwaiter().GetResult() to scenario 1 output_not_matches assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add using var to HttpResponseMessage in both async code examples - Remove .GetAwaiter().GetResult() from controller output_not_matches (too brittle for prose) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…, clarify queryAsync - After example now uses SendAsync to match the Before's Send(HttpRequestMessage) - MatrixMultiplier.csproj: net10.0 -> net8.0 for consistency with other fixtures - IAsyncEnumerable example: queryAsync(ct) -> db.QueryAsync() for clarity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…iter from IAsyncDisposable list, tighten CPU-bound regex - Add refactoring-to-async to plugins/dotnet/README.md skill inventory - Handle null from JsonSerializer.Deserialize with ?? [] - Remove StreamReader/Writer from IAsyncDisposable examples (they only implement IDisposable) - CPU-bound scenario: narrow regex to specifically detect Multiply being made async Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Removes contradictory Task.Run advice from anti-patterns table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SKILL.md: - Shorten frontmatter description (jaredpar) - Replace grep-style patterns with example method names (danmoseley) - Add Task.WhenAll concurrency caveat with sequential-await default (danmoseley) - Fix WinForms casing (danmoseley) - Expand ValueTask restrictions section (danmoseley) - Clarify IAsyncDisposable vs IDisposable for StreamReader/Writer (danmoseley) - Improve fire-and-forget guidance to recommend IHostedService (danmoseley) eval.yaml: - Make scenario prompts more natural to reduce overfitting (jaredpar/danmoseley) - Fix file I/O prompt to reliably trigger skill activation - Add Task.WhenAll concurrency caveat to controller rubric

mrsharm · 2026-04-07T21:32:15Z

/evaluate

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-04-07T21:38:59Z

Skill Validation Results

Skill	Scenario	Quality	Skills Loaded	Overfit	Verdict
refactoring-to-async	Refactor synchronous service to async	3.3/5 → 4.7/5 🟢	✅ refactoring-to-async; tools: skill, grep	✅ 0.10	✅
refactoring-to-async	Async refactoring should not apply to CPU-bound code	3.3/5 → 4.3/5 ⏰ 🟢	ℹ️ not activated (expected)	✅ 0.10	✅ [1]
refactoring-to-async	Refactor sync-over-async in ASP.NET Core controller	3.3/5 → 5.0/5 🟢	✅ refactoring-to-async; tools: skill, grep	✅ 0.10	✅ [2]
refactoring-to-async	Convert file I/O operations to async	3.0/5 → 4.7/5 🟢	✅ refactoring-to-async; tools: skill	✅ 0.10	✅
refactoring-to-async	Partial async conversion in large codebase	2.0/5 → 3.0/5 🟢	✅ refactoring-to-async; tools: skill	✅ 0.10	✅ [3]

[1] ⚠️ High run-to-run variance (CV=4.01) — consider re-running with --runs 5
[2] ⚠️ High run-to-run variance (CV=0.89) — consider re-running with --runs 5
[3] ⚠️ High run-to-run variance (CV=2.47) — consider re-running with --runs 5

⏰ timeout — run(s) hit the (120s) scenario timeout limit; scoring may be impacted by aborting model execution before it could produce its full output (increase via timeout in eval.yaml)

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

▶ Sessions Visualisation -- interactive replay of all evaluation sessions

mrsharm · 2026-04-07T21:40:56Z

@mrsharm if you click "update branch" here to rebase on latest main, you will have somewhat improved evaluation results next time, including a prompt and info for a local agent to optimize things. same for any of your PR's.

this one seems pretty good already :)

Thanks - done! Evals look good - can merge if things look good.

mrsharm requested review from a team and Copilot March 6, 2026 17:23

mrsharm requested review from dbreshears and timheuer as code owners March 6, 2026 17:23

Copilot started reviewing on behalf of mrsharm March 6, 2026 17:24 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

mrsharm commented Mar 6, 2026

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

dotnet deleted a comment from github-actions bot Mar 6, 2026

mrsharm requested review from agocke, artl93, danmoseley and stephentoub March 8, 2026 18:03

danmoseley added the skill label Mar 9, 2026

Copilot AI review requested due to automatic review settings March 10, 2026 17:57

mrsharm and others added 17 commits April 7, 2026 14:28

Migrate refactoring-to-async to plugins/ directory structure

880157f

Add expect_activation: false to CPU-bound negative test

303fc18

Addressed copilot feedback and more evals

111d62d

More eval

3686f04

Add @danmoseley as co-owner for refactoring-to-async skill

e0e775f

Fix DoIt()->Multiply() mismatch in CPU-bound scenario, WinForms casing

fa6432b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace Task.Run fire-and-forget with IHostedService guidance

7936a2d

Removes contradictory Task.Run advice from anti-patterns table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add ExecuteNonQueryAsync to sync-to-async mapping table

f5b43a0

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use GetAsync in After example to avoid undisposed HttpRequestMessage

28a04b6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 7, 2026 21:29

mrsharm force-pushed the musharm/refactoring-to-async-skill branch from e31c31d to 2fa9826 Compare April 7, 2026 21:29

Copilot started reviewing on behalf of mrsharm April 7, 2026 21:30 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

github-actions bot added a commit that referenced this pull request Apr 7, 2026

Update session data (PR #269)

ddfec3d

mrsharm disabled auto-merge April 7, 2026 21:40

mrsharm force-pushed the musharm/refactoring-to-async-skill branch from 2fa9826 to 00ae665 Compare April 7, 2026 21:41

This was referenced Apr 13, 2026

Log exceptions in AgentRunner.StopAllClients catch block #349

Draft

Add --run-in-docker to skill-validator to run Copilot CLI in a docker container #273

Closed

Conversation

mrsharm commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What the Skill Teaches

Uh oh!

mrsharm commented Mar 6, 2026

Migration Note

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrsharm commented Mar 6, 2026

Feedback carried over from #79 (refactoring-to-async)

Code Review Comments (10 threads)

Discussion Comments (5)

Skill Validation Results ΓÇö refactoring-to-async

Skill Validation Results ΓÇö refactoring-to-async

Uh oh!

Uh oh!

ViktorHofer commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Skill Validation Results

Uh oh!

mrsharm commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026

Uh oh!

mrsharm commented Mar 6, 2026

Uh oh!

ViktorHofer commented Mar 6, 2026

Uh oh!

mrsharm commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ViktorHofer commented Mar 8, 2026

Uh oh!

mrsharm commented Mar 8, 2026

Uh oh!

github-actions bot commented Mar 8, 2026

Uh oh!

mrsharm commented Mar 8, 2026

Uh oh!

ManishJayaswal commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrsharm commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions bot commented Apr 7, 2026

Skill Validation Results

Uh oh!

mrsharm commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mrsharm commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

mrsharm commented Mar 6, 2026 •

edited

Loading

ManishJayaswal commented Mar 10, 2026 •

edited

Loading