Add AGENTS.md and per-TFM MSBuild loader skills

.agents/skills/msbuild-loader-netcore/SKILL.md

@@ -0,0 +1,82 @@
+---
+name: msbuild-loader-netcore
+description: >-
+  How MSBuildLocator loads MSBuild assemblies and discovers the .NET SDK on .NET
+  / .NET Core (the net8.0 target / #if NETCOREAPP branches in src/MSBuildLocator).
+  Covers the AssemblyLoadContext.Default.Resolving handler, search-path probing,
+  the SDK environment variables set on registration, hostfxr-based SDK discovery,
+  the dotnet location probe order, and the AllowQueryAll* widening flags. Use when
+  editing or reviewing net8.0 loader, registration, or SDK-discovery code, or
+  diagnosing assembly-resolution behavior on .NET Core hosts.
+---
+
+# .NET / .NET Core (`net8.0` / `NETCOREAPP`) MSBuild loader
+
+Scope: `src/MSBuildLocator/MSBuildLocator.cs` `#if NETCOREAPP` branches, plus
+`DotNetSdkLocationHelper.cs` and `NativeMethods.cs`. For build/test and
+cross-cutting conventions, see `AGENTS.md`.
+
+SDK discovery is cross-platform, but executable names, hostfxr library loading, and symlink handling have OS-specific implementations.
+
+## Assembly-resolution handler
+- `s_registeredHandler` is a static
+  `Func<AssemblyLoadContext, AssemblyName, Assembly>`; registration hooks
+  `AssemblyLoadContext.Default.Resolving`.
+- Resolving can fire repeatedly; successful loads are cached in the local
+  `loadedAssemblies` dictionary keyed by `AssemblyName.FullName`.
+- Resolution is not thread-safe; keep the `loadedAssemblies` lock around cache
+  lookup, path probing, `Assembly.LoadFrom`, and cache insert.
+- The resolver receives `AssemblyName` directly — do not parse an event-args
+  name string (that is the net46 path).
+- For each registered `msbuildSearchPaths` entry, probe
+  `Path.Combine(msbuildPath, assemblyName.Name + ".dll")` and load with
+  `Assembly.LoadFrom(targetAssembly)`.
+
+## SDK environment variables (NETCOREAPP only)
+- Registration sets MSBuild SDK env vars via `ApplyDotNetSdkEnvironmentVariables`:
+  - `MSBUILD_EXE_PATH` = `<sdk>\MSBuild.dll`
+  - `MSBuildExtensionsPath` = `<sdk>`
+  - `MSBuildSDKsPath` = `<sdk>\Sdks`
+- `RegisterMSBuildPath(string)` applies these for that path before registering.
+- `RegisterMSBuildPath(string[])` applies these for the first search path only,
+  then registers all paths.
+- `RegisterInstance` applies these only when
+  `instance.DiscoveryType == DiscoveryType.DotNetSdk`.
+- Do not move this setup into net46; Framework has its own `MSBUILD_EXE_PATH`
+  compatibility branch under `#if NET46` (see the `msbuild-loader-netframework`
+  skill).
+
+## SDK discovery
+- Lives in `DotNetSdkLocationHelper`; instances are
+  `VisualStudioInstance(name: ".NET Core SDK", ..., DiscoveryType.DotNetSdk)`.
+- Uses `NativeMethods` hostfxr P/Invoke under `#if NETCOREAPP` only:
+  - `hostfxr_resolve_sdk2` — best SDK, honoring `global.json` via
+    `WorkingDirectory`.
+  - `hostfxr_get_available_sdks` — installed SDK enumeration.
+- `ResolveDotnetPathCandidates` preference order (tried in order):
+  `DOTNET_ROOT` (`DOTNET_ROOT(x86)` in a 32-bit process) → current process
+  directory when running under `dotnet` → `DOTNET_HOST_PATH` →
+  `DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR` → `PATH`.
+- A successful `hostfxr_resolve_sdk2` sets `DOTNET_HOST_PATH` (if empty) to
+  `<dotnetPath>\dotnet(.exe)`.
+- Default query returns the best SDK first, then unique SDK versions
+  newest-first from the available SDKs.
+
+## Widening flags
+- `AllowQueryAllRuntimeVersions` / `VisualStudioInstanceQueryOptions.AllowAllRuntimeVersions`
+  include SDKs whose major/minor runtime exceeds `Environment.Version`.
+- `AllowQueryAllDotnetLocations` / `VisualStudioInstanceQueryOptions.AllowAllDotnetLocations`
+  keep probing all dotnet candidate locations instead of stopping after the
+  first location that has SDKs.
+
+## What net8.0 does NOT do
+- No Developer Console or Visual Studio Setup COM discovery;
+  `FEATURE_VISUALSTUDIOSETUP` package references/constants are net46-only in the
+  csproj.
+
+## Register-before-load contract
+- `CanRegister` is false when already registered, or once any signed
+  `Microsoft.Build*` core assembly is loaded.
+- JIT caveat: JIT-compilation of a method referencing `Microsoft.Build` types is
+  enough to load those assemblies and break registration. Keep locator calls
+  isolated before any such reference.

.agents/skills/msbuild-loader-netframework/SKILL.md

@@ -0,0 +1,55 @@
+---
+name: msbuild-loader-netframework
+description: >-
+  How MSBuildLocator loads MSBuild assemblies and discovers installs on .NET
+  Framework (the net46 target / #if NET46 / non-NETCOREAPP branches in
+  src/MSBuildLocator). Covers the AppDomain.AssemblyResolve handler, search-path
+  probing, and Developer Console + Visual Studio Setup (COM) discovery. Use when
+  editing or reviewing net46 loader, registration, or VS-discovery code, or
+  diagnosing assembly-resolution behavior on .NET Framework hosts.
+---
+
+# .NET Framework (`net46`) MSBuild loader
+
+Scope: `src/MSBuildLocator/MSBuildLocator.cs` `#if NET46` and Framework (`#else`
+of `NETCOREAPP`) branches. `FEATURE_VISUALSTUDIOSETUP` is defined only when
+`TargetFramework == net46` in `Microsoft.Build.Locator.csproj`. For build/test
+and cross-cutting conventions, see `AGENTS.md`.
+
+## Assembly-resolution handler
+- `s_registeredHandler` is a static `ResolveEventHandler`; `IsRegistered` is
+  `s_registeredHandler != null`.
+- `RegisterMSBuildPathsInternally` stores the handler in the static field before
+  subscribing to `AppDomain.CurrentDomain.AssemblyResolve`; the event subscription
+  keeps the delegate alive, while the field tracks registration state.
+- `AssemblyResolve` can fire repeatedly for the same assembly; results are cached
+  in `loadedAssemblies` keyed by `AssemblyName.FullName`.
+- Resolution is explicitly not thread-safe; every cache lookup/load runs under
+  `lock (loadedAssemblies)`.
+- Handler path: parse `eventArgs.Name` with `new AssemblyName(eventArgs.Name)`;
+  for each registered search path, if `<msbuildPath>\<Name>.dll` exists, return
+  `Assembly.LoadFrom(targetAssembly)`.
+- Search paths come from `RegisterMSBuildPath(...)`, or from
+  `RegisterInstance(...)` as `instance.MSBuildPath` plus the VS NuGet path when
+  it exists.
+
+## Discovery sources (net46 only)
+- Developer command prompt: `GetDevConsoleInstance()` reads `VSINSTALLDIR`, then
+  parses `VSCMD_VER` (trimming any suffix after `-`), then falls back to
+  `VisualStudioVersion`; yields `DiscoveryType.DeveloperConsole`.
+- Visual Studio Setup COM API: under `FEATURE_VISUALSTUDIOSETUP`,
+  `VisualStudioLocationHelper.GetInstances()` enumerates VS 2017+ setup instances
+  with `Microsoft.Component.MSBuild`; yields `DiscoveryType.VisualStudioSetup`.
+- `DiscoveryType.DotNetSdk` exists in the enum but belongs to the Core path; net46
+  `GetInstances(...)` does not call SDK discovery.
+
+## What net46 does NOT do
+- No `AssemblyLoadContext`, `hostfxr`, or `.NET SDK` discovery — those are
+  `#if NETCOREAPP` paths (see the `msbuild-loader-netcore` skill).
+
+## Register-before-load contract
+- `CanRegister` is false once any strong-named `Microsoft.Build*` assembly in
+  `s_msBuildAssemblies` is loaded in the current `AppDomain`.
+- JIT caveat: a method that references `Microsoft.Build` types can trip the
+  contract when JIT-compiled, even if that reference never executes. Keep locator
+  calls isolated before any such reference.

.github/skills/code-review/SKILL.md

@@ -0,0 +1,26 @@
+---
+name: code-review
+description: >-
+  Repository-specific checks for Microsoft.Build.Locator code reviews. Use for
+  loader, discovery, public API, and package changes.
+license: MIT
+---
+
+# Microsoft.Build.Locator code review
+
+Read `AGENTS.md`. For loader, registration, or discovery changes, also read the
+applicable `msbuild-loader-netcore` or `msbuild-loader-netframework` skill; read
+both for common code. Verify that the skills accurately describe the code, and
+require them to change when behavior changes.
+
+Check these repository-specific invariants:
+
+- Common code must work on both `net46` and `net8.0`; runtime-specific behavior
+  must stay behind the correct conditional.
+- Registration must happen before any core `Microsoft.Build*` assembly loads.
+- .NET Framework discovers Visual Studio; .NET 8+ discovers the .NET SDK.
+- .NET Framework changes must remain compatible across supported Visual Studio
+  versions; do not assume only the latest MSBuild layout or behavior.
+- `Microsoft.Build.Locator.dll` must retain minimal framework-only dependencies.
+- Intentional public API changes require compatibility suppressions and an
+  appropriate version change.

AGENTS.md

@@ -0,0 +1,34 @@
+# Microsoft.Build.Locator — Copilot instructions
+
+This library exists to locate an MSBuild install (Visual Studio or .NET SDK) and register an assembly-resolution handler so the host app loads MSBuild's assemblies from that install. This is required for any use of the .NET API from an application that is not part of Visual Studio or the .NET SDK.
+
+A .NET Framework application can only locate MSBuild from a Visual Studio installation, and a .NET 8+ application can only locate MSBuild from a .NET SDK. This library's approach to finding the library and loading the assemblies is entirely different on the different runtimes. See .agents/skills/msbuild-loader-netcore/SKILL.md and .agents/skills/msbuild-loader-netframework/SKILL.md for details.
+
+The core `Microsoft.Build.Locator.dll` must have minimal dependencies—nothing outside the core libraries provided by .NET for the relevant TargetFramework.
+
+## Build / test (root `MSBuildLocator.sln`, .NET CLI)
+- `dotnet restore` / `dotnet build` / `dotnet test`
+- Single test: `dotnet test --filter "FullyQualifiedName~QueryInstancesTests"` or `--filter "Name=<Method>"`
+- Tests: xUnit + Shouldly, in `src/MSBuildLocator.Tests`.
+- Versioning: Nerdbank.GitVersioning. Use SemVer 2 and update `version.json` on breaking changes or feature additions.
+
+## Multi-targeting (central constraint)
+Library: `net46` + `net8.0`. Tests: `net472` + `net8.0`. Non-trivial code forks per TFM via `#if NETCOREAPP`, `#if NET46`, and `FEATURE_VISUALSTUDIOSETUP` (defined only for `net46`). Always check whether a change must be mirrored or excluded across these conditionals; for what each fork actually does, load the `msbuild-loader-netcore` or `msbuild-loader-netframework` skill.
+
+When changing behavior documented by either skill, update the skill in the same change.
+
+## Architecture (namespace `Microsoft.Build.Locator`)
+- `MSBuildLocator.cs` — entry point and handler registration. Both TFM forks live here. `Unregister()` is retained for compatibility but is a no-op.
+- `DotNetSdkLocationHelper.cs`, `NativeMethods.cs` — .NET SDK discovery (hostfxr); see `msbuild-loader-netcore`.
+- `VisualStudioLocationHelper.cs` — `net46`-only Visual Studio Setup discovery; see `msbuild-loader-netframework`.
+- `VisualStudioInstance.cs`, `VisualStudioInstanceQueryOptions.cs`, `DiscoveryType.cs` — result/option types.
+- `Utils/SemanticVersion*.cs`, `VersionComparer.cs` — internal SemVer parse/compare that is an implementation detail of .NET SDK discovery.
+- Props/targets ship from `src/MSBuildLocator/build/` to `build/` and `buildTransitive/`. Never ship MSBuild DLLs with an app: local copies load before Locator's handler. `EnsureMSBuildAssembliesNotCopied` reports **MSBL001**; fix the flagged `<PackageReference>` with `ExcludeAssets="runtime"` and `PrivateAssets="all"`.
+- Keep `EnsureMSBuildAssembliesNotCopied`'s hardcoded package list synchronized with MSBuild's redistributable assemblies so it catches new packages.
+
+## Conventions
+- Contract-stable public API: csproj `EnablePackageValidation` + `PackageValidationBaselineVersion` (1.6.1). Intentional API changes require updating `src/MSBuildLocator/CompatibilitySuppressions.xml` and an appropriate semver update.
+- XML doc comments on public members; match existing style.
+- Strong-name signed (`key.snk`) — don't remove signing.
+- Build settings centralized in `Directory.Build.props` / `Directory.Solution.props` / `Directory.Build.rsp` — edit there, not per-project.
+ - Register-before-load contract: callers must register via Locator BEFORE any core MSBuild assembly loads (`CanRegister` → false once loaded). Preserve this + the lazy-loading patterns protecting it when refactoring.