* Add skill-coverage PR workflow with slash command support
- Auto-triggers on PRs that change plugins/** or tests/**
- Detects specific plugin/skill pairs from the PR diff
- Runs Measure-SkillCoverage.ps1 and posts a coverage report as a PR comment
- Supports /skill-coverage slash command on PR comments:
/skill-coverage (runs on changed skills in the PR)
/skill-coverage <plugin> (runs on all skills in the plugin)
/skill-coverage <plugin> <skill> (runs on a specific skill)
* Address Copilot review: security, permissions, and robustness fixes
- Add permission check: require write access before running slash command
- Tighten /skill-coverage matching to avoid false triggers on similar commands
- Add issues: write permission (PR comments use Issues API)
- Use refs/pull/<num>/head for checkout to support fork PRs
- Remove unused \ variable
- Fix multi-line JSON parsing with Out-String
* Address PR #538 review: fork PR handling, reactions, and sourcing note
- Skip PR comment posting for fork PRs (read-only token); coverage still runs in step summary
- Add continue-on-error to reaction step to avoid 403 failures
- Add header comment noting issue_comment trigger sources from default branch
- Add continue-on-error to no-changes comment step
* Split into analyze + comment jobs; fix pagination and dedup
Security:
- Split into two jobs: analyze (contents: read only) and comment (write only)
- Analyze job checks out base branch for trusted tooling, PR head into
a separate git worktree - untrusted PR code is never executed
- Comment job only reads an uploaded artifact, no PR code checkout
Fixes:
- Use github.paginate() for listComments to handle PRs with >100 comments
- Deduplicate targets: plugin-wide changes replace per-skill targets for
that plugin instead of producing duplicates
* Copy PATs rotation from dotnet/core
* Recompile agentic workflows with gh-aw v0.67.1
Remove unsupported 'failure' safe-output (dropped in gh-aw v0.67.1).
Regenerate lock files with the new compiler version.
No functional changes.
* Add Copilot PAT pool rotation to agentic workflows
Use the select-copilot-pat action to randomly select from a pool of
COPILOT_GITHUB_TOKEN / COPILOT_GITHUB_TOKEN_2..8 secrets, distributing
Copilot API load across multiple PATs.
This is a stop-gap until organization-level billing is available for
Agentic Workflows. See .github/actions/select-copilot-pat/README.md.
* 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>
* Add stale PRs agentic workflow
* Add fork-guard to close-stale-prs workflow
Skip scheduled runs on forked repositories to avoid consuming fork
owner minutes and failing due to missing secrets/context, matching
the pattern used by other scheduled workflows in this repo.
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 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>
Reorganize the skill-validator project from layer-based grouping
(Commands/, Services/, Models/, Utilities/) to feature-based slices
(Check/, Evaluate/, Consolidate/, Shared/).
Key changes:
- Check/: CheckCommand, SkillProfiler, AgentProfiler, PluginValidator,
ExternalDependencyChecker, ReferenceScanner
- Evaluate/: EvaluateCommand, RejudgeCommand (now a subcommand of
evaluate), AgentRunner, Judge, PairwiseJudge, and other eval services
- Consolidate/: ConsolidateCommand
- Shared/: Models, SkillDiscovery, PluginParser (extracted from
PluginValidator), Reporter, and utilities
Split PluginValidator: parsing helpers (ParsePluginJson,
TryGetSafeSubdirectory) moved to Shared/PluginParser.cs; validation
logic stays in Check/PluginValidator.cs.
Move RawFrontmatter and RawAgentFrontmatter types from EvalSchema into
SkillDiscovery since they are only consumed by discovery.
Wire RejudgeCommand as subcommand: skill-validator evaluate rejudge.
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Integrate reference scanner into skill-validator CheckCommand
- Port reference scanner logic from scan.ps1 to C# ReferenceScanner service
- Add --known-domains option to CheckCommand for reference scanning
- Move known-domains.txt and allowed-external-deps.txt to eng/
- Remove standalone scan.ps1 and reference-validation.yml workflow
- Reference scanning now runs as part of skill-check.yml via skill-validator
- Add 34 unit tests for ReferenceScanner (all 463 tests pass)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR feedback: repoRoot derivation, early validation, .agents scanning, SRI policy
- Derive repoRoot from --known-domains file path instead of cwd
- Fail early with clear error when known-domains file is missing
- Add .agents/**/*.md scanning to DiscoverFiles (regression fix)
- Fix SRI policy loophole: SRI suppresses SCRIPT-NO-SRI only, domain
and HTTPS checks still apply
- Use actual known-domains file path in error messages instead of
hardcoded eng/known-domains.txt
- Update doc comments to reflect full set of scanned paths
- Add tests for .agents discovery and SRI domain enforcement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove repo-specific paths from ReferenceScanner
DiscoverFiles now only scans directories it is given -- no hardcoded
knowledge of .agents, agentic-workflows, eng, or README.md. The tool
is repo-agnostic; callers control what gets scanned via --plugin dirs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Drop repoRoot from ReferenceScanner API
ScanFile/ScanFiles no longer take a repoRoot parameter. Finding paths
use the file path as provided by the caller, keeping the scanner
completely decoupled from any repo layout assumptions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Prune stale domains and add json.schemastore.org to known-domains.txt
Removed 10 entries no longer referenced by any plugin content:
agentskills.io, visualstudio.com, mcr.microsoft.com, dotnet.github.io,
github.com/dotnet/docs, github.com/dotnet/dotnet-api-docs,
github.com/dotnet/skills, github.com/github/gh-aw,
github.com/modelcontextprotocol/csharp-sdk, developers.openai.com.
Added json.schemastore.org (referenced by dotnet-template-engine).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix case-insensitive regex, update docs, add tests
- Make UrlRegex and PipeToShellRegex case-insensitive to match the
original PowerShell behavior (HTTP://, CURL, Wget all now detected)
- Update DiscoverFiles doc comment to remove stale 'individual files' claim
- Narrow CONTRIBUTING.md scope description to match actual plugin-only scanning
- Add tests for mixed/upper-case URL and pipe-to-shell detection
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove IPv6 loopback support, fix pipe-to-shell allowlist case sensitivity
- Remove Ipv6LoopbackRegex and IPv6 test data (not needed)
- Add case-insensitive + culture-invariant matching for pipe-to-shell allowlist
- Add mixed-case allowlist test
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Make evaluate a subcommand with tests-dir as required argument
Change EvaluateCommand from RootCommand to a regular Command('evaluate')
so it is a subcommand like check, consolidate, and rejudge. Convert
--tests-dir from an optional Option to a required positional Argument
placed after the skill paths.
Update .csproj RunArguments, evaluation-run workflow, README, and
CONTRIBUTING.md to reflect the new CLI syntax.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Default --parallel-skills/scenarios/runs to 3
Update the default from 1 to 3 for all three parallelism options and
remove the now-redundant overrides from RunArguments in the .csproj.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Replace RunArguments with launchSettings.json, publish in skill-check
Add Properties/launchSettings.json with default evaluate args for local
development. Remove RunArguments from .csproj. Update skill-check.yml to
publish the validator and invoke the binary directly instead of using
dotnet run.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Keep --tests-dir as a required option, not positional
A variadic paths argument greedily consumes all positional values,
making a trailing positional tests-dir impossible to parse. Keep
--tests-dir as a named required option which matches the previous UX.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Move testsDirOpt after resultsDirOpt, drop evaluate from CONTRIBUTING examples
Reorder testsDirOpt declaration and registration to follow resultsDirOpt.
Remove the evaluate subcommand from CONTRIBUTING.md dotnet run examples
since the launch profile already provides it.
Co-authored-by: Copilot <223556219+Copilot@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>
* Initial plan
* Fix release job being skipped due to tainted dependency chain from discover
The release job was always skipped for schedule and workflow_dispatch
events because the discover job (a transitive dependency via build) is
skipped for non-PR events, which taints the dependency chain. Without
always() in its if condition, GitHub skips release without evaluating
the expression.
Add always() so the if expression is always evaluated, and add an
explicit needs.build.result == 'success' guard so the release only
runs when the build actually passed.
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>
* Refactor skill-validator: add check command, rename validate to evaluate
- Add new 'check' command for CI skill validation (frontmatter, structure)
- Rename ValidateCommand to EvaluateCommand to clarify its purpose
- Add skill-check.yml GitHub Actions workflow
- Refactor SkillDiscovery for improved skill detection
- Simplify OverfittingJudge and SkillProfiler
- Update tests to match refactored APIs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix check subcommand: use EvaluateCommand as root command
System.CommandLine 2.0.4 requires the primary command to be the
RootCommand. Restores the upstream pattern where the evaluate/validate
command is the root and check/consolidate/rejudge are subcommands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Pass all plugins in single check invocation
Build plugin args dynamically and pass them all at once instead of
looping per plugin.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review comments
- Remove 'evaluate' subcommand token from README, evaluation-run.yml,
and RunArguments in csproj (evaluate is now the root command)
- Wrap ParsePluginJson in try/catch for JsonException with clean error
- Add CheckCommandTests with aggregate description limit coverage
(under/at/over limit and multi-plugin cases)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+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>
When pwsh parses git diff --name-only "origin/main"...HEAD, the
partially-quoted argument is tokenized incorrectly, causing git to
receive malformed arguments and print its usage message.
Build the range string in a variable first so it is passed to git as
a single token.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a pull_request_target workflow that posts an advisory comment
when a fork PR is opened that modifies eng/ or .github/ files,
suggesting the author recreate the PR from an upstream branch.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PR-mode code path used --depth=1 when fetching the base branch,
which prevented git from finding the merge-base for the three-dot diff.
The git diff failure was silently swallowed because its stderr output
was piped through Where-Object and filtered out, causing the scanner
to report success without scanning anything.
Fix:
- Remove --depth=1 from git fetch (checkout already uses fetch-depth: 0)
- Check LASTEXITCODE after both git fetch and git diff
- Separate git diff from the Where-Object filter so exit code is captured
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>
* Add external reference scanner for skill/agent content
- PowerShell scanner (eng/reference-scanner/scan.ps1) checks for:
- http:// URLs (blocked except localhost and XML namespace URIs)
- External <script> tags without SRI integrity attribute
- Pipe-to-shell patterns (except dotnet install scripts)
- URLs referencing domains not in .github/known-domains.txt
- CI workflow runs on PRs and pushes touching skill/agent paths
- CONTRIBUTING.md updated with external references policy
- README.md: fixed http:// to https:// for agentskills.io
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Move known-domains.txt to eng/reference-scanner/, fix comments, add ollama.com
- Moved from .github/ to eng/reference-scanner/ (co-located with scanner)
- Updated header comments: errors not informational notices, removed misleading org-scoping language
- Added ollama.com (pre-existing reference in dotnet-ai skill)
- Updated all references in scan.ps1, CI workflow, CONTRIBUTING.md
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review feedback
- Add ? and # as valid URL terminators in path-scoped domain matching
- Strip # from host extraction in Test-KnownDomain and Test-HttpNotHttps
- Make script/SRI regex patterns case-insensitive
- Emit error finding on file read failure instead of silent skip
- Fix help comment (two levels up, not three)
- Unify CONTRIBUTING.md error language (all checks are CI-blocking)
- Add eng/reference-scanner/** to workflow path triggers
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add cdn.jsdelivr.net to known-domains (used by dashboard after #331 adds SRI)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove cdn.jsdelivr.net from known-domains
jsdelivr serves arbitrary npm packages -- keeping it off the allowlist
means any use requires both SRI and reviewer approval (two gates).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Skip domain check for scripts with SRI, fix single-line file handling
Scripts with integrity attributes are secured by hash -- the domain
is irrelevant. This means cdn.jsdelivr.net does not need to be in
known-domains.txt as long as SRI is present.
Also wrap Get-Content in @() to handle single-line files correctly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address round-2 review feedback
- Multi-line <script> tag detection: join file content so SRI attributes
spanning lines are correctly detected
- Safe relative path: use GetRelativePath() with fallback instead of
bare Substring that throws for out-of-repo files
- Remove dead notice plumbing: all findings are errors now
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Scan only changed files on PRs, full scan on push to main
PRs scan only changed .md/.html files so pre-existing issues on main
don't block unrelated PRs. Push to main runs the full -All scan.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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>
Jan Krivanek