-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add an api-proposal Copilot skill #124808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
eiriktsarpalis
wants to merge
7
commits into
dotnet:main
Choose a base branch
from
eiriktsarpalis:skill/api-proposal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+496
−0
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
91de6e8
Add an api-proposal skill
eiriktsarpalis fa44cee
Address initial feedback
eiriktsarpalis b13dea4
Delegate prototype build/test to copilot-instructions.md
eiriktsarpalis 5c9cb52
Apply suggestion from @Copilot
eiriktsarpalis d3e99bf
Address feedback
eiriktsarpalis a09dd41
Address feedback
eiriktsarpalis dc6a0db
Address feedback
eiriktsarpalis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,300 @@ | ||
| --- | ||
| name: api-proposal | ||
| description: Create prototype backed API proposals for dotnet/runtime. Use when asked to draft an API proposal or to refine a vague API idea into a complete proposal. | ||
| --- | ||
|
|
||
| # API Proposal Skill | ||
|
|
||
| Create complete, terse, and empirically grounded API proposals for dotnet/runtime. The output should have a high chance of passing the [API review process](https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md). | ||
|
|
||
| > 🚨 **NEVER** submit a proposal without a working prototype. The prototype is the evidence that the API works. Proposals without prototypes are speculative. | ||
|
|
||
| > 🚨 **NEVER** use `gh pr review --approve` or `--request-changes`. Only `--comment` is allowed. | ||
|
|
||
| ## When to Use This Skill | ||
|
|
||
| Use this skill when: | ||
| - Asked to propose a new API for dotnet/runtime | ||
| - Given a vague API idea or incomplete sketch that needs to be turned into a complete proposal | ||
| - Given an existing underdeveloped `api-suggestion` issue to refine | ||
| - Asked to prototype an API and draft a proposal | ||
| - Asked to "write an API proposal", "draft an api-suggestion", or "improve this proposal" | ||
|
|
||
| ## Core Principles | ||
|
|
||
| 1. **TERSENESS**: Proposals are reviewed live by humans during API review meetings who often lack prior context. Long text is counterproductive unless warranted by design complexity. Focus on WHAT problem and HOW to solve it. | ||
|
|
||
| 2. **Empirically grounded**: Build and test a working prototype BEFORE writing the proposal. The prototype validates the design, surfaces edge cases, and produces the exact API surface via ref source generation. | ||
|
|
||
| 3. **Claims backed by evidence**: Every motivating claim must have at least one concrete scenario. "This is useful" without showing *who* needs it and *how* they'd use it is the #1 reason proposals get sent back. | ||
|
|
||
| 4. **Context-driven depth**: The amount of supporting text should be proportional to how much **new information** the proposal introduces — not just API surface size. A small API introducing a novel concept needs more justification than a large API adding async counterparts to existing sync methods. | ||
|
|
||
| ## Modular Phases | ||
|
|
||
| This skill has 6 phases. Each can run independently (e.g., "just draft the proposal from my existing prototype"). When running the full pipeline, execute in order. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 0: Gather Input & Assess Context | ||
|
|
||
| 1. **Accept input** in any form: issue URL, text description, API sketch, or vague idea. | ||
|
|
||
| 2. **If the input is an existing GitHub issue**, read it in full and identify: | ||
| - What sections are missing or underdeveloped | ||
| - Whether the proposed API surface is concrete or still vague | ||
| - Any reviewer feedback in comments (especially if `api-needs-work` label is present) | ||
|
|
||
| 3. **Identify the target**: namespace, area label, affected types, target library under `src/libraries/`. | ||
|
|
||
| 4. **Assess novelty**: Is this a well-understood pattern extension (async variant, new overload, casing option) or something introducing a novel concept? This determines the depth of the proposal. | ||
|
|
||
| 5. **Evaluate existing workarounds**: Before proceeding, research what users can do TODAY without this API. | ||
| - Present the workaround(s) to the user for evaluation | ||
| - Explain trade-offs: performance penalty? excessive boilerplate? bad practices? | ||
| - **This is a checkpoint**: If a workaround is acceptable, the user may decide to shelve the proposal | ||
| - Only proceed to prototyping if workarounds are genuinely insufficient | ||
|
|
||
| 6. **Search for prior proposals** on the same topic: | ||
| - Search dotnet/runtime issues for related proposals (e.g., `api-suggestion`, `api-needs-work`, or closed issues) | ||
| - If duplicates exist, surface them — don't block work, but note them for linking later | ||
| - Look for clues in reviewer feedback: what caused a prior proposal to be marked `api-needs-work`? Why was it closed or stalled? Learn from that history to avoid repeating the same mistakes | ||
|
|
||
| 7. Ask clarifying questions if the proposal is too vague to prototype. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 1: Research | ||
|
|
||
| The skill contains baked-in examples and guidelines for writing good proposals (see [references/proposal-examples.md](references/proposal-examples.md) and [references/api-proposal-checklist.md](references/api-proposal-checklist.md)). The agent does NOT need to search for `api-approved` issues as templates — the baked-in references are sufficient. Phase 0's search for *related* issues on the same topic is a separate concern and is still required. | ||
|
|
||
| **What the agent DOES at runtime:** | ||
|
|
||
| 1. **Read the Framework Design Guidelines digest** at `docs/coding-guidelines/framework-design-guidelines-digest.md`. Validate that proposed names follow the conventions. | ||
|
|
||
| 2. **Read existing APIs in the target namespace** to ensure consistency: | ||
| - Naming patterns (e.g., `TryX` pattern, `XAsync` pattern, overload shapes) | ||
| - Type hierarchies and interface implementations | ||
| - Parameter ordering conventions | ||
|
|
||
| 3. **Read the reference documentation for updating ref source** at `docs/coding-guidelines/updating-ref-source.md`. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 2: Prototype | ||
|
|
||
| > **If the user already has a prototype**, ask for the published branch link and skip to Phase 3. | ||
|
|
||
| 1. Create a new branch: `api-proposal/<short-name>`. The prototype must be pushed as a **single commit** on this branch. | ||
|
|
||
| 2. Implement the API surface with: | ||
| - Complete triple-slash XML documentation on all public members | ||
| - Proper `#if` guards for TFM-specific APIs | ||
|
|
||
| 3. Write comprehensive tests: | ||
| - Use `[Theory]` with `[InlineData]`/`[MemberData]` where applicable | ||
| - Cover edge cases, null inputs, boundary conditions | ||
| - Test any interaction with existing APIs | ||
|
|
||
| #### Prototype Validation (all steps required) | ||
|
|
||
| > **Prerequisite:** Follow the build and test workflow in [`copilot-instructions.md`](/.github/copilot-instructions.md) — complete the baseline build, configure the environment, and use the component-specific workflow for the target library. All build and test steps below assume the baseline build has already succeeded. | ||
|
|
||
| **Step 1: Build and test** | ||
|
|
||
| Build the src and test projects, then run all tests for the target library using the workflow described in `copilot-instructions.md`. All tests must pass with zero failures. | ||
|
|
||
| Building the test project separately is critical for detecting **source breaking changes** that ApiCompat won't catch: | ||
| - New overloads/extension methods causing wrong method binding in existing code | ||
| - New generic overloads causing overload resolution ambiguity | ||
| - Pay attention to compilation **warnings**, not just errors | ||
|
|
||
| **Step 2: Check TFM compatibility** | ||
|
|
||
| Inspect the library's `.csproj` for `TargetFrameworks`. If it ships netstandard2.0 or net462 artifacts: | ||
| - Verify the prototype compiles for ALL target frameworks, not just `$(NetCoreAppCurrent)` | ||
| - Ensure .NET Core APIs form a **superset** of netstandard/netfx APIs | ||
| - Use `#if` guards where types like `DateOnly`, `IParsable<T>` restrict parts of the surface to .NET Core | ||
| - Failure to maintain superset relationship risks breaking changes on upgrade/type-forward | ||
|
|
||
| **Step 3: Generate reference assembly source** | ||
|
|
||
| ```bash | ||
| cd src/libraries/<LibraryName>/src | ||
| dotnet msbuild /t:GenerateReferenceAssemblySource | ||
| ``` | ||
|
|
||
| For System.Runtime, use `dotnet build --no-incremental /t:GenerateReferenceAssemblySource`. | ||
|
|
||
| This: | ||
| - Produces the **exact public API diff** to use in the proposal | ||
| - Validates that only intended APIs were added (no accidental public surface leakage) | ||
| - The `ref/` folder changes **must be committed** as part of the prototype | ||
|
|
||
| **The flow is**: vague input → working prototype → extract exact API surface from ref source → write the proposal. The prototype comes BEFORE the exact API proposal. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 3: Review (encapsulates code-review skill) — BLOCKING | ||
|
|
||
| 1. Invoke the **code-review** skill against the prototype diff. | ||
|
|
||
| 2. **All errors and warnings must be fixed** before proceeding to the draft phase. | ||
|
|
||
| 3. If the API change could affect performance (hot paths, allocations, new collection types), suggest running the **performance-benchmark** skill. | ||
|
|
||
| 4. Re-run tests after any review-driven changes to confirm nothing regressed. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 4: Draft Proposal | ||
|
|
||
| **Core principle: TERSENESS.** Focus on WHAT problem and HOW to solve it. Do not generate long text unless the design complexity warrants it. | ||
|
|
||
| Write the proposal matching the spirit of the [issue template](https://github.com/dotnet/runtime/blob/main/.github/ISSUE_TEMPLATE/02_api_proposal.yml). Skip inapplicable fields rather than filling them with "N/A". | ||
|
|
||
| #### Proposal Structure | ||
|
|
||
| **1. Background and motivation** | ||
|
|
||
| - WHAT concrete user problem are we solving? Show scenario(s). | ||
| - Reference prior art in other ecosystems where relevant. | ||
| - Briefly summarize existing workarounds and why they are insufficient, but do not repeat the full Phase 0 analysis. Keep this section focused on the problem and the high-level rationale for a new API. | ||
|
|
||
| **2. API Proposal** | ||
|
|
||
| The exact API surface, extracted from the `GenerateReferenceAssemblySource` output: | ||
|
|
||
| - **New self-contained types**: Clean declaration format (no diff markers). Example: | ||
|
|
||
| ```csharp | ||
| namespace System.Collections.Generic; | ||
|
|
||
| public class PriorityQueue<TElement, TPriority> | ||
| { | ||
| public PriorityQueue(); | ||
| public PriorityQueue(IComparer<TPriority>? comparer); | ||
| public int Count { get; } | ||
| public void Enqueue(TElement element, TPriority priority); | ||
| public TElement Dequeue(); | ||
| // ... | ||
| } | ||
| ``` | ||
|
|
||
| - **Additions to existing types**: Prefer `csharp` blocks when the proposal only adds new members and doesn't need to show existing APIs for context. Mark the containing type `partial` to emphasize it has other public members: | ||
|
|
||
| ```csharp | ||
| namespace System.Text.Json; | ||
|
|
||
| public partial class JsonNamingPolicy | ||
| { | ||
| public static JsonNamingPolicy SnakeLowerCase { get; } | ||
| public static JsonNamingPolicy SnakeUpperCase { get; } | ||
| } | ||
| ``` | ||
|
|
||
| When existing members ARE needed for context (e.g., to show sibling overloads), use `diff` blocks instead: | ||
|
|
||
| ```diff | ||
| namespace System.Text.Json; | ||
|
|
||
| public partial class JsonNamingPolicy | ||
| { | ||
| public static JsonNamingPolicy CamelCase { get; } | ||
| + public static JsonNamingPolicy SnakeLowerCase { get; } | ||
| + public static JsonNamingPolicy SnakeUpperCase { get; } | ||
| } | ||
| ``` | ||
|
|
||
| Rules: | ||
| - **No implementation code.** Ever. | ||
| - **No extensive XML docs.** Comments only as brief clarifications for the review board. | ||
| - Naming must follow the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/framework-design-guidelines-digest.md). | ||
|
|
||
| **3. API Usage** | ||
|
|
||
| Realistic, compilable code examples demonstrating the primary scenarios. Number and depth should match the novelty of the API, not just its size. A simple new overload may need one example; a new collection type may need several showing different use patterns. | ||
|
|
||
| **4. Design Decisions** (nontrivial only) | ||
|
|
||
| For any decision where reasonable alternatives exist, briefly explain the reasoning. Omit for self-evident decisions. List format works well: | ||
|
|
||
| - "Uses a quaternary heap instead of binary for better cache locality" | ||
| - "Does not implement `IEnumerable` because elements cannot be efficiently enumerated in priority order" | ||
|
|
||
| **5. Alternative Designs** | ||
|
|
||
| The agent has the burden of proof when claiming no viable alternatives exist. Show that alternatives were genuinely considered and explain why the proposed design is preferred. | ||
|
|
||
| **6. Risks** | ||
|
|
||
| The agent has the burden of proof when claiming absence of risks. Evaluate: | ||
| - Binary breaking changes (caught by ApiCompat) | ||
| - Source breaking changes (overload resolution, method binding) | ||
| - Performance implications | ||
| - TFM compatibility | ||
|
|
||
| **7. Open Questions** (if any) | ||
|
|
||
| List unresolved design questions with tentative answers. Surfacing uncertainty is a feature, not a weakness. Example from PriorityQueue: | ||
| - "Should we use `KeyValuePair` instead of tuples?" | ||
|
|
||
| **8. Scope considerations** (if applicable) | ||
|
|
||
| If the proposal could naturally extend to neighboring APIs (e.g., "should this also apply to `ToHashSet`?"), flag it as an open question. | ||
|
|
||
| **9. Related issues** | ||
|
|
||
| Link any related/duplicate proposals found during Phase 0 research. | ||
|
|
||
| **10. Prototype** | ||
|
|
||
| Link to the prototype commit (e.g., `https://github.com/<owner>/<repo>/commit/<sha>`). | ||
|
|
||
| #### After Drafting | ||
|
|
||
| Present the complete draft to the user for review. Iterate based on feedback before publishing. | ||
|
|
||
| --- | ||
|
|
||
| ### Phase 5: Publish | ||
|
|
||
| #### Step 1: Push and capture commit URL | ||
|
|
||
| Commit prototype changes and push the branch to the user's fork (default) or ask for an alternative remote. Capture the commit URL for inclusion in the proposal (e.g., `https://github.com/<owner>/<repo>/commit/<sha>`). | ||
|
|
||
| #### Step 2: Non-interactive mode (Copilot Coding Agent) | ||
|
|
||
| If the agent cannot prompt the user for input (e.g., running as Copilot Coding Agent), automatically post the API proposal as a comment on the associated pull request: | ||
|
|
||
| ```bash | ||
| gh pr comment <pr-number> --body-file proposal.md | ||
| ``` | ||
eiriktsarpalis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Skip the interactive options below. | ||
|
|
||
| #### Step 3: Interactive mode — offer publishing options | ||
|
|
||
| Present the user with the following options. Which options appear depends on context: | ||
|
|
||
| > **Note:** Always write the proposal text to a temporary file (e.g., `proposal.md`) and use `--body-file` instead of `--body` to avoid shell quoting/escaping issues with multi-line text. | ||
|
|
||
| 1. **Post as comment on existing issue/PR** — Only offer this when the user explicitly referenced an issue or PR in their original prompt. | ||
| ```bash | ||
| gh issue comment <number> --body-file proposal.md | ||
| # or | ||
| gh pr comment <number> --body-file proposal.md | ||
| ``` | ||
|
|
||
| 2. **Create a new issue** — Always offer this option. | ||
| ```bash | ||
| gh issue create --label api-suggestion --title "[API Proposal]: <title>" --body-file proposal.md | ||
| ``` | ||
| No area label — repo automation handles that. | ||
|
|
||
| 3. **Create a new draft PR with proposal in OP** — Always offer this option. | ||
| ```bash | ||
| gh pr create --draft --title "[API Proposal]: <title>" --body-file proposal.md | ||
| ``` | ||
|
|
||
| Include related issue links in the body for all options. | ||
78 changes: 78 additions & 0 deletions
78
.github/skills/api-proposal/references/api-proposal-checklist.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # API Proposal Quality Checklist | ||
|
|
||
| Use this checklist to validate an API proposal before publishing. Items are ordered by importance. | ||
|
|
||
| ## Background and Motivation | ||
|
|
||
| - [ ] **DO** state the concrete user problem with at least one real-world scenario | ||
| - [ ] **DO** evaluate existing workarounds and explain why they are insufficient (performance, boilerplate, bad practices) | ||
| - [ ] **DO** reference prior art in other ecosystems where relevant (e.g., Python, Java, Rust equivalents) | ||
| - [ ] **DO NOT** assume the reviewers are subject matter experts of the library being augmented (even though they're .NET experts) | ||
| - [ ] **DO NOT** make unsubstantiated claims (e.g., "this is commonly needed" without showing who needs it) | ||
| - [ ] **DO NOT** write motivation text that is longer than what the design complexity warrants | ||
|
|
||
| ## API Surface | ||
|
|
||
| - [ ] **DO** extract the exact API surface from `GenerateReferenceAssemblySource` output | ||
| - [ ] **DO** use clean declaration format for new self-contained types | ||
| - [ ] **DO** use `diff` blocks showing relevant context (sibling overloads) for additions to existing types | ||
| - [ ] **DO** validate all names against the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/framework-design-guidelines-digest.md) | ||
| - [ ] **DO** verify naming consistency with existing APIs in the target namespace | ||
| - [ ] **DO NOT** include implementation code in the API surface | ||
| - [ ] **DO NOT** include extensive XML documentation in the proposal diff — comments only as brief clarifications | ||
|
|
||
| ## Scope Completeness | ||
|
|
||
| - [ ] **DO** consider whether neighboring APIs need the same treatment (e.g., adding to `ToDictionary`? what about `ToHashSet`?) | ||
| - [ ] **DO** consider whether APIs require common parameters (e.g., `CancellationToken`, `IEqualityComparer<T>`) | ||
| - [ ] **DO** consider async counterparts if adding sync APIs, and vice versa | ||
| - [ ] **DO** include `System.Linq.Queryable` and `System.Linq.AsyncEnumerable` equivalents when proposing new `System.Linq` methods | ||
| - [ ] **DO** consider overload consistency with existing method families | ||
| - [ ] **DO NOT** propose a narrow addition without evaluating the broader scope — reviewers will ask about it | ||
|
|
||
| ## Prototype Validation | ||
|
|
||
| - [ ] **DO** verify the prototype builds for all target frameworks in the library's `.csproj` | ||
| - [ ] **DO** verify .NET Core APIs form a superset of netstandard/netfx APIs (if the library ships both) | ||
| - [ ] **DO** generate reference assembly source and commit the `ref/` changes | ||
| - [ ] **DO** build the test project separately to catch source breaking changes (overload resolution ambiguity, wrong method binding) | ||
| - [ ] **DO** verify all tests pass with zero failures | ||
| - [ ] **DO NOT** skip ApiCompat validation — binary compatibility must be maintained | ||
|
|
||
| ## Usage Examples | ||
|
|
||
| - [ ] **DO** provide realistic, compilable code examples | ||
| - [ ] **DO** demonstrate the primary scenarios the API is designed to address | ||
| - [ ] **DO** match the number of examples to the novelty of the API, not just its size | ||
| - [ ] **DO NOT** provide only trivial/toy examples that don't demonstrate real usage | ||
|
|
||
| ## Design Decisions | ||
|
|
||
| - [ ] **DO** explain the reasoning for nontrivial design choices | ||
| - [ ] **DO** list explicit trade-offs that were made (e.g., "no update support for 2-3x better performance") | ||
| - [ ] **DO NOT** explain self-evident decisions — this wastes reviewer time | ||
|
|
||
| ## Alternatives and Risks | ||
|
|
||
| - [ ] **DO** demonstrate that alternatives were genuinely considered | ||
| - [ ] **DO** evaluate risks specifically: binary breaking changes, source breaking changes, performance, TFM compatibility | ||
| - [ ] **DO NOT** write "N/A" without demonstrating you've actually evaluated — it signals lack of research | ||
| - [ ] **DO NOT** dismiss risks with hand-wavy language ("low risk", "unlikely to cause issues") | ||
|
|
||
| ## Open Questions | ||
|
|
||
| - [ ] **DO** list unresolved design questions with tentative answers | ||
| - [ ] **DO** surface uncertainty rather than hiding it — surprises during review are worse | ||
| - [ ] **DO NOT** leave fundamental design tensions unresolved without data to back a position | ||
|
|
||
| ## Common Reviewer Feedback Patterns (DO NOT) | ||
|
|
||
| The following patterns consistently result in proposals being sent back for rework: | ||
|
|
||
| - **DO NOT** state claims as facts without backing scenarios ("this is commonly needed" → show who and how) | ||
| - **DO NOT** propose a narrow scope without evaluating the full picture (reviewers will ask "what about X?") | ||
| - **DO NOT** use imprecise naming that violates Framework Design Guidelines conventions | ||
| - **DO NOT** propose APIs without checking for prior art ("has this been done before?" is a standard reviewer question) | ||
| - **DO NOT** oversimplify a complex design space — if the problem is nuanced, the proposal should acknowledge the nuance | ||
| - **DO NOT** hide open design questions — acknowledge them upfront with tentative answers | ||
| - **DO NOT** propose an API without a working prototype — speculation is not evidence |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.