Add MSBuild target authoring skills (target-authoring, property-patterns, item-management, extension-points)#669
Conversation
New skills: - target-authoring: three-level target chain, DependsOn extension, naming conventions - property-patterns: conditional defaults, composition, path normalization, TFM helpers - item-management: Include/Remove/Update, batching, transforms, FileWrites registration - extension-points: CustomBefore/After hooks, import gating, NuGet build extensions Each skill includes eval.yaml tests with anti-pattern fixtures. Closes dotnet#668
YuliiaKovalova
requested review from
a team,
JanKrivanek and
ViktorHofer
as code owners
|
/evaluate |
There was a problem hiding this comment.
Pull request overview
Adds four new authoring-focused skills to the dotnet-msbuild plugin (target authoring, property patterns, item management, and extension points) along with evaluation scenarios and anti-pattern fixture files to validate the agent’s review guidance.
Changes:
- Added 4 new skills under
plugins/dotnet-msbuild/skills/*documenting canonical MSBuild authoring patterns. - Added 4 new test suites under
tests/dotnet-msbuild/*witheval.yamlscenarios and fixture projects/files containing intentional anti-patterns. - Added supporting fixture assets (Directory.Build.props/targets, schemas, JSON data, and placeholder source files) used by the eval scenarios.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet-msbuild/skills/target-authoring/SKILL.md | New guidance for structuring custom targets, chaining, Returns vs Outputs, and templates. |
| plugins/dotnet-msbuild/skills/property-patterns/SKILL.md | New guidance for conditional defaults, composition, path handling, and property functions. |
| plugins/dotnet-msbuild/skills/item-management/SKILL.md | New guidance for Include/Remove/Update, batching, transforms, and common pitfalls. |
| plugins/dotnet-msbuild/skills/extension-points/SKILL.md | New guidance for CustomBefore/After hooks, import guards, Directory.Build discovery, and NuGet build extensions. |
| tests/dotnet-msbuild/target-authoring/TargetAuthoring.csproj | Fixture project containing target-authoring anti-patterns for review. |
| tests/dotnet-msbuild/target-authoring/Placeholder.cs | Minimal source file for the target-authoring fixture. |
| tests/dotnet-msbuild/target-authoring/eval.yaml | Eval scenario asserting correct feedback on target-authoring anti-patterns. |
| tests/dotnet-msbuild/property-patterns/PropertyPatterns.csproj | Fixture project for property-patterns eval context. |
| tests/dotnet-msbuild/property-patterns/Placeholder.cs | Minimal source file for the property-patterns fixture. |
| tests/dotnet-msbuild/property-patterns/eval.yaml | Eval scenario asserting correct feedback on property definition anti-patterns. |
| tests/dotnet-msbuild/property-patterns/Directory.Build.props | Fixture props file containing property anti-patterns. |
| tests/dotnet-msbuild/item-management/ItemManagement.csproj | Fixture project containing item-management anti-patterns for review. |
| tests/dotnet-msbuild/item-management/Placeholder.cs | Minimal source file for the item-management fixture. |
| tests/dotnet-msbuild/item-management/eval.yaml | Eval scenario asserting correct feedback on item/batching/generation issues. |
| tests/dotnet-msbuild/item-management/schemas/users.xsd | Fixture schema input for item-management scenarios. |
| tests/dotnet-msbuild/item-management/schemas/orders.xsd | Fixture schema input for item-management scenarios. |
| tests/dotnet-msbuild/item-management/data/users.json | Fixture data input for item-management scenarios. |
| tests/dotnet-msbuild/item-management/data/orders.json | Fixture data input for item-management scenarios. |
| tests/dotnet-msbuild/item-management/Constants.g.cs | Fixture generated-file example used by the item-management scenario. |
| tests/dotnet-msbuild/extension-points/ExtensionPoints.csproj | Fixture project for extension-points eval context. |
| tests/dotnet-msbuild/extension-points/Placeholder.cs | Minimal source file for the extension-points fixture. |
| tests/dotnet-msbuild/extension-points/eval.yaml | Eval scenario asserting correct feedback on import guards and extensibility patterns. |
| tests/dotnet-msbuild/extension-points/Directory.Build.targets | Fixture targets file containing extension-point anti-patterns. |
| tests/dotnet-msbuild/extension-points/Directory.Build.props | Fixture props file defining RepoRoot for the extension-points scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
- target-authoring: Reword DO NOT USE clause to exclude only deep incremental-build diagnostics, not basic Inputs/Outputs usage - property-patterns: Close unclosed XML elements in String Functions snippet (PropertyGroup, TargetFrameworkMoniker) - extension-points: Add missing CustomAfterMySDK property definition to match the import at bottom of example
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
plugins/dotnet-msbuild/skills/extension-points/SKILL.md:150
- The parent Directory.Build.props import example is missing an Exists(...) guard. Without it, the Import can fail when the file isn’t found (and the repo’s own directory-build-organization skill shows the guarded pattern). Consider adding an Exists condition to make the snippet robust and consistent.
```xml
<!-- src/Directory.Build.props -->
<Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)..\'))" />
**plugins/dotnet-msbuild/skills/property-patterns/SKILL.md:78**
* The IsPathRooted example passes $(MSBuildProjectExtensionsPath) unquoted into the property function. Quote the argument so paths containing spaces/special chars don’t break the function call, and keep function-call quoting consistent within the doc.
- extension-points: Use full MSBuildExtensionsPath expression and consistent Exists() casing in wildcard import example - property-patterns: Quote property arguments in NormalizePath call
|
/evaluate |
|
This looks good but before approving I would like to see an improvement in an evaluation run. My understanding was that the LLM with the public available documentation was good enough. |
|
/evaluate |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
Rewrite all 4 eval prompts as natural developer problem descriptions instead of skill-aligned checklists. Softens technique-prescriptive rubric items. Result: overfitting scores drop from 0.15-0.31 to 0.06-0.08 (all green). property-patterns now passes eval.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
plugins/dotnet-msbuild/skills/property-patterns/SKILL.md:110
- The
IsNullOrWhitespaceexample passes$(TargetFrameworkProfile)without quoting it inside the property function call. This can break if the property is empty or contains spaces/special characters; quote it as a string argument so the sample is copy/paste safe.
<!-- IsNullOrWhitespace -->
<TargetFrameworkMoniker
Condition="'$([System.String]::IsNullOrWhitespace($(TargetFrameworkProfile)))' != 'true'">
$(TargetFrameworkIdentifier),Version=$(TargetFrameworkVersion),Profile=$(TargetFrameworkProfile)
</TargetFrameworkMoniker>
|
/evaluate |
Skill Validation Results
[1]
Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
Add 'Only activate in MSBuild/.NET build context.' prefix to all 4 skills for consistency with existing skills in the plugin. Add explicit 'diagnosing and fixing' and 'reviewing' keywords to USE FOR sections so the SDK selects these skills over the broader msbuild-antipatterns skill when prompts ask to fix or review specific item/extension/property/target patterns. Add 'general MSBuild anti-pattern catalog (use msbuild-antipatterns)' to DO NOT USE FOR sections to help the SDK disambiguate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
tests/dotnet-msbuild/property-patterns/eval.yaml:52
- The rubric item about LangVersion being "impossible for projects to override" doesn’t match MSBuild evaluation order for this fixture: values set in Directory.Build.props are overridden by later
.csprojassignments. Align the rubric with the intended issue (e.g., earlier-layer/command-line overrides) or adjust the fixture so LangVersion is assigned late enough to block project overrides.
All 4 new skills had descriptions exceeding the 1024-character maximum enforced by the skill-validator check command. The Copilot SDK silently ignores skills with over-length descriptions, causing 0% activation in both isolated and plugin evaluation modes. Shortened all 4 descriptions while preserving key activation keywords (USE FOR / DO NOT USE FOR / 'Only activate in MSBuild/.NET build context'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/evaluate |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
- target-authoring eval.yaml: fix prompt to say 'Outputs attribute' (not 'Returns') since the fixture uses Outputs on a query target, which is the actual bug that causes MSBuild to skip re-execution - property-patterns eval.yaml: reword prompt and rubric — the real issue is unconditional assignment breaking parent-child Directory.Build.props inheritance, not project files being unable to override properties - extension-points SKILL.md: fix GetPathOfFileAbove example to factor the path into a property so Project= and Condition= use the same value consistently (avoiding the ..\\ vs ..\ discrepancy) - CustomSdk.targets: add WriteLinesToFile to CoreCodeGen so it actually creates the .g.cs output files (prevents compilation failure in fixture) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Head branch was pushed to by a user without write access
Scenario 1 (path merge bug): - PropertyPatterns.csproj: add OutputPath using CustomOutputDir without a trailing separator, making the merge bug observable (artifacts\binPropertyPatterns\ instead of artifacts\bin\PropertyPatterns\) Scenario 2 (multi-level hierarchy bugs): - hard/Directory.Build.props: make LangVersion unconditional so it overwrites the child src/Directory.Build.props LangVersion=preview, matching the rubric claim about parent unconditional assignments - hard/Directory.Build.props: add NoWarn=NU1702 (conditional default) so parent suppressions are visible but lost when the child's unconditional <NoWarn>CS1591;IDE0005</NoWarn> overwrites them after import Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/evaluate |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
Summary
Adds four new skills to the
dotnet-msbuildplugin covering MSBuild target/props authoring best practices derived from analysis of the MSBuild repo's own.targetsand.propsfiles.Closes #668
New Skills
1.
target-authoringThree-level target chain pattern (Before/Core/After), DependsOn chain extension, DependsOnTargets vs BeforeTargets/AfterTargets guidance, Returns vs Outputs, incremental build with Inputs/Outputs, naming conventions, and a complete target template.
2.
property-patternsConditional defaults, semicolon-delimited composition, path normalization, MSBuild string functions, TFM condition helpers, guard properties, feature gating, and fallback chains.
3.
item-managementInclude/Remove/Update semantics, batching (single-axis vs cross-product pitfall), item transforms, Exclude patterns, conditional item inclusion, PrivateAssets/ExcludeAssets metadata, and FileWrites registration for generated files.
4.
extension-pointsCustomBefore/CustomAfter hooks, wildcard import directories with alphabetic ordering, import gating with control properties, NuGet package build extension layout (build/buildTransitive), Directory.Build discovery and multi-level hierarchy, and the import guard pattern.
Tests
Each skill includes:
Checklist