Commit Graph

145 Commits

Author SHA1 Message Date
Viktor Hofer 3fff1f2573 Skill-validator: perf, cancellation, and reliability improvements (#482)
* 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>
2026-03-31 13:58:28 +02:00
Viktor Hofer 7747fcb427 Move skill-validator docs under src/ and include in packages (#476)
* 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>
2026-03-30 14:00:12 +00:00
Jan Krivanek 75f72efb0e Fix token usage collection (#471)
* Fix token usage collection

* Ensure evaluate is run on schedule

* Fix PR title issue

* Update eng/dashboard/token-usage.js

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

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-03-30 13:03:58 +00:00
Copilot 447e7367c8 Move investigation prompt to workflow summary, rename Full Results link (#473)
* Initial plan

* Move investigation prompt to workflow summary, remove InvestigatingResults.md callout, rename Full Results link

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/b5b537b4-3d90-48ff-ab2d-3298660ccc97

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Revert Reporter.cs change per review feedback

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/e5845ec8-06b3-4ffa-b282-fd63aa2a467c

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>
2026-03-30 14:33:46 +02:00
Copilot d8d3f3d369 Fix duplicate entries in Skills Loaded (and Agents Invoked) columns of evaluation table (#462)
* Initial plan

* Fix duplicate entries in Skills Loaded column of evaluation table

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/2430176d-bf62-461d-bb79-781e34d7d084

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>

* Remove accidentally committed .nuget binary

Agent-Logs-Url: https://github.com/dotnet/skills/sessions/2430176d-bf62-461d-bb79-781e34d7d084

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
2026-03-30 10:56:58 +00:00
Dan Moseley e1863cc526 Improve investigation guide: PR number in prompt, document reject_tools, baseline-already-good pattern (#451) 2026-03-27 07:09:29 +00:00
Jan Krivanek 0e47ef6545 Add more static checks (#453) 2026-03-26 14:00:24 +00:00
Dan Moseley 604c46c8c6 Fix eval investigation docs: download command, schema accuracy, example script (#449) 2026-03-25 15:24:33 -06:00
leslierichardson95 7286c31118 Add skills for C# MCP Server Development (#317)
* Add C# MCP server skills (create, debug, test, publish)

Four new skills for the C# MCP server development lifecycle:
- mcp-csharp-create: Scaffolding with dotnet new mcpserver, tools/prompts/resources, transport config
- mcp-csharp-debug: MCP Inspector, VS Code integration, breakpoint debugging, logging
- mcp-csharp-test: Unit tests with ClientServerTestBase, integration with WebApplicationFactory, evals
- mcp-csharp-publish: NuGet packaging, Docker/Azure deployment, MCP Registry publishing

Each skill includes SKILL.md with progressive disclosure references/ and eval.yaml tests.

* Fix mcp-csharp-create eval: use regex assertions for combined attribute syntax

Replace scaffolding-heavy scenarios with implementation-focused ones that
test MCP-specific features (resources, prompts, logging). Fix assertion
patterns to match combined C# attribute syntax [McpServerTool, Description()]
instead of requiring standalone [McpServerTool]. Increase timeouts to 180s
to account for skill-reading overhead.

Validator result: passed=True, improvement=44.6% (threshold=10%)

* Add CODEOWNERS entries for MCP C# skills (create, debug, publish, test)

* Update plugins/dotnet/skills/mcp-csharp-test/references/test-patterns.md

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

* Update plugins/dotnet/skills/mcp-csharp-debug/SKILL.md

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

* Update plugins/dotnet/skills/mcp-csharp-publish/SKILL.md

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

* Update plugins/dotnet/skills/mcp-csharp-publish/references/nuget-packaging.md

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

* Update plugins/dotnet/skills/mcp-csharp-debug/SKILL.md

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

* Remove evaluation step and guide from mcp-csharp-test skill

* Add CODEOWNERS entries for dotnet-maui skills

* Update CODEOWNERS to add @artl93 as co-owner for mcp-csharp skills

* Replace parent-directory file references with backtick skill names

Cross-skill references used ../sibling/SKILL.md paths which the
validator rejects. Replace with backtick-quoted skill names to match
the convention used by other skills in the repo.

* Add known domains for MCP server development skills

Add domains referenced by the mcp-csharp-* skills:
- github.com/modelcontextprotocol/registry
- github.com/open-telemetry/semantic-conventions
- npmjs.com, xunit.net, fluentassertions.com, nugettest.org

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

* Update Docker commands to use placeholder for registry and image names

* moved mcp skills and their evals into dotnet-ai plugin

* cleaned up moved files to dotnet-ai

* cleaned up files

* Update .github/CODEOWNERS

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

* Update nuget-packaging.md

* Update repository URL format in nuget-packaging.md

* Add github.com/ to known-domains to allow truncated placeholder URLs

The URL regex truncates at angle brackets (invalid in URIs), so
placeholder URLs like https://github.com/<yourusername>/<repo>
become https://github.com/ which didn't match any path-scoped entry.
Adding github.com/ (with trailing slash) covers only this case without
broadly allowing all github.com URLs.

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

* Update Docker tag command to use placeholder for server name

* Update known domains and fix GitHub username placeholders in documentation

* Remove McpServerToolType from mcp-csharp-create eval

McpServerToolType is only needed with WithToolsFromAssembly(), which is
discouraged because it is not Native AOT compatible. Updated eval assertions
and rubrics to prefer WithTools<T>() instead.

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

* Improve eval resilience for mcp-csharp-create and mcp-csharp-debug

- Increase mcp-csharp-create timeouts from 180s (or default 120s) to 360s
  for all 3 scenarios. These scenarios consistently time out because the
  model spends time on bash exploration/scaffolding before writing code.

- Add explicit timeout to scenario 3 which was relying on the 120s default.

- Broaden mcp-csharp-debug Inspector rubric to accept both the concise
  single CLI command style and the step-by-step UI configuration walkthrough,
  since both are correct approaches.

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

* Update eng/known-domains.txt

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

* Broaden debug 'failing tool' rubric to accept multiple debugging approaches

The rubric criterion 'Shows how to attach a debugger' was too narrow.
The skilled answer correctly focused on the #1 cause (stdout pollution)
but scored low because it didn't show a specific 'attach to process' flow.
Broadened to accept any valid debugging approach: attaching, Debugger.Launch(),
or launch.json configuration.

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

* Fix 4 eval failures in mcp-csharp-debug and mcp-csharp-test skills

mcp-csharp-debug:
- Trim SKILL.md verbosity (183->160 lines) and mcp-inspector.md (67->54 lines)
- Add Diagnosing Tool Errors section covering debugger, output panel, Inspector, common culprits
- Rebalance debugging narrative away from stderr-only focus
- Move HTTP logging config to ide-config.md reference

mcp-csharp-test:
- Add eval/evaluations keywords to frontmatter for activation
- Add HTTP tool invocation test pattern (tools/call via WebApplicationFactory)
- Trim test-patterns.md bloat (remove Test Categories, Coverage, Input Validation)
- Create references/evaluations.md with qa_pair format and read-only/deterministic guidance

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

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Dan Moseley <danmose@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-25 14:17:57 -06:00
Dan Moseley b39fe8541b Add evaluation troubleshooting guide for AI agents (#441)
* 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>
2026-03-25 16:10:17 +00:00
Dan Moseley 20ecb423c0 Include actionable guidance in scenario timeout warnings (#438)
* Include actionable guidance in scenario timeout warnings

The timeout messages now show the actual timeout value and how to
increase it via the 'timeout' field in eval.yaml. This applies to
the console summary, per-scenario detail, and markdown/PR comment
reporters.

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

* Show actual timeout values in summary and markdown messages

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

* Address review feedback: nullable TimeoutSeconds, centralize default constant, fix grammar

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/skills/sessions/478c8093-31bd-42df-b8d0-f75e4263d6d4

* Delete .nuget/nuget.exe

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Co-authored-by: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com>
2026-03-25 08:54:35 +00:00
Jan Krivanek c94bc5cdc3 Add Ability to author agent evals (#436)
* Add ability to author agent evals

* Adjust
2026-03-25 08:08:58 +01:00
Jan Krivanek 19b6cadddf Move token usage data into separate branch (#431)
* 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
2026-03-24 13:19:47 +01:00
Jan Krivanek 22542a5d75 Track and show subagents invocations (#427)
* Show subagents invocations

* Address PR feedback: fix XML doc and sanitize agent names in markdown

- Update ExtractSubagentActivation XML doc to accurately describe that
  all subagent event types are counted (not just Started/Completed)
- Add markdown table cell sanitization for agent names in FormatSubagentCell
  to prevent injection via pipe chars or newlines in plugin-controlled content
- Extract SanitizeMarkdownTableCell helper for reuse
2026-03-24 11:01:20 +00:00
Viktor Hofer b92dde1854 Refactor skill-validator to vertical slice architecture (#429)
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>
2026-03-24 09:41:55 +00:00
Viktor Hofer d97128e5d9 Move static analysis checks from eval to check command (#425)
* Move static analysis checks from eval to check command

Remove SkillProfiler.AnalyzeSkill from the evaluate command's per-skill
flow. All spec conformance checks (name, description, compatibility,
body line count, file reference depth, token size warnings, etc.) are
already enforced by the check command — running them again in eval was
redundant.

The eval-specific 'skill name in eval prompt' check is extracted into
EvaluateCommand.ValidateEvalPrompts and kept in eval since it requires
eval config context. The evalConfig parameter is removed from
SkillProfiler.AnalyzeSkill along with the dead ResourceFileCount field.

Tests updated to exercise ValidateEvalPrompts directly.

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

* [WIP] [WIP] Address feedback: Remove ProfileWarnings from vedict model (#426)

* Initial plan

* Remove ProfileWarnings from verdict model and reporter output

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/skills/sessions/9b7c3d0a-1693-4238-a397-2200f83634b5

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ViktorHofer <7412651+ViktorHofer@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 <198982749+Copilot@users.noreply.github.com>
2026-03-23 20:40:27 +01:00
Viktor Hofer c4c26175e2 Integrate reference scanner into skill-validator CheckCommand (#422)
* 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>
2026-03-23 13:05:08 +00:00
Jan Krivanek fcfa85a221 Mock out exponential backoff unit test (#421) 2026-03-23 09:52:42 +00:00
Viktor Hofer a895c226b3 Fix agents field in plugin.json to use explicit file paths (#419)
* Fix agents field in plugin.json to use explicit file paths

Claude Code's manifest validator rejects directory paths in the agents
field. Replace directory references with explicit .agent.md file paths
in all affected plugins (dotnet-diag, dotnet-msbuild, dotnet-template-engine).

Fixes https://github.com/dotnet/skills/issues/416

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

* Validate that agents paths are explicit files, not directories

Claude Code's manifest validator rejects directory paths in the agents
field. Update PluginValidator to error when an agent path resolves to a
directory, guiding authors to use explicit .agent.md file paths instead.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-23 08:36:20 +00:00
Youssef Victor c4937f8f68 Add 'run_command_and_assert' assertion (#401)
* Add 'run_command_and_assert' assertion

* Rename for clarity

* Add tests

* Fix race condition in command output handling

* Fix async output race condition in RunCommandAndAssert

WaitForExit(TimeSpan) does not guarantee that async output event
handlers have completed. Add parameterless WaitForExit() call after
the timed wait to ensure stdout/stderr are fully captured before
checking their contents. Also handle the timeout case explicitly
by killing the process.

* Address PR review: extract CommandAssertionArgs, add regex matching, configurable timeout

- Extract command-specific fields from Assertion into a dedicated
  CommandAssertionArgs record type for cleaner separation of concerns
- Add ExpectedStdOutMatches/ExpectedStdErrorMatches for regex-based
  stdout/stderr assertions (matches type asserts)
- Make command timeout configurable via assertion-level 'command_timeout'
  field, falling back to the scenario timeout when not specified
- Show actual timeout value in timeout error messages
- Update EvalSchema parsing, EvaluateCommand call sites, and all tests
- Add new tests for regex matching and timeout configuration

---------

Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com>
2026-03-20 21:48:52 +00:00
Copilot a68fa8da18 Add Codex CLI installation docs to README (#408)
* Initial plan

* Add Codex CLI installation section to README and allow developers.openai.com domain

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Remove Codex CLI example per review feedback

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Potential fix for pull request finding

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

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-03-20 10:29:03 +01:00
Copilot 6face98ff6 Change agents and skills fields in plugin.json to array format for Claude Code CLI compatibility (#407)
* Initial plan

* Remove agents string field from plugin.json and handle array format in parser

Claude Code CLI validates `agents` as array-of-strings per its plugin manifest
schema. Our plugin.json files had `"agents": "./agents/"` (a string), which
caused "agents: Invalid input" validation errors on install.

Remove the field from the 3 affected plugin.json files — agents are still
discovered by convention from the `agents/` directory. Also make ParsePluginJson
resilient to both string and array formats for forward-compatibility.

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Remove `agents` string field from plugin.json for Claude Code CLI compatibility

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Change agents field to array format in plugin.json, update validator to read array-first with string fallback

Plugin.json files now declare agents as an array of file paths:
  "agents": ["./agents/foo.agent.md", "./agents/bar.agent.md"]

ParsePluginJson reads the array first (preferred, Claude Code schema),
falls back to a string path (legacy). PluginInfo gains an AgentPaths
field. Validator and discovery use the array when present, else fall
back to directory-based convention.

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Simplify agents to directory-path array form ["./agents/"], handle directories in array entries

Per Copilot CLI spec, agents field values are "Path(s) to agent directories"
— no need to list individual files. Use ["./agents/"] to mirror how
skills uses "./skills/". Updated validator and discovery to handle both
directory and file entries in the array.

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Delete .nuget/nuget.exe

* Apply suggestions from code review

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

* Convert skills field to array format in plugin.json, update validator to handle both forms

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Align AgentPaths null checks to use { Count: > 0 } pattern for consistency

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Change `agents` and `skills` fields in plugin.json to array format for Claude Code CLI compatibility

Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>

* Delete .nuget/nuget.exe

* Consolidate SkillsPath/SkillPaths and AgentsPath/AgentPaths into single array fields

Remove dual string/array fields from PluginInfo. The parser now
normalizes legacy string values into single-element arrays, so all
downstream code uses only SkillPaths and AgentPaths (IReadOnlyList<string>).
This eliminates all array-vs-string fallback branching.

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>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-03-20 10:27:28 +01:00
Jan Krivanek 68d83b6ea8 Add parallelism configuration options to eval.yaml (#410) 2026-03-19 19:08:02 +00:00
Jan Krivanek cc8010f357 Include judge tokens, fix summaries (#405) 2026-03-19 10:23:09 +01:00
Jan Krivanek defd58af6a Add proper skills isolation (copying and allowlisting) (#402)
* Add proper skills isolation (copying and allowlisting)

* Address PR review: harden isolation, skip symlinks, precompute allowlist

- Pass null skillPath in isolated mode so the original skill tree
  (including siblings) is not accessible; only staged copies are allowed.
- Harden CopyDirectory to skip symlinks/reparse points and verify that
  recursed paths stay within the source root.
- Precompute additionalAllowedDirs once at session creation instead of
  allocating on every permission request.
- Add IsolatedStagingDoesNotExposeOriginalOrSiblingSkills test that
  verifies both directory-level and permission-level isolation.

* Fix unit test
2026-03-18 21:28:01 +00:00
Jan Krivanek 30c19eae13 Add tokens usage dashboard and data collection (#385) 2026-03-18 20:04:46 +00:00
dependabot[bot] 243f35a4bd Bump the microsoft-and-system group with 4 updates (#397)
Bumps Microsoft.Bcl.Memory from 9.0.14 to 10.0.5
Bumps Microsoft.Data.Sqlite from 10.0.0 to 10.0.5
Bumps Microsoft.Extensions.FileSystemGlobbing from 10.0.4 to 10.0.5
Bumps System.CommandLine from 2.0.4 to 2.0.5

---
updated-dependencies:
- dependency-name: Microsoft.Bcl.Memory
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Data.Sqlite
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Extensions.FileSystemGlobbing
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: System.CommandLine
  dependency-version: 2.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Bcl.Memory
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Data.Sqlite
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Extensions.FileSystemGlobbing
  dependency-version: 10.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: System.CommandLine
  dependency-version: 2.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
2026-03-18 15:35:43 +00:00
Viktor Hofer c5435dd3da Make evaluate a subcommand with tests-dir as required argument (#398)
* 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>
2026-03-18 08:59:03 +00:00
Jeff Handley 646b5d93b0 Add Skill: Convert to Central Package Management (convert-to-cpm) (#75)
* Add convert-to-cpm skill for NuGet Central Package Management conversion

Authoring process:
- Researched the Agent Skills standard at agentskills.io and reviewed repo conventions
- Studied the existing csharp-scripts skill and test structure as a reference implementation
- Studied Microsoft's CPM documentation at learn.microsoft.com
- Authored SKILL.md with a 9-step workflow covering scope determination through validation
- Created 5 test scenarios across simple, moderate, and advanced tiers
- Performed dry-run evaluation against all scenarios, identifying 15 findings
- Applied all 15 fixes (4 gaps, 6 ambiguities, 5 improvements) to SKILL.md
- Re-evaluated: 50 OK, 0 gaps, 0 ambiguities, 1 test fixture improvement
- Updated test fixture to resolve the remaining finding
- Final validation: 254 lines, 0 errors, 0 warnings

Skill implementation (skills/convert-to-cpm/SKILL.md):
- 9-step workflow: scope, CPM check, audit, create/update props, update projects, handle properties, restore/validate, cleanup, summary
- Handles single projects, solutions, and full repositories
- Audits packages with per-project detail table including version conflicts and security advisories
- Flags CVE/security vulnerabilities and recommends patched versions
- Supports MSBuild property-based versioning with inline vs keep decisions
- Covers conditional PackageReference items across target frameworks
- Discovers PackageReference in imported .props/.targets via Import chain scanning
- Provides cross-platform commands (Unix and Windows/PowerShell)
- Documents import order requirements for property references in Directory.Packages.props
- Mentions dotnet new packagesprops as preferred creation method (.NET 8+)
- Includes validation checklist and common pitfalls table

Test scenarios (tests/convert-to-cpm/):
- simple-single-project: 1 project, 3 packages, no conflicts
- simple-solution: 3 projects via .sln, 5 packages, PrivateAssets preservation
- moderate-version-conflicts: 4 projects, System.Text.Json version conflicts with CVE-2024-43485 advisory, AutoMapper major version conflict with VersionOverride
- moderate-msbuild-properties: 2 projects using MSBuild properties for versioning in Directory.Build.props, inline vs keep decisions, property cleanup verification
- advanced-multi-complexity: 5 projects combining version conflicts, MSBuild properties, conditional TFM PackageReference, shared Common.props imports, VersionOverride, and security advisory handling

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

* Refactor SKILL.md with references, baseline build, and package comparison

Condensed SKILL.md using progressive disclosure:
- Reduced from 254 lines / ~3937 tokens to 120 lines / ~1778 tokens
- Extracted detailed procedural instructions into 5 focused reference files
- Main file retains workflow decision logic with links to references loaded on demand
- Follows agentskills.io progressive disclosure pattern: metadata -> instructions -> resources

Added baseline build verification (new Step 2):
- Requires clean build (dotnet clean && dotnet build) before any changes
- Captures baseline.binlog for structured build artifact
- Captures baseline-packages.json via dotnet list package --format json
- Blocks conversion if baseline build fails

Added post-conversion baseline comparison (Steps 8 and 10):
- Step 8 produces after-cpm.binlog and after-cpm-packages.json from clean build
- Step 10 compares baseline vs post-conversion package lists per project
- Changes table shows version bumps, VersionOverride entries, added/removed packages
- Unchanged table confirms version-neutral conversion for unmodified packages
- Binlog files produced as artifacts for user review in MSBuild Structured Log Viewer

Reference files created:
- references/audit-complexities.md: 7 complexity categories for package auditing
- references/baseline-comparison.md: dotnet list package comparison procedure, binlog production, artifact guidance, table formats for changes and unchanged packages
- references/directory-packages-props.md: File creation (dotnet new packagesprops), placement rules, conditional versions, VersionOverride patterns
- references/msbuild-property-handling.md: Property usage search, inline vs keep decisions, import order requirements, cleanup verification
- references/validation-and-errors.md: NuGet error codes (NU1008, NU1010, NU1507), clean build validation, multi-TFM guidance, common pitfalls

Workflow is now 10 steps (was 9):
1. Determine scope
2. Establish baseline build (new)
3. Check for existing CPM
4. Audit package references
5. Create or update Directory.Packages.props
6. Update project files
7. Handle MSBuild version properties
8. Restore and validate (now with binlog + package list capture)
9. Clean up obsolete properties
10. Summary and baseline comparison (expanded with diff tables)

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

* Replace third-party packages with Microsoft equivalents, add baseline/comparison steps, add .gitignore for test solutions

Package replacements across all skill references and test scenarios:
- Serilog.AspNetCore -> OpenTelemetry.Extensions.Hosting 1.15.0
- Serilog (standalone) -> OpenTelemetry.Extensions.Hosting 1.15.0
- Swashbuckle.AspNetCore -> Microsoft.AspNetCore.OpenApi 8.0.24
- SerilogVersion MSBuild property -> OTelVersion
- DIVersion updated from 8.0.1 to 9.0.0 (DI/Abstractions alignment)
- Microsoft.AspNetCore.OpenApi uses 8.0.24 (not 9.x) for net8.0 compat

Test fixture improvements:
- Added Step 2 (baseline build with binlog) to all 5 with-skill.md files
- Added Step 10 (package comparison table + binlog artifact links) to all 5 with-skill.md files
- Expanded simple-solution without-skill.md with more realistic agent behavior
- Standardized security fix terminology: table markers cite CVEs, prose references CVE IDs
- Removed space-padded table formatting in favor of compact markdown with emojis

Test solution infrastructure:
- Added tests/convert-to-cpm/.gitignore to exclude src/ build artifacts
- Added generation prompt to test README for recreating buildable solutions
- All 5 test solutions verified to restore and build successfully

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

* Address review feedback on terminology, placement, and reference accuracy

- Rename Step 10 and validation checklist from 'binlog comparison' to 'package list comparison'
- Rewrite baseline-comparison.md: title to 'Baseline Comparison', lead with dotnet list package as primary comparison mechanism, position binlogs as supplementary artifacts for manual review/troubleshooting
- Add brief definition of binlogs before the artifacts section
- Fix Directory.Packages.props placement guidance: use first common ancestor of all .NET projects instead of assuming repository root (.git), since many repos nest source under src/ or similar
- Update SKILL.md scope input and Step 1 to reference directory-scoped conversion
- Standardize security fix terminology: table markers cite CVEs, prose uses 'security fix for CVE-...'

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

* Address review findings: sort PackageVersion, add packages.config scenario, fix commands

Review findings addressed:
1. Fixed BlobsVersion grep step description in advanced scenario
2. Sorted PackageVersion entries alphabetically in all Directory.Packages.props outputs
3. Consolidated baseline-comparison.md to single-table format
4. Added solution file arguments and multi-line commands in test fixtures
5. Added Directory.Packages.props placement question for single-project scenario
6. Created simple-packages-config test scenario (packages.config rejection)
7. Use dotnet new packagesprops in simple-single-project workflow
8. Fixed More Info link text to match reference file title

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

* Fix test scenario accuracy: sln/slnx mix, package counts, ImplicitUsings

- Use mix of .sln and .slnx across scenarios (simple-solution and
  moderate-msbuild-properties use .sln; moderate-version-conflicts and
  advanced-multi-complexity use .slnx)
- Fix simple-solution package count from 5 to 6
- Fix advanced user response #2 to reference Azure.Storage.Blobs version
- Add ImplicitUsings to advanced Directory.Build.props in audit and cleanup
- Complete the advanced unchanged table with all expected package entries

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

* Add skill evaluation section to test README

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

* Fix Inventory.sln references and make evaluation opt-in

- Fix 4 occurrences of Inventory.sln to Inventory.slnx in
  moderate-version-conflicts/with-skill.md for consistency with the
  scenario README and test solution
- Update test README to only run skill evaluation when the user
  explicitly requests it

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

* Condense SKILL.md with progressive disclosure

- Remove duplicated bash code blocks from Steps 2 and 8, linking to
  baseline-comparison.md instead
- Merge Steps 7 (MSBuild property decisions) and 9 (cleanup) into a
  single step since both reference msbuild-property-handling.md
- Condense Step 10 summary to reference baseline-comparison.md for
  procedure details
- Reduces from 120 to 97 lines (10 steps to 9 steps)

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

* Fix Enterprise.sln references to Enterprise.slnx in with-skill.md

Address code review feedback: fix 4 occurrences of Enterprise.sln to
Enterprise.slnx in advanced-multi-complexity/with-skill.md for
consistency with the scenario README and test solution.

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

* Improve convert-to-cpm skill: preserve whitespace, save summary report, fix links

- Instruct Step 6 to retain each file's existing indentation style and
  blank lines to keep diffs minimal
- Instruct Step 9 to save the full summary as convert-to-cpm.md alongside
  binlog and JSON artifacts, suggesting its contents for a PR description
- Strip en-us/ from learn.microsoft.com links in SKILL.md and
  validation-and-errors.md to make them culture-agnostic
- Update all with-skill.md test expectations to include the convert-to-cpm.md
  create step and artifact mention

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

* Indicate that the binlog viewer is for the user; not the tool/skill

* Move convert-to-cpm skill and tests to src/dotnet-msbuild layout

Move skill from skills/convert-to-cpm/ to src/dotnet-msbuild/skills/convert-to-cpm/
and tests from tests/convert-to-cpm/ to src/dotnet-msbuild/tests/convert-to-cpm/
to match the repository layout established on main.

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

* Convert tests from with/without-skill format to eval.yaml

Replace manual test approach (README.md + with-skill.md + without-skill.md)
with automated eval.yaml scenarios containing assertions and rubric items.
Add committed test fixture files (.csproj, .sln, .slnx, .props) for each
scenario with copy_test_files setup, replacing the git-ignored generated
solutions.

6 scenarios converted:
- simple-single-project: basic single project CPM conversion
- simple-solution: multi-project solution conversion
- simple-packages-config: negative test (packages.config rejection)
- moderate-version-conflicts: version conflict resolution
- moderate-msbuild-properties: MSBuild property version handling
- advanced-multi-complexity: combined complexities (conflicts, properties,
  conditionals, shared props)

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

* Consolidate eval.yaml files into single root eval.yaml

Move all 6 scenario definitions from individual subdirectory eval.yaml
files into a single src/dotnet-msbuild/tests/convert-to-cpm/eval.yaml
file, matching the skill-validator discovery convention that expects
{testsDir}/{skillname}/eval.yaml. Scenario prompts updated to reference
subdirectory-prefixed paths since copy_test_files now copies all fixture
subdirectories.

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

* Reorganize convert-to-cpm into plugins/tests folders

* Add post-conversion reporting rubrics to CPM eval scenarios

Add rubric items for baseline-vs-result comparison reporting, binlog
capture and guidance, version reconciliation, and risk/reassurance
messaging across all conversion scenarios. Increase complex scenario
timeout to 360s.

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

* Improve post-conversion reporting, conflict resolution, and skill activation

SKILL.md:
- Surface binlog capture commands explicitly in Steps 2 and 8
- Expand Step 4 with detailed version conflict presentation and
  resolution workflow
- Restructure Step 9 into 5-section post-conversion report template:
  overview, conflict resolutions, baseline comparison, risk assessment,
  and artifacts/next steps with binlog education
- Broaden activation triggers in description and When to Use to match
  update/bump/upgrade/align/sync terminology from ecosystem PRs
- Add activation for out-of-sync/conflicting/mismatched version scenarios

audit-complexities.md:
- Expand version conflict section with per-conflict presentation format,
  4 resolution options with trade-offs, and risk-level guidance

eval.yaml:
- Pre-specify conflict resolution strategy in prompts to avoid blocking
  on unanswered user prompts during single-turn evaluation
- Add 3 new scenarios testing CPM recommendation when user asks to
  update/align packages without mentioning CPM
- Update rubrics to match directed resolution strategy
- Increase complex scenario timeout to 360s

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

* Address PR feedback from kartheekp-ms

- Use GitHub markdown URL for CPM docs reference (SKILL.md)
- Use dotnet package list (renamed in .NET 10) with backward compat
  note for dotnet list package (SKILL.md, baseline-comparison.md)
- Use dotnet package list --format json for auditing (SKILL.md Step 4)
- Document Directory.Packages.props sub-folder support for test
  projects with different dependencies (directory-packages-props.md)
- Remove security advisory version bumping from CPM conversion scope;
  instead surface advisories as follow-up items in the post-conversion
  report (audit-complexities.md, SKILL.md Step 9 Section 5)
- Add Follow-up items section to report template covering security
  advisories, deprecated packages, version alignment opportunities,
  and test validation (SKILL.md Step 9)

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

* Improve convert-to-cpm skill activation, artifact preservation, and eval assertions

- Broaden skill description to trigger on version drift/sync scenarios and
  MSBuild property contexts even when CPM is not explicitly mentioned
- Add explicit instructions to preserve binlog and JSON artifacts (not delete)
- Make convert-to-cpm.md report a mandatory deliverable, not optional
- Replace fragile output_contains assertions (ManagePackageVersionsCentrally,
  Directory.Packages.props) with file_contains checks on the actual generated
  files across 5 conversion scenarios

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

* Extend timeouts and add CODEOWNERS entry

* Move convert-to-cpm skill into a new dotnet-nuget plugin

* Fix erroneous plugin msbuild/nuget plugin split change. Add dotnet-nuget to marketplace.

* Add NuGet package management skills to README

* Address PR feedback: clarify CPM sub-folder semantics, remove security-fix example, require explicit conflict strategy

- directory-packages-props.md: Clarify that sub-folder Directory.Packages.props
  files are independent and replace (not extend) the parent; require explicit
  MSBuild Import to chain files
- baseline-comparison.md: Replace security-fix upgrade example with version
  conflict alignment example, consistent with audit guidance that security
  advisories are follow-up items
- SKILL.md: Remove default 'use highest version' conflict strategy; require
  explicit user choice or confirmation when conflicts are detected

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

* Extend eval timeouts for scenarios hitting time limits

- Convert single project: 180s -> 270s
- Recommend CPM (version conflicts): 180s -> 270s
- Recommend CPM (complex repo): 360s -> 540s
- Convert complex repository: 360s -> 540s

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

* Add missing CODEOWNERS for dotnet-nuget plugin. Reorder convert-to-cpm evals and expand timeouts.

* Remove containing skill mentions from reference files; fix ordering

* Replace output_matches with deterministic file assertions in eval.yaml

Convert 6 non-deterministic output_matches assertions to 8 file-based
assertions (file_contains, file_not_contains) across conversion scenarios:

- Scenario 5: Verify PrivateAssets preserved in Tests.csproj
- Scenario 6: Verify OTelVersion/DIVersion removed from Directory.Build.props
- Scenario 7: Verify System.Text.Json and Azure.Identity in Directory.Packages.props
- Scenario 8: Verify System.Text.Json in Directory.Packages.props and
  BlobsVersion/HostingVersion removed from Directory.Build.props

Recommendation-only scenarios and conversational assertions (conflict
detection, conditionals, Common.props awareness) remain as output_matches.

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

* Reorganize skill description with USE FOR / DO NOT USE FOR markers

Restructure the description to improve activation reliability by
adopting the USE FOR / DO NOT USE FOR pattern used by other skills
in the repo (thread-abort-migration, binlog-failure-analysis).

Key changes:
- Front-load activation cues before capability list
- Add explicit USE FOR / DO NOT USE FOR section markers
- Add .sln/.slnx file extensions for solution-path matching
- Move DO NOT USE exclusions into description (was only in body)
- Use YAML folded scalar for multi-line readability

All existing activation keywords preserved. Length reduced from
882 to 849 chars (max 1024).

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

* Fix issues for external link references

* Increase timeouts

* Handle packages.config non-activation in convert-to-cpm skill

Set expect_activation: false on the packages.config eval scenario so
non-activation is reported as expected rather than flagged as a warning.

Add a packages.config guard in Step 1 of the skill workflow so that if
the skill is activated for a packages.config project, it stops early and
recommends migrating to PackageReference first.

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

* Remove leading blank lines from .sln test files

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

* Use imperative 'Read' instead of 'See' for reference file links

Ensures the agent explicitly loads and reads reference files rather than
treating them as optional 'see also' links. This aligns with the repo
convention of explicitly instructing agents to load reference content.

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

* Simplify dotnet-nuget CODEOWNERS

* Simplify packages.config guard

* Add minimal Program.cs entrypoints to Web/Worker test fixtures

Test fixture projects using Microsoft.NET.Sdk.Web and
Microsoft.NET.Sdk.Worker require a Program.cs entrypoint for dotnet build
to succeed. The skill workflow instructs the agent to run dotnet restore
and dotnet build for validation, so these fixtures must be buildable.

Adds minimal entrypoints (with explicit using directives since
ImplicitUsings is not enabled in these projects) to 6 projects:
- simple-solution/Web
- moderate-version-conflicts/Api, Worker
- moderate-msbuild-properties/Api
- advanced-multi-complexity/Api, Web

Also adds Microsoft.Extensions.Hosting package reference to the Worker
project, matching what the dotnet new worker template produces.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
2026-03-18 08:41:57 +01:00
Viktor Hofer 1a20e39a8d Refactor skill-validator: add check command, rename validate to evaluate (#388)
* 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>
2026-03-17 17:00:47 +01:00
Larry Ewing 1f127d4239 Add --keep-sessions flag and rejudge command to skill-validator (#149)
* Add --keep-sessions flag and rejudge command

- Add --keep-sessions flag to preserve agent session data in SQLite DB
- Create SessionDatabase service with WAL mode and thread-safe writes
- Update AgentRunner to separate config/work dir lifecycle
- Track session metadata, RunMetrics, judge and pairwise results
- Add rejudge command to re-run judges on saved sessions
- Add 8 tests for SessionDatabase (CRUD, concurrency, isolation)

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

* Add prompt and skill_content columns to sessions.db

Store the eval scenario prompt and SKILL.md content in the sessions
table so they are available for browsing and replay without needing
the original skill directory.

Both columns are optional (nullable) for backward compatibility.

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

* Replace skill_content with skill_sha for change detection

Compute a SHA-256 hash (truncated to 12 hex chars) over all files in
the skill directory, sorted by relative path for determinism. This
gives lightweight change detection without storing potentially large
skill content in the database.

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

* Address PR feedback: relative config_dir, schema_info table

- Use session ID as config dir folder name (sessions/<id>) so the
  DB record links directly to its files on disk
- Store config_dir as relative path for portable results directories
- Add schema_info table with type='skill-validator' and version='1'
  for easy DB detection by external tools like dotnet-replay
- Add GetSchemaInfo() method for consumers
- Extract GetSessions(whereClause) for reuse
- Add tests for schema_info and relative config_dir

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

* Fix: share timestamped results dir between sessions and reporters

When --keep-sessions is active, pass the already-created timestamped
directory to Reporter.ReportResults so sessions.db, session files, and
report outputs (results.json, summary.md) all live in the same folder
instead of creating two separate timestamped directories.

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

* Adapt rejudge for current validator model

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

* Address PR #149 review feedback

Persist the judge model for rejudge, use fresh judge workdirs, tighten session DB hashing/foreign key behavior, and cache skill SHA calculation during validation.

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

* Fix rejudge rubric persistence

Persist scenario rubrics in sessions.db so rejudge can reconstruct the original EvalScenario rubric, self-migrate older databases to the new schema, and validate --judge-mode inputs explicitly.

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

* Guard SessionDatabase reads with semaphore

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

* Fix validator session cleanup and reporting

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-16 18:22:47 -05:00
Amaury Levé ab201ee691 Fix eval source file resolution to use eval directory (#374)
* Fix eval source file resolution to use eval directory

The SetupWorkDir method resolved setup file 'source:' paths relative to the
skill directory (plugins/.../skills/<name>/), but eval.yaml files live under
tests/. The ../../../../ relative paths resolved correctly but were rejected
by the path traversal security check because the resolved path escaped the
skill directory boundary.

Fix by preferring the eval directory as the base for source resolution when
evalPath is available, falling back to skillPath for backward compatibility
with scenarios embedded in SKILL.md files.

Also update run-tests eval.yaml source paths from
'../../../../tests/dotnet-test/run-tests/fixtures/...' to 'fixtures/...'
(relative to the eval directory where the fixtures actually live).

This was causing all 11 source-based run-tests scenarios to get zero fixture
files, making the agent find an empty workspace and fail to evaluate.

* Address review: extract ResolveSourcePath helper, add warning, fix remaining eval.yaml paths

- Extract source-resolution and path-traversal check into internal
  ResolveSourcePath helper for testability
- Emit a clear warning when file.Source is set but no base directory
  is available (instead of silently continuing)
- Add unit tests covering: evalPath-based resolution, skillPath fallback,
  null paths warning, traversal rejection, and evalPath preference
- Fix path traversal patterns in analyzing-dotnet-performance and
  clr-activation-debugging eval.yaml files (same issue as run-tests)
2026-03-16 16:24:45 +00:00
Dan Moseley a0f77070f5 Reduce false positives in reference scanner (#360)
- Skip HTTP-not-HTTPS inside fenced code blocks (code examples use
  http:// listen addresses). EXTERNAL-DOMAIN checks still run inside
  fences since agents see raw text, not rendered markdown.
- CommonMark-compliant fence tracking (type, length, closing-only
  whitespace, max 3-space indent).
- Skip placeholder/template URLs by checking the host portion only
  (not the full URL) to prevent bypass via path components.
- Case-insensitive placeholder matching.
- Generalize path-scoped domain matching to all domains (not just
  github.com), enabling entries like npmjs.com/package/@org.
- Treat wildcard listen addresses (http://+:port, http://*:port) as
  local URLs.
- Add xunit.net to known-domains.txt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-14 07:47:37 +01:00
Dan Moseley b966ac4dec Add structural external dependency checker (#355)
* Add structural external dependency checker for skills, agents, and plugins

Add ExternalDependencyChecker to the skill-validator that produces advisory
warnings for structural dependencies that need human review:

- Script files in skill scripts/ directories (.ps1, .sh, .py, .bat, .cmd, .bash)
- INVOKES patterns referencing external scripts in skill descriptions
- #tool: references to non-built-in tools in skill/agent content
- Non-built-in tools declared in agent frontmatter tools array
- MCP server declarations in plugin.json files

URL scanning is intentionally excluded — it is handled by the reference
scanner (eng/reference-scanner/scan.ps1) with its known-domains.txt allowlist.

Also adds Tools property to AgentInfo record and RawAgentFrontmatter to
support parsing the tools array from agent YAML frontmatter.

Based on dotnet/skills#337 by ManishJayaswal, trimmed to structural checks
only (URL scanning superseded by #336).

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

* Add allowlist support for external dependency checker

Introduce an allowlist file (eng/skill-validator/allowed-external-deps.txt)
so known external dependencies can be pre-approved. Each finding generates
a deterministic key (type:name:detail) that is checked against the list.
Allowed items are silently suppressed; unrecognized entries produce warnings.

The allowlist is pre-seeded with the 5 findings currently on main so the
checker runs clean out of the box. Over time, entries can be removed as
issues are resolved, and checks can be promoted to hard errors.

Adds 7 tests covering allowlist loading and filtering.

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

* Address review feedback: naming, dedup, test robustness

- Rename 'errors' variables to 'findings' in all Check* methods to
  reflect advisory (warning) semantics, not hard failures.
- Rename test methods from FlagsError to FlagsWarning, NoError to
  NoWarning for consistency.
- De-duplicate #tool: reference warnings so repeated mentions of the
  same tool in a skill/agent produce only one finding.
- Use GUID-based temp path instead of hardcoded '/nonexistent/path.txt'
  in LoadAllowList_MissingFile test.
- Add 2 dedup tests (skill and agent) bringing total to 379.

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

* Fix comments: tool refs scan full content, not just body

Update comments in CheckSkill and CheckAgent to accurately reflect
that #tool: references are scanned in the full markdown content
(including frontmatter), not just the body.

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

---------

Co-authored-by: ManishJayaswal <ManishJayaswal@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-13 10:25:13 -06:00
Jan Krivanek 6666c9e5e8 [Infra] Add validation with whole plugin loaded (#316)
* Add plugin-aware skill validation with activation detection

* Address Copilot review comments (round 4)

- DESIGN doc: mark Targeted Skill Activation TODO as implemented
- ValidateCommand: fix misleading --plugin-dir comment
- Comparator: set PluginScore to null when no plugin runs exist
- ComparatorTests: copy missing fields in rehydrated ScenarioComparison
- AgentRunner: fix GetPluginClient docstring, route RunAgentCore through it
- AgentRunner: stage isolated skill to temp dir for true single-skill isolation
- dashboard.js: fix triple chart comment to match actual series names
- RunnerTests: update assertions for new isolated skill staging behavior

* Remove implementation doc
2026-03-12 22:44:36 +00:00
Abhitej John bb974f7360 Security Update (#331)
* 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>
2026-03-12 18:00:55 +00:00
Dan Moseley 64464e87e3 Add external reference scanner for skill/agent content (#336)
* 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>
2026-03-12 10:43:19 -06:00
Jan Krivanek 676523ca42 Bump dependency (#345) 2026-03-12 16:28:28 +00:00
Korolev Dmitry a2be8cce61 feat: noise-test evaluation option (#310)
* use BPE tokenizer

* noise-test impl

* address metrics calculation

* address copilot comments

* more separation & refactor

* cleanup safely

* nit
2026-03-11 20:39:15 +01:00
dependabot[bot] f8d6e3170f Bump the microsoft-and-system group with 2 updates (#324)
Bumps Microsoft.Extensions.FileSystemGlobbing from 10.0.3 to 10.0.4
Bumps System.CommandLine from 2.0.3 to 2.0.4

---
updated-dependencies:
- dependency-name: Microsoft.Extensions.FileSystemGlobbing
  dependency-version: 10.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: System.CommandLine
  dependency-version: 2.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: Microsoft.Extensions.FileSystemGlobbing
  dependency-version: 10.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
- dependency-name: System.CommandLine
  dependency-version: 2.0.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: microsoft-and-system
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
2026-03-11 09:43:38 +01:00
dependabot[bot] b482146c0a Bump the all-other-nuget group with 1 update (#325)
* Bump the all-other-nuget group with 1 update

Bumps GitHub.Copilot.SDK from 0.1.30 to 0.1.32

---
updated-dependencies:
- dependency-name: GitHub.Copilot.SDK
  dependency-version: 0.1.32
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: all-other-nuget
- dependency-name: GitHub.Copilot.SDK
  dependency-version: 0.1.32
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: all-other-nuget
...

Signed-off-by: dependabot[bot] <support@github.com>

* React to PermissionRequestResultKind breaking change in GitHub.Copilot.SDK 0.1.32 (#326)

* Initial plan

* Fix PermissionRequestResultKind breaking change in SDK 0.1.32

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>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
2026-03-11 09:41:39 +01:00
Korolev Dmitry 484d5d2c1f tokenization: use BPE instead of chars\4 approximation (#309)
* use BPE tokenizer

* address PR comments 1

* elaborate on model

* nit
2026-03-10 20:16:57 +01:00
Dan Moseley 70ea8cf225 Include error details in evaluation failure PR comments (#297)
* Include error details in evaluation failure PR comments

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

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

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

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

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

This commit should be reverted after verifying the error display.

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

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

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

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

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

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

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

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

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

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

* Sanitize @ in error output to prevent mention injection

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

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

* Distinguish errors vs warnings in evaluation failure comments

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

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

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

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

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

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

* Remove duplicate failure comment from fork workflow

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

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

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

This reverts commit 5f9056695a.

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

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

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

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

Removed redundant comments regarding failure comments for fork PRs.

* Refine comments on eval prompts and skill discovery

Clarify comment regarding eval prompts and skill discovery errors.

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

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

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

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

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

This reverts commit 331ed870c5.

* Fix test assertion to match updated markdown error format

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

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

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

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

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

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

* Link to newest results-table comment instead of oldest

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

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

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

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

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

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

* Guard consolidation step against missing validator binary

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

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

* Skip results table header when there are no scenario rows

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

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

* Display errors as bullet points in markdown summary

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

* Show validation errors with status emoji in heading

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

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

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

This reverts commit 59d5d8dfd4.

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

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

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

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-03-10 12:51:12 +01:00
Viktor Hofer b6bc33c926 Purge dashboard entries older than 14 days (#304)
* Purge dashboard entries older than 14 days

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

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

* Move stale file cleanup to workflow level

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

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

* Move stale file purge logic into the script

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

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

* Make retention days configurable via workflow env var

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

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

* Address review feedback: fix comment and add entries guard

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

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

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

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

* Update .github/workflows/evaluation.yml

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-03-10 11:00:19 +01:00
Dan Moseley abdd4d03d4 Improve evaluation results table readability and add verdict explanations (#299)
* Add verdict footnotes to evaluation results table

The results table showed a Verdict column (pass/fail) based on a weighted
composite score, but only displayed the raw quality delta. When the
composite disagreed with quality (e.g., quality improved but token/time
overhead made the composite negative), the verdict was unexplained.

Add a Notes column with footnote references [1], [2] etc. that link to
explanations below the table. Footnotes are only added when the verdict
disagrees with the quality delta direction, keeping the table clean for
straightforward cases.

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

* TEST: inflate csharp-scripts description to exercise verdict footnotes

Expands the skill description to trigger token overhead in evaluation,
which should produce a scenario where the verdict footnote feature
activates. **Revert before merge.**

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

* Address review feedback on BuildVerdictFootnote

- Return null for neutral verdicts (ImprovementScore == 0) and unknown
  quality delta (null), avoiding misleading footnotes for the yellow
  neutral icon or when the table shows delta as dash.
- Align quality direction threshold with the F1 rounding used in the
  delta column so footnotes only fire when the displayed value disagrees
  with the verdict.
- Use DefaultWeights.Values instead of hard-coded weight literals to
  avoid drift if weights change.

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

* Fix roundedDelta==0 footnote bug and improve test coverage

- Return null from BuildVerdictFootnote when rounded quality delta is zero,
  preventing misleading 'offset quality gain' text when the table shows 0.0
- Make test ImprovementScore values consistent with breakdown * DefaultWeights
- Add test for 'quality dropped but composite positive' branch
- Add test for roundedDelta==0 edge case

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

* Improve results table: collapse quality columns and clarify footnotes

- Merge Baseline, With Skill, and delta columns into a single Quality
  column showing 'base -> skill' with the better score bolded and an
  arrow indicator, reducing table width for narrow viewports
- Reword footnotes from cryptic 'composite -X% -- factors offset
  quality gain' to readable 'Quality improved but weighted score is
  -X% due to: factors'

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

* Show raw metric values in verdict footnotes

Footnotes now include the actual baseline and skill values for each
contributing metric, e.g.:
  tokens (1000 -> 8000), tool calls (5 -> 15), time (2.1s -> 8.5s)

This lets readers immediately see the magnitude of each factor without
digging into workflow logs.

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

* Use emoji arrows, align footnote gating with Quality column

- Replace tiny unicode arrows with emoji (📈/📉) in Quality column
- Use raw quality delta for footnote direction instead of rounded delta,
  matching FormatQualityCell which compares unrounded scores
- Rename test to MarkdownTable_NoFootnoteWhenQualityUnchanged with
  truly equal scores instead of near-zero delta

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

* Make footnote references bidirectional hyperlinks

Table [1] links jump to the footnote explanation at the bottom,
and footnote [1] links jump back to the table row.

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

* Restore dash assertion in ShowsDashWhenNoOverfitting test

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

* Fix flaky ReturnsEmptyForNonSkillDirectory test

Use a fresh empty temp directory instead of Path.GetTempPath() which
can contain stray SKILL.md files left by other tests or processes.

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

* Use <strong> instead of ** for bold inside HTML anchor tags

GitHub markdown does not render ** inside raw HTML elements.

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

* Merge Notes into Verdict column and add footnote for quality-unchanged cases

- Remove Notes column (7 -> 6 columns) to reduce table width; footnote
  reference now appears next to the verdict icon
- Add footnote when quality is unchanged but verdict is negative, with
  text: 'Quality unchanged but weighted score is -X% due to: ...'
- Update tests accordingly

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

* Revert "TEST: inflate csharp-scripts description to exercise verdict footnotes"

This reverts commit d86dee0eee.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
2026-03-10 09:27:13 +01:00
Jan Krivanek dcdad225a9 Fix 429 rate limiting in evaluation pipeline (#284)
* Fix 429 rate limiting in evaluation pipeline

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

Fixes dotnet/skills#168

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

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

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

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

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

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

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

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

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

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

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

* Remove unused agents path from plugins that have no agents

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

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

* Promote agent/plugin name validation from warnings to errors

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

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

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

* Address PR review feedback

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

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

* Detect orphaned test directories with no matching plugin/skill

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

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

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

* Address PR #279 review feedback

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

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

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

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

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

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

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

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

* Surface execution errors in validation summary

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

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

* Reduce parallelism for infra-change evaluation runs

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

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-08 15:40:06 +01:00
Dan Moseley 4cc0e46dc1 Promote skill spec conformance checks from warnings to errors (#278)
* Promote skill spec conformance checks from warnings to errors

The agentskills.io spec (https://agentskills.io/specification) uses
'Must' for all of these constraints:

- name: 'Must be 1-64 characters'
- name: 'May only contain lowercase alphanumeric and hyphens'
- name: 'Must not start or end with -'
- name: 'Must not contain consecutive hyphens'
- name: 'Must match the parent directory name'
- description: 'Must be 1-1024 characters' (non-empty)
- compatibility: 'Must be 1-500 characters if provided'

These were previously warnings following the lenient-loading guidance
from the client implementation guide, but that guidance is for client
loaders, not publishing validators. For a CI tool that gates what gets
published, spec violations should be blocking errors.

All 25 existing skills already comply — no changes needed to skills.

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

* Address PR review feedback: empty compatibility, empty name, test

- Error when compatibility field is provided but empty/whitespace,
  matching spec requirement of 1-500 characters when provided.
- Add explicit check for empty/whitespace name with targeted error
  message instead of falling through to regex mismatch.
- Add test for empty compatibility string rejection.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-07 09:36:44 +01:00
Dan Moseley 698b335ab2 Enforce description size limits and add spec conformance checks (#238)
* Enforce description size limits (per-skill 1024, per-plugin 15K)

Add infrastructure to reject skills with oversized descriptions, per issue #222:

- SkillProfiler: check description length, flag DescriptionTooLong
- ValidateCommand: block oversized descriptions (survives --verdict-warn-only)
- New eng/validate-descriptions.ps1: validates per-skill and per-plugin aggregate limits
- New CI workflow: runs validation on PRs touching plugins/
- build.ps1: adds size checks to local build validation
- Unit tests for all new SkillProfiler behavior

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

* Update --verdict-warn-only help text to mention description_too_long

Address review feedback: the option description now lists description_too_long
alongside execution errors and --require-evals as conditions that remain
fatal even when --verdict-warn-only is enabled.

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

* Add agentskills.io spec conformance checks to SkillProfiler

Validate name rules (max 64 chars, lowercase+hyphens, no edge/consecutive
hyphens, must match directory), compatibility field max 500 chars, body
max 500 lines, and file reference depth.

Scan of existing skills found 2 violations:
- dotnet-msbuild/msbuild-antipatterns: 582 lines (max 500)
- dotnet-msbuild/directory-build-organization: cross-skill ref 3 levels deep

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

* Address review feedback on spec conformance checks

- Fix MakeSkill SkillMdPath to derive from resolved path (SkillProfileTests.cs)
- Fix RepoRoot in build.ps1 to be actual repo root (was agentic-workflows/)
- Fix body line count off-by-one: trim trailing newlines before splitting
- Fix file ref depth: normalize paths, handle .. traversals, strip #anchor,
  compute depth from directory segments only
- Add tests for parent traversal, anchor fragments, ./ normalization,
  and trailing newline body count

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

* Consolidate description validation into skill-validator C#

Move the per-plugin aggregate description limit (15K chars) from the
standalone PowerShell script into ValidateCommand, where it runs after
skill discovery. This eliminates the separate validate-descriptions.ps1,
its CI workflow, its PowerShell tests, and the delegation from build.ps1.

Also rename FailureKind from "description_too_long" to the generic
"spec_conformance_failure" so future spec checks reuse the same kind.

Deleted:
  - eng/validate-descriptions.ps1
  - eng/tests/validate-descriptions.tests.ps1
  - .github/workflows/validate-descriptions.yml

Added:
  - ValidateCommand.CheckAggregateDescriptionLimits()
  - ValidateCommand.DerivePluginName()
  - 6 C# tests for aggregate limit + DerivePluginName

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

* Upgrade body-lines and file-ref-depth from warnings to blocking errors

Now that #247 has merged and fixed the two existing violations
(msbuild-antipatterns 582 lines, directory-build-organization cross-ref),
these spec conformance checks can be blocking.

Also replaces the DescriptionTooLong bool with a general Errors list on
SkillProfile, so all spec conformance violations (description length,
body lines, file ref depth) flow through the same mechanism.

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

* Revert unrelated lock file changes

The lock file diffs were side effects of running build.ps1 to verify
the validation changes — they picked up upstream content changes
unrelated to this PR.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-07 00:16:33 -07:00
Jan Krivanek e392d3cfde Fix flaky test (#258)
* Fix flaky test

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

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

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2026-03-06 12:32:53 +01:00