skills/dotnet__skills
1
0
Fork 0
mirror of https://github.com/dotnet/skills.git synced 2026-09-20 09:49:54 +08:00
Code Issues Packages Projects Releases Wiki Activity
213 Commits 131 Branches 2 Tags
dev/jankrivanek/test-plugin
Commit Graph

96 Commits

Author SHA1 Message Date
Jan Krivanek 4a4306f563 Disable scheduled workflows on forks (#318) 
* Disable schedules on forks

* Reflect PR comments
 2026-03-11 10:17:01 +01:00
Viktor Hofer ea6f594295 Enable "evaluation-status" as a required status check (#320) 
* Enable evaluation as a required status check

- Remove path filters from evaluation.yml pull_request trigger so the
  workflow runs on every PR
- Add statuses: write permission and a report-status job that posts a
  commit status with context 'evaluation' for non-fork PRs
- Change evaluation-fork-pr.yml commit status context from
  'evaluation (fork PRs)' to 'evaluation' so both workflows share the
  same status context
- The 'evaluation' status can now be configured as a required check
  in the GitHub ruleset

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

* Address review feedback: handle discover failure, least-privilege perms

- Remove workflow-level statuses: write (job already declares it)
- Broaden report-status condition to handle discover failure/cancellation
  instead of leaving PRs stuck with no status
- Exclude fork PRs (discover skipped) to avoid conflicting with
  evaluation-fork-pr.yml

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

* Handle fork PRs in evaluation.yml for required status check

- Remove fork guard from discover so it runs for all PRs
- Add is_fork output to detect fork PRs
- Skip run-evaluation for fork PRs (no access to secrets)
- Post 'pending' status for fork PRs with skill changes, prompting
  the author to use /evaluate
- Post 'success' for fork PRs with no skill changes
- This ensures every PR gets an 'evaluation' commit status, making
  it viable as a required check without fork PRs getting stuck

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

* Add statuses: write at workflow level (required for job-level grant)

Job-level permissions cannot exceed the workflow-level permissions.
The report-status job needs statuses: write but was getting read-only
because the workflow level didn't include it.

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

* Use pull_request_target for fork PR status posting

Fork PRs need an 'evaluation' commit status but pull_request triggers
have restricted GITHUB_TOKEN permissions for forks. Use
pull_request_target in evaluation-fork-pr.yml instead, which runs in
the base repo context with full permissions.

The new fork-status job:
- Triggers on pull_request_target for fork PRs only
- Checks if the PR changes any skills (git diff only, no code execution)
- Posts 'pending' status if skills changed (needs /evaluate)
- Posts 'success' if no skills to evaluate

evaluation.yml reverts to the original fork guard on discover (skips
fork PRs entirely). report-status excludes skipped discover results
so it only posts for non-fork PRs.

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

* Address review: fix header comment, use shallow checkout

- Correct security model comment: pull_request_target runs from the
  base branch, not necessarily the default branch
- Use default shallow checkout (depth 1) instead of fetch-depth: 0
- Diff against HEAD instead of base SHA since checkout is the base

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

* Simplify evaluation.yml: use plain check-run job instead of commit status API

Replace report-status job (which called gh api to post commit statuses)
with a plain evaluation job whose check run can be required directly.
Removes the need for statuses: write permission in this workflow.

Fork PRs are handled by evaluation-fork-pr.yml which still uses commit
statuses (necessary because issue_comment check runs are not associated
with the PR head SHA).

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

* Fix fork-status: pin checkout to base SHA, use merge-base for diff

- Pin actions/checkout to the PR base SHA to avoid checking out the
  merge ref which includes untrusted fork content
- Use explicit base/head SHAs with merge-base for reliable diff,
  consistent with the non-fork workflow's discovery logic

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

* Rename evaluation job and context to evaluation-status

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

* Address review: least-privilege permissions, full clone for merge-base

- Add permissions: {} to evaluation-status job (evaluation.yml)
- Use fetch-depth: 0 for fork-status checkout so merge-base works
- Remove --depth=1 from PR head fetch
- Keep statuses: write at workflow level (needed by reusable workflow)

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

* Fix header comment: evaluation → evaluation-status

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-11 09:48:50 +01:00
Viktor Hofer 7be01578bd Rename status job to skill-validator-status 2026-03-10 23:59:45 +01:00
Viktor Hofer 8fecbba151 Rename status job to skill-validator-status (#322) 
* Rename status job to skill-validator-status

* Update .github/workflows/skill-validator.yml

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

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-03-10 23:58:23 +01:00
Viktor Hofer 7bd004ee9f Enable skill-validator as a required status check (#321) 
* Enable skill-validator as a required status check

- Remove path filters from pull_request trigger
- Add discover job to skip the expensive 5-platform build matrix when
  no skill-validator files changed
- Add report-status job that posts commit status 'skill-validator':
  - No relevant changes → success
  - Build passed → success
  - Build failed → failure
- Schedule and workflow_dispatch triggers unchanged (discover is
  skipped, build runs unconditionally)

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

* Handle fork PRs with pull_request_target for status posting

Fork PRs have restricted GITHUB_TOKEN and can't post commit statuses
via pull_request trigger. Add pull_request_target trigger with a
fork-status job that posts success for fork PRs (they should never
change skill-validator).

- Add pull_request_target trigger
- Gate build to skip for pull_request_target events
- Gate report-status to non-fork PRs only
- Add fork-status job for fork PRs (always posts success)

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

* Enable skill-validator as a required check

- Remove path filters from pull_request trigger so the workflow runs
  on every PR
- Add discover job to skip the expensive 5-platform build matrix when
  no skill-validator files changed
- Add status job that can be required in the GitHub ruleset: succeeds
  when no relevant changes or when the build passes, fails otherwise
- No commit status API calls or pull_request_target needed — just a
  plain job whose check run is required

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-10 23:48:34 +01:00
Viktor Hofer c735d84f9d Improve CODEOWNERS validation: require sufficient owners and enable as required check (#314) 
* Improve CODEOWNERS validation workflow

- Remove path filters from pull_request trigger so it can be a required check
- Add owner sufficiency validation: each skill/test folder must have either
  2+ individual owners or 1+ team in CODEOWNERS
- Update issue creation to report both missing entries and insufficient owners
- Update failure step to trigger on either validation failure

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

* Update CODEOWNERS to add additional code owners

Added @agocke and @hoyosjs as code owners for migrate-nullable-references and dotnet-trace-collect skills.

* Update CODEOWNERS for dotnet-trace-collect

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-10 21:16:41 +01:00
Dan Moseley 70ea8cf225 Include error details in evaluation failure PR comments (#297) 
* Include error details in evaluation failure PR comments

When evaluation fails due to validation errors (e.g., description too long),
the PR comment previously only said 'Evaluation failed. View workflow run',
requiring a click-through to discover the actual error.

Two fixes:
- Reporter.cs: GenerateMarkdownSummary() now renders an Errors section for
  verdicts with FailureKind set, so the marker comment shows what went wrong.
- evaluation-fork-pr.yml: The report-status job now downloads result artifacts
  and extracts error reasons from results.json, including them directly in the
  failure comment posted to the PR.

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

* TEST: inflate build-perf-diagnostics description to trigger validation error

Temporarily makes the description 1221 chars (limit is 1024) to verify that
the new error reporting shows the validation failure in the PR comment.

This commit should be reverted after verifying the error display.

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

* Tighten error filter to pre-eval failures and add tests

Address review feedback:
- Restrict the Errors section in markdown summary to verdicts with no
  scenarios (pre-eval failures like spec_conformance_failure), avoiding
  duplication with threshold/regression failures already shown in the table.
- Add three tests: errors section appears for pre-eval failures, is omitted
  when all verdicts pass, and is omitted for failures that have scenarios.

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

* Fix error details in evaluation-run.yml post-completion-comment

The failure comment was posted by evaluation-run.yml's post-completion-comment
job (shared by both same-repo and fork PR workflows), not just by the fork PR
workflow. Apply the same artifact download + error extraction to this job so
error details appear for same-repo PRs too.

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

* Promote eval prompt skill-name mention from warning to error

When an eval scenario prompt explicitly references the skill name,
it biases baseline runs and forces activation instead of testing
organic discovery. Move the check from warnings to errors so it
blocks evaluation via the existing profile.Errors fail-fast path.
No existing evals on main have this pattern.

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

* Sanitize @ in error output to prevent mention injection

Neutralize @ characters with a zero-width space in all three locations
where error text from PR-controlled content is rendered into PR comments:
Reporter.cs, evaluation-run.yml, and evaluation-fork-pr.yml.

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

* Distinguish errors vs warnings in evaluation failure comments

Pre-eval structural errors (spec conformance, missing eval) get a
red X icon, while eval verdict failures (below threshold, completion
regression) get a warning icon. This makes it clear which failures
are blocking issues vs expected score variance.

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

* Only show pre-eval errors in failure comment, link to results table

The failure comment no longer repeats verdict warnings (below
threshold, completion regression) that are already visible in the
results table. It only shows pre-eval structural errors (spec
conformance failures) and links to the results table for full details.

Three cases:
- Pre-eval errors: show them + link to results table
- No errors but failed (verdict): just link to results table
- No results table: fallback with workflow run link

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

* Remove duplicate failure comment from fork workflow

The reusable evaluation-run.yml already posts failure comments when
pr-number is provided. The fork workflow was duplicating this by
posting its own failure comment, resulting in two comments per failure.

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

* Revert "TEST: inflate build-perf-diagnostics description to trigger validation error"

This reverts commit 5f9056695a.

* Address review feedback: boundary-aware match, markdown escaping, safe find loop

- SkillProfiler: use boundary-aware regex for skill-name-in-prompt check
  to avoid false positives on short skill names (e.g. 'go')
- Reporter: wrap skill name in inline code and escape backticks/newlines
  to prevent markdown injection from PR-controlled content
- evaluation-run.yml: use find -print0 with while read loop to handle
  paths with spaces; match Reporter inline-code format in jq output

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

* Clean up comments in evaluation-fork-pr.yml

Removed redundant comments regarding failure comments for fork PRs.

* Refine comments on eval prompts and skill discovery

Clarify comment regarding eval prompts and skill discovery errors.

* Update eng/skill-validator/src/Services/Reporter.cs

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

* Update .github/workflows/evaluation-run.yml

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

* Reapply "TEST: inflate build-perf-diagnostics description to trigger validation error"

This reverts commit 331ed870c5.

* Fix test assertion to match updated markdown error format

The Reporter.GenerateMarkdownSummary method was updated in 95ede45 to wrap
both skill name and reason in a single backtick pair for better markdown
injection prevention, but the test assertion was not updated to match.

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

* Update eng/skill-validator/src/Services/Reporter.cs

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

* Update .github/workflows/evaluation-run.yml

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

* Link to newest results-table comment instead of oldest

The GitHub comments API returns oldest-first, so head -1 was linking
the failure comment to the oldest results-table marker comment rather
than the most recent run. Use tail -1 to select the newest match.

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

* Consolidate PR comment jobs into a single comment-on-pr job

Merge comment-on-pr and post-completion-comment into one job that always
posts a new comment with the full results summary. When no result
artifacts exist (evaluate skipped or failed entirely), a short failure
message is posted instead.

This removes the find-and-update-existing-comment pattern, the separate
error extraction via jq, the PR_COMMENT_MARKER, and the comment_action
workflow output.

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

* Guard consolidation step against missing validator binary

When result artifacts download succeeds but the validator binary
download/extract fails, the consolidate command would fail silently
(step is continue-on-error) and no PR comment would be posted. Check
that the validator executable exists before attempting consolidation,
falling through to the generic failure message otherwise.

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

* Skip results table header when there are no scenario rows

Only emit the markdown table when at least one scenario row exists,
so error-only summaries show just the heading and errors section
without an empty table.

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

* Display errors as bullet points in markdown summary

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

* Show validation errors with status emoji in heading

Use '### ❌ Skill validation errors:' heading for the errors section
to make the failure status immediately visible.

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

* Revert "Reapply "TEST: inflate build-perf-diagnostics description to trigger validation error""

This reverts commit 59d5d8dfd4.

* Update eng/skill-validator/tests/OverfittingJudgeTests.cs

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

* Update .github/workflows/evaluation-run.yml

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-03-10 12:51:12 +01:00
Jan Krivanek b93d6c7d15 Prevent rep health description duplication (#306) 2026-03-10 12:07:20 +01:00
Viktor Hofer b6bc33c926 Purge dashboard entries older than 14 days (#304) 
* Purge dashboard entries older than 14 days

When updating plugin JSON files, filter out Quality and Efficiency
entries whose date is older than 14 days. If all entries are purged,
delete the JSON file so it is excluded from the components.json
manifest and no longer appears in the dashboard.

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

* Move stale file cleanup to workflow level

Address review feedback: the delete-if-empty branch in the script was
unreachable because fresh entries are always appended before purging.
Instead, add a workflow step that scans all existing data files after
per-plugin generation and removes any whose entries are entirely older
than 14 days.

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

* Move stale file purge logic into the script

Add a -PurgeStaleFiles/-DataDir parameter set so the workflow calls
the script instead of duplicating the retention logic inline. The
retention window is defined once in the script.

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

* Make retention days configurable via workflow env var

Add -RetentionDays parameter to the script (mandatory for
-PurgeStaleFiles, optional for generate mode) and define
DASHBOARD_RETENTION_DAYS env var at the workflow level so the value
is controlled in one place.

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

* Address review feedback: fix comment and add entries guard

- Fix workflow comment to accurately describe the purge behavior
- Guard against missing 'entries' key in purge mode to avoid silently
  skipping malformed plugin JSON files

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

* Update eng/dashboard/generate-benchmark-data.ps1

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

* Update .github/workflows/evaluation.yml

Co-authored-by: Copilot <175728472+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>
 2026-03-10 11:00:19 +01:00
Gerald Versluis ebe71e2f06 Add dotnet-maui plugin with dotnet-maui-doctor skill (#199) 
* Add dotnet-maui-doctor skill and refs

Add a new .agents/skills/dotnet-maui-doctor skill bundle. Includes SKILL.md describing an autonomous workflow to detect and remediate .NET MAUI environment issues and platform-specific reference docs for installation commands, platform requirements, troubleshooting, Microsoft OpenJDK guidance, and a WorkloadDependencies discovery guide that pulls authoritative versions from NuGet. Documents validation/remediation steps for .NET SDK, workloads, JDK, Android SDK, Xcode, and Windows SDK (macOS/Windows/Linux) and includes a temporary recommendation to prefer Microsoft OpenJDK (JDK 21) until manifests are updated.

* MAUI doesn't quite yet support dotnet run

* Add dotnet-maui plugin and dotnet-maui-doctor skill

Introduce the dotnet-maui plugin and move the dotnet-maui-doctor skill under plugins/ layout. Update marketplace, CODEOWNERS, and README accordingly.

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

* Add comprehensive eval scenarios for dotnet-maui-doctor

10 scenarios covering: macOS/Linux/Windows setup, JDK vendor enforcement,
workload update/repair guardrails, dynamic version discovery, JAVA_HOME
misconception, Android SDK diagnosis, out-of-scope decline, and stale
workload remediation.

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

* Refine eval scenarios: remove weak scenarios, fix overfitting

Remove 'Resist hardcoding' (penalizes skill — baseline gives more useful
direct answer) and 'Decline runtime bug' (skill makes agent refuse to
help). Rework Android SDK scenario to test CI-oriented package discovery
where baseline lacks knowledge. Replace vocabulary-level NuGet/API
assertions with outcome-focused rubric items across all scenarios.

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

* Remove hardcoded JDK version workarounds in favor of dynamic discovery

Replace temporary JDK 21 workaround notes with guidance to use the
version from WorkloadDependencies.json. Replace hardcoded JDK paths
in troubleshooting.md with {VERSION} placeholders.

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

* Improve eval scenarios: fix JAVA_HOME timeout, macOS tie, reduce overfitting

- JAVA_HOME: rewrite prompt to request conceptual answer (prevents tool timeout)
- macOS Xcode: add installation source guidance, remove equal-penalty assertion
- Remove irrelevant workload update/repair assertions from 3 off-topic scenarios
- Keep workload assertions in 5 scenarios where workload management is the topic

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

* Reframe JAVA_HOME scenario as prevent-harmful-action pattern

Rewrite prompt from conceptual yes/no question (which the agent answered
from wrong general knowledge without activating the skill) to a scenario
where the user is about to apply incorrect Stack Overflow advice. This
mirrors the successful workload guardrail pattern.

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

* Reduce overfitting: rephrase assertions and rubric to be outcome-focused

- Replace literal '--version' assertions with broader 'version|pin' pattern
- Rephrase Xcode rubric from skill-specific source to version control risk
- Rephrase version discovery rubric from 'NuGet API' to outcome-focused
- Rephrase workload install rubric from '--version flag' to 'pinned version'

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

* Inline critical JAVA_HOME auto-detection guidance into SKILL.md

The JAVA_HOME eval scenario scored poorly because the agent couldn't read
the reference file (references/microsoft-openjdk.md) and fell back on
general knowledge, which incorrectly says JAVA_HOME must be set.

Changes:
- Task 5: Add inline JAVA_HOME guidance with decision table showing that
  JAVA_HOME is NOT required and MAUI auto-detects JDK installations
- Common Pitfalls: Add 'Unnecessary JAVA_HOME' entry with unset commands

This ensures the agent knows JAVA_HOME is unnecessary even when reference
files can't be loaded. File stays at 210 lines (limit: 500).

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

* Inline JAVA_HOME auto-detection knowledge into SKILL.md

Add JAVA_HOME guidance directly to Task 5, Task 9, and Common Pitfalls
so the agent knows JAVA_HOME is not required even when reference files
can't be read. Based on Redth's original microsoft-openjdk.md reference.

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

* Address code review feedback from Copilot

- Fix cmd.exe syntax in installation-commands.md (use PowerShell $env:)
- Replace hardcoded simulator device/runtime with placeholders
- Add System.IO.Compression.FileSystem assembly load for PS 5.1 compat
- Clarify workload reinstall example is macOS-specific (omit ios on Linux)
- Replace hardcoded android-35 with $API_LEVEL placeholder

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

* Address jonathanpeppers review feedback

- Add Apple Developer login/2FA note for Xcode downloads
- Fix Windows SDK detection: extract KitsRoot10 and version subkeys
  instead of dumping full registry (was outputting noise)
- Replace manual NuGet API approach with dotnet workload search version
  CLI command for discovering workload versions
- Reframe JAVA_HOME guidance: report as anomaly rather than prescribing
  unset/set commands (per jonathanpeppers recommendation)
- Use $env:ProgramFiles instead of hardcoded C:\Program Files paths
  in PowerShell commands
- Update SKILL.md to match: softer JAVA_HOME guidance, improved
  Windows SDK detection command

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

* Address Redth review feedback

- Remove Windows SDK detection commands — installed automatically
  via MAUI workload/VS Installer (per Redth recommendation)
- Use dotnet workload search version --format json --take 1 for
  workload version discovery instead of NuGet search API (per both
  Redth and jonathanpeppers)
- Update workload-dependencies-discovery.md Step 2 to match
- Expand Android SDK detection with known filesystem paths
  instead of relying solely on environment variables

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

* Align eval rubric with reviewer feedback to reduce overfitting

Replace technique-specific rubric items with outcome-focused criteria:
- Replace PowerShell API query check with dotnet CLI discovery check
- Replace manifest extraction check with workload-version-based discovery
- Replace prescriptive version discovery with CLI/docs verification
- Convert workload update/repair regex assertions to rubric criteria
- Remove Invoke-RestMethod assertion (no longer using API queries)

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

* Align JAVA_HOME guidance with reviewer consensus, add Xcode download expectations

- Consistently use ⚠️ anomaly approach for non-Microsoft JAVA_HOME across
  microsoft-openjdk.md and troubleshooting.md (matching SKILL.md)
- Remove contradictory Note in microsoft-openjdk.md that conflicted with table
- Reframe unset commands as user's choice, not automatic action
- Add Xcode download size (~12GB) and time expectations per Redth's feedback

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

* Update plugins/dotnet-maui/skills/dotnet-maui-doctor/references/workload-dependencies-discovery.md

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>

* Update JDK example to version 21 per jonathanpeppers feedback

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

---------

Co-authored-by: redth <jondick@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>
 2026-03-09 13:34:19 -07:00
Jan Krivanek d709e0b55a Add checking for orphaned skills/plugins (#291) 2026-03-09 14:23:07 +01:00
Jan Krivanek a9335d4116 Workaround gh-aw concurrency group issue (#290) 
* Workaround gh-aw concurrency group issue

* Clarify concurrency bug comment per code review

Rephrase comment to make clear the bug causes mutual cancellation,
not a 2-run limit. The 2-run cap is the workaround, not the bug.

Co-authored-by: Copilot <copilot@github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
 2026-03-09 11:55:18 +01:00
Viktor Hofer a3f424de82 Add missing plugins to marketplace.json (#287) 2026-03-09 09:55:25 +01:00
Viktor Hofer 1fa63e60e6 Move completion comment into shared evaluation-run workflow (#286) 
Move the 'Post completion comment' step from evaluation-fork-pr.yml into
evaluation-run.yml as a dedicated post-completion-comment job, so both
evaluation.yml and evaluation-fork-pr.yml share the same logic.

The fork-pr workflow retains a slim step that only handles the skipped case
(no entries / discover failed) since evaluation-run.yml isn't invoked then.

Also extract the '<!-- skill-validator-results -->' marker into a workflow-
level PR_COMMENT_MARKER env var to eliminate duplication.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-09 09:53:44 +01:00
Viktor Hofer 7884803830 Fix infra change detection to use merge-base instead of base tip (#285) 
The evaluation workflow was using a two-point diff (base_sha..head_sha) to
detect changed files, which includes files changed on main that the PR branch
hasn't merged yet. This caused false 'Infrastructure changes detected' triggers
when evaluation workflow files were modified on main after a PR branch diverged.

Switch to computing the merge-base first, so only changes actually introduced
by the PR are considered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-09 09:53:23 +01:00
Jan Krivanek dcdad225a9 Fix 429 rate limiting in evaluation pipeline (#284) 
* Fix 429 rate limiting in evaluation pipeline

- P0: Add retry with exponential backoff to ListModelsAsync() in ValidateCommand
  using the existing RetryHelper (3 retries, 2s base delay, 60s budget)
- P1: Reduce scheduled-run parallelism to match infra-change settings (2/3/3)
  to avoid overloading the API when all 5 plugins run in parallel
- P2: Assign Copilot tokens deterministically by matrix job index instead of
  randomly, eliminating ~96% token collision probability
- P3: Add retry with backoff to AgentRunner.RunAgent() for mid-eval 429s
  (2 retries, 5s base delay, scenario timeout + 60s budget)

Fixes dotnet/skills#168

* Delete docs/fix-429-rate-limiting.md
 2026-03-08 21:11:02 +01:00
Dan Moseley 8e50fc8558 Add agent and plugin spec conformance validation (#279) 
* Add agent and plugin spec conformance validation

Extend the validation pipeline to check .agent.md files and plugin.json
files alongside skills. Previously only SKILL.md files were validated.

Agent validation (AgentProfiler):
- Frontmatter required (error)
- Name and description required (error)
- Name format: same rules as skills (lowercase alphanum + hyphens, 64 max)
- Name must match filename minus .agent.md suffix
- Description max 1024 chars
- Body max 500 lines

Plugin validation (PluginValidator):
- Name required, must match directory name
- Version and description required
- Description max 1024 chars
- Skills path required and must exist
- Agents path warned if specified but missing

Discovery (SkillDiscovery):
- DiscoverAgents() walks up from skill paths to plugin roots,
  scans agents/ directories for .agent.md files
- DiscoverPlugins() finds plugin.json from skill paths

Integration (ValidateCommand):
- Runs after skill discovery, before evaluation
- Errors block with exit code 1 (same as spec_conformance_failure)

All 4 existing agents and 2 plugins pass cleanly.
25 new tests (13 agent, 12 plugin).

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

* Code review fixes: spec comments, ValidateNameFormat, kind-specific messages

- Refactor ValidateName into ValidateNameFormat (format-only) + ValidateName
  (format + directory match) so agent/plugin validation doesn't abuse the
  skill directory-match check by passing name twice.
- Warning messages now say 'Agent name' or 'Plugin name' instead of
  'Skill name' when called from AgentProfiler/PluginValidator.
- Add spec URL comments to AgentProfiler and PluginValidator referencing
  agentskills.io/specification, VS Code agent-plugins docs, and Claude Code
  plugins-reference.
- Add missing agent name edge-case tests: leading/trailing hyphen,
  consecutive hyphens, and message-prefix assertions.

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

* Remove unused agents path from plugins that have no agents

The dotnet, dotnet-data, and dotnet-upgrade plugins declared an agents
path but had no agents/ directory, producing validation warnings. Remove
the field; it can be added back when agents are actually created.

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

* Promote agent/plugin name validation from warnings to errors

The agentskills.io spec uses 'Must' for all name constraints (length,
character set, no leading/trailing hyphens, no consecutive hyphens).
For a publishing validator these should block, not just warn. Agent
filename mismatch is also promoted since the IDE won't discover the
agent if the name doesn't match.

Skill name validation stays as warnings (lenient loading, not part
of this PR).

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

* Address PR review feedback

- Fix FrontmatterRegex: remove RegexOptions.Multiline so a horizontal
  rule (---) in the body is not mistaken for frontmatter. Detection and
  stripping now both anchor to start of string.
- Use filename as fallback identifier in AgentProfile when agent.Name
  is empty, so error output shows [agent:foo.agent.md] not [agent:].
- Add TryGetSafeSubdirectory to PluginValidator: rejects absolute paths
  and parent-directory traversal in skills/agents path fields. Used by
  both PluginValidator and SkillDiscovery.DiscoverAgents.
- Use DirectoryName as fallback for plugin result name when Name is
  empty, so error output shows [plugin:my-dir] not [plugin:].
- Add tests for path traversal rejection and name fallback behavior.

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

* Detect orphaned test directories with no matching plugin/skill

Add FindOrphanedTestDirectories to SkillDiscovery: checks that every
directory under tests/{plugin}/{skill}/ has a corresponding directory
under plugins/{plugin}/skills/{skill}/. Plugin-level orphans (tests/
directory with no matching plugins/ directory) are also flagged.

Integrated into ValidateCommand after plugin/agent validation so it
runs on every validation pass. Errors are blocking (exit 1).

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

* Address PR #279 review feedback

- Use case-sensitive (Ordinal) comparison for agent filename matching
  since agent names are required to be lowercase by spec.
- Replace OrdinalIgnoreCase path-under-root check in TryGetSafeSubdirectory
  with Path.GetRelativePath + '..' prefix check for correct behavior on
  case-sensitive filesystems.
- Let ParsePluginJson throw JsonException on malformed JSON instead of
  silently returning null. ValidateCommand catches it as a blocking error.
- Add test for malformed plugin.json.

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

* Increase eval timeouts for dump-collect and dotnet-trace-collect to 120s

The with-skill runs for these skills were hitting the 60s scenario timeout
in CI, causing the agent to be killed before producing output. Other skills
in the same plugin already use 120s. The 30s decline/negative-test scenario
is left unchanged.

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

* Fix fixture paths in clr-activation-debugging and analyzing-dotnet-performance evals

The source paths referenced tests/dotnet/ but the fixtures live under
tests/dotnet-diag/. This caused file-not-found exceptions at runtime,
which set hasRejections and caused unconditional exit 1 (bypassing
--verdict-warn-only).

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

* Surface execution errors in validation summary

Previously, skills that threw exceptions (e.g. missing fixture files)
were silently excluded from the verdict count. The summary showed
'2/4 skills passed' with no indication that 2 other skills crashed
entirely. Now the summary shows '2/6 skills passed (2 rejected due
to execution errors)' and lists the specific error messages near
the summary rather than buried in the middle of the log.

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

* Reduce parallelism for infra-change evaluation runs

When infrastructure changes trigger evaluation of all plugins
simultaneously, the high default parallelism (5x5x5) causes API
rate limits (429s) and scenario timeouts from contention. Reduce
to 2x3x3 for infra-change runs only; regular single-plugin PR
evaluations keep the default 5x5x5.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-08 15:40:06 +01:00
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>
 2026-03-06 21:01:52 -08:00
Viktor Hofer 750f56313b Fix fork PR evaluation concurrency cancelling runs on unrelated comments (#272) 
* Fix fork PR evaluation concurrency cancelling runs on unrelated comments

The concurrency group used the PR number alone, so any comment on a PR
(not just /evaluate) would cancel an in-progress evaluation. Now only
/evaluate comments share a group; other comments get a unique group
keyed by comment ID so they never cancel real evaluation runs.

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

* remove push trigger from skill-validator.yml

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-06 20:21:30 +01:00
Viktor Hofer 1370b4bda3 Add completion comments and emoji cleanup to fork PR evaluation workflow (#270) 
- Remove eyes reaction from trigger comment when workflow finishes
- Post a status comment on completion (success, failure, or skipped)
- On success, link to the evaluation summary table comment
- On failure, link to the workflow run
- On skip (no skills), explain why no evaluation ran
- Skip posting success comment if no table exists to avoid duplicates

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-06 19:48:10 +01:00
Copilot c50e97f148 Narrow evaluation workflow path triggers to only evaluation-related files (#262) 
* Initial plan

* Narrow evaluation workflow path triggers to only evaluation-related files

Replace broad '.github/workflows/**' trigger with specific evaluation
workflow files (evaluation.yml, evaluation-run.yml, evaluation-fork-pr.yml)
to prevent evaluation from running on PRs that only change unrelated workflows.

Also update the infra-change detection regex to match the same narrowed set.

Co-authored-by: JanKrivanek <3809076+JanKrivanek@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JanKrivanek <3809076+JanKrivanek@users.noreply.github.com>
 2026-03-06 14:16:53 +01:00
Jan Krivanek 941fedb937 [Infra] Harden the health check groomer (#260) 
* Harden the health check groomer

* Fix pr comment
 2026-03-06 13:47:38 +01:00
Viktor Hofer 05a3633f2a Fix create_release input check in skill-validator.yml (#261) 
* Add workflow_dispatch trigger to skill-validator.yml

Add a workflow_dispatch trigger with an optional create_release boolean
input so the workflow can be run manually from the Actions tab. When
create_release is true, the release job runs and retention is set to
90 days, matching the scheduled nightly behavior.

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

* Use string comparison for inputs and guard release to main

Compare inputs.create_release against 'true' (string) instead of true
(boolean) since workflow_dispatch inputs are surfaced as strings. Also
restrict the release job to refs/heads/main to prevent accidental
overwrites of the nightly tag from non-main branches.

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

* Fix create_release input check to use truthiness

workflow_dispatch inputs with type: boolean are actual booleans in the
inputs context, so comparing against the string 'true' never matches.
Use truthy evaluation (inputs.create_release) instead.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-06 13:00:41 +01:00
Viktor Hofer 83fb975ac7 Add workflow_dispatch trigger to skill-validator.yml (#259) 
* Add workflow_dispatch trigger to skill-validator.yml

Add a workflow_dispatch trigger with an optional create_release boolean
input so the workflow can be run manually from the Actions tab. When
create_release is true, the release job runs and retention is set to
90 days, matching the scheduled nightly behavior.

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

* Use string comparison for inputs and guard release to main

Compare inputs.create_release against 'true' (string) instead of true
(boolean) since workflow_dispatch inputs are surfaced as strings. Also
restrict the release job to refs/heads/main to prevent accidental
overwrites of the nightly tag from non-main branches.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-06 12:30:06 +01:00
Viktor Hofer 75263eeef4 Use non-zipped artifacts to preserve file permissions (#257) 
- Upload skill-validator-dist as tar.gz with archive: false to preserve
  execute permissions through artifact round-trip
- Remove chmod +x workarounds in evaluate and comment-on-pr jobs
- Include README.md in publish output via CopyToPublishDirectory

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-06 10:10:04 +01:00
Jan Krivanek e4c7b43b5b [Infra] Improve resiliency of health checks (#256) 
* Increase timeout and make sharing info resilient to cache eviction

* Fix path
 2026-03-06 10:06:31 +01:00
Viktor Hofer 78f1ef2d53 skill-validator: improve UX for published tool (#223) 
* skill-validator: improve UX for published tool

- Make paths argument required (OneOrMore arity) so invoking without
  args shows full help text instead of a cryptic error
- Add Installation section to README covering dotnet tool install from
  nightly release, platform-specific AOT packages, and pipeline artifacts
- Update all usage examples to use the skillvalidator command
- Update CI integration example to install from nightly release
- Include README.md as NuGet package readme

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

* Apply suggestion from @Copilot

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

* Address PR feedback: clarify release wording and chmod instructions

- Replace ambiguous 'GitHub pre-release artifacts' with a direct link
  to the skill-validator-nightly GitHub Release
- Use 'chmod +x' instead of 'give +x permissions'
- Capitalize NuGet consistently

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

* Align ToolCommandName with AssemblyName (skill-validator)

Fixes naming mismatch where dotnet tool install produced 'skillvalidator'
but the AOT binary was 'skill-validator'.

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

* Update README.md

* Publish tar.gz archives in skill-validator workflow

- Add tar.gz creation from dotnet pack publish output for each RID
- Upgrade upload-artifact to v7 with archive: false (no double-zip)
- Upgrade download-artifact to v8 for non-zipped artifact support
- Include tar.gz files in nightly GitHub release

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

* Update README

* Update README

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-03-06 09:50:40 +01:00
Viktor Hofer 54e6fe8485 Enable NativeAOT publishing for SkillValidator (#221) 
* Enable NativeAOT publishing for SkillValidator

Migrate all serialization to source-generated, AOT-compatible patterns:

- Add PublishAot to csproj and Vecc.YamlDotNet.Analyzers.StaticGenerator package
- Create SkillValidatorJsonContext with source-generated serializers for all types
- Create SkillValidatorYamlContext with static YAML deserialization for eval config
  and frontmatter parsing
- Change AgentEvent.Data from Dictionary<string, object?> to
  Dictionary<string, JsonNode?> for AOT-safe serialization
- Replace anonymous type in Reporter.cs with named ResultsOutput record
- Fix JsonStringEnumConverter to generic AOT-safe JsonStringEnumConverter<T>
- Move ConsolidateData to Models.cs for JSON context accessibility
- Switch EvalSchema and SkillDiscovery to StaticDeserializerBuilder
- Add RawFrontmatter model for strongly-typed YAML frontmatter parsing
- Update all JsonSerializer call sites to use context-based overloads

The published binary is a single 14.6 MB native executable.
All 198 existing tests pass.

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

* Add NativeAOT publish matrix and daily scheduled builds to CI

- Single build job with matrix for linux-x64, linux-arm64, win-x64, win-arm64, osx-arm64
- Each matrix entry: build, test, NativeAOT publish, pack NuGet, upload tarball
- collect-packages job: builds RID-agnostic package, merges with per-RID packages
  into a single skill-validator-nupkgs artifact
- Add daily schedule trigger (08:00 UTC) with 90-day retention
- Keep existing path-based triggers for push/PR

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

* Add NativeAOT publish matrix and daily scheduled builds to CI

- Single build job with matrix for linux-x64, linux-arm64, win-x64, win-arm64, osx-arm64
- Each matrix entry: build, test, NativeAOT publish, pack NuGet, upload binary
- Upload publish directory directly (Actions wraps in zip automatically)
- collect-packages job: builds RID-agnostic package, merges with per-RID packages
  into a single skill-validator-nupkgs artifact
- Add daily schedule trigger (08:00 UTC) with 90-day retention
- Keep existing path-based triggers for push/PR

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

* Fix flaky DelayClampedToRemainingBudget test

Increase totalTimeoutMs from 200ms to 2000ms. The tight budget was
causing the CTS to fire before the retry attempt on slower CI runners
(macOS arm64). The test still validates that the 60s base delay gets
clamped — the 5s wall-time assertion is the real check.

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

* Cancel in-progress workflow runs on new push

Add concurrency group so that pushing a new commit to a PR
automatically cancels any still-running workflow for that ref.

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

* Fix per-RID NuGet pack with --use-current-runtime

The pack step was producing the same agnostic package for every RID.
Adding --use-current-runtime ensures each matrix entry produces a
RID-specific nupkg that gets collected into the combined artifact.

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

* Remove redundant publish step; pack already produces native binary

dotnet pack --use-current-runtime already runs publish internally,
producing the native AOT binary in artifacts/publish/. The separate
dotnet publish step was redundant.

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

* Rename nupkgs artifact to nuget-bundled

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

* Remove redundant PublishTrimmed; implied by PublishAot

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

* Fix flaky ExponentialBackoff_DelaysIncrease test

Increase baseDelayMs from 50ms to 200ms so that OS scheduling jitter
on slow CI runners (macOS arm64) doesn't dominate the measured gaps.

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

* Remove per-RID publish directory artifacts; nupkgs suffice

NuGet packages are zip files containing the native binaries, so the
separate skill-validator-<rid> publish folder uploads are redundant.

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

* Remove collect-packages job; produce agnostic nupkg from linux-x64

Pack the RID-agnostic package from the linux-x64 matrix entry instead
of a separate downstream job, eliminating the bundle step entirely.

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

* Rename artifact names from nuget-* to skill-validator-*

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

* Rename agnostic artifact to skill-validator

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

* Simplify NuGet artifact uploads; remove rm -rf and collect-packages

Use precise glob patterns with the stable package name to distinguish
RID-specific from agnostic nupkgs. The agnostic upload uses a negation
pattern to exclude the RID-specific package on linux-x64.

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

* Extract retention-days and agnostic RID into workflow env vars

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

* Add nightly release job using gh release

On scheduled runs, downloads all nupkg artifacts and publishes them
as a rolling 'skill-validator-nightly' pre-release on the repo.

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

* Restore PropertyNameCaseInsensitive for JSON deserialization

The original MCPServerDef deserialization used PropertyNameCaseInsensitive
which was dropped when switching to source-generated context. Add it back
to the JsonSourceGenerationOptions to preserve flexible property casing.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-05 21:07:17 +01:00
Jan Krivanek 1f02fab51f [Infra] Improve the daily health check (#215) 
* Improve the daily health check

* Reflect PR comments
 2026-03-05 10:23:47 +01:00
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>
 2026-03-05 09:16:27 +01:00
Dan Moseley 3c3c7e051d Add CODEOWNERS entries for dotnet-aot-compat (#211) 2026-03-04 14:42:24 -08:00
Viktor Hofer 9bcc11c524 Fix pr-number input type: use string instead of number (#208) 
Reusable workflow inputs typed as 'number' reject string values
from job outputs. Change pr-number to type: string and update
the skip condition and default accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 20:40:35 +01:00
Viktor Hofer e3e9f489da Update evaluation-fork-pr.yml (#205) 
* Update evaluation-fork-pr.yml

* Address PR review: least-privilege permissions and resilient status reporting

- Remove statuses: write from workflow-level permissions (already on
  gate and report-status jobs)
- Make pending status step non-blocking with continue-on-error
- Use 'error' state for cancelled/infrastructure issues vs 'failure'
  for actual evaluation failures

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 19:39:53 +01:00
Jan Krivanek e1f513f486 [Infra] Improve judges retry and resiliency (#188) 
* Add better retry for judge

* Extract funtionality and guard against single scenario failing whole eval run

* Reflect PR comments

* Reflect PR comments

* Apply timeout and parallelism defaults to evaluation-run.yml

Upstream refactored the evaluation workflow to use a reusable workflow
(evaluation-run.yml). Move our timeout (60->90min) and parallelism
(3->5) changes to that file.
 2026-03-04 19:35:32 +01:00
Viktor Hofer 2d02f4664b Replace environment gate with /evaluate comment trigger for fork PRs (#203) 
* Replace environment gate with /evaluate comment trigger

Replace pull_request_target + environment approval with an issue_comment
trigger. Maintainers with write+ access post '/evaluate' on a fork PR
to start the evaluation. This avoids the deployment entries that GitHub
environments create on the repo page.

Security model:
- Workflow YAML from default branch (enforced by issue_comment trigger)
- Validator built from default branch
- Fork content is untrusted data only
- Only write+ users can trigger via permission check

Also removes the environment input from the reusable workflow since it
is no longer needed.

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

* Use PR base SHA for diff instead of hardcoded origin/main

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 19:19:16 +01:00
Viktor Hofer 7dd45ea1a8 Enable evaluation for fork PRs via reusable workflow (#198) 
* Enable evaluation for fork PRs via reusable workflow

Refactor the evaluation workflow into three files:

- evaluation-run.yml: reusable workflow with build-validator, evaluate,
  and comment-on-pr jobs (shared by both callers)
- evaluation.yml: thin caller for pull_request and schedule triggers;
  skips fork PRs which are handled by the new fork workflow
- evaluation-fork-pr.yml: thin caller using pull_request_target with
  environment-gated secrets for fork PRs

Also clear GITHUB_TOKEN from the process environment after the
CopilotClient reads it, so eval scenarios and setup commands cannot
access the credential.

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

* Address PR review: harden permissions and cleanup

- Add persist-credentials: false to untrusted content checkout
- Add explicit permissions (contents: read) to build-validator and
  evaluate jobs to limit blast radius
- Guard comment-on-pr against skipped evaluate result
- Remove unused is_infra input from reusable workflow

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

* Restrict discover job permissions and document workflow-level scope

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 18:18:21 +01:00
Jan Krivanek 29846573f6 [Infra] Improving health check aw (#187) 
* Update the issue update mask

* Improve pipeline issues detection

* Reflect PR comments
 2026-03-04 14:17:23 +01:00
Viktor Hofer 0040cd3a74 Update dependabot configuration for directories (#195) 2026-03-04 14:04:52 +01:00
dependabot[bot] 228faf6991 Bump the github-actions-dependencies group across 1 directory with 5 updates (#194) 
Bumps the github-actions-dependencies group with 5 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [actions/checkout](https://github.com/actions/checkout) | `4` | `6` |
| [actions/github-script](https://github.com/actions/github-script) | `7` | `8` |
| [actions/setup-dotnet](https://github.com/actions/setup-dotnet) | `4` | `5` |
| [actions/upload-artifact](https://github.com/actions/upload-artifact) | `4` | `7` |
| [actions/download-artifact](https://github.com/actions/download-artifact) | `4` | `8` |



Updates `actions/checkout` from 4 to 6
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](https://github.com/actions/checkout/compare/v4...v6)

Updates `actions/github-script` from 7 to 8
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](https://github.com/actions/github-script/compare/v7...v8)

Updates `actions/setup-dotnet` from 4 to 5
- [Release notes](https://github.com/actions/setup-dotnet/releases)
- [Commits](https://github.com/actions/setup-dotnet/compare/v4...v5)

Updates `actions/upload-artifact` from 4 to 7
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](https://github.com/actions/upload-artifact/compare/v4...v7)

Updates `actions/download-artifact` from 4 to 8
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](https://github.com/actions/download-artifact/compare/v4...v8)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions-dependencies
- dependency-name: actions/github-script
  dependency-version: '8'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions-dependencies
- dependency-name: actions/setup-dotnet
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions-dependencies
- dependency-name: actions/upload-artifact
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions-dependencies
- dependency-name: actions/download-artifact
  dependency-version: '8'
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: github-actions-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
 2026-03-04 13:58:58 +01:00
Viktor Hofer ef9c2d503c Exclude lock files from Dependabot updates (#193) 2026-03-04 13:55:40 +01:00
Viktor Hofer b88078ec8f [Infra] Add Dependabot configuration for NuGet and GitHub Actions (#190) 
* Add Dependabot configuration for NuGet and GitHub Actions

* Update dependabot.yml
 2026-03-04 13:43:11 +01:00
Viktor Hofer 7df034a3fc Skip token randomization on forked PRs (#186) 
* Skip token randomization on forked PRs

On forked PRs, the token randomization step is skipped and the
COPILOT_GITHUB_TOKEN secret is used directly as a fallback.

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

* Remove fork guard from skill-validator step

The step now runs on forked PRs using the COPILOT_GITHUB_TOKEN
fallback when the randomization step is skipped.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 12:02:21 +01:00
Copilot 61a4f0b540 Fix stale src/ paths in devops-health-check.md after folder rename to plugins/ (#175) 
* Initial plan

* Fix incorrect src/ paths to plugins/ in devops-health-check.md

Co-authored-by: timheuer <4821+timheuer@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: timheuer <4821+timheuer@users.noreply.github.com>
 2026-03-04 09:16:18 +01:00
Dan Moseley 30cd89132c Skip scheduled evaluation runs on forks (#179) 
Forks that activate Actions (by pushing a branch for a PR) inadvertently
start running the evaluation schedule every 2 hours. Each run builds and
uploads a ~95 MB artifact, consuming the fork owner's Actions minutes and
storage. The scheduled runs always fail on forks because the required
COPILOT_GITHUB_TOKEN secrets are not available.

Add a guard on the discover job to skip scheduled runs on forks. All
other jobs depend on discover, so this gates the entire workflow for
the schedule event. PR and workflow_dispatch triggers are unaffected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-04 07:20:57 +01:00
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>
 2026-03-03 08:15:32 -08:00
Jan Krivanek 66b8659ec3 [Infra] Fix devops aw (#167) 
* Fix devops-aw triggering

* fix: add pull_request to pre_activation gate so PR trigger works

* fix: enable deep investigation dispatches

- Fix COPILOT_GITHUB_TOKEN in investigate workflow to use _3 (fine-grained PAT)
- Strengthen Step 5 dispatch instructions to be MANDATORY
- Add first-run note: all critical findings must be dispatched
- Add verification checklist before finishing

* fix: update frontmatter hash to match .md source

* fix: investigation uses add-comment instead of update-issue

update-issue fails on workflow_dispatch (no triggering issue context).
Switch to add-comment with explicit item_number targeting the health issue.
Remove update_issue from safe-outputs config, tools, and validation.

* fix: address PR review feedback

- Fix concurrency group: use stable ref-based key with cancel-in-progress
  instead of run_id which defeated mutual exclusion (reviewer feedback)
- Remove temporary pull_request trigger and pre_activation gating
  (was only needed for testing, reviewer correctly flagged mismatch)
- Remove slash command references from docs and shared knowledge
  since slash_command trigger was removed (reviewer feedback)
 2026-03-03 16:21:10 +01:00
Aaron R Robinson dc85d453c4 Add missing CODEOWNERS entries for new skills (#161) 
* Add missing CODEOWNERS entries for new skills

Add folder-level CODEOWNERS entries for:
- android-tombstone-symbolication (@jeffschwMSFT)
- clr-activation-debugging (@marklio)
- microbenchmarking (@caaavik-msft)

Fixes #159

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

* Apply suggestion from @AaronRobinsonMSFT

* Apply suggestion from @AaronRobinsonMSFT

* Apply suggestion from @AaronRobinsonMSFT

* Apply suggestion from @AaronRobinsonMSFT

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-03-02 13:33:20 -08:00
Jan Krivanek 7b2708f8c7 [Infra] Add devops agentic workflow (#141) 
* Add initial plan

* add token strategy

* Add devops agentic workflow PoC

* Fix PR Comments

* Replace hand-written .yml wrappers with gh aw compile

- Move workflow .md files from eng/agentic-workflows/ to .github/workflows/
- Delete hand-written .yml wrappers (devops-health-check.yml, devops-health-investigate.yml)
- Generate .lock.yml files via gh aw compile
- Move compiled knowledge files to .github/aw/shared/ (required by gh-aw import security)
- Fix permissions: issues: write -> issues: read (strict mode uses safe-outputs)
- Update README with new file structure and setup instructions

Addresses PR #141 Comment 5 (ViktorHofer)

* Use fuzzy daily schedule, update README for gh-aw standard structure

* Recompile lock.yml with latest changes

* Add skills health

* Remove redundant file
 2026-03-02 21:59:28 +01:00
Jared Parsons 992dd7f3c9 GHA to validate ownership (#158) 
* GHA to validate ownership

Part of our contribution policy is that every skill / plugin has an associated owner with it. This GHA is designed to block PRs that produce content that violates this.

* Update .github/workflows/codeowners-folder-validation.yml

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

* Apply suggestions from code review

Co-authored-by: Jared Parsons <jared@paranoidcoding.org>

* better output

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-03-02 11:02:35 -08:00
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>
 2026-03-02 10:12:21 -08:00