Repository navigation
Fix msbuild skill spec conformance violations - #247
Merged
Merged
Conversation
danmoseley
requested review from
JanKrivanek,
ViktorHofer,
dbreshears and
timheuer
as code owners
March 5, 2026 21:46
Contributor
There was a problem hiding this comment.
Pull request overview
Updates dotnet-msbuild skills and the skill-validator tooling to conform to new agentskills.io spec checks (body line count + reference depth), while adding additional enforcement around SKILL.md frontmatter fields.
Changes:
- Move AP-16–AP-21 + checklist out of
msbuild-antipatterns/SKILL.mdinto a local references file to meet body-length limits. - Remove cross-skill reference from
directory-build-organizationby inlining AP-21 content into a local reference. - Extend
SkillProfiler/discovery/frontmatter parsing to validate name/description/compatibility and add CI tooling to validate description sizes.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet-msbuild/skills/msbuild-antipatterns/references/additional-antipatterns.md | New reference doc containing AP-16–AP-21 + checklist to reduce SKILL.md body length. |
| plugins/dotnet-msbuild/skills/msbuild-antipatterns/SKILL.md | Removes large sections and links to the new reference file. |
| plugins/dotnet-msbuild/skills/directory-build-organization/references/targetframework-props-pitfall.md | New local reference to replace a cross-skill link (AP-21 content). |
| plugins/dotnet-msbuild/skills/directory-build-organization/SKILL.md | Updates link to local reference to satisfy reference-depth rules. |
| eng/validate-descriptions.ps1 | Adds repo-wide script to validate per-skill and per-plugin aggregate description sizes. |
| eng/tests/validate-descriptions.tests.ps1 | Adds PowerShell tests for the description-size validation script. |
| eng/skill-validator/tests/SkillProfileTests.cs | Adds unit tests for name/description/compatibility/body-lines/ref-depth checks. |
| eng/skill-validator/src/Services/SkillProfiler.cs | Implements new spec conformance validations and adds DescriptionTooLong to profile. |
| eng/skill-validator/src/Services/SkillDiscovery.cs | Threads compatibility from SKILL.md frontmatter into SkillInfo. |
| eng/skill-validator/src/Services/EvalSchema.cs | Adds Compatibility to frontmatter schema. |
| eng/skill-validator/src/Models/Models.cs | Adds Compatibility field to SkillInfo. |
| eng/skill-validator/src/Commands/ValidateCommand.cs | Makes overlong descriptions a blocking failure (description_too_long). |
| agentic-workflows/dotnet-msbuild/build.ps1 | Calls the central description-size validator during workflow build validation. |
| .github/workflows/validate-descriptions.yml | Adds CI workflow to enforce description-size limits on PRs/pushes. |
| .github/CODEOWNERS | Updates ownership entries (unrelated to msbuild spec conformance). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
danmoseley
force-pushed
the
fix/msbuild-spec-conformance
branch
2 times, most recently
from
March 5, 2026 22:16
7e34628 to
bf1dc0b
Compare
Contributor
Author
|
@ViktorHofer I think you authored these skills. Note that in order to enforce the depth requirement for references, this duplicates the "antipatterns" content from the other MSBuild skill, rather than "reaching into" the other skill. I don't know how to avoid this, but it makes it conformant. Likely in use, the "reaching in" reference would not actually work. thoughts? |
- msbuild-antipatterns: Move AP-16 through AP-21 and checklist to references/additional-antipatterns.md (body: 384 lines, was 583) - directory-build-organization: Replace cross-skill reference to ../msbuild-antipatterns/SKILL.md with local reference file references/targetframework-props-pitfall.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danmoseley
force-pushed
the
fix/msbuild-spec-conformance
branch
from
March 5, 2026 22:29
bf1dc0b to
dedf961
Compare
Member
|
/evaluate |
Contributor
Skill Validation Results
Model: claude-opus-4.6 | Judge: claude-opus-4.6 |
ViktorHofer
approved these changes
Mar 6, 2026
danmoseley
pushed a commit
to danmoseley/skills
that referenced
this pull request
Mar 6, 2026
Now that dotnet#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>
danmoseley
pushed a commit
that referenced
this pull request
Mar 7, 2026
* 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two spec conformance violations flagged by the new checks in #238:
msbuild-antipatterns: Body was 583 lines (max 500). Moved AP-16 through AP-21 and the quick-reference checklist toreferences/additional-antipatterns.md. Body is now 384 lines.directory-build-organization: Had a cross-skill reference../msbuild-antipatterns/SKILL.md(3 levels deep, max is 1). Replaced with a localreferences/targetframework-props-pitfall.mdcontaining the same AP-21 content.After this merges and #238 merges, the body-lines and file-ref-depth checks in SkillProfiler should be upgraded from warnings to blocking errors. Change locations:
eng/skill-validator/src/Services/SkillProfiler.cs— look for commentswarning only — upgrade to blocking error after existing skills are fixed(around the body-lines and ref-depth checks)