Commit Graph

6 Commits

Author SHA1 Message Date
YuliiaKovalova be7b56028c Fix Codex discovery of the dotnet-msbuild binlog MCP server (#1070)
* Fix Codex discovery of the dotnet-msbuild binlog MCP server

.codex-plugin/plugin.json declared "mcpServers": "./.mcp.json", but the file
was packaged at .codex-plugin/.mcp.json. Codex resolves manifest resource
paths against the plugin root, so it looked for
plugins/dotnet-msbuild/.mcp.json and never found the binlog server.

Declare the server inline in .codex-plugin/plugin.json, matching the root
plugin.json and .claude-plugin/plugin.json, and drop the unreachable file.

Add a packaging regression check to skill-validator: every companion manifest
must declare the same MCP servers as the root plugin.json, and a manifest
referencing an external .mcp.json must resolve it from the plugin root the way
hosts do. skill-check.yml already runs `skill-validator check` over plugins/*
on every PR, so this now blocks in CI. A test also loads the shipped
dotnet-msbuild manifests and asserts binlog is present in each.

Fixes #1069

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

* Report non-object JSON roots as validation errors

JsonElement.TryGetProperty throws InvalidOperationException when the root
value is not an object, so a manifest or referenced .mcp.json that is valid
JSON but not an object (null, array, string) crashed skill-validator instead
of producing a validation error.

Check the root kind while reading and surface it as a structured error.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-08-27 17:58:26 +00:00
Amaury Levé 592a53008e skill-validator: restore 15K aggregate cap as the real Copilot CLI skill-menu budget (#803)
* skill-validator: restore 15K aggregate cap, document it as the real Copilot CLI skill-menu budget

The per-plugin aggregate description cap had been raised 15,000 -> 20,000
-> 22,000 under the belief that 15K was 'a local repo policy, NOT a
documented Copilot constraint'. That belief was wrong: the GitHub Copilot
CLI renders the model-facing <available_skills> menu under a hard 15,000-
char budget (the agent SDK's SKILL_CHAR_BUDGET, default 15e3, confirmed in
CLI 1.0.36 and 1.0.61). Skills are listed alphabetically and emitted with
their full <description> only until the budget is exhausted; every skill
past the cut-off collapses to a bare name with no description and can no
longer be reliably model-activated. Raising the validator cap merely
masked this silent menu truncation — e.g. dotnet-test's run-tests and
test-* skills stopped activating in plugin eval runs because they fell
into the name-only overflow.

Changes:
- SkillProfiler.MaxAggregateDescriptionLength: 22,000 -> 15,000, with the
  comment rewritten to document the real Copilot CLI budget (and correct
  the prior 'not a documented constraint' claim).
- CheckCommand aggregate now excludes skills marked
  'disable-model-invocation: true' — the CLI drops those from the menu, so
  they do not consume the budget. This makes the cap satisfiable by hiding
  reference / agent-orchestrated primitives rather than only by trimming.
- InvestigatingResults.md: document plugin-arm-only non-activation caused
  by skill-menu budget overflow, and how to fix it.

Note: dotnet-test currently exceeds 15K and must be slimmed below it
(via disable-model-invocation on reference/primitive skills plus
description trims) before this cap can go green repo-wide.

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

* skill-validator: use source-generated regex for disable-model-invocation check

Address review: replace Regex.IsMatch(pattern-string) with a
[GeneratedRegex] partial method (AOT-friendly, no per-call cache lookup),
matching FrontmatterParser's style. Runs once per skill during checks.

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

* skill-validator: parse disable-model-invocation via YAML to avoid block-scalar false positives

The regex-based check matched any line in the frontmatter, so a block-scalar description that merely mentioned 'disable-model-invocation: true' on its own line was wrongly treated as disabling model invocation. Parse the frontmatter with the existing YAML deserializer (which correctly handles block scalars) by adding a DisableModelInvocation field to SkillFrontmatter, and drop the regex entirely.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-25 14:59:05 +02:00
Aaron Powell 3134b816de Add JSON output for skill-validator check (#601)
* Add JSON output for skill-validator check

Fixes #600

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

* Fix empty check discovery results

Address PR feedback by failing when explicit skill or agent paths discover nothing, including the combined check path.

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

* Address Copilot review comments

- Replace AddPlainError with AddGeneralError (remove duplicate method)
- Key pluginSkills by DirectoryPath instead of display name to avoid
  mismatch when plugin.Name differs from directory name
- Use OS-aware path comparison in IsPathWithin (Ordinal on Unix,
  OrdinalIgnoreCase on Windows)
- Pre-build name lookup dictionaries in CreateJsonOutput to eliminate
  O(n*m) FirstOrDefault scans for external dependency attachment
- Extract CheckJsonSerializerContext scoped to check JSON output so
  UseStringEnumConverter does not affect unrelated JSON payloads

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

* Address latest Copilot PR feedback

- Attach external dependency warnings by stable path identifiers
  (skill SKILL.md path, agent path, plugin directory path)
- Use OS-aware comparer for plugin directory path matching
- Restore console warning formatting via SkillProfiler.FormatProfileWarnings
- Move CheckJsonSerializerContext into SkillValidator.Check namespace
- Precompute reference-attachment targets to avoid repeated full-path
  normalization and container recalculation per finding
- Add regression test covering duplicate skill names with external
  dependency warnings in JSON output

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

* Fix flaky duplicate-skill JSON test path

- Run duplicate-name external dependency assertion through plugin mode,
  which is where external dependency checks are executed
- Add plugin fixture helper for JSON output tests

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

* Address latest Copilot follow-up feedback

- Reintroduce PluginValidationResult as obsolete compatibility type
  while keeping PluginCheckResult as the primary model
- Avoid unnecessary profile-line formatting when verbose output is off
- Replace string discriminators for external dependency kind with enum
- Centralize JSON warning kind literals as constants

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-11 07:30:32 +00:00
Jan Krivanek 6ebb5616b2 Support checking skills and agents at single invocation (#533) 2026-04-17 08:32:42 +00:00
Jan Krivanek 0e47ef6545 Add more static checks (#453) 2026-03-26 14:00:24 +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