The publish-eval-data job iterates discover.outputs.plugins and looks for
"all-results/skill-validator-results-<plugin>", but when the discover job fans a
plugin into multiple shards the artifacts are named
"skill-validator-results-<plugin>--shard-<tag>". The exact-match lookup misses
all of them and the step logs "No run results found for dotnet-msbuild,
skipping" -- which is exactly what the 2026-05-27 scheduled run did (confirmed
in the publish-eval-data logs of run 26484154365).
Fix: locate every matching artifact dir (exact name OR --shard-* suffix),
collect the latest run dir's results.json from each, and -- when there are
multiple -- concat their verdicts arrays into a single synthetic results.json
before calling generate-benchmark-data.ps1. Calling the script per-shard would
have emitted N separate datapoints with the same commit/timestamp and skewed
the dashboard.
publish-token-data and publish-session-data were already shard-tolerant and
need no changes.
PR #473 moved the failure investigation prompt out of the PR comment and
only into the workflow run summary. This made it harder for users to
discover — they had to click through to the run details to find it.
Move it back to the PR comment so the copy-paste prompt is visible
inline on the PR, while keeping it in the workflow summary as well.
* Shard plugin evaluation via executionShard tag in eval.yaml
MCP-heavy plugins like dotnet-msbuild reliably exhaust the GitHub-hosted runner host after ~87 min of cumulative MCP traffic (Copilot CLI plus dotnet dnx Microsoft.AITools.BinlogMcp child processes), which has produced no dashboard data for dotnet-msbuild scheduled runs since 2026-05-20. Cutting per-runner concurrency alone was insufficient: at the default 18-way concurrency the runner OOMs in 17-30 min; at 3-way it survives longer but still hits a host-level termination at ~87 min.
Two mitigations, both required:
1. Sharding (this change). The discover step reads an optional top-level 'executionShard:' string from each tests/<plugin>/<skill>/eval.yaml and groups skills by tag, emitting one matrix entry per shard so each shard runs on a fresh runner. Untagged skills fall into a 'default' bucket. Plugins where all skills resolve to a single bucket emit one entry (unchanged behavior). The validator already calls IgnoreUnmatchedProperties so no Models.cs change is needed. Shard names follow '<plugin>--shard-<tag>' so the existing 'skill-validator-results-<plugin>--*' artifact glob keeps working.
2. Per-MCP-plugin concurrency caps (already in place). When plugin.json declares mcpServers, parallelism is forced to 1x3x1 with --judge-timeout 600 to avoid acute OOM within a shard.
Also adds a workflow_dispatch 'plugin' input for targeted re-runs of a single plugin, and tags 10 dotnet-msbuild eval.yamls (heavy/medium); the remaining 8 fall into 'default'. Empirically the three shards complete in parallel in ~30-40 min each, well inside the 180-min job timeout, and successfully upload dashboard data.
* Address PR review: lenient shard regex and validate dispatch plugin input
- executionShard regex now tolerates optional leading whitespace, so an accidentally indented key still groups correctly instead of silently falling back to the default bucket.
- workflow_dispatch 'plugin' input is now validated against ^[a-zA-Z0-9._-]+$ before any Join-Path/Test-Path use, preventing path-traversal style values (e.g. '../foo') from being injected into filesystem paths.
Real fix: detect plugins declaring mcpServers and force PARALLEL_SKILLS/SCENARIOS/RUNS=1 to avoid runner OOM when dnx-launched MCP children compete for memory (root cause of dotnet-msbuild scheduled-run failures since 2026-05-20).
TEMP (revert before PR): enable workflow_dispatch through gate/discover/build-validator, hardcoded to evaluate only dotnet-msbuild, for CI validation without merging to main.
* Dashboards series fix (followup)
* Fix deploy-dashboard condition for workflow_dispatch
The deploy-dashboard job required needs.discover.result == 'success',
but the discover job's condition does not include workflow_dispatch,
so it is always skipped for manual triggers. This made workflow_dispatch
unable to deploy the dashboard.
Split the condition so workflow_dispatch from main bypasses the
discover dependency, while scheduled runs still require it.
* Update deploy-dashboard comment to reflect workflow_dispatch support
* Make the infra eval gate faster
* Address PR feedback: handle infra+skill overlap, detect evaluation.yml changes
- When both infra and skill changes exist in a PR, evaluate the changed
skills normally instead of the random subset (fixes Copilot review).
- Expand infra detection to include .github/workflows/evaluation.yml
changes in all three detection sites (pr-status, fork-pr-status, discover).
The pinned actions/cache SHA (5a3ec84, v4) does not expose the
cache-primary-key output. This caused the evaluate job to fail with
'Input required and not supplied: key' because the build-validator
job output was always empty.
Fix: compute the cache key in a dedicated step and pass it as a job
output, instead of relying on an output that doesn't exist at this
action version.
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Skill-validator: perf, cancellation, and reliability improvements
Performance:
- Cache skill-validator-dist.tar.gz in CI (evaluation.yml and skill-check.yml)
to skip .NET SDK setup, build, and tar on cache hit. Evaluate and comment-on-pr
jobs restore from cache with artifact download fallback.
- Replace SemaphoreSlim with Lock in SessionDatabase for lighter-weight
synchronous locking.
Cancellation:
- Thread CancellationToken from System.CommandLine entry point through the
entire evaluate call chain: Run -> EvaluateTarget -> EvaluateSkill/EvaluateAgent
-> ExecuteScenario/ExecuteAgentScenario -> ExecuteRun/ExecuteAgentRun, plus
EvaluateSkillNoise -> ExecuteNoiseTest.
- Add CancellationToken parameter to AgentRunner.RunAgent, Judge.JudgeRun,
PairwiseJudge.Judge, and OverfittingJudge.Analyze, forwarding to
RetryHelper and LlmSession.SendAsync respectively.
- Pass CancellationToken to all ConcurrencyLimiter.RunAsync call sites so
semaphore waits are cancellable.
- Re-throw OperationCanceledException at the top-level skill evaluation loop
so Ctrl+C / CI cancellation propagates instead of being swallowed as a
skill failure.
Reliability:
- Smart timeline truncation in Judge and PairwiseJudge: error events
(session.error, runner.error) are always preserved during truncation
regardless of position, with a summary marker showing event type breakdown.
- Add jitter (±25%) to RetryHelper exponential backoff to prevent thundering
herd on rate-limit recovery.
- Add CancellationToken parameter to ConcurrencyLimiter.RunAsync<T>.
- Add ConcurrencyLimiter tests (6 tests) and RetryHelper jitter tests (2 tests).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review feedback
- RetryHelper: re-clamp delay after jitter so it never exceeds the
remaining budget or MaxSingleDelayMs.
- Judge/PairwiseJudge: handle edge case where error count alone exceeds
MaxTimelineEvents by trimming errors with a summary instead of
unconditionally appending all of them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add high-variance detection and refactor FailureKind to enum
- Add Statistics.CoefficientOfVariation() for measuring score consistency
- Flag scenarios with CV > 0.5 as high-variance in ScenarioComparison
- Show console warning and markdown footnote for high-variance scenarios
- Include high-variance scenario names in Comparator verdict reason
- Refactor FailureKind from magic strings to strongly-typed enum with
JsonStringEnumMemberName attributes for backwards-compatible JSON output
- Add 6 new CV unit tests
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Move skill-validator docs under src/ and include in packages
Move README.md to src/ and InvestigatingResults.md + OverfittingDetection.md
to src/docs/ so they ship in the NuGet package and .tar.gz archives.
- Include docs/*.md in NuGet pack and publish output via CopyToPublishDirectory
- Add LICENSE to publish output (tar.gz only, not NuGet — uses PackageLicenseExpression)
- Replace enumerated docOnlyFiles lists with a regex pattern in CI workflows
- Deduplicate eval.yaml docs between CONTRIBUTING.md and the shipping README
- Add test fixture files walkthrough to README (moved from CONTRIBUTING.md)
- Add AGENTS.md guidance: src/ is the packaging boundary
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update link format in failure diagnosis message
* Update AGENTS.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan
* Fix Action summary to show all columns and remove redundant Full results link
The comment-on-pr job was using the simplified PR comment table
(consolidated-comment.md) for the Action step summary. This change:
1. Writes the full summary-body.md (with all columns including
Quality (Plugin) and Agents Invoked) to GITHUB_STEP_SUMMARY.
2. Removes the "Full results" link from the Action summary since
it links to the same page.
3. Keeps the investigation prompt in both PR comment and Action summary.
Agent-Logs-Url: https://github.com/dotnet/skills/sessions/870c41f2-0e41-4e62-9a43-d1f7128bab06
Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
* Add evaluation troubleshooting guide and link from PR comments
Add InvestigatingResults.md with:
- How to find and download evaluation artifacts from workflow runs
- Full results.json schema documentation
- Common failure patterns (timeouts, empty output, variance, rubric
issues, skill not activated) with diagnosis steps and fixes
- Example analysis script for AI agents to parse results
Link the doc from:
- Reporter.cs: show a link in the PR comment when any skill fails
- skill-validator README.md: Output section
- CONTRIBUTING.md: CI evaluation section
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update InvestigatingResults.md
* Address review feedback: fix hard-coded URLs and optional plugin run docs
- Reporter.cs: Use repo-relative link instead of hard-coded GitHub URL
- evaluation.yml: Use github.sha instead of hard-coded 'main' branch
- InvestigatingResults.md: Clarify skilledPlugin run is optional
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* THROWAWAY: add trailing newline to trigger eval workflow
This change is intentionally trivial and will be reverted before merge.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR #441 review feedback (round 2)
- Reporter.cs: Remove leading / from relative link
- evaluation.yml: Use jq on results.json instead of grepping for emoji
- evaluation.yml: Use needs.gate.outputs.head_sha for revision-pinned URL
- InvestigatingResults.md: Mark pluginImprovementScore/pluginBreakdown as optional
- InvestigatingResults.md: Fix skill-not-activated symptoms to use
skillActivation fields instead of turnCount
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Use specific skillActivation field names and fix timestamp description
- Use skillActivationIsolated/skillActivationPlugin instead of generic skillActivation
- Clarify timestamp is when results were written, not run start
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix timestamp field description (was lost in prior commit)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Revert "THROWAWAY: add trailing newline to trigger eval workflow"
This reverts commit 83bd7f7601.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Move token usage data into separate branch
* Address PR review: add help doc, fix title injection, add token-usage fallback
* Add -SkipBenchmarkData switch, clean stale files on branch push
* Combine evaluation.yml and evaluation-run.yml into single workflow
Inline the three jobs from the reusable evaluation-run.yml workflow
(build-validator, evaluate, comment-on-pr) directly into evaluation.yml
and delete the now-unnecessary reusable workflow file.
Changes:
- Inline build-validator, evaluate, and comment-on-pr jobs
- Eliminate 8-secret forwarding block (direct secrets.* access)
- Replace inputs.* with direct needs.* references
- Add MODEL/JUDGE_MODEL workflow-level env vars
- Fix --verbose duplication in evaluate step
- Simplify infra-change detection regex
- Update dependency chains (report-status, publish-benchmark)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR feedback: fix consolidate subcommand, improve error reporting
- Fix consolidate invocation: use 'evaluate consolidate' subcommand
(was incorrectly changed to just 'consolidate' during inlining)
- Add build-validator to report-status needs so build failures surface
as a deterministic failure state instead of generic error
- Add discover to comment-on-pr needs and gate on has_entries so
/evaluate on a no-op PR doesn't post a misleading failure comment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix matrix fallback and header comment
- Use fromJson(... || '[]') fallback for matrix entries to prevent
errors when discover is skipped on non-evaluation events
- Update header comment to accurately describe the is_fork mechanism
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Require /evaluate command for PR evaluation
Update evaluation.yml to require an /evaluate command to trigger evaluation,
matching the behavior in evaluation-fork-pr.yml. This change:
- Adds pr-status job that posts initial commit status on PR open/sync:
- 'success' if no skills changed (PR not blocked)
- 'pending' if skills changed (maintainer must post /evaluate)
- Adds gate job to validate /evaluate command (permissions, same-repo check)
- Replaces evaluation-status check-run with report-status using commit status API
- Scheduled runs continue to work unchanged (daily evaluation of all plugins)
The evaluation-status required check still works because:
- PRs without skill changes get immediate success status
- PRs with skill changes get pending status until /evaluate is posted
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Merge evaluation and evaluation-fork-pr into single workflow
Consolidate evaluation.yml and evaluation-fork-pr.yml into one unified
workflow that handles both same-repo and fork PRs via conditionals:
- pr-status: posts initial commit status for same-repo PRs (pull_request)
- fork-pr-status: posts initial commit status for fork PRs (pull_request_target)
- gate: unified /evaluate command validation with is_fork output
- discover: unified skill discovery with conditional worktree for forks
- report-status: single job for posting final commit status
- Scheduled runs and publish-benchmark unchanged
Also fixes:
- run-evaluation now uses always() so scheduled runs aren't skipped
- report-status scoped to issue_comment events only
- Removed stale evaluation-fork-pr.yml reference from infra detection regex
- Updated evaluation-run.yml comment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review feedback
- Fetch PR head and use worktree for both fork and same-repo PRs in discover
- Always pass checkout-ref (PR head SHA) to evaluation-run for /evaluate
- Fix runs count: use event_name == 'schedule' instead of github.ref
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Require /evaluate for infra changes too
Expand pr-status and fork-pr-status discovery to detect evaluation workflow
and skill-validator changes in addition to skill/test changes. Both now post
pending status requiring /evaluate when infra files are modified.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Build skill-validator from PR branch for non-fork PRs
Add is-fork input to evaluation-run.yml. When false (same-repo PRs), the
validator is built from checkout-ref so skill-validator changes are tested.
When true (fork PRs), the validator is built from the base branch to prevent
untrusted code from modifying tooling.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review: fix security comment, add issues:write
- Update security model comment to reflect validator-ref behavior
- Add issues: write permission to gate and report-status jobs for
reaction and comment API calls
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Add prominent note about issue_comment trigger limitation
Changes to this workflow file in a PR won't take effect until merged
because issue_comment always runs from the default branch.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Don't require /evaluate for workflow YAML changes
Workflow YAML changes can't be tested via /evaluate anyway (issue_comment
always runs from main). Only eng/skill-validator/ changes now require
/evaluate in the pr-status jobs. The discover job still detects workflow
changes to trigger all-plugin evaluation when /evaluate is run.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Make publish-benchmark resilient to individual plugin eval failures
When any single evaluate matrix leg fails (e.g. OOM kill), the entire
run-evaluation reusable workflow reports failure. Previously this caused
publish-benchmark to be skipped, blocking dashboard updates for ALL
plugins -- even the ones that succeeded.
Changes:
- publish-benchmark: use !cancelled() so it runs when some legs fail
- download-artifact: continue-on-error so missing artifacts don't abort
- The existing Generate Benchmark Data step already handles missing
plugin results with Write-Warning and continue
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Make continue-on-error conditional on evaluation result
Only tolerate artifact download failures when run-evaluation didn't
fully succeed. When all legs passed, a download failure is a real error
that should fail the job.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Reduce scheduled evaluation from every 3h to daily, skip if no changes
The evaluation schedule was consuming ~/5 days in Copilot premium
requests from scheduled runs alone (~/month pace). This change:
1. Reduces the cron schedule from every 3 hours (8 runs/day) to once
daily at 08:00 UTC. This alone saves ~87% of scheduled evaluation
cost.
2. Adds a check-for-changes guard that skips the scheduled run entirely
if there have been no new commits to main since the last successful
scheduled evaluation. This avoids wasting tokens re-evaluating
identical code.
The PR trigger and all PR evaluation behavior is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address review feedback: harden skip-if-no-changes guard
- Add actions: read permission so the workflow run lookup succeeds
- Use status=completed with jq conclusion filter instead of status=success
- Make all gh api calls non-fatal (fallback to has_changes=true on error)
- Make compare API call best-effort for logging only
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Scope actions:read to discover job, increase per_page to 100
- Move actions:read from workflow-level to discover job permissions
for least-privilege (only the schedule guard needs it)
- Increase per_page from 10 to 100 so the skip guard finds the last
successful run even after long failure streaks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Key skip guard off most recent completed run, not just successful
Only skip when the most recent completed scheduled run was successful
AND at the same SHA. If it failed/cancelled at the current SHA, proceed
to retry. This prevents the guard from skipping indefinitely after a
transient failure.
Also simplifies to per_page=1 since we only inspect the latest run.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix jq filter to yield empty when no prior runs exist
Use // empty to guard against null when workflow_runs array is empty,
preventing 'null null' from leaking into LAST_SHA/LAST_CONCLUSION.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Bypass skip guard on manual reruns (run_attempt > 1)
When a maintainer clicks 'Re-run jobs' on a scheduled run, the event
is still schedule with the same SHA. Without this check, the guard
would skip the rerun if the previous attempt succeeded. Now reruns
always proceed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Security Update
Identified and remediated 15 vulnerabilities across the skill-validator
runtime, dashboard, and CI/CD pipelines.
Changes:
- Runtime (AgentRunner.cs): path traversal prevention, deny-by-default
permissions, env scrubbing, MCP server hardening
- Regex (AssertionEvaluator.cs): ReDoS timeout
- Logging (SkillDiscovery.cs): exception logging
- Dashboard (dashboard.html, dashboard.js): SRI hash, XSS escaping
- CI/CD (5 workflow files): SHA pinning, persist-credentials: false
- Tests (RunnerTests.cs): 45+ new security test cases
MCP Server Defense-in-Depth (5 layers):
1. Command allowlist: only dotnet, node, npx, python, python3, uvx
2. Path rejection: no /, \, or .. in command names
3. Args validation: per-runtime dangerous flag blocklist
4. Env sanitization: strips PATH, LD_PRELOAD, NODE_OPTIONS,
DOTNET_STARTUP_HOOKS, etc.
5. Cwd dropped: custom working directories not passed to SDK
Subprocess Isolation:
- Sensitive env vars + prefix patterns scrubbed from child processes
- 120-second timeout with Kill(entireProcessTree: true)
- Deny-by-default permission model
Supply Chain:
- All GitHub Action references pinned to full commit SHAs
- All checkout steps set persist-credentials: false
- Chart.js CDN pinned to v4.4.7 with SRI integrity hash
* Update .github/workflows/skill-validator.yml
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update eng/skill-validator/src/Services/AgentRunner.cs
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update eng/skill-validator/src/Services/AgentRunner.cs
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Update tests to use fullCommandText field name
Update ExtractsCommandProperty and PrefersPathOverCommand tests to use
the fullCommandText property instead of command, matching the updated
permission request schema in AgentRunner.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add 'dnx' to allowed MCP commands
* Address PR review feedback: security hardening fixes
- CheckPermission: normalize paths with TrimEndingDirectorySeparator to
prevent double-separator false denials when dirs have trailing slash
- SanitizeMcpArgs: reject prefix forms (-econsole.log) and --flag=value
variants that bypass exact-match blocklist checks
- Setup file path traversal: normalize canonicalWorkDir and
canonicalSkillPath to avoid double-separator in StartsWith checks
- IsAllowedMcpCommand: use GetFileName instead of
GetFileNameWithoutExtension to block dotnet.bat/node.cmd; only strip
.exe on Windows
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* prune
* fix
* Fix concurrency setting
* Add binlog generation and upload to skill-validator workflow
Produce binary logs for build, test, and pack steps using /bl: flag.
Upload binlogs as artifacts (with if: always()) for build diagnostics.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Improve CheckPermission diagnostics and fix AOT build error
Log allowed directories alongside denied paths to help diagnose
permission mismatches. Replace JsonSerializer.Serialize with manual
string building to fix IL3050 AOT compilation error.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Extract fileName from permission requests
Some permission requests use 'fileName' instead of 'path'. Handle
this property to avoid false denials for file-level operations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* nownar nu1903 temporarily
* Log config dir in verbose mode alongside work dir
Include skilled/baseline label for consistency with the work dir log.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add skilled/baseline label to permission denial logs
Helps identify which run type triggered the denial when debugging.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix empty runLabel brackets and add labels to judge permission checks
Conditionally format the label suffix to avoid empty () when no
runLabel is supplied. Pass 'judge' and 'pairwise-judge' labels from
the respective judge call sites.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Allow pairwise judge to read from skilled work directory
The pairwise judge compares baseline and skilled runs in one session,
but previously only allowed reads from the baseline work directory.
Add additionalAllowedDirs parameter to CheckPermission and pass the
skilled work directory through PairwiseJudgeOptions.SkilledWorkDir.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix tests to match allow-by-default for unrecognized permission requests
CheckPermission was changed to allow requests with no extractable
path/command (allow-by-default), but four tests still expected the
old deny-by-default behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove NoWarn
* Pin discover job checkout action to full commit SHA
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Dan Moseley <danmose@microsoft.com>
* 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>
* 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>
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>
* 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>
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>
* 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>
Jan Krivanek