* 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>
* 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>
* 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>
* 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>
* 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>
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>
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>
* 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)
Fixesdotnet/skills#168
* Delete docs/fix-429-rate-limiting.md
* 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>
* 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>
- 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>
* 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>
* 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>
* 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>
- 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>
* 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>
* 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>
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>
* 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>
* 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.
* 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>
* 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>
* 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>
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>
* 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)
* 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>
* Port skill-validator from TypeScript to .NET C#
Replace the TypeScript skill-validator with a .NET 10 C# implementation.
Uses System.CommandLine for CLI parsing, GitHub.Copilot.SDK for Copilot
communication, YamlDotNet for YAML, and xUnit v3 with MTP v2 for testing.
All 135 unit tests ported and passing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update CI workflows for .NET skill-validator
Replace Node.js setup and npm commands with dotnet build/test/publish.
Use global.json for SDK version instead of hardcoded dotnet-version.
Publish self-contained binary for evaluate runners.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix permission denied for Copilot SDK native binary in CI
Restore execute permissions on the native copilot binary after
artifact download, which strips file permissions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nit updates
* Apply upstream changes: --verdict-warn-only, safeDirName, fix benches
Port three origin/main commits to the C# skill-validator and workflow:
- Add --verdict-warn-only flag replacing --strict; verdict failures
(threshold, completion, activation) become warnings while execution
errors and --require-evals missing_eval failures remain fatal.
- Add FailureKind tracking to SkillVerdict and Comparator.
- Add SafeDirName sanitizer for skill directory names in Reporter.
- Add per-skill verdict.json generation in JSON reporter.
- Update evaluation.yml: remove continue-on-error, use --verdict-warn-only,
change default infra runs to 2, skip comment-on-pr on cancellation,
use ResultsFile for benchmark data generation.
- Update generate-benchmark-data.ps1 to read verdicts from results.json.
- Update README with new CLI flags and CI example.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update CONTRIBUTING.md for .NET skill-validator
Replace Node.js/npm prerequisites and commands with .NET 10 SDK
equivalents (dotnet run).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update test commands in CONTRIBUTING.md to use .csproj file
* Add RunArguments for default results-dir and verbose logging
- Set RunArguments in SkillValidator.csproj using MSBuild NormalizeDirectory
to pass --results-dir pointing to artifacts/TestResults/skill-validator/
- Log configured results dir at startup when --verbose is enabled
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix path
* Fix AgentRunner: capture tool completion events and implement permission checking
- Add ToolExecutionCompleteEvent handler to capture Success/Result data
from tool completions, fixing timeline showing all tools as FAIL
- Add ToolExecutionStartEvent arguments capture for timeline detail
- Implement CheckPermission with path extraction from PermissionRequest
ExtensionData, replacing the stub that always returned true
- Update Judge and PairwiseJudge to pass PermissionRequest directly
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix CheckPermission tests for cross-platform CI (Linux)
Use Path.GetTempPath() and Path.Combine() instead of hardcoded Windows
paths so tests pass on both Windows and Linux CI runners.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Capture skills & tokens usages
* Fix thread safety, resource leaks, and null safety in skill-validator
- Use ConcurrentBag and SemaphoreSlim with double-check locking for
_workDirs and _sharedClient in AgentRunner (race conditions under
parallel runs/scenarios)
- Make Spinner IDisposable; protect _frame/_message/_active with lock
and Interlocked; use 'using var' at call site
- Use 'using var' for timeout Timers in Judge and PairwiseJudge to
prevent leaks on exception paths
- Replace perRunPairwise[0] fallback with FirstOrDefault() to avoid
IndexOutOfRangeException when list is empty
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update eng/skill-validator/README.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Address PR review feedback
- Add per-type required field validation in ParseAssertion (e.g.
file_contains requires both path and value, output_matches requires
pattern)
- Fix JsonDocument leaks in LlmJson: use 'using' and Clone() on
RootElement in both ParseLlmJson and TryParseJson
- Pin Microsoft.Extensions.FileSystemGlobbing to 10.0.0 instead of
floating 10.* for reproducible builds
- Fix README Development section to reference SkillValidator.slnx
instead of .sln
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Add --warn-only option to skill-validator for non-fatal skill failures
This avoids the continue-on-error setting the yml which sometimes hides real issues.
* Change default runs from 1 to 2 in evaluation.yml
Updated the number of runs to 2 for better testing balance.
* Rename --warn-only to --threshold-warn-only and fail on execution errors
- Rename --warn-only to --threshold-warn-only to clarify it only applies
to threshold-based verdict failures, not execution errors.
- Track rejected evaluation promises and return exit code 1 for execution
errors even in --threshold-warn-only mode.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove redundant --strict option, clarify --threshold-warn-only
--strict only implied --require-evals, which already exists as its own
flag. Remove --strict to avoid confusion about its interaction with
--threshold-warn-only. Update --threshold-warn-only help text to clarify
it only suppresses threshold failures, not runtime/config errors.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Only post PR comment when evaluate matrix succeeds
When a new push triggers cancel-in-progress, the evaluate matrix jobs
get cancelled but comment-on-pr still ran due to always(). This caused
partial/stale results to be posted. Require evaluate to succeed before
posting the PR comment.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add --verdict-warn-only flag with failureKind tracking
--verdict-warn-only treats verdict failures (threshold, completion
regression, skill not activated) as warnings (exit 0), while execution
errors and --require-evals missing_eval failures remain fatal.
Add failureKind field to SkillVerdict to categorize failure reasons.
Remove --strict (redundant with --require-evals).
Update README and evaluation.yml with documentation.
Allow comment-on-pr to run on evaluate failure (skip only on cancel).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix empty benches arrays in benchmark data generation
The json reporter lost per-skill verdict.json generation during a
refactor (b461016), causing generate-benchmark-data.ps1 to find no
verdict files and produce empty benches arrays.
Fix by:
- Restoring verdict.json generation in the json reporter
- Updating generate-benchmark-data.ps1 to read verdicts directly from
results.json instead of scanning for per-skill verdict.json files
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify generate-benchmark-data to accept ResultsFile path directly
Replace ResultsDir parameter with ResultsFile since the script only
needs the results.json path. Update evaluation.yml caller accordingly.
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>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
When a PR modifies files under .github/workflows/ or
eng/skill-validator/, all component evaluations are triggered since
these infra changes can affect any evaluation result. To prevent
quota issues, infra-triggered evaluations use 1 run instead of the
default.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The skill-validator reporter creates timestamped subdirectories
(YYYYMMDD-HHMMSS format), not run-* directories. The benchmark
step was looking for run-* and never finding results. Also sort
descending to pick the latest run when multiple exist.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat: update build-validator job and README for Node.js version requirement
- Update an existing comment if it exists with the total summary
- Build the skill-validator just once and upload
- Clean-up when passing CLI args to skill-validator
* Update .github/workflows/evaluation.yml
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Prune dev dependencies before uploading validator artifact
Reduces artifact size by removing devDependencies (typescript, vitest, etc.)
from node_modules/ since the evaluate job only needs the built dist/.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Updates
* Consolidate step summary into comment-on-pr job
Move GITHUB_STEP_SUMMARY writing from per-matrix Generate Summary step
into the existing Consolidate summaries step in comment-on-pr job.
This produces a single unified summary instead of fragmented per-component
summaries. The job now runs for all event types, with the PR comment
step scoped to pull_request events only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Refactor reporters to use --results-dir for file output
- Remove --no-save-results option and saveRunResults function
- Reporters no longer accept inline paths (e.g., json:path)
- Add --results-dir option: when specified, file reporters (json, junit,
markdown) write to that directory; when omitted, they write to console
- Enhance markdown reporter with per-scenario judge reports including
rubric scores, reasoning, and agent output
- Update workflow, README, and CONTRIBUTING docs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Default --results-dir to .skill-validator-results and enable json/markdown reporters by default
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix unresolved matrix expression in skipped evaluate job name
Replace if: has_entries with if: !cancelled() && !failure() so the job
proceeds with an empty matrix, creating zero instances instead of a
skipped job with unresolved expressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update required nodejs version in CONTRIBUTING doc
* Address PR feedback: fix timeout message, improve reporter output handling
- Use effective timeout value in error message to avoid 'undefineds'
- Only create resultsDir when file-based reporters are selected
- Write results into timestamped subdirectory (yyyyMMdd-HHmmss) to avoid stale output
- Use scenario-level improvementScore for markdown verdict icons
- Update workflow to find summary.md in timestamped subdirs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update bin-obj clash skill
* Fix PR comment step: use gh api instead of gh pr comment
gh pr comment requires a git repo checkout, but comment-on-pr job
only downloads artifacts. Switch to gh api for creating comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Touch one more skill
* Address PR feedback: remove dead code, fix formatting, improve workflow
- Remove dead validation check in cli.ts (resultsDir always has default)
- Fix missing spaces in variable declarations in reporter.ts
- Quote command substitution in consolidation script
- Use explicit needs.build-validator.result == 'success' condition
- Strip duplicate headers when consolidating multi-component summaries
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Replace markdown concatenation with consolidate subcommand
Instead of fragile bash string manipulation to combine per-component
summary.md files (which caused duplicate headers, empty tables, and
repeated model lines), add a 'consolidate' subcommand that reads
results.json files and generates a single clean markdown summary.
The subcommand takes explicit file paths as arguments — the caller
controls exactly which files to merge. Runner-specific concerns
(comment markers, run URLs) stay in the workflow script.
- Extract generateMarkdownSummary() from reportMarkdown() in reporter.ts
- Add 'skill-validator consolidate' subcommand in cli.ts
- Update workflow to use the new subcommand
- Document consolidate in README.md
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Viktor Hofer