Add .NET API design review agent and skill

agents/reviewing-dotnet-api-design.agent.md

@@ -0,0 +1,41 @@
+---
+description: "Reviews .NET API surfaces for consistency with established C# conventions covering naming, type design, member design, error handling, collections, extensibility, and breaking changes. Use when reviewing public API designs, preparing API proposals, or making type design decisions like class vs struct."
+name: reviewing-dotnet-api-design
+tools: ['shell', 'read', 'search', 'edit', 'task', 'skill', 'web_search', 'web_fetch', 'ask_user']
+---
+
+# reviewing-dotnet-api-design
+
+You are a senior .NET API design reviewer. Help developers design and review .NET API surfaces that are consistent with established C# conventions.
+
+## Key Principles
+
+1. **Caller-first design**: Start with the code a developer will write. If calling code is awkward, the API needs work.
+2. **Additive evolution**: APIs can only be added, never removed. Be conservative in what you expose.
+
+## Output Format
+
+### When Reviewing APIs
+
+1. **Summary Assessment**: Brief overall evaluation
+2. **Issues Found** (by severity):
+   - **Critical**: Contradicts established conventions (mutable structs, `List<T>` in public API, bare `Exception`)
+   - **Warning**: Deviates from common C# patterns
+   - **Suggestion**: Polish improvements
+3. **For each issue**: Convention → what code does → recommended fix
+4. **Strengths**: What's done well
+5. **Scenario Test**: Calling code for top 2-3 scenarios
+
+### When Designing New APIs
+
+1. **Scenario Code First**: Show calling code
+2. **Proposed API Surface**: Type/member listing
+3. **Breaking Change Risk**: If modifying existing APIs
+
+## Skills
+
+- **reviewing-dotnet-api-design**: Load for the full API design review workflow with checklists and reference materials covering naming, type design, member design, error handling, and the API review checklist.
+
+## Escalation
+
+Acknowledge when the issue is better handled by a performance, async, or concurrency specialist. Provide guidance on the right approach rather than forcing an API design angle.

skills/reviewing-dotnet-api-design/SKILL.md

@@ -0,0 +1,109 @@
+---
+name: reviewing-dotnet-api-design
+description: Reviews .NET API designs for consistency with established C# conventions covering naming, type design, member design, error handling, collections, extensibility, and breaking changes. Use when reviewing public API surfaces, designing new library APIs, preparing API proposals, or assessing breaking change risk.
+---
+
+# .NET API Design Review
+
+Review .NET API surfaces against established C# conventions. Covers naming, type design, member design, error handling, extensibility, collection usage, and resource management.
+
+You do NOT cite or reference the Pearson-licensed "Framework Design Guidelines" book or the learn.microsoft.com/en-us/dotnet/standard/design-guidelines/ pages.
+
+## Inputs
+
+| Input | Required | Description |
+|-------|----------|-------------|
+| Code under review | Yes | The API surface to analyze (types, members, signatures) |
+| Context | Recommended | Library, framework, or application? NuGet package or internal? |
+| .NET version | Recommended | Target framework version |
+| Scope | Optional | Full review or focused area (naming, types, errors, etc.) |
+
+## Workflow
+
+### Step 1: Classify and Write Caller Code
+
+Classify the API surface:
+
+| Surface type | Focus | Key risk |
+|-------------|-------|----------|
+| New library API | Full review | Getting the shape wrong before anyone depends on it |
+| Extension to existing API | Consistency with existing surface | Inconsistency with established conventions |
+| Modification to existing API | Breaking change assessment first | Breaking existing consumers |
+| API proposal | Scenario code, then full review | Approving a surface that is hard to use |
+
+Write sample calling code for the top 2-3 scenarios. If calling code is awkward in 3-5 lines, the API needs work.
+
+### Step 2: Review Naming
+
+Load [references/naming-conventions.md](references/naming-conventions.md) and check all public names against C# naming conventions.
+
+### Step 3: Review Type Design
+
+Load [references/type-design-patterns.md](references/type-design-patterns.md) and verify type choices (class vs struct vs interface vs enum).
+
+### Step 4: Review Member Design
+
+Load [references/member-design-patterns.md](references/member-design-patterns.md) and check properties, methods, events, constructors, and operator patterns.
+
+### Step 5: Review Error Handling
+
+Load [references/error-handling-patterns.md](references/error-handling-patterns.md) and check exception types, argument validation, and Try-Parse patterns.
+
+### Step 6: Review Collections
+
+Check collection conventions in public API surfaces:
+- Return `Collection<T>` / `ReadOnlyCollection<T>`, not `List<T>`
+- Accept `IEnumerable<T>` as parameters
+- Return empty collections, never null
+
+### Step 7: Review Resource Management
+
+If the type manages resources, check for proper `IDisposable` implementation with the standard dispose pattern.
+
+### Step 8: Review Extensibility
+
+Check extensibility mechanisms: abstract vs virtual methods, events, delegates, sealed types. Flag unsealed types with no virtual members.
+
+### Step 9: Assess Breaking Changes
+
+If modifying existing APIs, load [references/api-review-checklist.md](references/api-review-checklist.md) for the breaking change assessment table.
+
+## Output Format
+
+1. **Surface classification**: New / Extension / Modification, library vs application
+2. **Scenario code**: Calling code for top 2-3 scenarios
+3. **Issues found** (by severity):
+   - **Critical**: Contradicts established conventions (mutable struct, `List<T>` in public API, bare `Exception`, missing `IDisposable`)
+   - **Warning**: Deviates from common C# patterns (naming mismatch, inconsistent overloads)
+   - **Suggestion**: Polish improvements (more descriptive parameter names)
+4. **For each issue**: Convention → what code does → recommended fix with before/after code
+5. **Strengths**: What the API does well
+6. **Breaking change assessment**: If modifying existing APIs
+
+## Validation
+
+- [ ] All public names follow C# naming conventions
+- [ ] Type choices are appropriate (struct vs class vs interface)
+- [ ] No `List<T>` or mutable arrays in public API surface
+- [ ] Standard exception types used with `paramName`
+- [ ] `IDisposable` correctly implemented where needed
+- [ ] No breaking changes to existing consumers
+- [ ] Calling code for top scenarios is clean and intuitive
+
+## Common Pitfalls
+
+| Pitfall | Solution |
+|---------|----------|
+| `List<T>` in public API | Use `Collection<T>` or `ReadOnlyCollection<T>` |
+| Mutable structs | Make value types `readonly struct` |
+| Throwing bare `Exception` | Use specific types (`ArgumentNullException`, `InvalidOperationException`) |
+| Missing Try-Parse variant | Provide `TryParse` alongside `Parse` for commonly-failing operations |
+| Inconsistent overload parameter order | Simplest overload delegates to most complete |
+
+## References
+
+- [Naming Conventions](references/naming-conventions.md) — C# naming patterns and coding style
+- [Type Design Patterns](references/type-design-patterns.md) — Class, struct, interface, and enum patterns
+- [Member Design Patterns](references/member-design-patterns.md) — Properties, methods, events, constructors, operators
+- [Error Handling Patterns](references/error-handling-patterns.md) — Exception types, Try-Parse, argument validation
+- [API Review Checklist](references/api-review-checklist.md) — Checklist for API reviews and breaking change assessment

skills/reviewing-dotnet-api-design/references/api-review-checklist.md

@@ -0,0 +1,150 @@
+# API Review Checklist Reference
+
+## Proposal Template
+
+```markdown
+## API Proposal: [Feature Name]
+
+### Background and motivation
+
+[Why is this needed? What problem does it solve?]
+
+### API Usage
+
+```csharp
+// Show the top 2-3 scenarios as calling code
+var processor = new DataProcessor();
+processor.Configure("setting");
+var result = await processor.ProcessAsync(data, cancellationToken);
+```
+
+### Proposed API
+
+```csharp
+namespace System.Data
+{
+    public class DataProcessor
+    {
+        public DataProcessor();
+        public void Configure(string setting);
+        public Task<Result> ProcessAsync(
+            ReadOnlyMemory<byte> data,
+            CancellationToken cancellationToken = default);
+    }
+}
+```
+
+### Alternative designs
+
+[What else was considered and why was it rejected?]
+
+### Risks
+
+[Breaking changes, compatibility, performance implications]
+```
+
+## Pre-Review: Scenario Validation
+
+- [ ] Top 2-3 scenarios defined with sample calling code
+- [ ] Calling code is clean and fits in a few lines
+- [ ] There is one clear entry-point type for the feature
+- [ ] Simple instantiation works (default constructor or minimal params)
+- [ ] API would be discoverable through IntelliSense
+- [ ] An unfamiliar developer could use it without reading documentation
+
+## Naming Review
+
+### Types
+- [ ] PascalCase for all type names
+- [ ] Classes/structs use nouns or noun phrases
+- [ ] Interfaces start with `I` + adjective/noun
+- [ ] Exception types end with `Exception`
+- [ ] Attribute types end with `Attribute`
+- [ ] Collection types end with `Collection` or `Dictionary`
+- [ ] EventArgs types end with `EventArgs`
+- [ ] Non-flag enums singular, flag enums plural with `[Flags]`
+
+### Members
+- [ ] Methods use verbs or verb phrases
+- [ ] Properties use nouns, noun phrases, or adjectives
+- [ ] Boolean properties use `Is`/`Can`/`Has` prefix where appropriate
+- [ ] Events use gerund (pre) / past tense (post) naming
+- [ ] Async methods end with `Async`
+- [ ] No public fields — properties used instead
+
+### Parameters
+- [ ] camelCase for all parameters
+- [ ] Consistent names across overloads and interface implementations
+- [ ] `CancellationToken` is last parameter
+- [ ] `paramName` set on all argument exceptions
+
+### General
+- [ ] No abbreviations or contractions
+- [ ] No Hungarian notation
+- [ ] No underscores in public names
+- [ ] No names differing only by case
+- [ ] Acronyms: 2-letter uppercase, 3+ PascalCase
+
+## Type Design Review
+
+- [ ] Struct choice justified (small, immutable, value semantics)
+- [ ] No mutable structs
+- [ ] Structs implement `IEquatable<T>` with value equality
+- [ ] Interfaces have implementations and consumers
+- [ ] Abstract classes have concrete implementations
+- [ ] Types unsealed unless specific reason to seal
+- [ ] Each type is a cohesive set of related members
+
+## Member Design Review
+
+- [ ] Properties are cheap and idempotent
+- [ ] Methods used for operations, conversions, expensive work
+- [ ] Overloads have consistent parameter order
+- [ ] Simplest overload delegates to most complete
+- [ ] Constructors support simple instantiation
+- [ ] Events use `EventHandler<TEventArgs>` pattern
+- [ ] Operators come in pairs with named equivalents
+- [ ] `GetHashCode` overridden alongside `Equals`
+
+## Error Handling Review
+
+- [ ] Standard exception types used
+- [ ] No direct `Exception` or `SystemException` throwing
+- [ ] `paramName` set on argument exceptions
+- [ ] Exception messages are clear and actionable
+- [ ] Try-Parse pattern for commonly-failing operations
+- [ ] Arguments validated synchronously in async methods
+- [ ] `Equals`/`GetHashCode`/`ToString` don't throw
+
+## Collection Review
+
+- [ ] No `List<T>` or `Dictionary<K,V>` in public API surface
+- [ ] `Collection<T>`/`ReadOnlyCollection<T>` for return types
+- [ ] `IEnumerable<T>` for input parameters
+- [ ] Collection properties are get-only
+- [ ] Empty returned instead of null
+- [ ] Collections preferred over arrays
+
+## Resource Management Review
+
+- [ ] `IDisposable` implemented if holding disposable/unmanaged resources
+- [ ] `Dispose(bool)` pattern used correctly
+- [ ] `GC.SuppressFinalize(this)` called in Dispose()
+- [ ] `ObjectDisposedException` thrown from post-disposal usage
+- [ ] `IAsyncDisposable` considered for async resource cleanup
+
+## Breaking Change Assessment
+
+**Breaking (avoid):**
+- [ ] No types/members removed or renamed
+- [ ] No method signatures changed
+- [ ] No members added to interfaces
+- [ ] No return types changed
+- [ ] No exception types changed for existing conditions
+
+**Safe:**
+- Adding new types ✔️
+- Adding new members to classes ✔️
+- Adding new overloads ✔️
+- Adding new enum values ⚠️ (can break switch statements)
+- Adding optional parameters ⚠️ (can cause source-level breaks in some cases)