mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
add-dotnet-breaking-changes-skill
21 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Wendy Breiding (She/Her)
|
438f7aab1b | Add dotnet breaking changes skill | ||
|
Abhitej John
|
78054c1161 |
Migrate LLM evals to the Vally harness (#877)
* Make Vally the sole LLM eval engine, retiring skill-validator evaluate Collapses the parallel skill-validator + Vally eval pipeline into a single Vally-only path while preserving every capability skill-validator provided: PR gate, PR comment, dotnet/skills-data historical push, and the dashboard. The skill-validator `check` linter is retained (skill-check.yml) pending microsoft/vally #463. - evaluation.yml: delete build-validator and the skill-validator `evaluate` job; single `evaluate:` job now uses vally-evaluation.yml. Forward the COPILOT_PAT_0..9 pool onto it (from #911) so the reusable workflow's token gate is satisfied. Rewire comment-on-pr / report-status / publish-* / deploy-dashboard onto `evaluate` and vally-results-* artifacts. - vally-evaluation.yml: pin @microsoft/vally-cli@0.9.0; keep the #911 prepare guard; write the generated per-plugin experiment to $GITHUB_WORKSPACE so relative eval globs resolve. - adapt.mjs: reconcile #887's runCompareWithRetry + conclusive/unmatched hardening with the consolidation helpers (nonActivation, compareByStim, roleToDashboard, etc.). Add consolidate.mjs and gen-experiment.mjs. - build-replay-sessions.ps1: read Vally executor-session-logs/events.jsonl instead of skill-validator session output. - Rename all tests/*/*/eval.vally.yaml to eval.yaml (Vally format is now the only eval format); author Vally evals for the four skills that lacked one. - CONTRIBUTING.md + InvestigatingResults: point contributors at the Vally harness; drop non-public links. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix setup-local-sdk eval for Vally schema The hand-authored setup-local-sdk eval used skill-validator grader/environment schema that Vally rejects at validation (no stimuli executed): - file-contains/file-not-contains graders require 'value:' not 'substring:' (output-contains correctly keeps 'substring:'). - environment.files entries require 'src:'/'dest:' (copy a fixture) rather than inline 'path:'/'content:'. Moved the global.json body into fixtures/global.json. Verified with 'vally lint --eval-spec' (0 errors) across all 97 eval.yaml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix markdownlint violations in migration docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review feedback on eval migration - Consolidate run-vally-evals.sh into a single neutral entrypoint eng/run-skill-evals.sh (deletes the thin wrapper; adapt.mjs stays in eng/vally-adapter/ since CI invokes it directly) - Rename reusable workflow vally-evaluation.yml -> evaluation-run.yml and update all references/self-trigger regexes - Rename local output dir vally-results -> eval-results - Bake prerequisite preflight (Node 20+, GITHUB_TOKEN/gh) into the runner - Simplify CONTRIBUTING "Running tests locally" prerequisites - Scrub harness name from consumer-facing surfaces Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reinstate overfitting detection under the Vally harness Overfitting detection previously ran inside skill-validator's `evaluate` command, which the Vally migration replaced, so it silently stopped running. Reinstate it as a standalone step wired into the Vally pipeline: - Add `skill-validator overfitting` command that discovers skills, parses each eval.yaml, runs the existing OverfittingJudge, and emits [{plugin, skill, overfittingResult}] JSON (per-skill failures non-fatal, bounded parallelism). - Add a Vally-format parser bridge (ParseEvalConfigFlexible) so the judge handles the current stimuli/graders eval.yaml format; the legacy scenarios-only parser rejected all 98 evals, so the judge never ran. - adapt.mjs: merge overfitting results onto each verdict via --overfitting <file> (keyed by plugin/skill); output is byte-identical when the flag is absent. - evaluation-run.yml: build skill-validator (shared cache with check), run the judge per leg reading GITHUB_TOKEN, and pass --overfitting to adapt.mjs so the dashboard/skills-data consume it unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Reconcile #919 Vally eval configs into the sole eval.yaml convention PR #919 landed on main adding fresh Vally specs for three coverage-gap skills (setup-local-sdk, convert-blazor-server-to-webapp, dotnet-webapi) under the interim eval.vally.yaml filename. This branch collapses to a single Vally spec named eval.yaml, so the merge left each skill with both my migrated eval.yaml and #919's newer eval.vally.yaml (a git-clean but semantic duplicate the pipeline would not discover). Adopt #919's reviewed Vally content as the canonical eval.yaml for all three skills and drop the redundant eval.vally.yaml files, preserving the 'sole eval.yaml, zero eval.vally.yaml' invariant. Content is byte-identical to #919's blobs ( |
||
|
Abhitej John
|
90e21cddd9 |
test: add cross-family Vally eval configs for 3 coverage-gap skills (#919)
Close the coverage gap where these skills ship a base eval.yaml but had no eval.vally.yaml, so the cross-family evaluation harness silently never scored them (root cause behind #903): - dotnet/setup-local-sdk (deterministic, network-free stimuli: host-version gating, team install-script generation, dry-run install-plan composition; live SDK/workload install stays covered by the interactive base eval.yaml) - dotnet-aspnetcore/dotnet-webapi - dotnet-aspnetcore/convert-blazor-server-to-webapp Verified in cross-family run 29785538175 (executors opus/sonnet46/gpt, n=3, judge != executor family, 0 errored trials): setup-local-sdk and dotnet-webapi pass on all three executors; convert-blazor is under-powered (single stimulus) and its per-executor pass flags should not be over-read. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Wendy Breiding
|
4975793af0 |
Moving niche skills to dotnet-advanced (#807)
* Add csharp-development skill and update CODEOWNERS and README.md - Introduced a new skill for C# development guidance. - Updated CODEOWNERS to include reviewers for the new skill. - Modified README.md to list the new csharp-development skill. - Added evaluation scenarios for the csharp-development skill. * Refactor C# evaluation scenarios to enhance production code review and validation guidance * Refactor production code review scenarios to improve clarity and focus on async error handling * Update CODEOWNERS to reflect current ownership and streamline reviewer assignments * Remove csharp-development skill documentation and evaluation tests * Remove csharp-development skill from README * Fix formatting in Constants.g.cs by adding missing semicolon * Revert Constants.g.cs to main * Remove csharp-development skill from CODEOWNERS * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update CODEOWNERS to include additional reviewers for dotnet-ai skills * Move dotnet skills to dotnet-util to keep dotnet plugin focused and targeted. * Add CODEOWNERS, SKILL.md, package-types.md, and publish-workflow.md for NuGet trusted publishing; implement eval.yaml for dotnet-pinvoke and dotnet-util skills * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update README files to enhance plugin descriptions and remove outdated LSP section * Rename repository title from 'dotnet' to 'dotnet-util' in README.md for clarity. * Refine README.md to clarify included .NET skills and remove redundant descriptions * Add 'dotnet-util' plugin for utility skills in .NET development * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add 'dotnet-util' plugin for utility skills in .NET development * Add dotnet-util to Codex marketplace manifest * Align dotnet-util marketplace descriptions * Refactor CODEOWNERS and update README for nuget-trusted-publishing; add evaluation scenarios and workflow template for trusted publishing * Add P/Invoke diagnostics and type mapping references; implement NuGet trusted publishing setup - Created diagnostics.md for common pitfalls and failure modes in P/Invoke. - Added type-mapping.md for native-to-.NET type mappings. - Introduced nuget-trusted-publishing skill for setting up OIDC-based NuGet publishing. - Added package-types.md detailing structural requirements for various NuGet package types. - Created publish-workflow.md as a template for GitHub Actions workflows for NuGet publishing. * Add evaluation YAML files for csharp-scripts, dotnet-pinvoke, and nuget-trusted-publishing skills * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Complete nuget-trusted-publishing move: remove from dotnet, add to dotnet-specialized README * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Remove eval.vally.yaml and eval.yaml files for nuget-trusted-publishing skill * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add NuGet trusted publishing skill and related references - Introduced `nuget-trusted-publishing` skill for setting up OIDC-based trusted publishing on GitHub Actions. - Added detailed documentation in SKILL.md covering prerequisites, process, safety rules, and troubleshooting. - Created `package-types.md` to outline structural requirements for various NuGet package types. - Developed `publish-workflow.md` as a template for GitHub Actions workflows for NuGet publishing. - Removed obsolete `dotnet-specialized` plugin and updated `dotnet` plugin description. - Added evaluation tests for `nuget-trusted-publishing` skill to ensure proper guidance for users. - Implemented evaluation tests for `dotnet-pinvoke` and `csharp-scripts` skills to validate functionality. * Update dotnet plugin description to include high-level .NET development skills * Add NuGet trusted publishing and P/Invoke documentation - Introduced type mapping reference for native-to-.NET types in dotnet-pinvoke. - Added a comprehensive guide for setting up NuGet trusted publishing using OIDC in GitHub Actions. - Created a reference document detailing structural requirements for various NuGet package types. - Provided a template for GitHub Actions workflow for publishing NuGet packages with trusted publishing. - Implemented evaluation scenarios for testing the dotnet-pinvoke and nuget-trusted-publishing skills, including prompts and assertions for expected outputs. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Add CODEOWNERS entries for setup-local-sdk * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update dotnet README to list setup-local-sdk skill * Add setup-local-sdk to skills section in README * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
Gerald Versluis
|
72c0d2c7b7 |
Add setup-local-sdk skill for global.json paths feature (#508)
* Rebase setup-local-sdk skill Replay PR #508 onto current main and address the remaining setup-local-sdk review feedback. The install examples now use fail-fast temporary downloads, the team scripts preserve existing global.json content, and eval assertions cover the critical paths/version/workload behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address setup-local-sdk review comments Add the missing skill license metadata and fix the PowerShell .gitignore regex examples so they correctly detect an existing .dotnet/ entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stabilize setup-local-sdk evals Reduce overfit-prone assertions, make incompatible host handling explicit, configure base SDK resolution before slow workload installs, and tighten exact-version/team-script validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address setup-local-sdk follow-up review Include the shared dotnet skill reviewer team in CODEOWNERS, make .gitignore appends newline-safe, and tighten the incompatible-host eval to reward stopping before local install commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tighten exact local SDK eval Measure project-level SDK resolution and exact roll-forward behavior using global.json artifact assertions so the scenario distinguishes PATH-based local installs from SDK paths setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address install script review feedback Use the documented lowercase dotnet-install quality value and call out both bash and PowerShell official install script URLs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Increase team script eval timeout Give the team install script scenario the same realistic long-running timeout as other scenarios that install preview SDK bits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Windows local SDK check Document the PowerShell equivalent for detecting an existing local SDK and give exact SDK installs enough time to complete in eval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stabilize team script eval Focus the team script scenario on script/config generation, create reproducible setup guidance before long downloads, and avoid brittle transcript assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make mktemp usage portable Use BSD/macOS-compatible mktemp templates in setup-local-sdk bash snippets and generated team install script. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Jan Jones
|
3aefa0f10a |
Update file-based app skill to mention features from 10.0.300 (#602)
Co-authored-by: Copilot <copilot@github.com> |
||
|
Aditya Mandaleeka
|
9433a31e67 | Initial port of evals to Vally (#615) | ||
|
Dan Moseley
|
042a9d2b2c |
Reorganize plugins into domain-specific groupings (#274)
* Reorganize plugins per agreed structure (#271) Split the dotnet plugin into domain-specific plugins per discussion in #271: - dotnet: narrowed to common everyday C#/.NET coding tasks - dotnet-diag: perf investigations, debugging, incident analysis - dotnet-data: data access and Entity Framework - dotnet-aspnet, dotnet-wpf, dotnet-winforms, dotnet-maui: scaffolded empty to convey intent for future framework-specific skills Moved skills, tests, and CODEOWNERS entries accordingly. No file content was changed; all skill/test moves are pure renames. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve dotnet-diag and dotnet-winforms descriptions - dotnet-diag: add '.NET' qualifier to clarify scope - dotnet-winforms: use official 'Windows Forms (WinForms)' name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add .NET qualifier to dotnet-data description Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove empty plugins; list them in CONTRIBUTING.md as future options Remove the four empty placeholder plugins (dotnet-aspnet, dotnet-wpf, dotnet-winforms, dotnet-maui) that had only plugin.json and no skills. Add a 'Plugin organization' section to CONTRIBUTING.md that describes the domain-specific plugin groupings introduced in this PR and lists the four removed plugin names as reserved candidates for future contributions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Create dotnet-upgrade plugin with migration/upgrade skills Move migrate-nullable-references, thread-abort-migration, and dotnet-aot-compat from dotnet to the new dotnet-upgrade plugin. Update CODEOWNERS paths (same owners), CONTRIBUTING.md plugin table, and fix the thread-abort-migration SKILL.md link in CONTRIBUTING.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Larry Ewing
|
f7abee2099 |
Add nuget-trusted-publishing skill (#210)
* Add nuget-trusted-publishing skill Knowledge-driven skill for setting up NuGet trusted publishing (OIDC) on GitHub Actions repos, replacing long-lived API keys with short-lived tokens. Replaces #34 which could not be reopened after history rewrite. * Address review: move to plugins/tests layout, fix phase sequencing, add CODEOWNERS - Move skill from src/dotnet/skills/ to plugins/dotnet/skills/nuget-trusted-publishing/ - Move eval from src/dotnet/tests/ to tests/dotnet/nuget-trusted-publishing/ - Fix Phase 3/4 sequencing conflict: clarify that drafting the workflow is OK before policy confirmation, but publishing/removing secrets should wait - Add CODEOWNERS entry for nuget-trusted-publishing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Brian Robbins
|
f4d6d8eea3 |
Add networking investigation support to dotnet-trace-collect skill (#184)
* Add networking investigation support to dotnet-trace-collect skill - Add Networking Issues section with System.Net.* EventSource providers for modern .NET (Http, NameResolution, Security, Sockets) - Clarify .NET Framework needs only PerfView /ThreadTime for networking - Add Kestrel providers to Slow Requests section for ASP.NET Core - Use /Providers syntax for PerfView command-line examples - Update reference files with networking provider examples - Add two networking eval scenarios (HTTP 500s, timeouts) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix networking trace commands to explicitly specify --profile when using --providers When --providers is specified, it overrides the default profiles. Updated the dotnet-trace collect command to include --profile dotnet-common,dotnet-sampled-thread-time and the collect-linux command to include --profile dotnet-common,cpu-sampling,thread-time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix dotnet-trace reference to show correct default profiles The default profiles are dotnet-common and dotnet-sampled-thread-time, not CPU sampling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Andy Gocke
|
38da20aa3a |
Add skill for AOT-compatibility (#122)
* Add skill for AOT-compatibility Also adds a real-world project conversion between the before and after tests. * Update skills/dotnet-aot-compat/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve skill focus * Move skill into plugin directory and delete tests * Add references section * Improve dotnet-aot-compat skill and add eval tests - Expand SKILL.md with routing signals, severity markers, stop signals, triage step, batch-fix strategy, IJsonModel guidance, and sub-agent dispatch pattern - Extract polyfill code block to references/polyfills.md - Add eval scenario and test fixtures at tests/dotnet/dotnet-aot-compat/ following repo convention (moved from skills/ to tests/<plugin>/) - Eval validates at +25-47% improvement across multiple runs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve dotnet-aot-compat skill consistency - Warn against adding external package types to JsonSerializerContext (prevents the ResponseError compile-error trap) - Strengthen anti-exploration directive with concrete negative examples - Push sub-agent parallelization after 2 build-fix cycles Validated with 3-run eval (Opus 4.6): pass, rubric 4.4/5. Cross-judge rejudge (Sonnet 4.5, GPT-5.1-Codex, Gemini 3 Pro): all pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Steve Pfister <stpfiste@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Steve Pfister
|
3bbe47c66e |
Remove noisy tombstone eval scenarios and fix activation flag (#171)
- Add expect_activation: false to iOS crash log scenario (negative test) - Remove 3 script-runner scenarios that never activated the skill and created persistent CI noise (they tested PowerShell script correctness, not skill activation) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Brian Robbins
|
2a9a48b982 |
Add dotnet-trace-collect skill for .NET performance diagnostics (#124)
* Add dotnet-trace-collect skill for .NET performance diagnostics Add a new skill that guides developers through capturing diagnostic artifacts for production .NET performance issues. Covers tool selection, data collection commands, and analysis recommendations across Windows, Linux, containers, and Kubernetes for both modern .NET and .NET Framework. Includes eval.yaml with 12 test scenarios covering high CPU, memory leaks, excessive GC, hangs, slow requests, native call stacks, Windows containers, long-running repros, and environment-specific tool trade-offs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove overly broad eval assertion. * Add workload context check to Step 1 Ask whether the agent is running in the context of the workload. If so, run diagnostic commands directly; otherwise provide guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prefer console tools over dotnet-monitor in K8s when in workload context When running in the context of the workload with console access, prefer dotnet-trace, dotnet-trace collect-linux, and dotnet-dump over dotnet-monitor to avoid authentication setup. Fall back to dotnet-monitor when console access is not available or it is already deployed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update tool reference: prefer console tools over dotnet-monitor in workload context Update dotnet-counters container comment and dotnet-monitor trade-offs to note that console tools are preferred when running in the workload context with console access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Delegate dump collection to dump-collect skill This skill focuses on trace collection. All dump collection guidance and execution is now delegated to the dump-collect skill. References to dotnet-dump, procdump, createdump, gcore, and Task Manager for dump collection are replaced with delegation to dump-collect. Dump analysis rows removed from Step 4 analysis table since dump artifacts are handled by the other skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dotnet-dump section and dump references from tool reference The entire dotnet-dump section is removed since dump collection is delegated to the dump-collect skill. Also removed dump/gcdump endpoints from dotnet-monitor REST API examples (replaced gcdump with GC trace endpoint), and cleaned up remaining procdump/dotnet-dump references in PerfView and dotnet-monitor trade-offs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update hangs guidance: start with a trace before a dump Reorder the hangs workflow to capture a trace first. A trace can reveal livelocks, thread starvation, or partial forward progress. Only escalate to a dump if the trace does not explain the hang, which indicates a potential true deadlock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix dotnet-trace collect profile: use dotnet-sampled-thread-time cpu-sampling is only available for collect-linux. Replace with dotnet-sampled-thread-time in the dotnet-trace collect example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename dotnet-trace section header to dotnet-trace collect Clarify the section heading to distinguish dotnet-trace collect from dotnet-trace collect-linux, which has its own dedicated section. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Split tool-compatibility.md into per-tool reference files Split the monolithic tool-compatibility.md (603 lines) into 6 individual reference files, one per tool: - dotnet-counters.md (47 lines) - dotnet-trace-collect.md (66 lines) - dotnet-trace-collect-linux.md (76 lines) - dotnet-monitor.md (93 lines) - perfview.md (125 lines) - perfcollect.md (177 lines) This reduces context usage since the skill only needs to load 1-2 tool references per scenario (50-250 lines) instead of all 603 lines. The tool selection logic remains entirely in SKILL.md; references are only needed after the tool is chosen. Updated SKILL.md to replace 3 generic tool-compatibility.md links with an environment-to-reference-file lookup table. Follows the same pattern used by the dump-collect skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add collect-linux limitations: symbol resolution and cross-namespace Document two additional limitations for dotnet-trace collect-linux: - Native symbols must be on disk during capture for resolution - Cannot trace cross-namespace processes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add aka.ms direct download links for dotnet-trace Add curl/wget download option using aka.ms links for Linux x64, Arm64, musl x64, and Windows x64. This avoids the need for a .NET SDK, which is useful in container environments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dotnet-trace report from analysis tools table Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update evals to cover recent skill changes Updated existing scenarios: - Hang diagnosis: expect trace-first (livelock/starvation check) before escalating to dump for true deadlocks - Memory leak (Linux + .NET Framework): expect delegation to dump-collect skill instead of direct dotnet-dump commands Added new scenarios: - K8s with console access: prefer console tools over dotnet-monitor - Container without SDK: aka.ms direct download for dotnet-trace - collect-linux cross-namespace limitation awareness Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove dotnet-counters from skill dotnet-counters was causing confusion where the skill would recommend dotnet-counters but download dotnet-trace via aka.ms links. Since traces provide richer diagnostic data, remove all dotnet-counters references and focus the skill on trace collection. - Remove dotnet-counters.md reference file - Remove dotnet-counters from lookup table, container tools list, live monitoring section, and analysis table in SKILL.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update K8s eval to prefer dotnet-trace over dotnet-monitor The skill recommends console-based tools (dotnet-trace) as the preferred approach when the user has console access to a container. Update the High CPU in Kubernetes scenario to match: expect dotnet-trace as primary, dotnet-monitor as alternative for no-console-access cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add missing context to eval prompts to prevent user prompting Several eval prompts were missing required inputs (deployment context, admin privileges, symptom, .NET version) that the skill needs to select the right tool. This caused the validator to hang waiting for clarifying questions that never come in eval mode. Changes: - Add 'running directly on a VM/server' to non-container scenarios - Add symptom to .NET 10 native stacks and container installation prompts - Add .NET version and admin access to slow requests prompt - Add admin access to PerfView triggers prompt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen dump-collect delegation language to 'invoke' not 'delegate' The skill was using passive 'delegate to dump-collect skill' phrasing which LLMs may interpret as merely mentioning the skill rather than actually invoking it. Changed all references to use 'invoke the dump-collect skill' with explicit 'do not provide dump commands directly' in key sections. Updated eval rubrics to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove mention of dotnet-monitor for Windows Container CPU eval The result should be completely focused on PerfView usage and not dotnet-monitor. * Clarify StopOn triggers must fire on interesting event, not recovery Strengthened the circular buffer + StopOn guidance to explain WHY triggering on recovery is wrong: the circular buffer continuously overwrites old data, so the interesting behavior may be lost by the time collection stops. Added 'Critical' callout and 'Never trigger on recovery' in the pitfalls table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Explain why console tools are preferred over dotnet-monitor When recommending console-based tools over dotnet-monitor in containers, explicitly state the reason: dotnet-monitor requires authentication configuration and sidecar deployment, making console tools easier to set up when you have direct access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add kernel requirement to cross-namespace eval rubric The judge was flagging the kernel >= 6.4 with CONFIG_USER_EVENTS=y detail as oddly specific, but it is accurate per the collect-linux reference documentation. Add rubric item so the judge recognizes this as correct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Scope dump-collect to modern .NET only, provide direct commands for .NET Framework The dump-collect skill only supports modern .NET (.NET Core 3.0+). Updated all .NET Framework references to provide dump commands directly (procdump -ma, Task Manager) instead of invoking dump-collect. Updated the Memory dumps header, decision matrix, .NET Framework sections, and the .NET Framework memory leak eval to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove .NET 10 collect-linux cross-namespace eval scenario Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove auth setup rubric from K8s console access eval Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move skill to standard directory structure and update CODEOWNERS Moved skill from src/dotnet/skills/ to plugins/dotnet/skills/ and tests from src/dotnet/tests/ to tests/dotnet/ to match the existing directory structure. Updated CODEOWNERS to add the new skill with @brianrob as owner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
Jeff Schwartz
|
ac12ae8a6a |
Move android-tombstone-symbolication skill and tests from src/dotnet to plugins/dotnet and tests/dotnet (#157)
Move the android-tombstone-symbolication skill from src/dotnet/skills/ to plugins/dotnet/skills/ and its tests from src/dotnet/tests/ to tests/dotnet/ to align with the standard plugin directory structure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Dan Moseley
|
63c6ee1eec |
Nullable reference migration skill (#39)
* Add nullable-reference-migration skill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: annotate based on design intent, not warnings Annotations should reflect the intended nullability contract of each member. Deciding intent first and then fixing warnings avoids the common mistake of over-annotating with ? or scattering ! to silence the compiler. Learned from dotnet/runtime nullable annotation guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: ?. (null-conditional) changes behavior unlike ? and ! During nullable migration it is tempting to change obj.Method() to obj?.Method() to silence a warning. Unlike ? on a type or ! on an expression, ?. changes the generated IL and silently skips the call when the value is null, which can introduce subtle regressions. Learned from dotnet/runtime PR #57418, #65262, #67198 review comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: adding ? to value types changes the actual type For reference types, ? is a metadata-only annotation. For value types (enums, structs, int, etc.), ? wraps the type in Nullable<T>, which changes the method signature and binary layout. This was caught as a breaking change in dotnet/runtime PR #65262 when LogLevel was accidentally made nullable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: Debug.Assert as compiler hint and reducing ! usage Debug.Assert(x != null) informs the nullable flow analyzer just like an if-check, eliminating the need for ! operators in the subsequent scope. Also adds guidance that if every call site of an internal method needs !, the parameter should probably be nullable instead. Learned from dotnet/runtime PR #67198 and PR #67198 review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enhance CS8618 guidance with [MemberNotNull] for helper methods When a helper method called from the constructor initializes fields, [MemberNotNull(nameof(field))] tells the compiler those fields are guaranteed non-null after the call, eliminating CS8618 warnings without resorting to ! or making fields nullable. Learned from dotnet/runtime PR #67198 where this pattern eliminated many suppression operators. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for struct default fields and post-Dispose state Struct reference-type fields are null in default(T). Annotate based on whether default is valid usage: nullable if yes, non-nullable if not. Also adds guidance that post-Dispose nullability should not weaken annotations, since post-Dispose use is a contract violation. Learned from dotnet/runtime PR #528 (struct fields), nullability.md (InterfaceMapping vs CancellationToken), and PR #65262 (Dispose). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: prioritize public API annotations over internal code Public and protected APIs define the contract consumers depend on and should be annotated first and most carefully. Internal code can tolerate ! more liberally since it does not affect callers. Learned from dotnet/runtime PR #67198 review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: do not remove ArgumentNullException checks Non-nullable parameters still need runtime null validation because callers may not have NRT enabled (other languages, older C#, suppressed warnings). Removing existing checks during migration can introduce regressions for those callers. Learned from dotnet/runtime nullability.md guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enhance Try method annotation patterns with specific recipes Clarifies the distinction between generic and non-generic Try method out parameters: use [NotNullWhen(true)] out T? for non-generic, and [MaybeNullWhen(false)] out T for generic. Also notes that Equals(object?) overrides should use [NotNullWhen(true)]. Learned from dotnet/runtime nullability.md DO guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for override covariance and IEquatable<T?> Override return types can be stricter (non-nullable) than the base even if the base returns T?. Also adds that reference types should implement IEquatable<T?> and IComparable<T?> since callers commonly pass null to Equals. Learned from dotnet/runtime PR #67198 and nullability.md guidelines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add three-pass code review checklist for nullable changes Nullable PRs need broader review than a typical diff: (1) verify no behavior/IL changes, (2) review explicit annotation changes for correct contracts, (3) scan unchanged APIs whose implicit nullability changed when the nullable context was enabled. Learned from dotnet/runtime nullability.md Code Review Guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add link to dotnet/runtime nullable annotation guidelines This is the authoritative internal document used when annotating the .NET libraries and contains detailed guidance on gray areas, virtual methods, struct defaults, and attribute usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen null validation pitfall: NRTs are compile-time only Nullable annotations do not prevent null at runtime. Callers using older C#, other .NET languages, reflection, dynamic, or the ! operator can still pass null. Emphasizes that runtime validation on public APIs remains essential for correctness and security. Learned from community blog posts (Maarten Balliauw, JetBrains) and dotnet/runtime nullability.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Define NRT abbreviation at first use in body text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Note that [MaybeNull] should be rare — prefer T? when possible The .NET runtime codebase has only ~7 uses of [MaybeNull]. In most cases T? is a better choice. Reserve [MaybeNull] for edge cases like AsyncLocal<T>.Value where T? would incorrectly allow setting to null when T is non-nullable. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance to use [NotNullWhen(true)] on Equals overrides When Equals(object? obj) returns true, the argument is guaranteed non-null. Annotating with [NotNullWhen(true)] lets callers skip redundant null checks after equality tests. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for widely-overridden virtual return types For virtual/abstract methods with many overrides, check whether existing overrides return null in practice. The .NET runtime annotated Object.ToString() as string? (overrides commonly return null) but Exception.Message as string (null overrides vanishingly rare). When in doubt, prefer T?. Source: dotnet/runtime nullability guidelines (ToString case study) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prefer multiple [MemberNotNull] attributes over params overload The params string[] overload of MemberNotNull is not CLS-compliant and may require #pragma warning disable. Use separate attributes for each field instead. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Warn that compiler does not flag misapplied nullable attributes Attributes like [DisallowNull] on a non-nullable parameter or [MaybeNull] on a by-value input parameter are silently ignored. Even Roslyn team members get tripped up by this. Source: dotnet/roslyn#36073 (jcouv, RikkiGibson) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Warn about false safety from nullable-oblivious libraries Unannotated dependencies produce no nullable warnings at all, giving a false sense of safety. Return values from oblivious APIs should be treated as potentially null. Source: dotnet/roslyn#44063 (jcouv), dotnet/roslyn#38941 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify Try method annotation: generic vs non-generic out params Non-generic out params: declare nullable with [NotNullWhen(true)]. Generic out params: keep non-nullable with [MaybeNullWhen(false)]. Using T? on generic params would change value-type signatures to Nullable<T>. Source: dotnet/runtime nullability guidelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance for unconstrained generics and LINQ null filtering For generic methods returning default on unconstrained T, use [return: MaybeNull] T instead of T? to avoid changing value-type signatures to Nullable<T>. LINQ Where(x => x != null) does not narrow T? to T. Use a WhereNotNull() helper (as Roslyn itself does) or OfType<T>(). Source: dotnet/roslyn#30953 (jcouv, roji), dotnet/roslyn#39586 (sharwell) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pitfall: var is always considered nullable by the compiler The compiler treats var as nullable regardless of the assigned expression — flow analysis determines the actual null-state. This can surprise developers who expect var to inherit non-nullability from the right-hand side. Learned from Maarten Balliauw's nullable migration blog series. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add strategy guidance: start at the center, work outward Annotating core domain models, DTOs, and shared utilities first eliminates cascading warnings across the codebase and gives the biggest return on effort. Applies to all three rollout strategies. Learned from Maarten Balliauw's JetBrains 2025 talk and blog series. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add dedicated EF Core section with schema migration warning EF Core infers column nullability from NRT annotations. Enabling NRTs can silently generate migrations that make previously nullable columns required, risking data loss. Consolidates all EF Core guidance (navigation properties, schema impact, migration review) into a dedicated section and removes the scattered EF references. Learned from Maarten Balliauw's JetBrains 2025 talk, Damir Arh's blog, and EF Core documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: warn that #nullable disable warnings is insufficient for entity files #nullable disable warnings only suppresses compiler diagnostics but leaves the annotation context active. EF Core reads annotations via reflection, so properties without ? are still treated as required, potentially altering database schema. Use #nullable disable to fully opt entity files out. Source: dotnet/efcore#33245 (confirmed by EF team member roji). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: add DbSet<T> initialization guidance DbSet properties should remain non-nullable since EF Core always initializes them. EF Core 7.0+ suppresses CS8618 automatically; older versions can use = null! or expression body => Set<T>(). Source: official EF Core NRT docs page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: expand navigation property guidance with three approaches Replace single-sentence guidance with the three patterns from the official EF Core NRT docs: (a) non-nullable with null!, (b) nullable when code checks load state, (c) non-nullable wrapping nullable backing field that throws. Also note collection navigations must always be non-nullable (empty, never null). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * EF Core: add LINQ query guidance for optional navigations EF Core translates LINQ to SQL so null navigations don't throw at runtime, but the compiler still warns. Use ! in Where/Include expressions to suppress false positives. From official EF Core docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: MVC treats non-nullable properties as [Required] When NRTs are enabled, ASP.NET Core MVC/Web API implicitly adds [Required(AllowEmptyStrings = true)] to every non-nullable reference type property in DTOs and view models. This can cause previously accepted requests to return 400 Bad Request. Controllable via SuppressImplicitRequiredAttributeForNonNullableReferenceTypes. Source: official ASP.NET Core model validation docs, aspnetcore#22656. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: System.Text.Json RespectNullableAnnotations and limits STJ (.NET 9+) can enforce NRT annotations at serialization time via RespectNullableAnnotations, but cannot enforce collection element nullability (List<string> vs List<string?> are indistinguishable in IL), top-level types, or generic properties. Document the opt-in and its limitations. Source: official STJ nullable annotations docs, aspnetcore#57234 (confirmed by eiriktsarpalis and terrajobst). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ASP.NET Core: warn about #nullable disable warnings on model files Same pitfall as EF Core: #nullable disable warnings only suppresses compiler diagnostics but leaves annotations active. MVC reads them via reflection to infer [Required], so model files not yet migrated must use #nullable disable to fully opt out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Expand rollout strategy guidance and add suppression analysis use case Add 'analyzing existing suppressions' to When to Use section. Expand the start-at-center strategy with guidance to work outward through layers and keep PRs scoped per project or layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add helper extension methods section with WhereNotNull Add a Helper Extension Methods section with WhereNotNull for both reference and value types. This is the pattern used by the Roslyn compiler codebase to filter nulls with correct type narrowing, which .Where(x => x != null) cannot provide. Update the inline reference in Step 3 to link to the new section. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add incremental build checkpoints to NRT migration workflow Add explicit build checkpoint instructions after Steps 2-6 so the AI builds and checks warning counts after each phase rather than attempting all fixes blind. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add commit strategy guidance to NRT migration workflow Tell the AI when to commit during the migration: at each logical boundary (enable, dereference fixes, annotations, attributes, cleanup) to keep diffs reviewable and prevent lost work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add 'when to ask the user' guidance for annotation decisions Tell the AI to ask the user before changing public API contracts, deciding nullable vs required for ambiguous properties, or choosing between null checks and suppressions when design intent is unclear. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add inline anti-pattern callouts with warning markers Add prominent ❌ and ⚠️ callouts inline in Steps 3 and 4 where dangerous mistakes are most likely: using ?. to silence warnings, sprinkling !, adding ? to value types, letting warnings drive annotations, and removing existing null argument validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add decision flowchart for resolving nullable warnings Add a structured decision tree in Step 3 that guides the AI through each warning: is null valid by design? Can you prove non-null? This prevents defaulting to ! or over-annotating with ?. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add scope-based prioritization guidance to Step 3 Tell the AI to work in dependency order (core models first, then consumers) and prioritize public/protected members within each file to minimize cascading warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add verify-before-claiming guidance to validation step Tell the AI to verify annotations match design intent, check for accidental ?. operators, confirm no removed null checks, and audit ! usage before reporting the migration as complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add conditional behavior guidance based on project type Tell the AI to tailor its approach based on whether the project is a library (public API contracts first), application (boundaries first), or test project (more liberal use of !). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Flag public API methods missing runtime null validation Tell the AI to check for missing ArgumentNullException guards on non-nullable public/protected parameters during annotation. Without runtime checks, null passed at runtime causes NRE instead of ANE. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Purpose paragraph to match recommended SKILL.md structure Add a one-paragraph Purpose description under the top-level heading, describing the outcome of using this skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Couple notes * Add missing reference links to More Info section Add links to Nullable reference types overview, C# language reference page, and null-forgiving operator reference. These are foundational docs referenced throughout the skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add breaking changes section for library NRT annotations Warn that incorrect NRT annotations are source-breaking changes for library consumers. Covers implicit non-nullability, parameters that should be nullable, return types, and versioning guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add guidance: methods tolerating null should accept nullable params If a method handles null gracefully (returns null, returns default, returns a failure result) rather than throwing, the parameter should be T?, not T. Based on BCL conventions and NodaTime design discussions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make build/test references build-system agnostic Not all repos use dotnet build/test directly — some use build scripts, msbuild, cake, nuke, etc. Add build/test command as optional inputs, update Step 1 to detect the build system, and replace hardcoded 'dotnet build'/'dotnet test' with generic 'build'/'test' in checkpoints and validation checklist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove cake/nuke from build script examples These are uncommon enough to not warrant explicit mention. Keep the list focused on what an agent would actually encounter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Recommend WarningsAsErrors in project file instead of CLI flag <WarningsAsErrors>nullable</WarningsAsErrors> in the .csproj or Directory.Build.props permanently prevents nullable regressions, which is better than relying on a CLI flag that CI might miss. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Track and document breaking changes for public libraries When annotating a public library, the AI should create a nullable-breaking-changes.md listing all public API changes that could affect consumers (return types narrowed/widened, parameters changed, value type nullability, new guards). This file serves as a review artifact and basis for release notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add trigger phrases to skill description for discovery Add CS8602, CS8618, CS86xx, #nullable enable, annotating APIs, and null-forgiving operator keywords so agents discover this skill for a wider range of nullable-related user requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move framework-specific sections to references/ for progressive disclosure Move EF Core, ASP.NET Core, breaking changes, and helper extension method content into references/ files. SKILL.md retains one-line summaries with links so the agent loads detailed content only when relevant. Reduces SKILL.md from 218 to 194 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add NRT readiness scanner script Add scripts/Scan-NullableReadiness.ps1, a PowerShell Core script that scans a project or solution and reports NRT migration status: project settings (Nullable, LangVersion, TargetFramework, WarningsAsErrors) and source-level counts (#nullable directives, ! operators, #pragma CS86xx suppressions). Supports -Path, -Json, and -Recurse parameters. Update SKILL.md Steps 1 and 6 to reference the script as an optional automation step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix strict mode .Count errors in Scan-NullableReadiness.ps1 Wrap pipeline results in @() before accessing .Count to avoid 'property Count cannot be found' errors under Set-StrictMode -Version Latest when a pipeline returns a single object instead of an array. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add automated tests for Scan-NullableReadiness.ps1 Test script runs the scanner against three fixture projects: - nrt-disabled: no <Nullable>, has ! operators and #pragma suppressions - nrt-enabled: <Nullable>enable, clean code - nrt-partial: <Nullable>enable but files with #nullable disable and ! Validates JSON output matches expected NRT readiness state (19 assertions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Recommend clean builds for validation checkpoints Incremental builds only recompile changed files, hiding nullable warnings in untouched files. All build checkpoints should use clean builds (e.g., --no-incremental) to surface all warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move nullable-reference-migration to src/dotnet/ layout Align with upstream repo restructuring that moved skills and tests under src/dotnet/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve ! operator counting by stripping comments and strings The scanner now removes block comments, line comments, verbatim strings, interpolated strings, and regular strings before counting null-forgiving operators. This eliminates false positives from ! in prose like 'Important!' or 'Value required!'. Tests updated with exact assertions and fixtures enriched with ! in comments, XML docs, and string literals to verify filtering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix misleading 'var is always nullable' wording var does not inherently introduce nullability — the inferred type carries nullability from the right-hand side expression. Updated the pitfall description to reflect this accurately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make STJ RespectNullableAnnotations limitations more explicit Clarify that the feature is opt-in, affects both serialization and deserialization, and explicitly list what it does not cover: collection elements, dictionary values, top-level types, and generic type parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add caveat that Debug.Assert is stripped from Release builds Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify that required does not enforce during deserialization The required keyword is a compile-time and object-initializer constraint. It does not replace runtime validation unless the serializer explicitly supports it (e.g., STJ with [JsonRequired]). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add explicit version callouts for required, DbSet, JsonRequired Consistently tag version requirements so the skill adapts across repos targeting different .NET versions: - required (C# 11+) on all mentions - EF Core 7.0+ (.NET 7) for DbSet CS8618 suppression - [JsonRequired] (.NET 7+) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add DTO vs domain model nullability guidance DTOs should be nullable by default (they cross trust boundaries), domain models should prefer non-nullable invariants. This distinction is where migrations most often go wrong. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add gray area heuristics for parameter nullability decisions From dotnet/runtime nullability guidelines: when a parameter is neither validated nor documented for null, check usage patterns in the codebase, whether null acts as a default/nop, and whether similar methods accept null. When in doubt, prefer nullable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Never infer nullability intent from usage frequency or naming Explicit guard against AI hallucination: if intent is not clear from code or documentation, ask the user rather than guessing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen serialization bypass warning in pitfalls table Reworded to anchor the rule: serializers bypass constructors, non-nullable properties on DTOs do not imply runtime safety. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stop and ask user if lang version or TFM is insufficient for NRTs If the project targets C# 7.x or a framework that defaults below C# 8.0, explicitly inform the user what needs to change and ask whether to update or abort. Do not silently proceed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add = null! guidance for late-initialized fields in NRT skill Add recommended technique for CS8618 warnings on fields set after construction: use = null! on the field declaration instead of adding ! at every use site. Document the pattern in the CS8618 table row, the annotation decision flowchart, and the Step 3 guidance section. Motivated by exercising the skill on System.Text.RegularExpressions, where = null! reduced ! operator count from 423 to 178 compared to the original 2019 NRT annotation PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Categorize ! operators as null!/default! vs assertions in scanner Break down the ! operator count into two categories: - null!/default! initializers (safe, expected for late-initialized fields) - assertions (value! patterns that warrant scrutiny) This gives a more nuanced readiness assessment — a project with 30 null! and 5 assertions is in better shape than one with 35 assertions, even though the total is similar. Update fixtures with null!/default! patterns and add 6 new test assertions (25 total, up from 19). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add pre-migration effort estimate for CS8618 warnings Count uninitialised reference-type fields and auto-properties as a rough predictor of CS8618 warnings before NRT is enabled. Excludes value types, events, const, and 'required' properties. Shown only when NRT is not yet enabled, labeled as approximate. In the corefx regex migration, CS8618 was 47% of all warnings (236 of 502), making this the single most useful pre-migration signal. Adds 3 test assertions (28 total). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add per-file ! breakdown in scanner output Show null!/default! vs assertion counts for each flagged file in both human-readable and JSON output. This helps reviewers quickly see which files have mostly safe initializers vs genuine assertions that warrant scrutiny. Example: 'RegexCompiler.cs: 31 ! (24 null!/default!, 7 assertions)' tells the reviewer only 7 uses need attention, not 31. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Show per-file uninit fields pre-migration, suppress post-migration Flag files with >5 uninitialised reference-type fields in 'Files needing attention' so the pre-migration report shows where the most CS8618 work will be. Suppress this metric when NRT is already enabled since it's no longer actionable. Also skip empty lines in per-file output when a file only triggered the uninit threshold but NRT is enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Context-aware section headings in scanner output Replace generic 'Files needing attention' with context-dependent labels: - Pre-migration (NRT not enabled): 'Migration work needed' - Post-migration with remaining directives: 'Remaining cleanup' - Post-migration clean: 'Suppression audit (Step 6)' — frames ! operators as review candidates, not problems Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove Step 6 reference from scanner output to avoid sync issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add before/after example output to scanner script header Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Advocate RespectNullableAnnotations as best practice for .NET 9+ Reframe guidance from neutral opt-in to prescriptive best practice, add motivation for why it matters, and recommend companion setting RespectRequiredConstructorParameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add zero-IL-change principle as hard rule for NRT migration Add prominent top-level principle: NRT migration must never change runtime behavior or generated IL. Missing null guards should be flagged as TODO comments (with user consent), not added inline. Update the existing null validation guidance to align. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add TFM caveat to Debug.Assert null-state guidance Debug.Assert acts as a nullable flow-analysis hint only on .NET Core 3.0+ where it has [DoesNotReturnIf(false)]. On .NET Framework/netstandard2.0 it lacks this annotation and does not affect null-state analysis. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert Debug.Assert TFM caveat The skill targets .NET Core 3.0+ where Debug.Assert has [DoesNotReturnIf(false)]. No need to document .NET Framework behavior since we don't encourage NRTs on older TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add return-type semantic nullability guidance to NRT skill Round-trip experiment on Dapper (51 files, 984 warnings) revealed the agent systematically removed ? from 40 public return types (OrDefault, ExecuteScalar) to satisfy the compiler, but those methods can return null by design. Added explicit guidance that return types must reflect semantic nullability, not just compiler satisfaction, with specific patterns to watch for. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strip string literals before comments in scanner Reorder the regex passes so strings are removed before comments. A string like "http://..." contains // that was previously mis-parsed as a line comment, corrupting downstream text. Add a URL-in-string test case to the nrt-enabled fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add NRT skill guidance from real-world evaluation Tested the skill against NimblePros/eShopOnWeb (ASP.NET Core + EF Core). Stripped NRT annotations from 44 files, re-annotated using the skill, and compared against the originals. Found 3 areas for improvement: ef-core.md: - Add guidance to always pair #pragma warning disable CS8618 with #pragma warning restore CS8618 to prevent suppression leakage aspnet-core.md: - Add Razor Pages [BindProperty] initialization guidance - Add collection property default initialization guidance - Call out the ?.+! anti-pattern (contradictory null handling) SKILL.md: - Add guard clause library awareness (Ardalis.GuardClauses, Dawn.Guard use [NotNull] attributes that narrow null state — avoid redundant !) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename skill to migrate-nullable-references, wire scanner tests into eval.yaml, merge main - Rename nullable-reference-migration -> migrate-nullable-references per CONTRIBUTING.md naming convention (lead with action verb) - Update YAML front-matter name field - Update scanner script and test script references to new directory name - Add scanner script eval scenario to eval.yaml using test fixtures - Merge latest main for CI/eval infrastructure updates Fixes #38 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove en-us locale from learn.microsoft.com URLs in NRT skill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove scanner eval scenario (script tests stay in Test-ScanNullableReadiness.ps1) The scanner script scenario doesn't work as an eval scenario because the baseline run has no access to skill resources (source: files aren't resolved when skill=null). Scanner tests are validated separately via Test-ScanNullableReadiness.ps1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve eval rubric for correctness and fix code formatting Replace generic throughput rubric with 5 correctness-focused criteria: - Value-type avoidance (int, bool, DateTime) - Nullable return types for FirstOrDefault methods - Late-init field handling (_header in CsvExporter) - Interface/implementation consistency - Zero-warning build Also fix C# code formatting in setup files (namespace/class declarations were collapsed onto single lines). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve migrate-nullable-references skill routing and deduplication - Add structured USE FOR / DO NOT USE FOR / INVOKES to description - Add multi-project solution migration order note in Step 2 - Deduplicate serialization/DTO pitfall (cross-ref Step 4) - Shorten redundant ArgumentNullException pitfall row Multi-model validation: Sonnet 4.8/5, GPT-5.1 4.6/5, Haiku 4.8/5 (avg 4.73) No regressions detected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add minimal API parameter optionality guidance per halter73 review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix contradiction in CS86xx fix guidance per eerhardt review The warning fix table recommended adding null checks/guards, contradicting the zero-runtime-behavior-change rule in line 35. Reworded to prioritize annotation-only fixes (? and !) and note that null checks change behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skill: prefer [NotNullWhen(true)] over ! at call sites When a boolean-returning helper guarantees a nullable parameter is non-null, annotating the parameter with [NotNullWhen(true)] is better than sprinkling ! at every call site. This is metadata-only (no behavior change) and gives the compiler real flow information. Based on feedback from reviewing skill-guided NRT migration output on dotnet/roslyn#58107 — the agent used ! without comment instead of annotating the helper method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove WhereNotNull helper extensions; refine skill scope - Delete helper-extensions.md (WhereNotNull was rejected as a BCL API and is only used internally in Roslyn) - Update LINQ guidance to recommend OfType<T>() instead - Add 'fixing a handful of warnings' to DO NOT USE FOR in description Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename Scan-NullableReadiness to Get-NullableReadiness per convention Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ASP.NET Core + EF Core eval scenario for NRT migration Exercises EF Core navigation property nullability, DbSet handling, and ASP.NET model validation behavior changes when enabling NRTs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add file_not_contains assertion and file-by-file eval scenario Add file_not_contains assertion type to skill-validator, mirroring file_contains but asserting the value is absent. Includes 3 tests. Add eval scenario testing file-by-file NRT migration: agent must modify only the targeted file while leaving others unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix YAML frontmatter: use block scalar for description with colons The colons in USE FOR:/DO NOT USE FOR:/INVOKES: caused YAML parse failures in skill-validator. Use > block scalar syntax per lewing review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Strengthen CS8603 return-type guidance and add return null! anti-pattern Clarify that non-nullable return type is a promise to callers. Add explicit warning against return null!/default! pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract nullable attributes table to optional reference Move the System.Diagnostics.CodeAnalysis attributes table from Step 5 inline content to references/nullable-attributes.md, reducing the core skill by ~500 tokens. The inline step retains a summary listing the key attribute names so the agent knows what's available and when to load the reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tweak ASP.NET/EF eval scenario prompt wording Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move skill to plugins/tests layout, add CODEOWNERS for migrate-nullable-references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: remove spoon-fed build commands, refocus rubric on skill value - Remove explicit 'Build with:' lines from eval prompts so the skill's Step 1 (discover build command) is actually exercised - Reword file-by-file rubric to test the skill's migration strategy rather than general instruction compliance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Larry Ewing <lewing@microsoft.com> |
||
|
Mark Miller
|
3aa3ddbf1c | Skill for debugging .NET Framework runtime activation issues using the activation logs added in the in-proc SxS days. (#152) | ||
|
Cameron Aavik
|
4d11e331dd |
Add microbenchmarking skill (#63)
* Add microbenchmarking skill * Add eval.yaml * Merge in latest changes + apply review comments * Use stronger wording for reference file loading * Remove DoNotParallelize * Move skill and tests to new location |
||
|
Viktor Hofer
|
4d484a18ce |
Move src/dotnet content into plugins/ and tests/ (#144)
Move skills from src/dotnet/skills/ to plugins/dotnet/skills/ and tests from src/dotnet/tests/ to tests/dotnet/, then remove the now-empty src/ directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
8023c59a07 |
Fix remaining relative fixture paths in analyzing-dotnet-performance eval
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
0d4396e1f9 |
Fix relative fixture path in analyzing-dotnet-performance eval
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Viktor Hofer
|
6e2ad53e88 | Move plugin tests out of plugins folder into a root tests/ folder |