[mthalman]

Demo reproduction of dotnet#49288 (Multi-RID tool packages usability tweaks) at the exact commit Copilot reviewed (89f3885), to compare default Copilot Code Review vs. the code-review skill. Category #2 (MSBuild property/item/target/path semantics).

[Copilot]

Pull request overview

This PR adjusts how .NET SDK tool packages are packed when producing multiple RID-specific tool packages, including changes to MSBuild targets that orchestrate RID builds and updates to the tool configuration schema/consumer to no longer track per-RID package versions.

Changes:

• Gate Microsoft.NET.PackTool.targets import on PackAsTool=true and update MSBuild logic to orchestrate RID-specific tool package creation.
• Shift multi-RID tool package selection from item-based @(ToolPackageRuntimeIdentifier) to property-based $(ToolPackageRuntimeIdentifiers).
• Update tool settings generation and CLI deserialization/downloading to remove RID-package version metadata and rely on the base package version.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets	Imports tool-pack targets only when packing as a tool.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Common.targets	Switches RID tool package type detection to $(ToolPackageRuntimeIdentifiers).
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets	Major rework of tool packing logic: publish dependencies, package layout, and RID package orchestration.
src/Tasks/Microsoft.NET.Build.Tasks/ResolveToolPackagePaths.cs	Removes the MSBuild task previously used to map publish outputs to package paths.
src/Tasks/Microsoft.NET.Build.Tasks/GenerateToolsSettingsFile.cs	Removes RID-package version emission from the generated tool settings XML.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAPublishRelativePathToPackPackagePath.cs	Removes unit tests tied to the deleted ResolveToolPackagePaths task.
src/Cli/dotnet/ToolPackage/ToolPackageDownloaderBase.cs	Downloads RID-specific packages using the base tool package version.
src/Cli/dotnet/ToolPackage/ToolConfigurationDeserializer.cs	Deserializes RID-specific packages without requiring a per-RID version.
src/Cli/dotnet/ToolPackage/ToolConfigurationDeserialization/DotNetCliToolRuntimeIdentifierPackage.cs	Removes the Version attribute from the RID-package schema model.

[mthalman]

Default Copilot Code Review vs. Code-Review Skill — Category #2 (MSBuild path/target semantics)

This PR faithfully reproduces dotnet/sdk#49288 ("Multi-RID tool packages usability tweaks") at the exact commit Copilot reviewed (89f3885), 228 lines of Microsoft.NET.PackTool.targets / Microsoft.NET.Sdk.targets restructuring. The base branch here carries .github/skills/code-review/SKILL.md, so Copilot auto-loads the skill.

On the real PR, default Copilot Code Review produced 0 inline findings. With the skill, Copilot produced 5 findings, all in PackTool.targets — squarely the MSBuild property/item/target/path semantics this repo is most prone to and that default review consistently overlooks:

1. <_ToolPackageRuntimeIdentifier Include="$(ToolRuntimeIdentifiers)" /> references a property (ToolRuntimeIdentifiers) that is defined nowhere — everywhere else uses ToolPackageRuntimeIdentifiers. The item ends up empty, so the generated tool settings file never lists RID-specific packages and dotnet tool install/restore won't download them.
2. PackToolImplementation runs unconditionally and pulls in $(PublishDepsFilePath) / $(PublishRuntimeConfigFilePath), which may not exist for the outer RID-agnostic build.
3. The publish dependency (_PackToolPublishDependency) is gated on '$(RuntimeIdentifier)' != '', so normal tool packs (and GeneratePackageOnBuild=true) skip the publish pipeline and can produce a package without the tool implementation.
4. _PublishFiles is likewise gated on '$(RuntimeIdentifier)' != '', so a plain PackAsTool nupkg can be missing its tools/<tfm>/any/*.dll binaries.
5. ToolEntryPoint switches to the native apphost name for any RID-specific build with UseAppHost=true, even when the runner is framework-dependent dotnet — pointing dotnet at an .exe entry point.

These aren't speculative. The final merged version of dotnet#49288 fixes exactly these areas: the ToolRuntimeIdentifiers typo is gone, and the RuntimeIdentifier-based gating on implementation inclusion was replaced with a dedicated _ToolPackageShouldIncludeImplementation property (commit "restore RID-agnostic-only tool packaging"). The skill surfaced, at review time, several of the bugs the author subsequently had to chase down.