[WIP] feat: Async refactor

.skills/csharp-async-best-practices/SKILL.md

@@ -0,0 +1,120 @@
+---
+name: csharp-async-best-practices
+description: Use when reviewing, writing, refactoring, or designing c# async code that uses task, task-generic, valuetask, cancellationtoken, task.whenall, task.whenany, task.run, configureawait, async void, or fire-and-forget patterns. Trigger on `.result`, `.wait()`, deadlocks, cancellation propagation, asp.net core background work, ui responsiveness, exception flow, and performance-sensitive async api design.
+metadata:
+  category: technique
+  triggers:
+    - c#
+    - async
+    - task
+    - valuetask
+    - cancellationtoken
+    - configureawait
+    - .result
+    - .wait()
+    - async void
+    - fire-and-forget
+    - task.run
+    - whenall
+    - whenany
+    - asp.net core
+    - deadlock
+---
+
+# C# Async Best Practices
+
+## Overview
+
+Apply evidence-backed async guidance with this priority order:
+
+1. correctness and cancellation semantics
+2. context-specific API design
+3. concurrency behavior and failure handling
+4. performance tuning only when the hot path is real
+
+Treat blanket advice as suspect. Separate official behavior from expert interpretation and from your own recommendation.
+
+## Workflow
+
+1. Classify the code before judging it.
+   - **I/O-bound async**: network, file, database, timers, async waits
+   - **CPU-bound work**: expensive computation
+   - **Context**: library, UI app, ASP.NET Core app, background service, test code
+   - **Pressure**: hot path or ordinary path
+2. Prefer the least surprising correct design.
+3. Only optimize allocations or scheduling after the correctness story is sound.
+4. Load the matching reference file before making strong claims.
+   - **General rules and code review defaults**: `references/core-guidance.md`
+   - **Context-sensitive rules**: `references/context-and-tradeoffs.md`
+   - **Source notes and authority breakdown**: `references/source-notes.md`
+
+## Review defaults
+
+Start from these defaults unless the case-specific evidence says otherwise:
+
+| Topic | Default judgment |
+|---|---|
+| Blocking on async | usually a defect or interop boundary smell |
+| `async void` | only acceptable for event handlers |
+| `ValueTask` | avoid by default; justify with measurements or a very hot path |
+| `ConfigureAwait(false)` | good library default, not an app-wide default |
+| `Task.Run` | use to offload CPU work when needed, not to fake async I/O |
+| Fire-and-forget | assume unsafe until lifecycle, scope, and exception handling are explicit |
+| `Task.WhenAll` | prefer for independent concurrent operations |
+| `Task.WhenAny` | always inspect winner and define what happens to losers |
+| Cancellation | accept and propagate token until the point of no cancellation |
+
+## Output contract
+
+When you review or design code, label your reasoning like this:
+
+- **Fact**: official runtime or API behavior
+- **Expert guidance**: interpretation from strong experts when it adds design meaning
+- **Synthesis**: your recommendation for this exact case
+
+Do not present contextual advice as a universal law.
+
+## Common traps
+
+- Calling `.Result`, `.Wait()`, or `GetAwaiter().GetResult()` inside normal async-capable code
+- Recommending `ConfigureAwait(false)` everywhere because “it is .NET Core” or “it prevents deadlocks”
+- Recommending `Task.Run` inside ASP.NET Core request code just to make code “more async”
+- Recommending `ValueTask` for every hot-looking method without checking completion behavior, call frequency, or single-consumer assumptions
+- Ignoring cancellation after plumbing a `CancellationToken`
+- Using `Task.WhenAny` without awaiting the returned winner task or handling the remaining tasks
+- Treating fire-and-forget as harmless when it touches scoped services, `HttpContext`, or unobserved failures
+
+## Rationalization traps
+
+| Rationalization | Better reasoning |
+|---|---|
+| “It works, so `.Result` is fine.” | Lack of failure under one context does not make blocking safe or scalable. |
+| “`ValueTask` is always faster.” | It trades simplicity for niche allocation wins and stricter consumption rules. |
+| “`ConfigureAwait(false)` everywhere is modern guidance.” | Library and app code have different constraints. Blanket rules are weak. |
+| “`Task.Run` makes server code asynchronous.” | It only queues work; it does not turn blocking I/O into true async I/O. |
+| “Fire-and-forget is okay because logging exists.” | Logging does not solve scope lifetime, shutdown, retries, or error propagation. |
+
+## Deliverable shape
+
+For code review or implementation help, prefer:
+
+1. a short context classification
+2. the concrete problem
+3. the corrected pattern
+4. the context-dependent tradeoff, if any
+5. the smallest safe code change
+
+## API shape and testability
+
+- Prefer `Async` suffixes for awaitable-returning methods unless an established contract or event pattern dictates otherwise.
+- Prefer `Task`-returning seams over hidden background work so tests can await completion, faults, and cancellation.
+- For timers, queues, retries, or background pipelines, recommend abstractions that let tests control time and observe completion.
+- When reviewing an async API, ask whether callers can compose it, cancel it, await it, and assert its failure behavior.
+
+## Hard boundaries
+
+- Do not endorse sync-over-async as a normal design choice.
+- Do not suggest `async void` except for event handlers.
+- Do not suggest `ValueTask` unless the constraints are understood.
+- Do not claim `ConfigureAwait(false)` is always needed or always unnecessary.
+- Do not approve fire-and-forget unless ownership, exception handling, and lifetime are explicit.

.skills/csharp-async-best-practices/references/context-and-tradeoffs.md

@@ -0,0 +1,82 @@
+---
+description: >-
+  Context-specific async guidance for library code, ui apps, asp.net core,
+  background work, task.run, configureawait, and performance-sensitive design.
+metadata:
+  tags: [configureawait, task.run, asp.net core, ui, library, performance]
+  source: mixed
+---
+
+# Context and Tradeoffs
+
+## Library code versus app code
+
+### General-purpose library code
+- Prefer APIs that expose true async for I/O-bound work.
+- Do not add async wrappers around purely compute-bound methods just to look modern. Expose sync compute APIs and let callers decide whether to offload.
+- `ConfigureAwait(false)` is a strong default when the library does not need the caller’s context.
+- Avoid ambient assumptions about a UI thread, request context, or test framework behavior.
+
+### App code
+- Prefer the style that fits the app model.
+- UI code often needs the original context after `await`.
+- ASP.NET Core request code normally does not need `Task.Run` just to stay responsive, because it already runs on thread pool threads.
+- Do not present “ASP.NET Core has no synchronization context” as proof that every `ConfigureAwait(false)` discussion is obsolete.
+
+## `Task.Run` boundaries
+
+### Good uses
+- Offload CPU-bound work so a UI thread can stay responsive.
+- Offload CPU work from a caller when that scheduling boundary is deliberate.
+
+### Weak uses
+- Wrapping synchronous I/O to pretend it is true async I/O.
+- Calling `Task.Run` and immediately awaiting it in ASP.NET Core request handling when no CPU offload goal exists.
+- Using `Task.Run` to hide blocking APIs instead of fixing the underlying API choice.
+
+## Fire-and-forget
+
+### Assume unsafe until proven otherwise
+A background task needs answers for all of these:
+- Who owns its lifetime?
+- How are exceptions observed?
+- How does shutdown cancel it?
+- Does it touch scoped services or request-bound objects?
+- Does work need retries, backpressure, or queueing?
+
+### Safer alternatives
+- Await the task normally.
+- Queue work to an owned background component.
+- In ASP.NET Core, prefer hosted services or a dedicated background queue pattern for long-lived work.
+- If scoped services are required in background processing, create an explicit scope instead of capturing request scope objects.
+
+## `ConfigureAwait`
+
+### Strong recommendation
+- In general-purpose libraries, use `ConfigureAwait(false)` unless the continuation must run in the captured context.
+
+### Weak recommendation
+- “Always use it in app code.”
+- “Never use it on .NET Core.”
+- “Use it once at the first await and you are done.”
+
+### Review note
+If code after the `await` needs a specific context, say so explicitly. If it does not, the recommendation depends on whether the code is app-level or general-purpose library code.
+
+## Performance guidance
+
+### Correctness first
+Do not trade API clarity for speculative micro-optimizations.
+
+### `ValueTask` is performance-specialized
+Recommend it only when most of these are true:
+1. the method is called very frequently
+2. it often completes synchronously or from a reusable source
+3. allocation reduction matters on measurements
+4. consumers can respect single-consumer semantics
+5. task combinator ergonomics are not central to the API
+
+### Throttling and concurrency control
+- `Task.WhenAll` expresses concurrency; it does not limit it.
+- For bounded concurrency, use an async gate such as `SemaphoreSlim.WaitAsync`, or platform helpers such as `Parallel.ForEachAsync` when the workload fits.
+- Always define what happens to remaining work after the first completion or first failure.

.skills/csharp-async-best-practices/references/core-guidance.md

@@ -0,0 +1,105 @@
+---
+description: >-
+  Source-backed core guidance for task, valuetask, cancellation, exception flow,
+  blocking, and concurrency in c# async code reviews and implementations.
+metadata:
+  tags: [csharp, async, task, valuetask, cancellation, exceptions, concurrency]
+  source: mixed
+---
+
+# Core Guidance
+
+## Facts from official .NET documentation
+
+### 1. Return types and `async void`
+- Async methods should normally return `Task` or `Task<T>`.
+- `async void` is intended for event handlers; callers cannot await it and exception handling differs.
+- TAP methods that return awaitable types conventionally use the `Async` suffix.
+
+### 2. Blocking on async
+- `Task<T>.Result` is blocking. Prefer `await` in most cases.
+- Blocking can deadlock in context-bound environments and reduces scalability even when it does not deadlock.
+- `await` on a faulted task rethrows one exception directly; `.Wait()` and `.Result` wrap failures in `AggregateException`.
+
+### 3. `Task` versus `ValueTask`
+- Default to `Task` or `Task<T>` unless there is a demonstrated reason not to.
+- `ValueTask` has stricter usage rules. A given instance should generally be awaited only once.
+- Do not await the same `ValueTask` multiple times, call `AsTask()` multiple times, or mix consumption techniques on the same instance.
+- For synchronously successful `Task`-returning methods, `Task.CompletedTask` is the normal zero-result completion value.
+
+### 4. Cancellation
+- If a TAP method supports cancellation, expose a `CancellationToken`.
+- Pass the token to nested operations that should participate in cancellation.
+- If an async method throws `OperationCanceledException` associated with the method’s token, the returned task transitions to `Canceled`.
+- After a method has completed its work successfully, do not report cancellation instead of success.
+
+### 5. Exception flow and task combinators
+- `Task.WhenAll` does not block the calling thread.
+- If any supplied task faults, the `WhenAll` task faults and aggregates the unwrapped exceptions from the component tasks.
+- If none fault and at least one is canceled, the `WhenAll` task is canceled.
+- `Task.WhenAny` returns a task that completes successfully with the first completed task as its result, even when that winning task itself is faulted or canceled.
+- After `WhenAny`, await the returned winner task to propagate its outcome.
+- The remaining tasks continue unless you cancel or otherwise handle them.
+
+## Expert guidance that is strong and technically grounded
+
+### Stephen Toub
+- Use `ConfigureAwait(false)` as the general default for general-purpose library code, because library code should not depend on an app model’s context.
+- App-level code is different. UI code often needs the captured context. ASP.NET Core also changes the deadlock discussion because it does not install the classic ASP.NET style synchronization context, but that does not make blanket `ConfigureAwait` advice strong.
+- `ValueTask<T>` exists mainly to avoid allocations on frequently synchronous success paths. It is not a general replacement for `Task<T>` because `Task` is more flexible for multiple awaits, caching, and combinators.
+
+### Andrew Arnott
+- Propagate the token until the point of no cancellation.
+- Validate arguments before cancellation checks when argument validation should always run.
+- Prefer catching `OperationCanceledException` rather than `TaskCanceledException` in general-purpose logic.
+- Keep `CancellationToken` last in the parameter list; make it optional mainly on public APIs, not necessarily on internal methods.
+
+### Stephen Cleary
+- “Async all the way” is a strong design guideline, not an absolute law of physics. Sync bridges exist, but they are specialized boundary decisions, not a normal code review recommendation.
+- `async void` and sync-over-async both create real observability and composition problems even when a sample appears to work.
+
+## Naming and testability
+
+### Naming
+- TAP methods that return awaitable types conventionally use the `Async` suffix. Do not force renames when an interface, base class, or event pattern already dictates the name.
+
+### Testability
+- Favor awaitable APIs over hidden work so tests can await completion, assert faults, and drive cancellation deterministically.
+- Prefer explicit background components, injected clocks, and owned queues over ad hoc fire-and-forget logic that tests cannot observe.
+
+## Synthesis for agents
+
+### Code review defaults
+- Treat `.Result`, `.Wait()`, and `GetAwaiter().GetResult()` as likely defects unless the code is a deliberate sync boundary and the caller explicitly cannot be async.
+- Prefer `Task`/`Task<T>` for API design. Require an explicit reason before recommending `ValueTask`.
+- Require cancellation behavior to be coherent: accepted, propagated, and not silently dropped.
+- Prefer `await Task.WhenAll(...)` for independent operations started before awaiting.
+- Treat `Task.WhenAny(...)` as incomplete until the winner is awaited and losers are canceled, observed, or intentionally left running.
+
+### Minimal examples
+
+#### Avoid sync-over-async
+```csharp
+// bad
+var user = client.GetUserAsync(id).Result;
+
+// better
+var user = await client.GetUserAsync(id);
+```
+
+#### Use `Task.WhenAll` for parallel I/O
+```csharp
+var userTask = repo.GetUserAsync(id, ct);
+var ordersTask = repo.GetOrdersAsync(id, ct);
+await Task.WhenAll(userTask, ordersTask);
+return new Dashboard(await userTask, await ordersTask);
+```
+
+#### Be conservative with `ValueTask`
+```csharp
+// default
+Task<Item?> GetAsync(string key, CancellationToken ct);
+
+// specialized hot path only when justified
+ValueTask<Item?> TryGetCachedAsync(string key);
+```

.skills/csharp-async-best-practices/references/source-notes.md

@@ -0,0 +1,59 @@
+---
+description: >-
+  Authority notes and citations for the c# async best practices skill, separating
+  official documentation, expert interpretation, and synthesized guidance.
+metadata:
+  tags: [sources, citations, authority, notes]
+  source: external
+---
+
+# Source Notes
+
+## Official facts
+
+- Microsoft Learn, "Implementing the Task-based Asynchronous Pattern"
+  - https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/implementing-the-task-based-asynchronous-pattern
+  - Return types, cancellation behavior, `Task.Run` boundaries, and TAP implementation guidance.
+- Microsoft Learn, "Consuming the Task-based Asynchronous Pattern"
+  - https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/consuming-the-task-based-asynchronous-pattern
+  - `await`, `WhenAll`, `WhenAny`, cancellation propagation, and exception behavior.
+- Microsoft Learn, "Async return types"
+  - https://learn.microsoft.com/en-us/dotnet/csharp/asynchronous-programming/async-return-types
+  - `Task`, `Task<T>`, `async void`, generalized async return types.
+- Microsoft Learn, `ValueTask` API reference
+  - https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.valuetask
+  - single-consumer warnings and default-to-`Task` guidance.
+- Microsoft Learn, ASP.NET Core best practices
+  - https://learn.microsoft.com/en-us/aspnet/core/fundamentals/best-practices
+  - avoid blocking calls, avoid unnecessary `Task.Run`, background-work cautions.
+- Microsoft Learn, hosted services in ASP.NET Core
+  - https://learn.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services
+  - safe long-lived background work and cancellation during shutdown.
+
+## Expert guidance used only when technically grounded
+
+- Stephen Toub, ".NET Blog: ConfigureAwait FAQ"
+  - https://devblogs.microsoft.com/dotnet/configureawait-faq/
+  - best source for context capture semantics and library-vs-app guidance.
+- Stephen Toub, ".NET Blog: Understanding the Whys, Whats, and Whens of ValueTask"
+  - https://devblogs.microsoft.com/dotnet/understanding-the-whys-whats-and-whens-of-valuetask/
+  - performance rationale and tradeoffs behind `ValueTask<T>`.
+- Stephen Toub, ".NET Blog: Await, and UI, and deadlocks! Oh my!"
+  - https://devblogs.microsoft.com/dotnet/await-and-ui-and-deadlocks-oh-my/
+  - canonical deadlock explanation for context-bound code.
+- Stephen Toub, ".NET Blog: Task Exception Handling in .NET 4.5"
+  - https://devblogs.microsoft.com/dotnet/task-exception-handling-in-net-4-5/
+  - explains `await` versus blocking exception shape and why `WhenAll` matters.
+- Andrew Arnott, "Recommended patterns for CancellationToken"
+  - https://devblogs.microsoft.com/premier-developer/recommended-patterns-for-cancellationtoken/
+  - practical cancellation design heuristics; useful, but not treated as a language/runtime spec.
+- Stephen Cleary, "Async/Await - Best Practices in Asynchronous Programming"
+  - https://learn.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming
+  - useful design interpretation, but older and treated as contextual guidance rather than current official policy.
+
+## Where the skill is intentionally cautious
+
+- `ConfigureAwait`: strong guidance exists for libraries, weaker guidance for app code. Blanket rules are rejected.
+- `Task.Run`: valid for deliberate CPU offload, weak as a server-side patch for blocking I/O.
+- `ValueTask`: supported and useful, but easy to misuse. The skill defaults to `Task` unless evidence is present.
+- Fire-and-forget: acceptable only with explicit ownership and lifecycle design, especially in server code.