From 1d37231d286caa57bd0731559deacd3acd3be0d3 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Mon, 9 Nov 2026 10:17:26 +0000 Subject: [PATCH 1/2] feat: added agent skill update integration targets --- .config/dotnet-tools.json | 4 +- .editorconfig | 2 +- AGENTS.md | 27 +- CHANGELOG.md | 35 + Justfile | 26 +- README.md | 36 +- docs/wiki/Agent-Skills.md | 196 +++-- docs/wiki/Getting-Started.md | 2 +- docs/wiki/Home.md | 7 +- package.json | 2 +- skills/purview-buildsdk-adoption/SKILL.md | 11 +- .../purview-buildsdk-configuration/SKILL.md | 13 + .../references/properties.md | 5 +- skills/purview-buildsdk-justfile/SKILL.md | 143 +++ .../SKILL.md | 21 +- .../SKILL.md | 102 ++- .../references/repository-root.md | 4 +- src/src/BuildSdk/Sdk/Props/Defaults.props | 18 +- src/src/BuildSdk/Sdk/Sdk.props | 14 +- src/src/BuildSdk/Sdk/Sdk.targets | 833 +++++++++++++++++- .../PurviewSkillInstallCommandTests.cs | 29 +- .../PurviewSkillRecommendationTests.cs | 18 +- .../PurviewSkillTargetIsolationTests.cs | 152 ++++ .../PurviewSkillUpdateTests.cs | 313 +++++++ templates/global.json | 2 +- 25 files changed, 1843 insertions(+), 172 deletions(-) create mode 100644 skills/purview-buildsdk-justfile/SKILL.md create mode 100644 src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs create mode 100644 src/tests/BuildSdk.IntegrationTests/PurviewSkillUpdateTests.cs diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 8881711..cafa6cb 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -10,14 +10,14 @@ "rollForward": false }, "dotnet-inspect": { - "version": "0.26.0", + "version": "0.27.0", "commands": [ "dotnet-inspect" ], "rollForward": false }, "purview.build": { - "version": "0.6.1", + "version": "0.8.2", "commands": [ "purview-build" ], diff --git a/.editorconfig b/.editorconfig index f8b3050..e5f6016 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,4 +1,4 @@ -# Purview.BuildSdk 1.3.0 - managed file, do not edit +# Purview.BuildSdk 1.3.1 - managed file, do not edit # # This file is written by Purview.BuildSdk and is replaced whenever the SDK version changes. The version # on the first line is how the SDK recognises a file it owns: a .editorconfig without that marker is diff --git a/AGENTS.md b/AGENTS.md index b1e7906..7b7f450 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,7 @@ The authored set, and what each one answers: | [`purview-buildsdk-skill-tests`](./skills/purview-buildsdk-skill-tests/SKILL.md) | authoring a skill, and validating it with `Purview.Skills.Testing` | | [`purview-buildsdk-skill-installation`](./skills/purview-buildsdk-skill-installation/SKILL.md) | installing the skills a package ships, and the `PRSKL*` diagnostics | | [`purview-buildsdk-repository-review`](./skills/purview-buildsdk-repository-review/SKILL.md) | auditing a repository against the SDK's conventions | +| [`purview-buildsdk-justfile`](./skills/purview-buildsdk-justfile/SKILL.md) | adding or updating `just` recipes that run the skill targets | After editing one, run `just skills-validate` for the pack-time check and `just test` for the full rule set. @@ -128,13 +129,15 @@ dotnet msbuild -t:InstallPurviewSkills # delegates to the skills CLI That makes version compatibility exact by construction: the resolved ref is the commit the *restored package version* was built from, so an older package reference gets that version's skills rather than whatever is on the default branch. A package whose nuspec has no GitHub repository URL, or no commit, cannot be pinned — it is reported as `PRSKL0002` rather than installed from a guessed branch. `PRSKL0001` is raised when `project.assets.json` is missing, because resolution reads the restore graph and deliberately does not trigger a restore of its own. -Installation itself is delegated to the skills CLI (`npx skills`) rather than reimplemented, because the CLI owns the agent-specific destinations and their discovery conventions — which differ per agent and change independently of this SDK — and owns `skills-lock.json`, so a repository can restore the same set later with `npx skills experimental_install`. `PurviewSkillsAgents` (default `*`) narrows the target agents. +Installation itself is delegated to the skills CLI (`bun x skills` by default) rather than reimplemented, because the CLI owns the agent-specific destinations and their discovery conventions — which differ per agent and change independently of this SDK — and owns `skills-lock.json`, so a repository can restore the same set later with `skills experimental_install`. + +`PurviewSkillsAgents` is **empty by default**, which passes no `--agent` at all, so the CLI auto-detects agents. Set, it becomes one `--agent` per name (`_ResolvePurviewSkillsAgents` splits on `,`, `;` and spaces) — the CLI's `--agent` does not split a comma-separated value, so the old `--agent "claude-code,codex"` form named one agent that does not exist. `dotnet msbuild -p:` itself splits on `,` and `;`, so a command line needs `%2C` or a quoted value; tests use `%2C`. ### Package runners and the repository root Two invariants govern how the CLI is launched, and both are load-bearing. -**Any package runner, defaulting to npx.** The CLI is published to npm and each runner spells "run this npm package" differently. `PurviewSkillsRunner` (default `npx`, because npx ships with Node itself) selects one; the mapping lives in the `PurviewSkillsRunnerLauncher` item group in `Sdk/Sdk.targets`, which carries an `Exec` value (version-pinned, for `InstallPurviewSkills`) and a `Display` value (unpinned, for recommendation text) per runner. A runner with no entry falls through to ` $(PurviewSkillsPackage)`, so an unknown runner still works. Each invocation form was verified against the real tool — `npx -y`, `bunx`, `bun x`, `pnpm dlx`, `yarn dlx` (Yarn 2+ only), `deno run -A npm:` — because a wrong form is a command that simply does not run. +**Any package runner, defaulting to bun.** The CLI is published to npm and each runner spells "run this npm package" differently. `PurviewSkillsRunner` (default `bun`, i.e. `bun x skills@1`) selects one; the mapping lives in the `PurviewSkillsRunnerLauncher` item group in `Sdk/Sdk.targets`, which carries an `Exec` value (version-pinned, for `InstallPurviewSkills`) and a `Display` value (unpinned, for recommendation text) per runner. A runner with no entry falls through to ` $(PurviewSkillsPackage)`, so an unknown runner still works. Each invocation form was verified against the real tool — `npx -y`, `bunx`, `bun x`, `pnpm dlx`, `yarn dlx` (Yarn 2+ only), `deno run -A npm:` — because a wrong form is a command that simply does not run. `DetectPurviewSkillsCli` (inline task) probes `PATH` for `skills` first, then `$(PurviewSkillsRunner)`, then `$(PurviewSkillsRunnerCandidates)`, honouring `PATHEXT` on Windows where these are `.cmd` shims. `PATH` is read directly rather than by spawning a version check: deterministic, no process, cannot hang on a cold download. The runner actually found wins over the configured preference. Nothing found is `PRSKL0004`; `PurviewSkillsCommand` overrides the launcher entirely and skips detection. @@ -204,10 +207,23 @@ cost was pure duplication. The remaining work is now paid only when somebody ask ```bash dotnet msbuild -t:RecommendPurviewSkills # report, never fail dotnet msbuild -t:VerifyPurviewSkills # report, and fail on anything unverifiable -dotnet msbuild -t:InstallPurviewSkills # install what is missing +dotnet msbuild -t:InstallPurviewSkills # install every shipped skill at its package's commit +dotnet msbuild -t:UpdatePurviewSkills # re-add every skills-lock.json source, then 'skills update' +dotnet msbuild -t:CheckPurviewSkills # only the PurviewSkillsAgents directories exist +dotnet msbuild -t:FixPurviewSkills # remove the CLI's links from any other agent directory dotnet msbuild -t:ListPurviewSkills # what the restored packages ship ``` +### Updating, checking and fixing + +**Install, update and fix run only when named — never automatically.** Two layers, both tested in `PurviewSkillTargetIsolationTests`: no SDK target hooks or depends on them (a structural scan of every `.props`/`.targets` under `Sdk/`, including `*DependsOn` properties), and `_GuardExplicitPurviewSkillsTarget` — first in each one's `DependsOnTargets` — fails with `PRSKL0009` when `BuildingProject`, `DesignTimeBuild` or `MSBuildIsRestoring` is true, which catches a consumer's own `AfterTargets="Build"` hook. Do not add a `BeforeTargets`/`AfterTargets` to any of them, and do not weaken the guard. + +`UpdatePurviewSkills` (`ReadPurviewSkillsLockSources` inline task) re-adds each unique lockfile source once, naming only the skills the lockfile records for it. A source a restored package ships is re-added at **that package's commit**, never unpinned — re-adding it at latest would turn a satisfied entry into `PRSKL0011` drift. Every other source is re-added at latest. Then `skills update -y -p` runs per `PurviewSkillsUpdateMode` (`Auto`: only when the lockfile hash changed; `Always`; `Never`). `skills update` re-fetches each entry at its recorded ref — verified in the CLI source — so it cannot undo the pin. The re-adds use target batching (`_UpdatePurviewSkillsSource`) so each failure is attributed to its source (`PRSKL0008`, `ErrorAndContinue` so all are named); `-p:PurviewSkillsDryRun=true` prints and runs nothing. + +`CheckPurviewSkills`/`FixPurviewSkills` (`EnforcePurviewSkillDirectories`) do nothing unless `PurviewSkillsAgents` names agents. Then the only agent skill directories allowed are those agents', per the `PurviewSkillsAgentDirectory` item group (copied from the CLI's agent table — extend it, don't hard-code), plus `.agents/skills`, the canonical store every agent directory links into. The authored `skills/` folder is protected even though openclaw installs there. Fix deletes links only (never their targets — `RemoveDirectory` on Windows, `unlink` on Unix) and folders left empty; a real file is kept and reported as `PRSKL0015`, because it cannot be told from hand-written content. Note the CLI writes copies, not links, for some agent selections, so those are kept too. + +Tests live in `src/tests/BuildSdk.IntegrationTests/PurviewSkillUpdateTests.cs`. + `VerifyPurviewSkills` is the gate a CI job calls. It escalates `PRSKL0010` (missing) and `PRSKL0011` (drifted) from warning to **error** — both are factual, both are fixed by one command, and neither can fire without a referenced package that really ships skills. `PRSKL0012` stays a warning, because a @@ -225,7 +241,7 @@ Drift **never** recommends `npx skills update`: `update` installs the newest pub Three things shape the output, and all of them are load-bearing: -- missing and drifted are warnings (actionable in one command), while unverifiable and orphaned are messages — and `VerifyPurviewSkills` is what turns them into failures; and +- missing and drifted are warnings (actionable in one command), while unverifiable and orphaned are messages — and `VerifyPurviewSkills` is what turns missing and drifted into failures; - a correctly installed set produces no output at all. - there is no deduplication stamp any more (`EnablePurviewSkillRecommendations`, `PurviewSkillRecommendationsInCI` and `PurviewSkillRecommendationStampPath` are retired with it). A stamp existed so a per-build hook could report one state once per repository; a target a person invokes should answer every time. @@ -296,6 +312,9 @@ Prefer `just` tasks: - `just pack` - `just skills-validate` — the pack-time check over the authored skills - `just skills-add ` — install a skill from another repository into `.agents/skills/` +- `just skills-update` — `UpdatePurviewSkills` then `FixPurviewSkills` +- `just skills-check` / `just skills-fix` — enforce the `PurviewSkillsAgents` layout +- `just skills-recommend` — `RecommendPurviewSkills` (was `skills-check`) - `just version-sync` — rewrite the `Purview.BuildSdk` pins in the docs, `templates/global.json` and `.editorconfig` marker after bumping `package.json` - `just version-sync-check` — the same check without writing; what the pre-commit hook runs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d1289f..cbbda3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,41 @@ release summarise what the commits recorded rather than what was written at the ## [Unreleased] +### Added + +- **`UpdatePurviewSkills`** re-adds every unique source in `skills-lock.json`, naming only the skills + recorded for it, then runs `skills update -y -p`. A source a restored package ships is re-added at + that package's commit, so package skills stay matched to the version referenced; every other source + is re-added at latest. `PurviewSkillsUpdateMode` (`Auto` default, `Always`, `Never`) decides when + `skills update` runs, and `PurviewSkillsDryRun=true` prints every command without running it (also + honoured by `InstallPurviewSkills`). New diagnostics `PRSKL0006`–`PRSKL0008`. +- **`CheckPurviewSkills` and `FixPurviewSkills`** enforce `PurviewSkillsAgents` when it is set: only + those agents' skill directories (plus the canonical `.agents/skills`) may exist at the repository + root (`PRSKL0015`). Fix removes the skills CLI's links — never their targets — and folders left + empty, and keeps and reports real files. The agent-to-directory map is the extensible + `PurviewSkillsAgentDirectory` item group; an unknown agent is `PRSKL0016`. +- **Install, update and fix are explicit-only, enforced.** `InstallPurviewSkills`, + `UpdatePurviewSkills` and `FixPurviewSkills` fail with `PRSKL0009` when pulled into a build, a + restore or a design-time build — including by a consumer's own `AfterTargets="Build"` hook. Run + them by name. +- New skill **`purview-buildsdk-justfile`**: adding or updating `just` recipes that run the skill + targets, and replacing hand-written skill scripts with them. +- `just skills-update`, `just skills-fix`; `just skills-check` now runs `CheckPurviewSkills`, and the + old recommendation report moved to `just skills-recommend`. + +### Changed (breaking) + +- **`PurviewSkillsRunner` defaults to `bun`** (`bun x skills@1`) instead of `npx`, and + `PurviewSkillsRunnerCandidates` to `bun;npx;bunx;pnpm;yarn;deno`. Recommendation text now reads + `bun x skills …`. Set `PurviewSkillsRunner=npx` to keep the previous behaviour. +- **`PurviewSkillsAgents` defaults to empty instead of `*`**: no `--agent` is passed and the skills CLI + auto-detects agents. Set it to choose. + +### Fixed + +- Several agents in `PurviewSkillsAgents` were passed as one `--agent "a,b"`, which the skills CLI + reads as a single agent named `a,b`. Each agent is now its own `--agent` flag. + ## [1.3.0] - 2026-11-08 ### Changed (breaking) diff --git a/Justfile b/Justfile index e0f62fc..45607a2 100644 --- a/Justfile +++ b/Justfile @@ -158,9 +158,33 @@ skills-list *args: # Report missing, drifted or unverifiable skills without installing anything [group('Agent Skills')] -skills-check *args: +skills-recommend *args: dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:RecommendPurviewSkills -v:n {{ args }} +# Update the agent skills by re-adding every unique source in skills-lock.json, then 'skills update' +# +# Skills a referenced package ships are re-added at that package's commit. When PurviewSkillsAgents +# is set, the agent directories it does not select are cleaned up afterwards (FixPurviewSkills); +# when it is not, the skills CLI auto-detects agents and the cleanup does nothing. +# +# just skills-update -p:PurviewSkillsUpdateMode=Always -p:PurviewSkillsDryRun=true +# just skills-update -p:PurviewSkillsRunner=npx -p:PurviewSkillsAgents=universal +# +# Update the agent skills by re-adding every unique source in skills-lock.json +[group('Agent Skills')] +skills-update *args: + dotnet msbuild src/src/BuildSdk/BuildSdk.csproj "-t:UpdatePurviewSkills;FixPurviewSkills" -v:n {{ args }} + +# Check that only the skill directories of the agents in PurviewSkillsAgents exist +[group('Agent Skills')] +skills-check *args: + dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:CheckPurviewSkills -v:n {{ args }} + +# Remove the skills CLI's links from agent directories PurviewSkillsAgents does not select +[group('Agent Skills')] +skills-fix *args: + dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:FixPurviewSkills -v:n {{ args }} + # Format the code with CSharpier [group('Utilities')] lint-fix: diff --git a/README.md b/README.md index 79012d0..4b1de60 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ A reusable MSBuild SDK NuGet package that delivers standardised .NET project def "runner": "Microsoft.Testing.Platform" }, "msbuild-sdks": { - "Purview.BuildSdk": "1.3.0" + "Purview.BuildSdk": "1.3.1" } } ``` @@ -201,7 +201,7 @@ to a low-importance message) and only a copy that still fails after every retry The bootstrapped `.editorconfig` opens with a version marker: ```editorconfig -# Purview.BuildSdk 1.3.0 - managed file, do not edit +# Purview.BuildSdk 1.3.1 - managed file, do not edit ``` That line is how the SDK recognises a file **it** wrote. When the SDK version changes, a file carrying @@ -283,11 +283,21 @@ and recommends the command that fixes the difference. It never installs, updates as a side effect of a restore or a build. ```bash -npx skills add purview-dev/build-sdk # install, the ordinary way +bun x skills add purview-dev/build-sdk # install, the ordinary way dotnet msbuild -t:RecommendPurviewSkills # what is missing, drifted or unverifiable dotnet msbuild -t:InstallPurviewSkills # explicit; delegates to the CLI +dotnet msbuild -t:UpdatePurviewSkills # re-add every skills-lock.json source, then 'skills update' +dotnet msbuild -t:CheckPurviewSkills # only the PurviewSkillsAgents directories exist +dotnet msbuild -t:FixPurviewSkills # remove the CLI's links from any other agent directory ``` +These run only when named: install, update and fix fail with `PRSKL0009` if anything pulls them into +a build, a restore or a design-time build. `UpdatePurviewSkills` re-adds each source a restored package ships at the commit that package version +was built from, so after upgrading a package reference it lands on the matching skills. Every other +source is re-added at its latest. `PurviewSkillsAgents` is empty by default, so the CLI auto-detects +agents; set it (for example `universal`) to choose, and `CheckPurviewSkills`/`FixPurviewSkills` then +enforce that only those agents' skill directories exist. + Compatibility is pinned by commit: a package's nuspec names the commit it was built from, the CLI accepts `owner/repo#` and records it, so a project on an older package version is never advised to install skills describing a newer API. @@ -298,8 +308,8 @@ to a different ref — the second is what a CI job calls. Reading the restore gr every build of every project was measured as the largest avoidable cost in an up-to-date build, so it no longer happens unless somebody asks for it. -The CLI is launched through a package runner. `PurviewSkillsRunner` defaults to `npx` — which ships -with Node itself — and also accepts `bunx`, `bun`, `pnpm`, `yarn`, `deno` or `skills`; anything else +The CLI is launched through a package runner. `PurviewSkillsRunner` defaults to `bun` (`bun x +skills`) and also accepts `npx`, `bunx`, `pnpm`, `yarn`, `deno` or `skills`; anything else falls through to ` skills@1`. Because `skills-lock.json` and the agent directories belong to the repository rather than to a project, every skill target runs **from the repository root, always**, and reports `PRSKL0003` rather than falling back to a project folder when that root cannot @@ -422,9 +432,11 @@ Non-packable projects (including web applications) default `WarnOnPackingNonPack | -- | -- | -- | | `PurviewSkillsLockFile` | `/skills-lock.json` | Overrides where the skills CLI's lockfile is read from. | | `PurviewSkillsSourceFolder` | `/skills` | Repository folder holding authored skills. | -| `PurviewSkillsAgents` | `*` | Agents that `InstallPurviewSkills` installs to, and that recommended commands name. | -| `PurviewSkillsRunner` | `npx` | Package runner used to launch the skills CLI (`npx`, `bunx`, `bun`, `pnpm`, `yarn`, `deno`, `skills`, or any other). | -| `PurviewSkillsRunnerCandidates` | `npx;bunx;bun;pnpm;yarn;deno` | Runners probed on `PATH` after `PurviewSkillsRunner`. | +| `PurviewSkillsAgents` | *(empty — the CLI auto-detects)* | Comma-separated agents the skill targets install to, and that recommended commands name. When set, `CheckPurviewSkills`/`FixPurviewSkills` allow only those agents' skill directories. | +| `PurviewSkillsUpdateMode` | `Auto` | When `UpdatePurviewSkills` runs `skills update`: `Auto` (only when `skills-lock.json` changed), `Always` or `Never`. | +| `PurviewSkillsDryRun` | `false` | When `true`, `InstallPurviewSkills` and `UpdatePurviewSkills` print the CLI commands without running them. | +| `PurviewSkillsRunner` | `bun` | Package runner used to launch the skills CLI (`bun`, `npx`, `bunx`, `pnpm`, `yarn`, `deno`, `skills`, or any other). | +| `PurviewSkillsRunnerCandidates` | `bun;npx;bunx;pnpm;yarn;deno` | Runners probed on `PATH` after `PurviewSkillsRunner`. | | `PurviewSkillsPackage` | `skills@1` | Package spec the executed launcher pins to. | | `PurviewSkillsInstallRoot` | *(resolved)* | Overrides the repository root every skill target runs from. | | `PurviewRepositoryRootMarkers` | `.git/;.github/;.gitattributes;LICENSE.md;AGENTS.md;.agents/;CLAUDE.md;.claude/;package.json` | Ordered markers identifying a repository root. The first is definitive and searched to the top; the rest resolve nearest-first. | @@ -524,9 +536,11 @@ The SDK now exports its properties via `CompilerVisibleProperty`, so analyzers a | `PurviewAutoSdkPack` | When `true`, automatically packs the `Sdk/` folder contents into the NuGet package with the correct root-level paths. | | `PurviewSkillsLockFile` | Where the skills CLI's `skills-lock.json` is read from (default `/skills-lock.json`). | | `PurviewSkillsSourceFolder` | Repository folder holding authored agent skills (default `/skills`). | -| `PurviewSkillsAgents` | Agents that `InstallPurviewSkills` installs to (default `*`). | -| `PurviewSkillsRunner` | Package runner used to launch the skills CLI (default `npx`; also `bunx`, `bun`, `pnpm`, `yarn`, `deno`, `skills`). | -| `PurviewSkillsRunnerCandidates` | Runners probed on `PATH` after `PurviewSkillsRunner` (default `npx;bunx;bun;pnpm;yarn;deno`). | +| `PurviewSkillsAgents` | Comma-separated agents the skill targets install to (default empty: the CLI auto-detects). | +| `PurviewSkillsUpdateMode` | When `UpdatePurviewSkills` runs `skills update`: `Auto` (default), `Always` or `Never`. | +| `PurviewSkillsDryRun` | When `true`, `InstallPurviewSkills` and `UpdatePurviewSkills` print commands without running them. | +| `PurviewSkillsRunner` | Package runner used to launch the skills CLI (default `bun`; also `npx`, `bunx`, `pnpm`, `yarn`, `deno`, `skills`). | +| `PurviewSkillsRunnerCandidates` | Runners probed on `PATH` after `PurviewSkillsRunner` (default `bun;npx;bunx;pnpm;yarn;deno`). | | `PurviewSkillsPackage` | Package spec the executed launcher pins to (default `skills@1`). | | `PurviewSkillsInstallRoot` | Overrides the repository root every skill target runs from. | | `PurviewRepositoryRootMarkers` | Ordered markers identifying a repository root (default `.git/;.github/;.gitattributes;LICENSE.md;AGENTS.md;.agents/;CLAUDE.md;.claude/;package.json`). | diff --git a/docs/wiki/Agent-Skills.md b/docs/wiki/Agent-Skills.md index fd0918a..eaf20b4 100644 --- a/docs/wiki/Agent-Skills.md +++ b/docs/wiki/Agent-Skills.md @@ -2,14 +2,16 @@ [Agent Skills](https://skills.sh/) are folders of `SKILL.md` guidance that coding agents load on demand. Purview publishes them in the standard format and installs them with the standard -[`skills` CLI](https://skills.sh/) — `npx skills add`, `npx skills update`, `npx skills remove`. +[`skills` CLI](https://skills.sh/) — `bun x skills add`, `bun x skills update`, `bun x skills remove`. `Purview.BuildSdk` deliberately does **not** reimplement any of that. It knows one thing the CLI cannot know: which skills are relevant to the NuGet packages a project actually references, and at which version. So the SDK **discovers and recommends**, and the CLI **installs**. -> The SDK recommends commands. It never runs them. Nothing on this page installs, updates or removes -> a skill as a side effect of a restore, a build, a design-time build or a test run. +> The reporting targets recommend commands and never run them. Only `InstallPurviewSkills` and +> `UpdatePurviewSkills`, invoked by name, run the CLI, and only `FixPurviewSkills` deletes anything. +> Nothing on this page installs, updates or removes a skill as a side effect of +> a restore, a build, a design-time build or a test run. ## The boundary @@ -21,11 +23,11 @@ flowchart TD SDK["Purview Build SDK"] Decision{"Skill status"} - Add["Suggest: npx skills add owner/repo#commit"] - Update["Suggest: pinned re-add
(not a blind npx skills update)"] - List["Suggest: npx skills list"] - Restore["Suggest: npx skills experimental_install"] - Use["Optional: npx skills use"] + Add["Suggest: bun x skills add owner/repo#commit"] + Update["Suggest: pinned re-add
(not a blind bun x skills update)"] + List["Suggest: bun x skills list"] + Restore["Suggest: bun x skills experimental_install"] + Use["Optional: bun x skills use"] OK["No recommendation"] CLI["Standard skills CLI"] @@ -56,7 +58,7 @@ flowchart TD | Component | Owns | | -- | -- | | Purview libraries | Their API guidance, authored as skills in their own repository | -| Skill publication | Making those skills reachable by `npx skills add` | +| Skill publication | Making those skills reachable by `bun x skills add` | | Package-to-skill metadata | Which package ships which skills, and at which commit | | Purview Build SDK | Reading package references, evaluating local state, recommending commands | | Skills CLI | Installing, updating, removing, source tracking, agent-specific destinations | @@ -71,9 +73,9 @@ That is the only route. No NuGet package carries skills: a package holds .NET ar CLI delivers skills from git. ```bash -npx skills add purview-dev/build-sdk # every skill -npx skills add purview-dev/build-sdk --skill purview-buildsdk-testing -npx skills add purview-dev/build-sdk --list # see what is there, install nothing +bun x skills add purview-dev/build-sdk # every skill +bun x skills add purview-dev/build-sdk --skill purview-buildsdk-testing +bun x skills add purview-dev/build-sdk --list # see what is there, install nothing ``` Skills live **alongside the library they describe**, in that library's repository, rather than in @@ -82,7 +84,7 @@ which library, and would drift from the libraries it documents. ## Authoring -`npx skills init ` scaffolds a skill; move it under `skills//` and adapt the frontmatter +`bun x skills init ` scaffolds a skill; move it under `skills//` and adapt the frontmatter to Purview conventions. `ValidatePurviewSkills` runs `BeforeTargets="Pack"`, so a malformed skill fails the build of the repository that owns it rather than reaching the CLI, which would silently skip it. It checks that: @@ -115,9 +117,9 @@ reported rather than installed from a guessed default branch. ## Recommendations -Recommendations are the one part of this feature that runs during an ordinary build. They read the -restore graph and the CLI's own `skills-lock.json`, both local; they recommend commands and never -run them; and they never fail a build. +Recommendations read the restore graph and the CLI's own `skills-lock.json`, both local; they +recommend commands and never run them; and `RecommendPurviewSkills` never fails a build. They run +only when a target is named — never during an ordinary build (see [When it runs](#when-it-runs)). ```bash dotnet msbuild -t:RecommendPurviewSkills # explicit, always reports @@ -128,11 +130,11 @@ Every skill a referenced package ships is classified: | State | Diagnostic | Severity | Recommendation | | -- | -- | -- | -- | | Installed at the referenced commit | *(none)* | — | — | -| Not installed | `PRSKL0010` | Warning | `npx skills add "#" --skill …` | +| Not installed | `PRSKL0010` | Warning | `bun x skills add "#" --skill …` | | Installed from a different ref | `PRSKL0011` | Warning | pinned re-add at the referenced commit | -| Compatibility unverifiable | `PRSKL0012` | Message | `npx skills list` | -| From a repository no longer referenced | `PRSKL0013` | Message | `npx skills remove ""` | -| Lockfile present, nothing restored | `PRSKL0014` | Message | `npx skills experimental_install` | +| Compatibility unverifiable | `PRSKL0012` | Message | `bun x skills list` | +| From a repository no longer referenced | `PRSKL0013` | Message | `bun x skills remove ""` | +| Lockfile present, nothing restored | `PRSKL0014` | Message | `bun x skills experimental_install` | Severity splits on what a developer can act on. A missing or drifted skill is one command away, so it is a warning. An unverifiable one usually is not — a package without SourceLink metadata cannot @@ -167,7 +169,10 @@ because somebody named it. | -- | -- | | `RecommendPurviewSkills` | Reports the state. Never fails a build. | | `VerifyPurviewSkills` | The same report, escalating `PRSKL0010`/`PRSKL0011` to an error. The CI gate. | -| `InstallPurviewSkills` | Installs what is missing, via the skills CLI. | +| `InstallPurviewSkills` | Installs every skill the restored packages ship, pinned to each package's commit, via the skills CLI. | +| `UpdatePurviewSkills` | Re-adds every source in `skills-lock.json` — package skills at their package's commit — then runs `skills update`. See [Updating](#updating). | +| `CheckPurviewSkills` | Fails when an agent skill directory exists for an agent `PurviewSkillsAgents` does not name. See [Choosing agents](#choosing-agents). | +| `FixPurviewSkills` | Removes the CLI's links from those directories, keeping and reporting anything real. | | `ListPurviewSkills` | Lists what the restored packages ship. Installs nothing. | A correctly installed set produces no output at all from either reporting target. @@ -177,9 +182,11 @@ A correctly installed set produces no output at all from either reporting target | Property | Default | Description | | -- | -- | -- | | `PurviewSkillsLockFile` | `/skills-lock.json` | Overrides where the CLI's lockfile is read from. | -| `PurviewSkillsAgents` | `*` | Agents named in the recommended install command. | -| `PurviewSkillsRunner` | `npx` | Package runner used to launch the CLI. | -| `PurviewSkillsRunnerCandidates` | `npx;bunx;bun;pnpm;yarn;deno` | Probed in order after `PurviewSkillsRunner`. | +| `PurviewSkillsAgents` | *(empty — the CLI auto-detects)* | Agents the skill targets install to and recommended commands name; when set, the only agent skill directories `CheckPurviewSkills` allows. | +| `PurviewSkillsUpdateMode` | `Auto` | When `UpdatePurviewSkills` runs `skills update`: `Auto` (only when the lockfile changed), `Always` or `Never`. | +| `PurviewSkillsDryRun` | `false` | Print the install and update commands without running them. | +| `PurviewSkillsRunner` | `bun` | Package runner used to launch the CLI. | +| `PurviewSkillsRunnerCandidates` | `bun;npx;bunx;pnpm;yarn;deno` | Probed in order after `PurviewSkillsRunner`. | | `PurviewSkillsPackage` | `skills@1` | Package spec the executed launcher pins to. | | `PurviewSkillsInstallRoot` | *(resolved)* | Overrides the repository root every skill target runs from. | | `PurviewRepositoryRootMarkers` | `.git/;.github/;.gitattributes;LICENSE.md;AGENTS.md;.agents/;CLAUDE.md;.claude/;package.json` | Ordered markers that identify a repository root; the first is definitive. | @@ -202,19 +209,19 @@ package Purview.Example 1.2.3 → nuspec commit 0123456… skills-lock.json: purview-alpha → ref aaaaaaa… ← drift ``` -This is why **`npx skills update` is not recommended on drift**. `update` installs the newest +This is why **`bun x skills update` is not recommended on drift**. `update` installs the newest published skill, which is correct only when the newest skill happens to describe the referenced package version — something that cannot be known offline, and that is false for any project not on the latest package. `PRSKL0011` therefore names the referenced package's own commit instead: ```bash -npx skills add "purview-dev/purview-example#0123456789abcdef0123456789abcdef01234567" \ - --skill "purview-alpha" --agent "*" -y +bun x skills add "purview-dev/purview-example#0123456789abcdef0123456789abcdef01234567" \ + --skill "purview-alpha" -y ``` -`npx skills update` remains the right command when the intent is to track the latest published -skills — for example right after upgrading the package reference, once the new version is restored -and the SDK reports the state as compatible again. +A bare re-add or `bun x skills update` is only right for skills no package version speaks for. +`UpdatePurviewSkills` does both, but re-adds a package's skills at that package's commit first — and +`skills update` re-fetches each entry at the ref the lockfile records, so it does not undo the pin. > Note the repeated `--skill` flag. The CLI does not split a comma-separated value: given one, it > matches nothing and reports "No matching skills found". @@ -223,7 +230,7 @@ and the SDK reports the state as compatible again. `skills-lock.json` is the CLI's own record of what is installed and where it came from. The SDK reads it and introduces **no competing lockfile**. Reading it rather than parsing CLI output is -deliberate: the JSON listing (`npx skills list --json`) is machine-readable but omits the `ref`, and +deliberate: the JSON listing (`bun x skills list --json`) is machine-readable but omits the `ref`, and the ref is the entire question. The lockfile carries it, and reading a file costs no Node tooling — which is what keeps recommendations working on a machine that can build .NET but has no Node. @@ -232,28 +239,106 @@ A lockfile is a record of an install, not proof of one: a fresh clone carries a agent skill directory anywhere at the repository root — and reports `PRSKL0014`: ```bash -npx skills experimental_install # restore everything in skills-lock.json +bun x skills experimental_install # restore everything in skills-lock.json ``` This command is **experimental**, so it is suggested and never required, never invoked -automatically, and always offered with the supported fallback (`npx skills add`). Prefer a stable +automatically, and always offered with the supported fallback (`bun x skills add`). Prefer a stable restore command if the CLI gains one. The detection looks for any root dot-folder containing a `skills` folder rather than hard-coding agent directories, which belong to the CLI and change independently of this SDK. '.agents/' is included in that scan: it is the CLI's universal install location, and nothing else writes there any more. ## Installing from the SDK -Installation is **explicit**. Neither target has `BeforeTargets`/`AfterTargets`: +Installation is **explicit**. None of these targets has `BeforeTargets`/`AfterTargets`: ```bash dotnet msbuild -t:ListPurviewSkills # diagnostics only, installs nothing dotnet msbuild -t:InstallPurviewSkills # delegates to the skills CLI +dotnet msbuild -t:UpdatePurviewSkills # re-add every lockfile source, then 'skills update' +dotnet msbuild -t:CheckPurviewSkills # only the PurviewSkillsAgents directories exist +dotnet msbuild -t:FixPurviewSkills # remove the CLI's links from any other agent directory ``` +`-p:PurviewSkillsDryRun=true` makes install and update print every command and run none. + +They run **only when named**. Nothing in the SDK hooks or depends on them, and install, update and fix +refuse to run when pulled into a build, a restore or a design-time build (`PRSKL0009`) — a +consumer's own `AfterTargets="Build"` hook included. Name one project, not the solution: on a +solution each target runs once per project. For `just` recipes, see the +[`purview-buildsdk-justfile` skill](../../skills/purview-buildsdk-justfile/SKILL.md). + `InstallPurviewSkills` shells out to the CLI rather than reimplementing it, because the CLI owns the agent-specific destinations and their discovery conventions — which differ per agent and change independently of this SDK — and owns `skills-lock.json`. +`InstallPurviewSkills` installs every skill the restored packages ship, not only the missing ones. +Re-running it is idempotent: the CLI skips a skill already installed from the same source and ref. + +### Updating + +```bash +dotnet restore # after bumping a PackageReference +dotnet msbuild -t:RecommendPurviewSkills # optional: shows PRSKL0011 for what will change +dotnet msbuild -t:UpdatePurviewSkills +``` + +`UpdatePurviewSkills` reads `skills-lock.json` and re-adds each unique source once, naming only the +skills the lockfile records for it — an update never installs a skill nobody asked for: + +| Source | Re-added as | +| -- | -- | +| shipped by a restored package | `#` — the commit that package version was built from, the same pinned add `PRSKL0011` recommends | +| anything else | `` — its latest | + +Then it runs `skills update -y -p` according to `PurviewSkillsUpdateMode`: `Auto` (default) only when +the re-adds changed the lockfile, `Always`, or `Never`. Every source is attempted even when one fails; +each failure is named as `PRSKL0008`, and `skills update` is skipped. No lockfile is `PRSKL0007` — +install first. A CI job can then call `-t:VerifyPurviewSkills` to confirm nothing is left drifted. + +`InstallPurviewSkills` is also enough to move package skills to a new package version: it re-adds +each at its package's commit, and the CLI skips one already at that ref. + +### Choosing agents + +`PurviewSkillsAgents` is **empty by default**: no `--agent` is passed and the CLI auto-detects the +agents installed on the machine (installing to all of them if it detects none). Set it to choose. +Each agent becomes its own `--agent` flag, because the CLI does not split a comma-separated value. + +```xml + + universal + +``` + +On a command line, `dotnet msbuild -p:` itself splits on `,` and `;`, so escape the separator +(`-p:PurviewSkillsAgents=claude-code%2Ccodex`) or quote the whole value +(`-p:PurviewSkillsAgents=\"claude-code codex\"`); spaces separate agents too. + +Once agents are named, `CheckPurviewSkills` enforces them: any agent skill directory at the repository +root that belongs to an agent not named fails as `PRSKL0015`. Two directories are always allowed — +`.agents/skills`, the CLI's canonical store that every agent directory links into, and the authored +`skills/` folder, even though openclaw installs into a root `skills/`. With `PurviewSkillsAgents` +empty, every agent directory is a legitimate destination and both targets report that there is +nothing to enforce. + +`FixPurviewSkills` removes what is provably the CLI's: links (symlinks, and junctions on Windows) — +the link only, never its target — and then any folder left empty. A real file is kept and reported, +because it cannot be told from hand-written content; note that for some agent selections the CLI +writes copies rather than links, and those are kept too. + +The agent-to-directory map is the `PurviewSkillsAgentDirectory` item group, taken from the CLI's +agent table. The CLI adds agents independently of this SDK, so an agent the map does not know is +`PRSKL0016` (a warning), fixed by adding an item: + +```xml + + + +``` + +### Installing from an explicit source + To install a Purview skill the project has no `PackageReference` for, name the source directly. The caller owns the ref in that case; append `#` to pin it: @@ -264,14 +349,13 @@ dotnet msbuild -t:InstallPurviewSkills -p:PurviewSkillsSource=purview- ### Package runners The CLI is published to npm, and every package runner spells "run this npm package" its own way. -`PurviewSkillsRunner` picks one; it defaults to **`npx`**, because npx ships with Node itself and is -therefore the one runner any machine with Node at all already has. +`PurviewSkillsRunner` picks one; it defaults to **`bun`** (`bun x skills`). | `PurviewSkillsRunner` | Executed | Recommended in diagnostics | | -- | -- | -- | -| `npx` *(default)* | `npx -y skills@1` | `npx skills …` | +| `bun` *(default)* | `bun x skills@1` | `bun x skills …` | +| `npx` | `npx -y skills@1` | `npx skills …` | | `bunx` | `bunx skills@1` | `bunx skills …` | -| `bun` | `bun x skills@1` | `bun x skills …` | | `pnpm` | `pnpm dlx skills@1` | `pnpm dlx skills …` | | `yarn` | `yarn dlx skills@1` | `yarn dlx skills …` | | `deno` | `deno run -A npm:skills@1` | `deno run -A npm:skills …` | @@ -289,7 +373,7 @@ most new runners adopt. `yarn dlx` needs Yarn 2+; Yarn 1 has no `dlx`. The executed launcher pins the CLI to a major so a breaking release cannot change what `InstallPurviewSkills` does — `PurviewSkillsPackage` (default `skills@1`) overrides the pin. Recommendation text deliberately drops it: that is a command for a -person to run, and `npx skills add` is the form they know. +person to run, and `bun x skills add` is the form they know. ### Installing in the repository that authors the skills @@ -354,14 +438,14 @@ Installation probes `PATH` rather than assuming the configured runner is present 1. `skills` — used as-is if it is already on `PATH`. No download, works offline. 2. `$(PurviewSkillsRunner)` — the configured preference. -3. `$(PurviewSkillsRunnerCandidates)` — `npx;bunx;bun;pnpm;yarn;deno` by default. +3. `$(PurviewSkillsRunnerCandidates)` — `bun;npx;bunx;pnpm;yarn;deno` by default. So a machine with bun but no Node still installs, rather than reporting that nothing was found. The runner that is actually found wins over the configured preference, because it is the one that can really run. `PATH` is read directly rather than by spawning a version check: it is deterministic, costs no process, and cannot hang on a cold download. -- Nothing found is `PRSKL0004`, naming the fix rather than surfacing a raw `'npx' is not recognized`. +- Nothing found is `PRSKL0004`, naming the fix rather than surfacing a raw `'bun' is not recognized`. - `PurviewSkillsCommand` overrides the launcher entirely and skips detection. `ListPurviewSkills` and `RecommendPurviewSkills` **never** error on a missing runner. Resolving, @@ -424,7 +508,7 @@ unaffected, because it sits below it. To consult a skill once without installing it: ```bash -npx skills use purview-dev/build-sdk@purview-buildsdk-testing | claude +bun x skills use purview-dev/build-sdk@purview-buildsdk-testing | claude ``` This is a good fit for a one-time operation. It is not a substitute for installing the skills a @@ -433,12 +517,12 @@ project needs throughout development, and the SDK's recommendations do not sugge ## npm distribution -`npx skills experimental_sync` discovers skills shipped inside npm packages, by reading +`bun x skills experimental_sync` discovers skills shipped inside npm packages, by reading `node_modules`. **Purview does not use it, and should not.** Purview libraries ship as NuGet packages; their skills are already described by those packages' metadata, and are already installable -from git by `npx skills add`. Publishing a parallel npm package would mean: +from git by `bun x skills add`. Publishing a parallel npm package would mean: - a second release pipeline to version, publish and keep in step with the NuGet one, - a `package.json` and a `node_modules` restore in .NET repositories that otherwise need neither, @@ -457,15 +541,15 @@ Verified against the `skills` CLI v1. | Command | Purpose | Role here | | -- | -- | -- | -| `npx skills add ` | Install skills | The installation workflow; accepts `owner/repo#` | -| `npx skills list` | List installed skills | Diagnostics; `--json` is machine-readable but omits `ref` | -| `npx skills find [query]` | Search for skills | Discovery fallback where no package mapping exists | -| `npx skills update` | Update to latest | Correct only when latest matches the referenced package | -| `npx skills remove` | Remove skills | Developer-managed | -| `npx skills use @` | Use without installing | One-off guidance | -| `npx skills experimental_install` | Restore from `skills-lock.json` | Optional bootstrap; experimental | -| `npx skills init [name]` | Scaffold a skill | Authoring | -| `npx skills experimental_sync` | Sync from `node_modules` | Not used by Purview — see above | +| `bun x skills add ` | Install skills | The installation workflow; accepts `owner/repo#` | +| `bun x skills list` | List installed skills | Diagnostics; `--json` is machine-readable but omits `ref` | +| `bun x skills find [query]` | Search for skills | Discovery fallback where no package mapping exists | +| `bun x skills update` | Update to latest | Correct only when latest matches the referenced package | +| `bun x skills remove` | Remove skills | Developer-managed | +| `bun x skills use @` | Use without installing | One-off guidance | +| `bun x skills experimental_install` | Restore from `skills-lock.json` | Optional bootstrap; experimental | +| `bun x skills init [name]` | Scaffold a skill | Authoring | +| `bun x skills experimental_sync` | Sync from `node_modules` | Not used by Purview — see above | ## Diagnostics @@ -476,11 +560,17 @@ Verified against the `skills` CLI v1. | `PRSKL0003` | The repository root could not be resolved, so there is nowhere correct to install | | `PRSKL0004` | No package runner was found on `PATH` | | `PRSKL0005` | An authored skill is malformed | +| `PRSKL0006` | `PurviewSkillsUpdateMode` is not `Auto`, `Always` or `Never` | +| `PRSKL0007` | `UpdatePurviewSkills` found no `skills-lock.json` to update from | +| `PRSKL0008` | Re-adding a lockfile source failed; `skills update` was skipped | +| `PRSKL0009` | Install, update or fix was pulled into a build, a restore or a design-time build | | `PRSKL0010` | Skills a referenced package ships are not installed | | `PRSKL0011` | Installed skills are pinned to a different ref than the referenced package | | `PRSKL0012` | Compatibility cannot be verified | | `PRSKL0013` | A skill is installed from a repository no longer referenced | | `PRSKL0014` | `skills-lock.json` exists but no skills have been restored | +| `PRSKL0015` | An agent skill directory exists for an agent `PurviewSkillsAgents` does not name | +| `PRSKL0016` | `PurviewSkillsAgents` names an agent with no known skills directory | ## See also diff --git a/docs/wiki/Getting-Started.md b/docs/wiki/Getting-Started.md index fc5f045..1d534d2 100644 --- a/docs/wiki/Getting-Started.md +++ b/docs/wiki/Getting-Started.md @@ -14,7 +14,7 @@ Add `Purview.BuildSdk` to the `msbuild-sdks` section so MSBuild can resolve the "runner": "Microsoft.Testing.Platform" }, "msbuild-sdks": { - "Purview.BuildSdk": "1.3.0" + "Purview.BuildSdk": "1.3.1" } } ``` diff --git a/docs/wiki/Home.md b/docs/wiki/Home.md index c35c6a8..2e3bfe5 100644 --- a/docs/wiki/Home.md +++ b/docs/wiki/Home.md @@ -56,9 +56,10 @@ repo root inherits everything automatically. (`PDS0001`, `PDS0005`, `PDS0006`, `PDS0007`), plus IDE code fixes for naming and extensions-namespace conventions. - **Agent skills** — the SDK resolves which skills a restored package ships and the exact commit it was - built from, and reports what is missing or has drifted (`PRSKL0010`–`PRSKL0014`) during an ordinary - build. Installing them is a separate, explicit step (`dotnet msbuild -t:InstallPurviewSkills`) that - delegates to the skills CLI. The SDK itself writes nothing into an agent directory: the build-time + built from, and reports what is missing or has drifted (`PRSKL0010`–`PRSKL0014`) when asked + (`-t:RecommendPurviewSkills`, or `-t:VerifyPurviewSkills` in CI) — never during an ordinary build. + Installing or updating them is a separate, explicit step (`dotnet msbuild -t:InstallPurviewSkills`) + that delegates to the skills CLI. The SDK itself writes nothing into an agent directory: the build-time `.agents/**` mirror was removed outright, because the CLI owns those paths and two writers on one set of paths meant the mirror silently overwriting a pinned install. diff --git a/package.json b/package.json index f496cfd..8ec7966 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "build-sdk", - "version": "1.3.0", + "version": "1.3.1", "homepage": "https://purview.dev/projects/build-sdk/", "bugs": { "url": "https://github.com/purview-dev/build-sdk/issues" diff --git a/skills/purview-buildsdk-adoption/SKILL.md b/skills/purview-buildsdk-adoption/SKILL.md index b1f222c..5a4caf1 100644 --- a/skills/purview-buildsdk-adoption/SKILL.md +++ b/skills/purview-buildsdk-adoption/SKILL.md @@ -100,9 +100,14 @@ silently gets no runner, no coverage, no `InternalsVisibleTo` and no category. ## 5. Agent skills are a separate, explicit step -The SDK does not install agent skills, and nothing it does during a build writes to an agent -directory. It reads which skills each referenced package ships and recommends the command that -installs them; the skills CLI does the installing. See `purview-buildsdk-skill-installation`. +Nothing the SDK does during a restore or a build touches agent skills. Installing, updating and +cleaning them up are separate targets a person runs by name — `InstallPurviewSkills`, +`UpdatePurviewSkills`, `CheckPurviewSkills`, `FixPurviewSkills` — and the ones that change the +repository refuse to run inside a build (`PRSKL0009`). Do not hook them into one. The skills CLI does +the installing, through `bun x` by default; with `PurviewSkillsAgents` unset it auto-detects agents. + +If the repository uses `just`, add recipes for them — see `purview-buildsdk-justfile`. For what each +target does and the `PRSKL*` diagnostics, see `purview-buildsdk-skill-installation`. ## Constraints diff --git a/skills/purview-buildsdk-configuration/SKILL.md b/skills/purview-buildsdk-configuration/SKILL.md index d82768b..bcd1e2e 100644 --- a/skills/purview-buildsdk-configuration/SKILL.md +++ b/skills/purview-buildsdk-configuration/SKILL.md @@ -105,6 +105,19 @@ the repository; that is gone, along with `EnableAgentFolderInPackage`, `AgentPac and the `PurviewAgentFolder*` properties. Skills have exactly one installer, the skills CLI — see `purview-buildsdk-skill-installation`. +The skill targets read their settings when they run, so these can go in `Directory.Build.props` or +on the command line, and a repository-wide choice belongs in `Directory.Build.props`: + +| Property | Default | Controls | +| -- | -- | -- | +| `PurviewSkillsRunner` | `bun` (`bun x skills`) | Which package runner launches the skills CLI | +| `PurviewSkillsAgents` | empty — the CLI auto-detects | Agents to install to; when set, the only agent directories `CheckPurviewSkills` allows | +| `PurviewSkillsUpdateMode` | `Auto` | When `UpdatePurviewSkills` runs `skills update`: `Auto`, `Always`, `Never` | +| `PurviewSkillsDryRun` | `false` | Install and update print commands instead of running them | + +On a command line, `dotnet msbuild -p:` splits values on `,` and `;` — write +`-p:PurviewSkillsAgents=claude-code%2Ccodex`. For `just` recipes, see `purview-buildsdk-justfile`. + ## Guidance when changing SDK configuration 1. Preserve existing defaults unless the task explicitly changes product behaviour. diff --git a/skills/purview-buildsdk-configuration/references/properties.md b/skills/purview-buildsdk-configuration/references/properties.md index 3b22bc8..d45ca48 100644 --- a/skills/purview-buildsdk-configuration/references/properties.md +++ b/skills/purview-buildsdk-configuration/references/properties.md @@ -135,7 +135,8 @@ nothing else was left for the mirror to deliver. Covered in full by `purview-buildsdk-skill-installation`: `PurviewSkillsRunner`, `PurviewSkillsRunnerCandidates`, `PurviewSkillsCommand`, `PurviewSkillsPackage`, -`PurviewSkillsAgents`, `PurviewSkillsSource`, `PurviewSkillsInstallRoot`, `PurviewSkillsRepoRoot`. +`PurviewSkillsAgents`, `PurviewSkillsSource`, `PurviewSkillsInstallRoot`, `PurviewSkillsRepoRoot`, +`PurviewSkillsUpdateMode`, `PurviewSkillsDryRun`, and the `PurviewSkillsAgentDirectory` item group. ## Compiler-visible properties @@ -162,6 +163,6 @@ Exported to analyzers and generators as `build_property.`: - `PurviewSuppressCopyRetryWarnings` - `PurviewSkillsSourceFolder`, `PurviewSkillsAgents`, `PurviewSkillsInstallRoot`, `PurviewRepositoryRootMarkers`, - `PurviewSkillsRunner` + `PurviewSkillsRunner`, `PurviewSkillsUpdateMode`, `PurviewSkillsDryRun` - `PurviewBuildSdkVersionForGlobalJson`, `CurrentYear`, `AutoGeneratedAssemblyInfoFile`, `AutoGeneratedAssemblyIdentifierFile` diff --git a/skills/purview-buildsdk-justfile/SKILL.md b/skills/purview-buildsdk-justfile/SKILL.md new file mode 100644 index 0000000..5e03ce2 --- /dev/null +++ b/skills/purview-buildsdk-justfile/SKILL.md @@ -0,0 +1,143 @@ +--- +name: purview-buildsdk-justfile +description: Add or update Justfile recipes that run Purview.BuildSdk's agent-skill targets - skills-update, skills-check, skills-fix, skills-install, skills-recommend and skills-verify. Use when a repository wants a just recipe to update, install, check or clean up its agent skills, when replacing hand-written skill scripts (update-agent-skills.ts, ensure-universal-skills.ts, package.json skills:* scripts) with the SDK targets, or when an existing skills recipe runs the wrong project, the wrong runner or the wrong agents. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Justfile recipes for the agent-skill targets + +The recipes are thin: each one names one SDK target. The behaviour lives in the targets, which are +covered by `purview-buildsdk-skill-installation`; this skill is about wiring them into a `Justfile` +correctly. + +## The rule that matters most + +**Every skill recipe is run by a person, on purpose.** `InstallPurviewSkills`, `UpdatePurviewSkills` +and `FixPurviewSkills` change the repository, and the SDK refuses to run them inside a build, a +restore or a design-time build (`PRSKL0009`). Keep the Justfile consistent with that: + +- never make a skills recipe a dependency of `build`, `test`, `default` or any other recipe; +- never call `skills-update`, `skills-install` or `skills-fix` from a git hook (`lefthook.yml`, + `.husky/`) or a CI job that commits; +- `skills-verify` is the one recipe CI may call: it is read-only and fails on missing or drifted skills. + +## Recipes + +Add them under one group, after the build recipes. Replace `src/MyApp/MyApp.csproj` as described below. + +```just +skills_project := "src/MyApp/MyApp.csproj" + +# Update agent skills: re-add every skills-lock.json source, then 'skills update' +[group('Agent skills')] +skills-update *args: + dotnet msbuild "{{ skills_project }}" "-t:UpdatePurviewSkills;FixPurviewSkills" -v:n {{ args }} + +# Check that only the skill directories of the agents in PurviewSkillsAgents exist +[group('Agent skills')] +skills-check *args: + dotnet msbuild "{{ skills_project }}" -t:CheckPurviewSkills -v:n {{ args }} + +# Remove the skills CLI's links from agent directories PurviewSkillsAgents does not select +[group('Agent skills')] +skills-fix *args: + dotnet msbuild "{{ skills_project }}" -t:FixPurviewSkills -v:n {{ args }} + +# Install the skills the referenced packages ship (pass -p:PurviewSkillsSource=owner/repo for others) +[group('Agent skills')] +skills-install *args: + dotnet msbuild "{{ skills_project }}" -t:InstallPurviewSkills -v:n {{ args }} + +# Report missing, drifted or unverifiable skills without changing anything +[group('Agent skills')] +skills-recommend *args: + dotnet msbuild "{{ skills_project }}" -t:RecommendPurviewSkills -v:n {{ args }} + +# Fail when a referenced package's skills are missing or drifted - the CI gate +[group('Agent skills')] +skills-verify *args: + dotnet msbuild "{{ skills_project }}" -t:VerifyPurviewSkills -v:n {{ args }} +``` + +Only add the recipes the repository will use. `skills-update` and `skills-check` are the usual pair. + +### Details that break a recipe if missed + +- **Name one project, never the solution.** `dotnet msbuild -t:UpdatePurviewSkills` runs + the target once per project, re-adding every source N times. Every skill target works from the + repository root whichever project runs it, so pick one that imports the SDK. If a package ships + skills, choose a project that references it: only packages in *that* project's restore graph are + pinned to their commit. +- **Restore first.** The targets read `project.assets.json` and never restore on their own + (`PRSKL0001`). A fresh clone needs `dotnet restore` (or `just restore`) before `skills-update`. +- **Quote a multi-target `-t:`.** `"-t:UpdatePurviewSkills;FixPurviewSkills"` — unquoted, `sh` treats + the `;` as the end of the command and runs `FixPurviewSkills` as a separate, failing command. +- **Keep `*args` last** so a caller can pass properties: `just skills-update -p:PurviewSkillsDryRun=true`. +- `FixPurviewSkills` after `UpdatePurviewSkills` does nothing unless `PurviewSkillsAgents` is set, so + it is safe in the recipe either way. + +## Runner and agents belong in MSBuild, not in the recipe + +Set repository-wide choices once in `Directory.Build.props`, not as flags repeated in each recipe, +so a plain `dotnet msbuild -t:...` behaves the same as the recipe: + +```xml + + + bun + + universal + +``` + +One-off overrides go on the command line. `dotnet msbuild -p:` splits values on `,` and `;`, so +escape the separator: `just skills-update -p:PurviewSkillsAgents=claude-code%2Ccodex`. + +Useful properties to pass through `*args`: + +| Property | Effect | +| -- | -- | +| `PurviewSkillsDryRun=true` | Print every CLI command; run none | +| `PurviewSkillsUpdateMode=Always` | Run `skills update` even when the lockfile did not change (`Auto` default, `Never`) | +| `PurviewSkillsRunner=npx` | Launch the CLI through another runner (`bunx`, `pnpm`, `yarn`, `deno`) | +| `PurviewSkillsSource=owner/repo#ref` | `skills-install` from an explicit source with no package behind it | + +## Replacing hand-written skill scripts + +Repositories often carry their own scripts for this. The SDK targets replace them: + +| Hand-written | Replace with | +| -- | -- | +| `scripts/update-agent-skills.ts`, `"skills:update"` | `skills-update` (`UpdatePurviewSkills`) | +| `--update` / `--no-update` | `-p:PurviewSkillsUpdateMode=Always` / `Never` | +| `--dry-run` | `-p:PurviewSkillsDryRun=true` | +| `scripts/ensure-universal-skills.ts check`, `"skills:check"` | `skills-check` with `PurviewSkillsAgents=universal` | +| `scripts/ensure-universal-skills.ts fix`, `"skills:fix"` | `skills-fix` | +| a hard-coded `-a universal` | `PurviewSkillsAgents` — or leave it empty to auto-detect | + +Two behaviours differ, deliberately — mention them when migrating: + +- A source a referenced package ships is re-added **at that package's commit**, not at latest, so its + skills keep matching the package version. Other sources are re-added at latest, as before. +- Only the skills `skills-lock.json` records for a source are re-added, not `-s '*'`, so an update + never installs a skill nobody chose. + +After switching, delete the scripts and their `package.json` entries, and remove `package.json` +itself if nothing else uses it. Run `just skills-update -p:PurviewSkillsDryRun=true` and compare the +printed commands with what the old script ran before deleting anything. + +## Updating an existing Justfile + +1. Read the whole `Justfile` first: match its group names, comment style, and any `set shell` + (a PowerShell shell still needs the `-t:` value quoted). +2. Keep recipe names the repository already uses; change only what they run. If `skills-check` + already means "report", rename it to `skills-recommend` rather than silently changing its meaning, + and say so. +3. Check nothing depends on a changing recipe: `just --list`, then search the Justfile, CI workflows + and hook configuration for the recipe name. +4. Verify: `just --list` shows the group, `just skills-update -p:PurviewSkillsDryRun=true` prints the + commands, and `just skills-check` passes or names a real problem. diff --git a/skills/purview-buildsdk-repository-review/SKILL.md b/skills/purview-buildsdk-repository-review/SKILL.md index ce1fd29..b4f56fa 100644 --- a/skills/purview-buildsdk-repository-review/SKILL.md +++ b/skills/purview-buildsdk-repository-review/SKILL.md @@ -1,6 +1,6 @@ --- name: purview-buildsdk-repository-review -description: Review a repository that uses Purview.BuildSdk for naming, placement and test-structure alignment, and report what the SDK can and cannot do automatically as a result. Use when asked to audit, rationalise or health-check a repo against the SDK's conventions, when onboarding an existing codebase onto Purview.BuildSdk, or when deciding whether a layout quirk is worth changing. +description: Review a repository that uses Purview.BuildSdk for naming, placement, test-structure and agent-skill-management alignment, and report what the SDK can and cannot do automatically as a result. Use when asked to audit, rationalise or health-check a repo against the SDK's conventions, when onboarding an existing codebase onto Purview.BuildSdk, or when deciding whether a layout quirk is worth changing. license: MIT metadata: package: Purview.BuildSdk @@ -89,7 +89,24 @@ go stale: is not any more. - A package the SDK already adds, restated in a project file, is maintenance cost with no effect. -## 6. Explain consequences, not just mismatches +## 6. Check how agent skills are managed + +Only relevant when the repository has a `skills-lock.json`, an agent skills directory, or a package +reference that ships skills: + +- **Anything that runs `InstallPurviewSkills`, `UpdatePurviewSkills` or `FixPurviewSkills` + implicitly** — an `AfterTargets`/`BeforeTargets` hook, a `*DependsOn` property, a git hook, a + `just` recipe dependency — is structural debt: the SDK fails it with `PRSKL0009`, and skill changes + should only happen when somebody asks. +- **Hand-written skill scripts** (`update-agent-skills.ts`, `ensure-universal-skills.ts`, + `package.json` `skills:*` entries) are an incremental opportunity: the SDK targets replace them, and + pin package skills to their package's commit where the scripts install latest. See + `purview-buildsdk-justfile`. +- **Drift**: run `dotnet msbuild -t:RecommendPurviewSkills` and quote any `PRSKL0010`/`PRSKL0011`. +- **Layout**: if `PurviewSkillsAgents` is set, run `-t:CheckPurviewSkills` and quote any `PRSKL0015`. + If it is not set, every agent directory is legitimate — that is acceptable variance, not a finding. + +## 7. Explain consequences, not just mismatches For each finding, name the specific automatic behaviour that is lost: `RootNamespace`, `AssemblyName`, `PackageId`, `TargetProjectName`, the automatic target-project reference, the diff --git a/skills/purview-buildsdk-skill-installation/SKILL.md b/skills/purview-buildsdk-skill-installation/SKILL.md index 59bfb54..1fbd729 100644 --- a/skills/purview-buildsdk-skill-installation/SKILL.md +++ b/skills/purview-buildsdk-skill-installation/SKILL.md @@ -1,6 +1,6 @@ --- name: purview-buildsdk-skill-installation -description: Install, list and diagnose the agent skills a Purview.BuildSdk package ships. Use for the InstallPurviewSkills, ListPurviewSkills and RecommendPurviewSkills MSBuild targets, for any PRSKL0001-PRSKL0014 diagnostic, for choosing a package runner (npx, pnpm dlx, bunx, yarn dlx, deno), for skills-lock.json, or when a package ships skills but no agent can see them. +description: Install, update, list, check and diagnose the agent skills a Purview.BuildSdk package ships. Use for the InstallPurviewSkills, UpdatePurviewSkills, CheckPurviewSkills, FixPurviewSkills, ListPurviewSkills, RecommendPurviewSkills and VerifyPurviewSkills MSBuild targets, for updating skills after a package upgrade, for choosing agents with PurviewSkillsAgents, for any PRSKL diagnostic, for choosing a package runner (bun x, npx, pnpm dlx, bunx, yarn dlx, deno), for skills-lock.json, or when a package ships skills but no agent can see them. license: MIT metadata: package: Purview.BuildSdk @@ -9,7 +9,8 @@ metadata: # Installing the skills a package ships -For *authoring* skills, use `purview-buildsdk-skill-tests`. +For *authoring* skills, use `purview-buildsdk-skill-tests`; for `just` recipes that run these +targets, `purview-buildsdk-justfile`. ## The one thing to understand first @@ -30,10 +31,19 @@ restore, a build, a design-time build or a test run. ```bash dotnet msbuild -t:ListPurviewSkills # diagnostics only, installs nothing dotnet msbuild -t:InstallPurviewSkills # delegates to the skills CLI +dotnet msbuild -t:UpdatePurviewSkills # re-add every skills-lock.json source, then 'skills update' +dotnet msbuild -t:CheckPurviewSkills # only the PurviewSkillsAgents directories exist +dotnet msbuild -t:FixPurviewSkills # remove the CLI's links from any other agent directory ``` -Neither target has `BeforeTargets` or `AfterTargets`. If skills are appearing or changing on their -own, something has been wired up that should not be. +They run **only when named**. None has `BeforeTargets` or `AfterTargets`, nothing in the SDK depends +on them, and `InstallPurviewSkills`, `UpdatePurviewSkills` and `FixPurviewSkills` refuse to run when +something else pulls them into a build, a restore or a design-time build (`PRSKL0009`). Do not hook +them into a build, a `*DependsOn` property or a git hook. If skills are changing on their own, +something has been wired up that should not be. + +Name **one project**, never the solution: on a solution each target runs once per project. For +`just` recipes that run these targets, see `purview-buildsdk-justfile`. ## How a skill is resolved, and why the version is always right @@ -48,6 +58,58 @@ So the resolved ref is the commit the *restored package version* was built from. `PackageReference` gets that version's skills, not whatever is on the default branch. Compatibility is exact by construction rather than by convention. +## Updating + +```bash +# after bumping the PackageReference +dotnet restore +dotnet msbuild -t:UpdatePurviewSkills +``` + +`UpdatePurviewSkills` re-adds every unique source recorded in `skills-lock.json`, naming only the +skills the lockfile records for it: + +- a source a restored package ships is re-added at **that package's commit** — the same pinned `add` + `PRSKL0011` recommends, so the skills match the package version referenced; +- every other source is re-added at its latest. + +Then it runs `skills update -y -p`, which re-fetches each entry at the ref the lockfile records, so it +cannot undo a pin. `PurviewSkillsUpdateMode` decides when: `Auto` (default — only when the re-adds +changed the lockfile), `Always` or `Never`. `-p:PurviewSkillsDryRun=true` prints every command and +runs none. Each failed source is named (`PRSKL0008`) and `skills update` is then skipped. + +`InstallPurviewSkills` also works as an update for package skills alone: it re-adds each at its +package's commit, and the CLI skips one already at that ref. Do not reach for a bare +`skills update` instead; see below. + +## Choosing agents + +`PurviewSkillsAgents` is **empty by default**, which passes no `--agent` and lets the CLI auto-detect +the agents on the machine. Set it to choose — in a project file `universal`, +or on a command line with the separator escaped or the value quoted, because `dotnet msbuild -p:` +itself splits on `,` and `;`: + +```bash +dotnet msbuild -t:UpdatePurviewSkills -p:PurviewSkillsAgents=claude-code%2Ccodex +``` + +Each agent becomes its own `--agent` flag; the CLI does not split a comma-separated value. + +When `PurviewSkillsAgents` is set, `CheckPurviewSkills` fails (`PRSKL0015`) for any agent skill +directory at the repository root that belongs to an agent it does not name. `.agents/skills` is always +allowed — it is the CLI's canonical store — and the authored `skills/` folder is never touched. +`FixPurviewSkills` removes links (symlinks, junctions) from those directories, never their targets, +and then any folder left empty; a real file is kept and reported, because it is not provably the +CLI's. The agent-to-directory map is the `PurviewSkillsAgentDirectory` item group, taken from the +CLI's own agent table; an agent it does not know is `PRSKL0016`, fixed by adding an item: + +```xml + +``` + +With `PurviewSkillsAgents` empty, both targets say so and do nothing: every agent directory is then a +legitimate destination. + ## Installing from an explicit source For a skill you want but have no `PackageReference` for: @@ -66,13 +128,19 @@ The caller owns the ref here — append `#` to pin it. This path needs no r | `PRSKL0001` | `project.assets.json` is missing | Restore. Resolution reads the restore graph and deliberately does not trigger a restore of its own | | `PRSKL0002` | The package's nuspec has no GitHub repository URL, or no commit | The package cannot be pinned. It is reported rather than installed from a guessed branch | | `PRSKL0003` | No repository root resolved | Pass `-p:PurviewSkillsInstallRoot=` | -| `PRSKL0004` | No skills CLI and no package runner found | Install Node, or set `PurviewSkillsCommand` | +| `PRSKL0004` | No skills CLI and no package runner found | Install Bun or Node, or set `PurviewSkillsCommand` | | `PRSKL0005` | An authored skill is malformed | See `purview-buildsdk-skill-tests` | -| `PRSKL0010` | A skill a referenced package ships is missing | Warning. `npx skills add ...` | +| `PRSKL0006` | `PurviewSkillsUpdateMode` is not `Auto`, `Always` or `Never` | Fix the value | +| `PRSKL0007` | `UpdatePurviewSkills` found no `skills-lock.json` | Install first with `-t:InstallPurviewSkills` | +| `PRSKL0008` | Re-adding a lockfile source failed | Read the CLI output above it; `skills update` was skipped | +| `PRSKL0009` | Install, update or fix was pulled into a build, a restore or a design-time build | Remove the hook; run the target on its own | +| `PRSKL0010` | A skill a referenced package ships is missing | Warning. `bun x skills add ...` | | `PRSKL0011` | An installed skill is pinned to a different ref than the referenced package | Warning. Install the package's own commit | | `PRSKL0012` | A skill cannot be verified | Message | | `PRSKL0013` | An orphaned lockfile entry | Message | | `PRSKL0014` | Recorded in the lockfile but never restored | Message | +| `PRSKL0015` | An agent skill directory exists for an agent `PurviewSkillsAgents` does not name | `-t:FixPurviewSkills`, then remove anything it kept by hand | +| `PRSKL0016` | `PurviewSkillsAgents` names an agent with no known skills directory | Warning. Add a `PurviewSkillsAgentDirectory` item | ## Nothing about skills runs during a build @@ -102,7 +170,8 @@ messages. Failing over something the caller cannot clear is how a gate gets swit Three things shape the output: - Missing and drifted are **warnings** (fixable in one command); unverifiable and orphaned are - **messages**. `VerifyPurviewSkills` is what turns them into failures. + **messages**. `VerifyPurviewSkills` turns missing and drifted into failures, and promotes + unverifiable to a warning. - A correctly installed set produces no output at all. - There is no deduplication stamp any more. `EnablePurviewSkillRecommendations`, `PurviewSkillRecommendationsInCI` and `PurviewSkillRecommendationStampPath` were retired with the @@ -113,22 +182,23 @@ Orphan detection is **owner-scoped on purpose**: a lockfile cannot record who as only an entry under an owner the project references — but from a repository it no longer references — is reported. A skill from any other owner is the developer's own, and is never mentioned. -### Drift never recommends `npx skills update` +### Drift never recommends a bare `skills update` -`update` installs the *newest* published skill, which is only correct when the newest skill happens -to describe the referenced package version. `PRSKL0011` names the referenced package's own commit in -a pinned `npx skills add` instead. +Re-adding a source without a ref installs the *newest* published skill, which is only correct when +the newest skill happens to describe the referenced package version. `PRSKL0011` names the referenced +package's own commit in a pinned `skills add` instead — which is what `-t:InstallPurviewSkills` and +`-t:UpdatePurviewSkills` run for package skills. ## Package runners The CLI is published to npm, and every runner spells "run this npm package" differently. -`PurviewSkillsRunner` selects one, defaulting to `npx` because npx ships with Node itself. +`PurviewSkillsRunner` selects one, defaulting to `bun` (`bun x skills`). | Runner | Invocation | | -- | -- | +| `bun` *(default)* | `bun x skills@1` | | `npx` | `npx -y skills@1` | | `bunx` | `bunx skills@1` | -| `bun` | `bun x skills@1` | | `pnpm` | `pnpm dlx skills@1` | | `yarn` | `yarn dlx skills@1` — Yarn 2+ only | | `deno` | `deno run -A npm:skills@1` | @@ -167,11 +237,9 @@ two-pass behaviour and its home-directory bound are both load-bearing. The CLI owns the agent-specific destinations and their discovery conventions, which differ per agent and change independently of this SDK. It also owns `skills-lock.json`, so a repository can restore -the same set later with `npx skills experimental_install`. - -`PurviewSkillsAgents` (default `*`) narrows the target agents. +the same set later with `bun x skills experimental_install`. -The lockfile is read **directly** rather than by parsing `npx skills list --json`, because that +The lockfile is read **directly** rather than by parsing `skills list --json`, because that listing omits the `ref` — and the ref is the whole compatibility question. Reading a file also needs no Node tooling, which is what keeps recommendations working on a machine that can build .NET but has no Node. There is deliberately no competing lockfile. diff --git a/skills/purview-buildsdk-skill-installation/references/repository-root.md b/skills/purview-buildsdk-skill-installation/references/repository-root.md index e90b695..d556174 100644 --- a/skills/purview-buildsdk-skill-installation/references/repository-root.md +++ b/skills/purview-buildsdk-skill-installation/references/repository-root.md @@ -44,7 +44,9 @@ an agent directory somewhere that is not the repository. A property a dependency computes is still empty in the depending target's `Condition`. That is why the unresolved-root guard sits on the `EvaluatePurviewSkillState` task rather than on -`RecommendPurviewSkills` — and equally why the build hook's CI and design-time gating works at all. +`RecommendPurviewSkills`, and why the explicit-only guard (`PRSKL0009`) is a dependency target with +its own `Error` rather than a `Condition` on `InstallPurviewSkills`, `UpdatePurviewSkills` or +`FixPurviewSkills`. ## The managed port diff --git a/src/src/BuildSdk/Sdk/Props/Defaults.props b/src/src/BuildSdk/Sdk/Props/Defaults.props index b793c16..543e70d 100644 --- a/src/src/BuildSdk/Sdk/Props/Defaults.props +++ b/src/src/BuildSdk/Sdk/Props/Defaults.props @@ -64,7 +64,8 @@ true @@ -72,14 +73,21 @@ which is where the CLI writes it for a project-scope install. --> - npx + bun is the default ('bun x skills'); set npx, bunx, pnpm, yarn or deno to use another. --> + bun npx;bunx;bun;pnpm;yarn;deno + >bun;npx;bunx;pnpm;yarn;deno + + + + Auto diff --git a/src/src/BuildSdk/Sdk/Sdk.props b/src/src/BuildSdk/Sdk/Sdk.props index 77b1666..9907ed6 100644 --- a/src/src/BuildSdk/Sdk/Sdk.props +++ b/src/src/BuildSdk/Sdk/Sdk.props @@ -411,10 +411,16 @@ Repository folder holding authored agent skills. Defaults to the repository-root skills/ folder. - Comma-separated agents that InstallPurviewSkills installs to. Defaults to '*' (every agent the skills CLI detects). + Comma-separated agents the skill targets install to. Empty (the default) lets the skills CLI auto-detect; when set, CheckPurviewSkills and FixPurviewSkills allow only those agents' skill directories. + + + When UpdatePurviewSkills runs 'skills update' after re-adding every lockfile source: Auto (only when skills-lock.json changed), Always or Never. Defaults to Auto. + + + When true, InstallPurviewSkills and UpdatePurviewSkills print the skills CLI commands without running them. - Overrides how the skills CLI is invoked. Empty detects 'skills' then 'npx' on PATH. + Overrides how the skills CLI is invoked. Empty detects 'skills', then PurviewSkillsRunner, then PurviewSkillsRunnerCandidates on PATH. Installs from an explicit git source (owner/repo, optionally '#ref') instead of from referenced packages. @@ -429,10 +435,10 @@ Ordered markers that identify a repository root, most authoritative first. The first is definitive and searched to the top; the rest resolve nearest-first. A trailing slash marks a directory. - Package runner used to launch the skills CLI (npx, bunx, bun, pnpm, yarn, deno, skills, or any other). Defaults to npx. + Package runner used to launch the skills CLI (bun, npx, bunx, pnpm, yarn, deno, skills, or any other). Defaults to bun ('bun x skills'). - Runners probed on PATH after PurviewSkillsRunner. Defaults to npx;bunx;bun;pnpm;yarn;deno. + Runners probed on PATH after PurviewSkillsRunner. Defaults to bun;npx;bunx;pnpm;yarn;deno. Package spec the executed launcher pins to. Defaults to skills@1. diff --git a/src/src/BuildSdk/Sdk/Sdk.targets b/src/src/BuildSdk/Sdk/Sdk.targets index 1094c3f..7250f5f 100644 --- a/src/src/BuildSdk/Sdk/Sdk.targets +++ b/src/src/BuildSdk/Sdk/Sdk.targets @@ -607,9 +607,17 @@ agent.Trim()) + .Where(agent => agent.Length > 0) + .Select(agent => " --agent \"" + agent + "\"") + ); + // How the repository launches the CLI - 'bun x skills', 'npx skills', and so on. + var cli = string.IsNullOrEmpty(CommandPrefix) ? "bun x skills" : CommandPrefix; // ---- Read the CLI's lockfile ------------------------------------------------- // Entries are flat objects, so one regex over the 'skills' object is enough. A @@ -786,7 +794,7 @@ // matches nothing when given one comma-separated value. var commandFor = new Func, string>(selected => cli + " add \"" + sourceWithRef + "\" --skill \"" - + string.Join("\" --skill \"", selected) + "\" --agent \"" + agents + "\" -y"); + + string.Join("\" --skill \"", selected) + "\"" + agentArguments + " -y"); var addItem = new Func, List, ITaskItem>((bucket, selected) => { @@ -880,6 +888,336 @@ + + + + + + + + + + + + + + + + + + + + + (); + LockFileHash = string.Empty; + LockFileFound = File.Exists(LockFilePath); + + var lockText = string.Empty; + if (LockFileFound) + { + try { lockText = File.ReadAllText(LockFilePath); } + catch (IOException) { LockFileFound = false; } + catch (UnauthorizedAccessException) { LockFileFound = false; } + } + + if (LockFileFound) + { + using (var sha = SHA256.Create()) + { + LockFileHash = string.Concat( + sha.ComputeHash(Encoding.UTF8.GetBytes(lockText)).Select(value => value.ToString("x2")) + ); + } + + // Same reading as EvaluatePurviewSkillState: walk to the brace that closes the + // 'skills' object, then one regex per flat entry. + var namesBySource = new Dictionary>(StringComparer.OrdinalIgnoreCase); + var skillsKey = lockText.IndexOf("\"skills\"", StringComparison.Ordinal); + var open = skillsKey < 0 ? -1 : lockText.IndexOf('{', skillsKey); + if (open >= 0) + { + var depth = 0; + var inString = false; + var escaped = false; + var close = -1; + for (var index = open; index < lockText.Length; index++) + { + var character = lockText[index]; + if (escaped) { escaped = false; continue; } + if (character == '\\' && inString) { escaped = true; continue; } + if (character == '"') { inString = !inString; continue; } + if (inString) continue; + if (character == '{') depth++; + else if (character == '}') + { + depth--; + if (depth == 0) { close = index; break; } + } + } + + if (close > open) + { + var body = lockText.Substring(open, close - open + 1); + var entryPattern = new Regex("\"(?[^\"]+)\"\\s*:\\s*\\{(?[^{}]*)\\}", RegexOptions.Singleline); + var sourcePattern = new Regex("\"source\"\\s*:\\s*\"(?[^\"]*)\""); + + foreach (Match entry in entryPattern.Matches(body)) + { + var sourceMatch = sourcePattern.Match(entry.Groups["entry"].Value); + var source = sourceMatch.Success ? sourceMatch.Groups["value"].Value.Trim() : string.Empty; + if (source.Length == 0) + continue; + + List names; + if (!namesBySource.TryGetValue(source, out names)) + namesBySource[source] = names = new List(); + names.Add(entry.Groups["name"].Value); + } + } + } + + var packages = SkillSources ?? new ITaskItem[0]; + foreach (var pair in namesBySource.OrderBy(pair => pair.Key, StringComparer.OrdinalIgnoreCase)) + { + var package = packages.FirstOrDefault(item => + string.Equals(item.GetMetadata("Source"), pair.Key, StringComparison.OrdinalIgnoreCase)); + + ITaskItem created; + if (package != null) + { + created = new TaskItem(package.GetMetadata("SourceWithRef")); + created.SetMetadata("Skills", package.GetMetadata("Skills")); + created.SetMetadata("Origin", package.GetMetadata("PackageId") + " " + package.GetMetadata("PackageVersion")); + } + else + { + created = new TaskItem(pair.Key); + created.SetMetadata( + "Skills", + string.Join(";", pair.Value.OrderBy(name => name, StringComparer.OrdinalIgnoreCase)) + ); + created.SetMetadata("Origin", "skills-lock.json"); + } + + updates.Add(created); + } + } + + UpdateSources = updates.ToArray(); + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + (); + var removed = new List(); + var unknown = new List(); + var windows = Path.DirectorySeparatorChar == '\\'; + var pathComparer = windows ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; + + var selected = (Agents ?? string.Empty) + .Split(new[] { ';', ',' }, StringSplitOptions.RemoveEmptyEntries) + .Select(agent => agent.Trim()) + .Where(agent => agent.Length > 0) + .ToList(); + + // '*' is every agent, so every directory is allowed. + if (selected.Count > 0 && !selected.Contains("*")) + { + var root = Path.GetFullPath(RepositoryRoot).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + var toRelative = new Func(full => + full.Substring(root.Length).TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar).Replace('\\', '/')); + + var directories = (AgentDirectories ?? new ITaskItem[0]) + .Select(item => new + { + Relative = item.ItemSpec.Replace('\\', '/').Trim('/'), + Agents = (item.GetMetadata("Agents") ?? string.Empty) + .Split(new[] { ';', ',' }, StringSplitOptions.RemoveEmptyEntries) + .Select(agent => agent.Trim()) + .ToArray(), + }) + .Where(directory => directory.Relative.Length > 0) + .ToList(); + + var known = new HashSet(directories.SelectMany(directory => directory.Agents), StringComparer.OrdinalIgnoreCase); + foreach (var agent in selected.Where(agent => !known.Contains(agent))) + unknown.Add(new TaskItem(agent)); + + var allowed = new HashSet(pathComparer) { ".agents/skills" }; + foreach (var directory in directories) + { + if (directory.Agents.Any(agent => selected.Contains(agent, StringComparer.OrdinalIgnoreCase))) + allowed.Add(directory.Relative); + } + + var protectedPaths = new HashSet( + (ProtectedDirectories ?? new ITaskItem[0]) + .Where(item => !string.IsNullOrWhiteSpace(item.ItemSpec)) + .Select(item => Path.GetFullPath(item.ItemSpec).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)), + pathComparer + ); + + // GetAttributes does not follow a link, so a dangling one still counts as present. + var exists = new Func(path => + { + try { File.GetAttributes(path); return true; } + catch (IOException) { return false; } + catch (UnauthorizedAccessException) { return false; } + }); + + var deleteLink = new Action((path, attributes) => + { + // On Windows a directory link or junction is removed by RemoveDirectory, which + // removes the link and never its target. On Unix, unlink does the same for any + // symlink, whatever it points at. + if (windows && (attributes & FileAttributes.Directory) != 0) + Directory.Delete(path, false); + else + File.Delete(path); + }); + + var kept = new List(); + Action prune = null; + prune = path => + { + var attributes = File.GetAttributes(path); + if ((attributes & FileAttributes.ReparsePoint) != 0) + { + deleteLink(path, attributes); + removed.Add(new TaskItem(toRelative(path))); + return; + } + + if ((attributes & FileAttributes.Directory) == 0) + { + kept.Add(toRelative(path)); + return; + } + + foreach (var child in Directory.GetFileSystemEntries(path).OrderBy(child => child, StringComparer.Ordinal)) + prune(child); + + if (Directory.GetFileSystemEntries(path).Length == 0) + { + Directory.Delete(path, false); + removed.Add(new TaskItem(toRelative(path))); + } + }; + + foreach (var directory in directories + .Where(directory => !allowed.Contains(directory.Relative)) + .GroupBy(directory => directory.Relative, pathComparer) + .OrderBy(group => group.Key, StringComparer.Ordinal)) + { + var full = Path.GetFullPath(Path.Combine(root, directory.Key.Replace('/', Path.DirectorySeparatorChar))); + if (protectedPaths.Contains(full) || !exists(full)) + continue; + + kept.Clear(); + if (Fix) + { + prune(full); + + // A folder that only ever held the agent's skills is left empty; remove it too, + // but never climb to the repository root. + var parent = Path.GetDirectoryName(full); + while (!exists(full) && parent != null && parent.Length > root.Length + && Directory.Exists(parent) && Directory.GetFileSystemEntries(parent).Length == 0) + { + Directory.Delete(parent, false); + removed.Add(new TaskItem(toRelative(parent))); + parent = Path.GetDirectoryName(parent); + } + + if (!exists(full)) + continue; + } + + var violation = new TaskItem(directory.Key); + violation.SetMetadata("Agents", string.Join(", ", directory.SelectMany(entry => entry.Agents))); + // The CLI copies rather than links for some agent selections, so a kept folder can + // hold every file of every skill; name enough to recognise, then count the rest. + violation.SetMetadata( + "Kept", + string.Join(", ", kept.Take(5)) + (kept.Count > 5 ? " and " + (kept.Count - 5) + " more" : string.Empty) + ); + violations.Add(violation); + } + } + + Violations = violations.ToArray(); + Removed = removed.ToArray(); + UnknownAgents = unknown.ToArray(); + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - * - - false + <_PurviewSkillsAgentList>$([System.Text.RegularExpressions.Regex]::Replace('$(PurviewSkillsAgents)', '^[\s,;"'']+|[\s,;"'']+$', '')) + <_PurviewSkillsAgentList>$([System.Text.RegularExpressions.Regex]::Replace('$(_PurviewSkillsAgentList)', '[\s,;]+', ';')) + <_PurviewSkillsAgentArguments> + <_PurviewSkillsAgentArguments Condition="'$(_PurviewSkillsAgentList)' != ''" + > --agent "$(_PurviewSkillsAgentList.Replace(';', '" --agent "'))" + - - - - $(PurviewSkillsSource) - * - (explicit source) - - - - - - + 'skills' on PATH is tried first because it needs no download at all, then the configured + runner, then the remaining candidates - so a machine with Node but no bun still installs + rather than reporting that nothing was found. --> + @@ -2215,14 +2625,72 @@ + + + @(PurviewSkillsRunnerLauncher->WithMetadataValue('Identity', '$(PurviewSkillsRunner)')->'%(Exec)') + $(PurviewSkillsRunner) $(PurviewSkillsPackage) + + + + + + + + + + + + + + $(PurviewSkillsSource) + * + (explicit source) + + + + <_PurviewSkillsCommandRequired Condition="'@(PurviewSkillSource)' != ''" + >true + + + + + + + false + + + + + + + $([MSBuild]::NormalizePath('$(_PurviewSkillsRepositoryRoot)', 'skills-lock.json')) + Auto + + + + + + + + + + <_PurviewSkillsCommandRequired Condition="'@(_PurviewSkillsUpdateSource)' != ''" + >true + + + + + + + <_PurviewSkillsUpdateCommand>$(PurviewSkillsCommand) add "%(_PurviewSkillsUpdateSource.Identity)" --skill "$([System.String]::Copy('%(_PurviewSkillsUpdateSource.Skills)').Replace(';', '" --skill "'))"$(_PurviewSkillsAgentArguments) -y + <_PurviewSkillsUpdateExitCode>0 + + + + + + + <_PurviewSkillsUpdateFailure + Include="%(_PurviewSkillsUpdateSource.Identity)" + Condition="'$(_PurviewSkillsUpdateExitCode)' != '0'" + > + $(_PurviewSkillsUpdateExitCode) + + + + + + + + + + <_PurviewSkillsUpdateFailed Condition="'@(_PurviewSkillsUpdateFailure)' != ''" + >true + + + + <_PurviewSkillsUpdateCommand>$(PurviewSkillsCommand) update -y -p + + + + + + + + <_PurviewSkillsRunUpdate>false + <_PurviewSkillsRunUpdate Condition="'$(PurviewSkillsUpdateMode)' == 'Always'">true + <_PurviewSkillsRunUpdate + Condition="'$(PurviewSkillsUpdateMode)' == 'Auto' AND '$(_PurviewSkillsLockHashAfter)' != '$(_PurviewSkillsLockHashBefore)'" + >true + + + + + + + + + + + + + + <_PurviewSkillsProtectedDirectory Include="$([MSBuild]::NormalizeDirectory('$(_PurviewSkillsRepositoryRoot)', 'skills'))" /> + <_PurviewSkillsProtectedDirectory + Include="$(PurviewSkillsSourceFolder)" + Condition="'$(PurviewSkillsSourceFolder)' != ''" + /> + + + + + + + + + + + + + + + + + <_PurviewSkillsFixDirectories>true + + + + + + + + + @@ -2298,7 +3033,6 @@ $([MSBuild]::NormalizePath('$(_PurviewSkillRecommendationRoot)', 'skills-lock.json')) - * @@ -2323,7 +3057,7 @@ SkillSources="@(PurviewSkillSource)" UnresolvableSkillSources="@(PurviewUnresolvableSkillSource)" LockFilePath="$(PurviewSkillsLockFile)" - Agents="$(PurviewSkillsAgents)" + Agents="$(_PurviewSkillsAgentList)" RepositoryRoot="$(_PurviewSkillRecommendationRoot)" > @@ -2394,7 +3128,10 @@ -t:RecommendPurviewSkills report the state, never fail -t:VerifyPurviewSkills the same state, as a build failure - -t:InstallPurviewSkills install what is missing + -t:InstallPurviewSkills install or update, pinned to each package's commit + -t:UpdatePurviewSkills re-add every lockfile source, then 'skills update' + -t:CheckPurviewSkills only the selected agents' skill directories exist + -t:FixPurviewSkills remove the CLI's links from any other agent directory -t:ListPurviewSkills what the restored packages ship VerifyPurviewSkills is what a CI job calls. Because it is invoked by diff --git a/src/tests/BuildSdk.IntegrationTests/PurviewSkillInstallCommandTests.cs b/src/tests/BuildSdk.IntegrationTests/PurviewSkillInstallCommandTests.cs index ed58eb6..9e3d54a 100644 --- a/src/tests/BuildSdk.IntegrationTests/PurviewSkillInstallCommandTests.cs +++ b/src/tests/BuildSdk.IntegrationTests/PurviewSkillInstallCommandTests.cs @@ -139,7 +139,7 @@ CancellationToken cancellationToken } [Test] - public async Task SkillsRunner_DefaultsToNpx(CancellationToken cancellationToken) + public async Task SkillsRunner_DefaultsToBun(CancellationToken cancellationToken) { using var harness = await ProjectHarness.CreateAsync("SkillRunner", cancellationToken: cancellationToken); @@ -149,8 +149,31 @@ public async Task SkillsRunner_DefaultsToNpx(CancellationToken cancellationToken ); await Assert.That(exitCode).IsZero().Because(TestHelpers.GenerateError(stdOut, stdErr)); - // npx ships with Node itself, so it is the one runner a machine with Node at all has. - await Assert.That(stdOut.Trim()).IsEqualTo("npx"); + await Assert.That(stdOut.Trim()).IsEqualTo("bun"); + } + + [Test] + public async Task InstallPurviewSkills_WithoutAgents_LetsTheCliAutoDetect(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillInstaller", cancellationToken: cancellationToken); + + var packagesRoot = Path.Combine(harness.SolutionDirectory, "packages"); + WriteFakePackage(packagesRoot, "Purview.Example", "1.2.3", ["purview-alpha"]); + + var assetsPath = Path.Combine(harness.SolutionDirectory, "obj", "project.assets.json"); + WriteAssetsFile(assetsPath, "Purview.Example", "1.2.3"); + + var (exitCode, stdOut, stdErr) = await harness.RunMSBuildAsync( + "-t:InstallPurviewSkills -v:n " + + $"-p:ProjectAssetsFile=\"{assetsPath}\" -p:NuGetPackageFolders=\"{packagesRoot}\" " + + $"-p:PurviewSkillsInstallRoot=\"{harness.SolutionDirectory}\" " + + "-p:PurviewSkillsCommand=echo", + cancellationToken + ); + + await Assert.That(exitCode).IsZero().Because(TestHelpers.GenerateError(stdOut, stdErr)); + await Assert.That(stdOut).Contains("--skill \"purview-alpha\" -y"); + await Assert.That(stdOut).DoesNotContain("--agent"); } /// diff --git a/src/tests/BuildSdk.IntegrationTests/PurviewSkillRecommendationTests.cs b/src/tests/BuildSdk.IntegrationTests/PurviewSkillRecommendationTests.cs index 42f3983..d6a7dca 100644 --- a/src/tests/BuildSdk.IntegrationTests/PurviewSkillRecommendationTests.cs +++ b/src/tests/BuildSdk.IntegrationTests/PurviewSkillRecommendationTests.cs @@ -86,7 +86,7 @@ public async Task MissingSkills_AreReportedWithAPinnedInstallCommand(Cancellatio await Assert .That(output) .Contains( - $"npx skills add \"{Source}#{SkillPackageFixtures.DefaultCommit}\" " + $"bun x skills add \"{Source}#{SkillPackageFixtures.DefaultCommit}\" " + "--skill \"purview-alpha\" --skill \"purview-beta\"" ); } @@ -124,11 +124,11 @@ public async Task DriftedSkills_RecommendAPinnedInstallRatherThanAnUpdate(Cancel await Assert.That(code).IsZero().Because(output); await Assert.That(output).Contains("PRSKL0011"); await Assert.That(output).Contains(OtherCommit); - // The recommendation is pinned to the referenced package's own commit. 'npx skills update' + // The recommendation is pinned to the referenced package's own commit. 'bun x skills update' // installs the newest published skill regardless of package version, so it is named only to // explain why it is not the recommendation. - await Assert.That(output).Contains($"npx skills add \"{Source}#{SkillPackageFixtures.DefaultCommit}\""); - await Assert.That(output).Contains("Prefer this over 'npx skills update'"); + await Assert.That(output).Contains($"bun x skills add \"{Source}#{SkillPackageFixtures.DefaultCommit}\""); + await Assert.That(output).Contains("Prefer this over 'bun x skills update'"); await Assert.That(output).DoesNotContain("PRSKL0010"); } @@ -145,7 +145,7 @@ public async Task SkillsInstalledWithoutARef_AreReportedAsUnverifiable(Cancellat await Assert.That(code).IsZero().Because(output); await Assert.That(output).Contains("PRSKL0012"); - await Assert.That(output).Contains("npx skills list"); + await Assert.That(output).Contains("bun x skills list"); // Unverifiable is not the same as wrong, so it is never reported as drift or as missing. await Assert.That(output).DoesNotContain("PRSKL0011"); await Assert.That(output).DoesNotContain("PRSKL0010"); @@ -191,7 +191,7 @@ CancellationToken cancellationToken await Assert.That(code).IsZero().Because(output); await Assert.That(output).Contains("PRSKL0013"); await Assert.That(output).Contains("purview-legacy"); - await Assert.That(output).Contains("npx skills remove \"purview-legacy\""); + await Assert.That(output).Contains("bun x skills remove \"purview-legacy\""); // Removal is the developer's, so it is only ever suggested. await Assert .That(await File.ReadAllTextAsync(LockFilePath(harness), cancellationToken)) @@ -259,9 +259,9 @@ CancellationToken cancellationToken await Assert.That(code).IsZero().Because(output); await Assert.That(output).Contains("PRSKL0014"); - await Assert.That(output).Contains("npx skills experimental_install"); + await Assert.That(output).Contains("bun x skills experimental_install"); // The command is experimental, so the fallback is named alongside it. - await Assert.That(output).Contains("npx skills add"); + await Assert.That(output).Contains("bun x skills add"); } [Test] @@ -370,7 +370,7 @@ public async Task Recommendations_WorkWithoutAnyNodeTooling(CancellationToken ca await Assert.That(code).IsZero().Because(output); await Assert.That(output).DoesNotContain("PRSKL0004"); await Assert.That(output).Contains("PRSKL0010"); - await Assert.That(output).Contains("npx skills add"); + await Assert.That(output).Contains("bun x skills add"); } [Test] diff --git a/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs b/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs new file mode 100644 index 0000000..2be3469 --- /dev/null +++ b/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs @@ -0,0 +1,152 @@ +using System.Xml.Linq; +using Purview.BuildSdk.Harness; + +namespace Purview.BuildSdk; + +/// +/// InstallPurviewSkills, UpdatePurviewSkills and FixPurviewSkills change the +/// repository - they run the skills CLI or delete files - so they must only ever run because a person +/// named them. Two layers keep it that way, and each is tested: nothing in the SDK hooks or depends on +/// them, and a guard refuses to run when something else has pulled one into a build, a restore or a +/// design-time build. +/// +sealed class PurviewSkillTargetIsolationTests +{ + static readonly string[] ExplicitTargets = + [ + "InstallPurviewSkills", + "UpdatePurviewSkills", + "FixPurviewSkills", + "CheckPurviewSkills", + ]; + + static IEnumerable SdkProjectFiles() => + Directory + .EnumerateFiles(SdkPaths.SdkDirectory, "*.*", SearchOption.AllDirectories) + .Where(path => + path.EndsWith(".props", StringComparison.OrdinalIgnoreCase) + || path.EndsWith(".targets", StringComparison.OrdinalIgnoreCase) + ); + + static string[] SplitTargetList(string? value) => + (value ?? string.Empty).Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + + [Test] + public async Task ExplicitTargets_AreNeverHookedOrDependedOn() + { + var problems = new List(); + + foreach (var file in SdkProjectFiles()) + { + var document = XDocument.Load(file); + var name = Path.GetFileName(file); + + foreach (var target in document.Descendants().Where(element => element.Name.LocalName == "Target")) + { + var targetName = (string?)target.Attribute("Name") ?? string.Empty; + + if (ExplicitTargets.Contains(targetName, StringComparer.OrdinalIgnoreCase)) + { + foreach (var hook in new[] { "BeforeTargets", "AfterTargets" }) + { + if (target.Attribute(hook) is not null) + problems.Add($"{name}: {targetName} has {hook}, so it would run without being named"); + } + + continue; + } + + foreach (var attribute in new[] { "BeforeTargets", "AfterTargets", "DependsOnTargets" }) + { + foreach (var referenced in SplitTargetList((string?)target.Attribute(attribute))) + { + if (ExplicitTargets.Contains(referenced, StringComparer.OrdinalIgnoreCase)) + problems.Add($"{name}: {targetName} references {referenced} in {attribute}"); + } + } + } + + // $(BuildDependsOn), $(CoreBuildDependsOn) and friends are the other way to splice a + // target into an ordinary build. + foreach ( + var property in document + .Descendants() + .Where(element => element.Name.LocalName.EndsWith("DependsOn", StringComparison.Ordinal)) + ) + { + foreach (var referenced in SplitTargetList(property.Value)) + { + if (ExplicitTargets.Contains(referenced, StringComparer.OrdinalIgnoreCase)) + problems.Add($"{name}: {property.Name.LocalName} includes {referenced}"); + } + } + } + + await Assert.That(problems).IsEmpty().Because(string.Join(Environment.NewLine, problems)); + } + + [Test] + [Arguments("UpdatePurviewSkills", "BuildingProject")] + [Arguments("InstallPurviewSkills", "BuildingProject")] + [Arguments("FixPurviewSkills", "BuildingProject")] + [Arguments("UpdatePurviewSkills", "DesignTimeBuild")] + [Arguments("InstallPurviewSkills", "DesignTimeBuild")] + [Arguments("InstallPurviewSkills", "MSBuildIsRestoring")] + public async Task ExplicitTargets_RefuseToRunInsideABuild( + string target, + string context, + CancellationToken cancellationToken + ) + { + using var harness = await ProjectHarness.CreateAsync("SkillGuard", cancellationToken: cancellationToken); + SkillPackageFixtures.WriteLockFile( + Path.Combine(harness.SolutionDirectory, "skills-lock.json"), + ("web-design", "vercel-labs/agent-skills", null) + ); + + // 'echo' would succeed, so reaching the CLI at all would show up as a pass. + var (code, stdOut, stdErr) = await harness.RunMSBuildAsync( + $"-v:n -t:{target} -p:{context}=true " + + $"-p:PurviewSkillsInstallRoot=\"{harness.SolutionDirectory}\" " + + "-p:PurviewSkillsSource=vercel-labs/agent-skills -p:PurviewSkillsAgents=universal " + + "-p:PurviewSkillsCommand=purview-guard-reached-the-cli", + cancellationToken + ); + + var output = stdOut + stdErr; + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0009"); + await Assert.That(output).DoesNotContain("purview-guard-reached-the-cli add"); + } + + [Test] + public async Task AConsumerHook_CannotRunUpdateDuringABuild(CancellationToken cancellationToken) + { + // The shape somebody might reasonably try: chaining update onto every build. + // The default telemetry packages would need Central Package Management versions the throwaway + // solution does not declare, and are not part of this scenario. + using var harness = await ProjectHarness.CreateAsync( + "SkillHook", + extraProps: """ + true + true + """, + cancellationToken: cancellationToken + ); + await File.WriteAllTextAsync( + Path.Combine(harness.SolutionDirectory, "Directory.Build.targets"), + $""" + + + + + """, + cancellationToken + ); + + var (success, output, errors) = await harness.BuildAsync(restore: true, cancellationToken: cancellationToken); + + await Assert.That(success).IsFalse(); + await Assert.That(output + errors).Contains("PRSKL0009"); + } +} diff --git a/src/tests/BuildSdk.IntegrationTests/PurviewSkillUpdateTests.cs b/src/tests/BuildSdk.IntegrationTests/PurviewSkillUpdateTests.cs new file mode 100644 index 0000000..81efdbc --- /dev/null +++ b/src/tests/BuildSdk.IntegrationTests/PurviewSkillUpdateTests.cs @@ -0,0 +1,313 @@ +using Purview.BuildSdk.Harness; + +namespace Purview.BuildSdk; + +/// +/// Verifies UpdatePurviewSkills, CheckPurviewSkills and FixPurviewSkills. The +/// skills CLI is stood in for by echo, so the assertions are about the commands handed to it - +/// never letting a test reach bun x or npx, which would download from the network. +/// +sealed class PurviewSkillUpdateTests +{ + const string PackageId = "Purview.Example"; + const string PackageVersion = "1.2.3"; + const string PackageSource = "purview-dev/purview-example"; + const string OtherSource = "vercel-labs/agent-skills"; + const string OldCommit = "aaaaaaaabbbbbbbbccccccccddddddddeeeeeeee"; + + static string LockFilePath(ProjectHarness harness) => Path.Combine(harness.SolutionDirectory, "skills-lock.json"); + + /// + /// One package shipping purview-alpha, installed at an old ref, plus two skills from a + /// repository no package speaks for. + /// + static void GivenInstalledSkills(ProjectHarness harness) + { + var packagesRoot = Path.Combine(harness.SolutionDirectory, "packages"); + SkillPackageFixtures.WritePackage(packagesRoot, PackageId, PackageVersion, ["purview-alpha"]); + SkillPackageFixtures.WriteAssetsFile( + Path.Combine(harness.SolutionDirectory, "obj", "project.assets.json"), + (PackageId, PackageVersion) + ); + SkillPackageFixtures.WriteLockFile( + LockFilePath(harness), + ("purview-alpha", PackageSource, OldCommit), + ("web-design", OtherSource, null), + ("frontend-design", OtherSource, null) + ); + } + + static async Task<(int Code, string Output)> RunAsync( + ProjectHarness harness, + string target, + string extraArguments = "", + CancellationToken cancellationToken = default + ) + { + var packagesRoot = Path.Combine(harness.SolutionDirectory, "packages"); + var assetsPath = Path.Combine(harness.SolutionDirectory, "obj", "project.assets.json"); + + var (code, stdOut, stdErr) = await harness.RunMSBuildAsync( + $"-v:n \"-t:{target}\" " + + $"-p:ProjectAssetsFile=\"{assetsPath}\" -p:NuGetPackageFolders=\"{packagesRoot}\" " + + $"-p:PurviewSkillsInstallRoot=\"{harness.SolutionDirectory}\" {extraArguments}", + cancellationToken + ); + + return (code, stdOut + stdErr); + } + + [Test] + public async Task Update_ReAddsEachSource_PinningThePackagesOwnCommit(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "-p:PurviewSkillsCommand=echo", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + // The package's source moves to the commit the referenced version was built from, not to the + // newest skill and not left at the old ref. + await Assert + .That(output) + .Contains($"add \"{PackageSource}#{SkillPackageFixtures.DefaultCommit}\" --skill \"purview-alpha\""); + // Every other source is re-added once, naming only the skills the lockfile records for it. + await Assert.That(output).Contains($"add \"{OtherSource}\" --skill \"frontend-design\" --skill \"web-design\""); + await Assert.That(output).DoesNotContain(OldCommit + "\" --skill"); + // No agents named: the CLI auto-detects, so no agent flag is passed. + await Assert.That(output).DoesNotContain("--agent"); + // 'echo' leaves the lockfile untouched, so Auto skips 'skills update'. + await Assert.That(output).Contains("skipped 'echo update -y -p'"); + } + + [Test] + public async Task Update_NamedAgents_BecomeOneFlagEach(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "-p:PurviewSkillsCommand=echo -p:PurviewSkillsAgents=claude-code%2Ccodex", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + // 'dotnet msbuild -p:' splits on a bare comma, so the separator is escaped; the CLI does not + // split a comma-separated agent list either, so each agent gets its own flag. + await Assert.That(output).Contains("--agent \"claude-code\" --agent \"codex\" -y"); + await Assert.That(output).DoesNotContain("claude-code,codex\""); + } + + [Test] + public async Task Update_ModeAlways_RunsSkillsUpdate(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "-p:PurviewSkillsCommand=echo -p:PurviewSkillsUpdateMode=Always", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(output).Contains("Agent skills: echo update -y -p"); + await Assert.That(output).Contains("refreshed and updated"); + } + + [Test] + public async Task Update_DryRun_RunsNothing(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + + // A command that cannot run proves nothing was executed. + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "-p:PurviewSkillsCommand=purview-no-such-command -p:PurviewSkillsDryRun=true", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(output).Contains($"purview-no-such-command add \"{OtherSource}\""); + await Assert.That(output).Contains("dry run, 2 source(s), nothing changed"); + } + + [Test] + public async Task Update_AFailedSource_IsNamed_AndSkillsUpdateIsSkipped(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + + // 'dotnet ' exits non-zero on every platform without touching the network. + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "\"-p:PurviewSkillsCommand=dotnet purview-no-such-command\" -p:PurviewSkillsUpdateMode=Always", + cancellationToken + ); + + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0008"); + await Assert.That(output).Contains($"re-adding '{OtherSource}' failed"); + await Assert.That(output).Contains($"re-adding '{PackageSource}#{SkillPackageFixtures.DefaultCommit}' failed"); + await Assert.That(output).DoesNotContain("Agent skills: dotnet purview-no-such-command update"); + } + + [Test] + public async Task Update_WithoutALockFile_ReportsHowToInstall(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillUpdate", cancellationToken: cancellationToken); + GivenInstalledSkills(harness); + File.Delete(LockFilePath(harness)); + + var (code, output) = await RunAsync( + harness, + "UpdatePurviewSkills", + "-p:PurviewSkillsCommand=echo", + cancellationToken + ); + + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0007"); + await Assert.That(output).Contains("InstallPurviewSkills"); + } + + [Test] + public async Task Check_WithoutAgents_EnforcesNothing(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillLayout", cancellationToken: cancellationToken); + Directory.CreateDirectory(Path.Combine(harness.SolutionDirectory, ".claude", "skills", "purview-alpha")); + + var (code, output) = await RunAsync(harness, "CheckPurviewSkills", cancellationToken: cancellationToken); + + // The CLI auto-detects agents, so an agent directory is expected, not a violation. + await Assert.That(code).IsZero().Because(output); + await Assert.That(output).Contains("PurviewSkillsAgents is not set"); + await Assert.That(output).DoesNotContain("PRSKL0015"); + } + + [Test] + public async Task Check_WithAgents_ReportsOnlyUnselectedAgentDirectories(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillLayout", cancellationToken: cancellationToken); + var root = harness.SolutionDirectory; + Directory.CreateDirectory(Path.Combine(root, ".agents", "skills", "purview-alpha")); + Directory.CreateDirectory(Path.Combine(root, ".claude", "skills", "purview-alpha")); + Directory.CreateDirectory(Path.Combine(root, ".windsurf", "skills", "purview-alpha")); + // The authored folder, which openclaw would also install into, is never a violation. + Directory.CreateDirectory(Path.Combine(root, "skills", "my-skill")); + + var (code, output) = await RunAsync( + harness, + "CheckPurviewSkills", + "-p:PurviewSkillsAgents=claude-code", + cancellationToken + ); + + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0015"); + await Assert.That(output).Contains("'.windsurf/skills'"); + // '.agents/skills' is the canonical store, and claude-code was selected. + await Assert.That(output).DoesNotContain("'.agents/skills'"); + await Assert.That(output).DoesNotContain("'.claude/skills'"); + await Assert.That(output).DoesNotContain("'skills'"); + } + + [Test] + public async Task Check_UnknownAgent_IsAWarning(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillLayout", cancellationToken: cancellationToken); + + var (code, output) = await RunAsync( + harness, + "CheckPurviewSkills", + "-p:PurviewSkillsAgents=universal%2Cpurview-imaginary", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(output).Contains("PRSKL0016"); + await Assert.That(output).Contains("purview-imaginary"); + } + + [Test] + public async Task Fix_RemovesEmptyAgentDirectories_AndKeepsRealContent(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillLayout", cancellationToken: cancellationToken); + var root = harness.SolutionDirectory; + // Emptied of links already: nothing real left, so the folder - and its now-empty parent - go. + Directory.CreateDirectory(Path.Combine(root, ".windsurf", "skills", "purview-alpha")); + // Real content beside the skills folder is not the CLI's, and neither is a real file inside it. + Directory.CreateDirectory(Path.Combine(root, ".claude", "skills", "hand-written")); + await File.WriteAllTextAsync(Path.Combine(root, ".claude", "settings.json"), "{}", cancellationToken); + await File.WriteAllTextAsync( + Path.Combine(root, ".claude", "skills", "hand-written", "SKILL.md"), + "---\nname: hand-written\n---\n", + cancellationToken + ); + + var (code, output) = await RunAsync( + harness, + "FixPurviewSkills", + "-p:PurviewSkillsAgents=universal", + cancellationToken + ); + + await Assert.That(Directory.Exists(Path.Combine(root, ".windsurf"))).IsFalse().Because(output); + await Assert.That(File.Exists(Path.Combine(root, ".claude", "settings.json"))).IsTrue(); + await Assert.That(File.Exists(Path.Combine(root, ".claude", "skills", "hand-written", "SKILL.md"))).IsTrue(); + // What could not be removed fails the target and names what was kept. + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0015"); + await Assert.That(output).Contains(".claude/skills/hand-written/SKILL.md"); + } + + [Test] + public async Task Fix_RemovesLinks_NeverTheirTargets(CancellationToken cancellationToken) + { + using var harness = await ProjectHarness.CreateAsync("SkillLayout", cancellationToken: cancellationToken); + var root = harness.SolutionDirectory; + var canonical = Path.Combine(root, ".agents", "skills", "purview-alpha"); + Directory.CreateDirectory(canonical); + await File.WriteAllTextAsync( + Path.Combine(canonical, "SKILL.md"), + "---\nname: purview-alpha\n---\n", + cancellationToken + ); + Directory.CreateDirectory(Path.Combine(root, ".claude", "skills")); + + var link = Path.Combine(root, ".claude", "skills", "purview-alpha"); + try + { + Directory.CreateSymbolicLink(link, canonical); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // Windows only allows symlinks with Developer Mode or elevation; CI on Linux always runs this. + Skip.Test($"Symbolic links cannot be created here: {exception.Message}"); + } + + var (code, output) = await RunAsync( + harness, + "FixPurviewSkills", + "-p:PurviewSkillsAgents=universal", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(Directory.Exists(Path.Combine(root, ".claude", "skills"))).IsFalse(); + // The canonical copy the link pointed at is untouched. + await Assert.That(File.Exists(Path.Combine(canonical, "SKILL.md"))).IsTrue(); + await Assert.That(output).Contains("removed .claude/skills/purview-alpha"); + } +} diff --git a/templates/global.json b/templates/global.json index afbf464..4cf8f53 100644 --- a/templates/global.json +++ b/templates/global.json @@ -3,6 +3,6 @@ "runner": "Microsoft.Testing.Platform" }, "msbuild-sdks": { - "Purview.BuildSdk": "1.3.0" + "Purview.BuildSdk": "1.3.1" } } From f94a7e2b19fa291656eefa3e71d0a54567ac1099 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Mon, 9 Nov 2026 10:58:07 +0000 Subject: [PATCH 2/2] feat: simplififed the skills target selection --- AGENTS.md | 30 ++ CHANGELOG.md | 9 + Justfile | 65 +--- Justfile.skills.just | 73 ++++ docs/wiki/Agent-Skills.md | 8 +- skills/purview-buildsdk-justfile/SKILL.md | 50 ++- .../SKILL.md | 8 +- src/src/BuildSdk/Sdk/Sdk.targets | 338 ++++++++++++++++-- .../PurviewSkillSolutionTests.cs | 196 ++++++++++ .../PurviewSkillTargetIsolationTests.cs | 6 + 10 files changed, 673 insertions(+), 110 deletions(-) create mode 100644 Justfile.skills.just create mode 100644 src/tests/BuildSdk.IntegrationTests/PurviewSkillSolutionTests.cs diff --git a/AGENTS.md b/AGENTS.md index 7b7f450..abd2c24 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -214,6 +214,36 @@ dotnet msbuild -t:FixPurviewSkills # remove the CLI's links fr dotnet msbuild -t:ListPurviewSkills # what the restored packages ship ``` +### Once per solution: the leader + +Pointed at a solution, MSBuild runs a custom target in **every** project, so each skill target used +to re-add every source and re-run the CLI once per project. Every public skill target is now a thin +wrapper: `_ResolvePurviewSkillsProjects` reads the solution's project list from +`CurrentSolutionConfigurationContents` (the same value in every project, for `.sln` and `.slnx`; +outside a solution it is the project itself), asks each project for its assets file through +`_GetPurviewSkillsProjectInfo`, and elects as leader the first, by sorted path, that answered. The +wrapper then calls `_Core` on the leader through the `MSBuild` task. MSBuild runs one +project's target once per set of global properties and returns the cached result to every later +caller, so the work runs once — no lock and no stamp file. + +Four things are load-bearing: + +- **The leader is a consumer project, not a `.proj` shipped in the package.** The skill settings live + in the consumer's `Directory.Build.props`, which a project in the NuGet cache would never import. +- **Every wrapper passes identical global properties** (`_PurviewSkillsLeaderRemoveProperties` strips + `TargetFramework`/`RuntimeIdentifier`). Anything that varies per caller would give each caller its + own cache entry and quietly bring back the fan-out. +- **The query's reply has its own item type** (`_PurviewSkillsProjectInfoReply`). Asking the calling + project runs the query in the caller's own instance, so reusing the caller's item put the caller + first in its own list, and every project elected itself. +- `ResolvePurviewSkillsTarget` resolves across **every** answering project's assets file, so a package + only a test project references is found; packages are de-duplicated by id and version, and one id at + two versions is `PRSKL0017`. The old per-framework condition is gone: it made a multi-targeted + project's outer build resolve nothing. + +Tests live in `src/tests/BuildSdk.IntegrationTests/PurviewSkillSolutionTests.cs`, which writes its +own `.slnx` because the harness's lists projects as `` entries. + ### Updating, checking and fixing **Install, update and fix run only when named — never automatically.** Two layers, both tested in `PurviewSkillTargetIsolationTests`: no SDK target hooks or depends on them (a structural scan of every `.props`/`.targets` under `Sdk/`, including `*DependsOn` properties), and `_GuardExplicitPurviewSkillsTarget` — first in each one's `DependsOnTargets` — fails with `PRSKL0009` when `BuildingProject`, `DesignTimeBuild` or `MSBuildIsRestoring` is true, which catches a consumer's own `AfterTargets="Build"` hook. Do not add a `BeforeTargets`/`AfterTargets` to any of them, and do not weaken the guard. diff --git a/CHANGELOG.md b/CHANGELOG.md index cbbda3c..bbba994 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,15 @@ release summarise what the commits recorded rather than what was written at the `UpdatePurviewSkills` and `FixPurviewSkills` fail with `PRSKL0009` when pulled into a build, a restore or a design-time build — including by a consumer's own `AfterTargets="Build"` hook. Run them by name. +- **Skill targets run once per solution.** Pointed at a solution, every skill target used to run in + every project — re-adding each source and re-running the CLI once per project, racing on + `skills-lock.json`. Each project now hands the work to one elected leader (the first solution + project, by path, that imports the SDK), and MSBuild's result cache runs it once. Resolution reads + every solution project's restore graph, so a package only a test project references is found + without choosing a project for it. Point recipes at the solution; no repository changes are needed + otherwise. Two projects on different versions of one skill-shipping package are `PRSKL0017`. + Package resolution also works in a multi-targeted project now: its outer build used to fail the + per-framework condition and resolve nothing. - New skill **`purview-buildsdk-justfile`**: adding or updating `just` recipes that run the skill targets, and replacing hand-written skill scripts with them. - `just skills-update`, `just skills-fix`; `just skills-check` now runs `CheckPurviewSkills`, and the diff --git a/Justfile b/Justfile index 45607a2..2f35a0f 100644 --- a/Justfile +++ b/Justfile @@ -1,5 +1,7 @@ set quiet +import "Justfile.skills.just" + solution_file := "src/BuildSdk.slnx" build_configuration := "Debug" artifacts_folder := "./artifacts" @@ -122,69 +124,6 @@ pack artifact_folder=artifacts_folder *args: echo "==> Packing {{ BLUE }}{{ solution_file }}{{ NORMAL }} ({{ GREEN }}{{ current_version }}{{ NORMAL }}) to {{ YELLOW }}{{ artifact_folder }}{{ NORMAL }}" dotnet pack "{{ solution_file }}" --configuration "{{ build_configuration }}" --output "{{ artifact_folder }}" {{ args }} -# Install skills from another repository into .agents/skills/, through the skills CLI -# -# This repository's *own* skills are not installed here. They are authored at skills//SKILL.md -# for other repositories to install, and are read from that path - AGENTS.md links to it, and -# `npx skills add purview-dev/build-sdk` is how a consumer gets them. .agents/skills/ is reserved for -# skills installed from other sources, and skills-lock.json is the committed record of those. -# -# This goes through the SDK's own InstallPurviewSkills target rather than calling the CLI directly, -# so the repository exercises the path it ships to consumers. -# -# just skills-add purview-dev/engineering-playbook -# -# Install skills from another repository into .agents/skills/ (append '#' to pin) -[group('Agent Skills')] -skills-add source *args: - echo "==> Installing skills from {{ BLUE }}{{ source }}{{ NORMAL }} into the agent directories" - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:InstallPurviewSkills -p:PurviewSkillsSource={{ source }} {{ args }} - -# Validate the authored skills in skills/ against the Agent Skills specification -# -# The same check the pack runs (PRSKL0005). The broader rule set - names, descriptions, optional -# fields, bundled links, body size - runs as RepositorySkillTests in `just test`. -# -# Validate the authored skills in skills/ against the Agent Skills specification -[group('Agent Skills')] -skills-validate *args: - echo "==> Validating authored skills in {{ BLUE }}skills/{{ NORMAL }}" - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:ValidatePurviewSkills -v:n {{ args }} - -# Report which skills the restored packages ship, and whether a runner is available -[group('Agent Skills')] -skills-list *args: - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:ListPurviewSkills -v:n {{ args }} - -# Report missing, drifted or unverifiable skills without installing anything -[group('Agent Skills')] -skills-recommend *args: - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:RecommendPurviewSkills -v:n {{ args }} - -# Update the agent skills by re-adding every unique source in skills-lock.json, then 'skills update' -# -# Skills a referenced package ships are re-added at that package's commit. When PurviewSkillsAgents -# is set, the agent directories it does not select are cleaned up afterwards (FixPurviewSkills); -# when it is not, the skills CLI auto-detects agents and the cleanup does nothing. -# -# just skills-update -p:PurviewSkillsUpdateMode=Always -p:PurviewSkillsDryRun=true -# just skills-update -p:PurviewSkillsRunner=npx -p:PurviewSkillsAgents=universal -# -# Update the agent skills by re-adding every unique source in skills-lock.json -[group('Agent Skills')] -skills-update *args: - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj "-t:UpdatePurviewSkills;FixPurviewSkills" -v:n {{ args }} - -# Check that only the skill directories of the agents in PurviewSkillsAgents exist -[group('Agent Skills')] -skills-check *args: - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:CheckPurviewSkills -v:n {{ args }} - -# Remove the skills CLI's links from agent directories PurviewSkillsAgents does not select -[group('Agent Skills')] -skills-fix *args: - dotnet msbuild src/src/BuildSdk/BuildSdk.csproj -t:FixPurviewSkills -v:n {{ args }} - # Format the code with CSharpier [group('Utilities')] lint-fix: diff --git a/Justfile.skills.just b/Justfile.skills.just new file mode 100644 index 0000000..7ac2bea --- /dev/null +++ b/Justfile.skills.just @@ -0,0 +1,73 @@ +# Agent-skill recipes, imported by the Justfile. +# +# Every skill target runs once for the whole solution, however many projects it holds, and reads +# every project's restore graph - so the recipes name the solution, not a hand-picked project. It is +# the Justfile's own solution_file, so the two cannot drift apart. +skills_project := solution_file + +# ValidatePurviewSkills is the pack-time check, and runs in each project it is invoked on, so it names +# the one project that packs the skills. +skills_validate_project := "src/src/BuildSdk/BuildSdk.csproj" + +# Install skills from another repository into .agents/skills/, through the skills CLI +# +# This repository's *own* skills are not installed here. They are authored at skills//SKILL.md +# for other repositories to install, and are read from that path - AGENTS.md links to it, and +# `npx skills add purview-dev/build-sdk` is how a consumer gets them. .agents/skills/ is reserved for +# skills installed from other sources, and skills-lock.json is the committed record of those. +# +# This goes through the SDK's own InstallPurviewSkills target rather than calling the CLI directly, +# so the repository exercises the path it ships to consumers. +# +# just skills-add purview-dev/engineering-playbook +# +# Install skills from another repository into .agents/skills/ (append '#' to pin) +[group('Agent Skills')] +skills-add source *args: + echo "==> Installing skills from {{ BLUE }}{{ source }}{{ NORMAL }} into the agent directories" + dotnet msbuild "{{ skills_project }}" -t:InstallPurviewSkills -p:PurviewSkillsSource={{ source }} {{ args }} + +# Validate the authored skills in skills/ against the Agent Skills specification +# +# The same check the pack runs (PRSKL0005). The broader rule set - names, descriptions, optional +# fields, bundled links, body size - runs as RepositorySkillTests in `just test`. +# +# Validate the authored skills in skills/ against the Agent Skills specification +[group('Agent Skills')] +skills-validate *args: + echo "==> Validating authored skills in {{ BLUE }}skills/{{ NORMAL }}" + dotnet msbuild "{{ skills_validate_project }}" -t:ValidatePurviewSkills -v:n {{ args }} + +# Report which skills the restored packages ship, and whether a runner is available +[group('Agent Skills')] +skills-list *args: + dotnet msbuild "{{ skills_project }}" -t:ListPurviewSkills -v:n {{ args }} + +# Report missing, drifted or unverifiable skills without installing anything +[group('Agent Skills')] +skills-recommend *args: + dotnet msbuild "{{ skills_project }}" -t:RecommendPurviewSkills -v:n {{ args }} + +# Update the agent skills by re-adding every unique source in skills-lock.json, then 'skills update' +# +# Skills a referenced package ships are re-added at that package's commit. When PurviewSkillsAgents +# is set, the agent directories it does not select are cleaned up afterwards (FixPurviewSkills); +# when it is not, the skills CLI auto-detects agents and the cleanup does nothing. +# +# just skills-update -p:PurviewSkillsUpdateMode=Always -p:PurviewSkillsDryRun=true +# just skills-update -p:PurviewSkillsRunner=npx -p:PurviewSkillsAgents=universal +# +# Update the agent skills by re-adding every unique source in skills-lock.json +[group('Agent Skills')] +skills-update *args: + dotnet msbuild "{{ skills_project }}" "-t:UpdatePurviewSkills;FixPurviewSkills" -v:n {{ args }} + +# Check that only the skill directories of the agents in PurviewSkillsAgents exist +[group('Agent Skills')] +skills-check *args: + dotnet msbuild "{{ skills_project }}" -t:CheckPurviewSkills -v:n {{ args }} + +# Remove the skills CLI's links from agent directories PurviewSkillsAgents does not select +[group('Agent Skills')] +skills-fix *args: + dotnet msbuild "{{ skills_project }}" -t:FixPurviewSkills -v:n {{ args }} diff --git a/docs/wiki/Agent-Skills.md b/docs/wiki/Agent-Skills.md index eaf20b4..3bf8c95 100644 --- a/docs/wiki/Agent-Skills.md +++ b/docs/wiki/Agent-Skills.md @@ -264,8 +264,11 @@ dotnet msbuild -t:FixPurviewSkills # remove the CLI's links from They run **only when named**. Nothing in the SDK hooks or depends on them, and install, update and fix refuse to run when pulled into a build, a restore or a design-time build (`PRSKL0009`) — a -consumer's own `AfterTargets="Build"` hook included. Name one project, not the solution: on a -solution each target runs once per project. For `just` recipes, see the +consumer's own `AfterTargets="Build"` hook included. Name the solution: each target runs once +however many projects it holds, because every project hands the work to the same elected leader +project and MSBuild's result cache runs it once. Package resolution reads every project in the +solution, so a package only a test project references is still found; a single project sees only its +own packages. For `just` recipes, see the [`purview-buildsdk-justfile` skill](../../skills/purview-buildsdk-justfile/SKILL.md). `InstallPurviewSkills` shells out to the CLI rather than reimplementing it, because the CLI owns the @@ -571,6 +574,7 @@ Verified against the `skills` CLI v1. | `PRSKL0014` | `skills-lock.json` exists but no skills have been restored | | `PRSKL0015` | An agent skill directory exists for an agent `PurviewSkillsAgents` does not name | | `PRSKL0016` | `PurviewSkillsAgents` names an agent with no known skills directory | +| `PRSKL0017` | Projects in the solution reference one skill-shipping package at different versions | ## See also diff --git a/skills/purview-buildsdk-justfile/SKILL.md b/skills/purview-buildsdk-justfile/SKILL.md index 5e03ce2..edab74a 100644 --- a/skills/purview-buildsdk-justfile/SKILL.md +++ b/skills/purview-buildsdk-justfile/SKILL.md @@ -26,10 +26,19 @@ restore or a design-time build (`PRSKL0009`). Keep the Justfile consistent with ## Recipes -Add them under one group, after the build recipes. Replace `src/MyApp/MyApp.csproj` as described below. +Put them in their own file beside the `Justfile`, and import it from the `Justfile`: ```just -skills_project := "src/MyApp/MyApp.csproj" +# Justfile +import "Justfile.skills.just" +``` + +The only line that differs between repositories is `skills_project`. Work it out from the repository +as described in [Choosing `skills_project`](#choosing-skills_project) — do not ask for it, and do not +leave a placeholder: + +```just +skills_project := solution_file # or a path, when the Justfile has no such variable # Update agent skills: re-add every skills-lock.json source, then 'skills update' [group('Agent skills')] @@ -64,13 +73,37 @@ skills-verify *args: Only add the recipes the repository will use. `skills-update` and `skills-check` are the usual pair. +### Choosing `skills_project` + +Take the first of these that applies: + +1. **The `Justfile` already names the solution in a variable** — `solution_file`, `solution`, `sln` + or similar, typically used by `build`, `test` and `restore`. Reference it: + `skills_project := solution_file`. An imported file shares the importing file's variables, so the + skill recipes then follow the solution if it is ever renamed or moved, with nothing to keep in step. +2. **An existing recipe runs `dotnet build`, `test` or `restore` on a literal solution path.** Use that + path. If several recipes repeat it, offer to lift it into a `solution_file` variable and reference + that, but do not restructure the `Justfile` unasked. +3. **No `Justfile`, or one that never names a solution.** Search for `*.slnx` and `*.sln`, ignoring + `bin/`, `obj/`, `node_modules/` and `artifacts/`. If there is exactly one, use its path relative to + the `Justfile`. With several, prefer the one the CI workflow or README builds; ask only if that + still leaves more than one. +4. **No solution at all.** Name a project that imports the SDK and references every package that + ships skills, and say in a comment that it sees only its own packages. + +An existing `Justfile.skills.just` pointed at a hand-picked project gets the same treatment: replace +the project with the solution, chosen as above. Remove any comment explaining why that project was +picked, because the reason no longer holds. + ### Details that break a recipe if missed -- **Name one project, never the solution.** `dotnet msbuild -t:UpdatePurviewSkills` runs - the target once per project, re-adding every source N times. Every skill target works from the - repository root whichever project runs it, so pick one that imports the SDK. If a package ships - skills, choose a project that references it: only packages in *that* project's restore graph are - pinned to their commit. +- **Name the solution.** Every skill target runs once for the whole solution, however many projects + it holds: each project hands the work to the same elected leader, and MSBuild runs it once. Package + resolution reads every project in the solution, so a package only a test project references is + still pinned. There is no project to choose and nothing to keep in step as projects come and go. + A single project also works, but sees only that project's packages. (SDK releases before this + behaviour ran the target once per project; there, name one project that references every package + that ships skills.) - **Restore first.** The targets read `project.assets.json` and never restore on their own (`PRSKL0001`). A fresh clone needs `dotnet restore` (or `just restore`) before `skills-update`. - **Quote a multi-target `-t:`.** `"-t:UpdatePurviewSkills;FixPurviewSkills"` — unquoted, `sh` treats @@ -136,7 +169,8 @@ printed commands with what the old script ran before deleting anything. (a PowerShell shell still needs the `-t:` value quoted). 2. Keep recipe names the repository already uses; change only what they run. If `skills-check` already means "report", rename it to `skills-recommend` rather than silently changing its meaning, - and say so. + and say so. A recipe pointed at a hand-picked "small" project, to avoid running once per project, + can point at the solution instead. 3. Check nothing depends on a changing recipe: `just --list`, then search the Justfile, CI workflows and hook configuration for the recipe name. 4. Verify: `just --list` shows the group, `just skills-update -p:PurviewSkillsDryRun=true` prints the diff --git a/skills/purview-buildsdk-skill-installation/SKILL.md b/skills/purview-buildsdk-skill-installation/SKILL.md index 1fbd729..e5b93d6 100644 --- a/skills/purview-buildsdk-skill-installation/SKILL.md +++ b/skills/purview-buildsdk-skill-installation/SKILL.md @@ -42,8 +42,11 @@ something else pulls them into a build, a restore or a design-time build (`PRSKL them into a build, a `*DependsOn` property or a git hook. If skills are changing on their own, something has been wired up that should not be. -Name **one project**, never the solution: on a solution each target runs once per project. For -`just` recipes that run these targets, see `purview-buildsdk-justfile`. +Name **the solution**. Skills belong to the repository, so each target runs once however many +projects the solution holds: every project hands the work to the same elected leader, and MSBuild +runs it once. Package resolution reads every project in the solution, so a package only one project +references is still found. A single project works too, but sees only its own packages. For `just` +recipes that run these targets, see `purview-buildsdk-justfile`. ## How a skill is resolved, and why the version is always right @@ -141,6 +144,7 @@ The caller owns the ref here — append `#` to pin it. This path needs no r | `PRSKL0014` | Recorded in the lockfile but never restored | Message | | `PRSKL0015` | An agent skill directory exists for an agent `PurviewSkillsAgents` does not name | `-t:FixPurviewSkills`, then remove anything it kept by hand | | `PRSKL0016` | `PurviewSkillsAgents` names an agent with no known skills directory | Warning. Add a `PurviewSkillsAgentDirectory` item | +| `PRSKL0017` | Projects in the solution reference one skill-shipping package at different versions | Warning. Align the versions; only one version's skills can be installed | ## Nothing about skills runs during a build diff --git a/src/src/BuildSdk/Sdk/Sdk.targets b/src/src/BuildSdk/Sdk/Sdk.targets index 7250f5f..cf45960 100644 --- a/src/src/BuildSdk/Sdk/Sdk.targets +++ b/src/src/BuildSdk/Sdk/Sdk.targets @@ -391,6 +391,12 @@ Returns one item per package, with the skill names as metadata, for an explicit install target to act on. Resolution never runs during a normal build. + + Takes every project's assets file at once, because skills belong to + the repository: a package referenced by two projects is one package, + and is reported once. Two projects on *different* versions of one + package are both returned, and named in VersionConflicts, because + only one version's skills can be installed into the repository. ===================================================================== --> - + + @@ -415,27 +422,37 @@ (); var unresolved = new List(); + // One package at one version is one result, however many projects reference it. + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - if (File.Exists(ProjectAssetsFile)) + var roots = (NuGetPackageFolders ?? string.Empty) + .Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) + .Select(root => root.TrimEnd('\\', '/')) + .Where(root => root.Length > 0) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + + // Same "libraries" shape DiscoverAgentPackageFolders reads; the version is the + // segment after the id, and is reported so a diagnostic can name it. + var libraryPattern = new Regex( + "\"(?[^\"/]+)/(?[^\"]+)\"\\s*:\\s*\\{[^{}]*?\"type\"\\s*:\\s*\"package\"[^{}]*?\"path\"\\s*:\\s*\"(?[^\"]+)\"", + RegexOptions.Singleline + ); + // NuGet writes the repository element on one line; url and commit may appear in + // either order, so match the element then pull each attribute out of it. + var repositoryElementPattern = new Regex("]*/?>", RegexOptions.IgnoreCase); + var urlPattern = new Regex("\\burl\\s*=\\s*\"(?[^\"]*)\"", RegexOptions.IgnoreCase); + var commitPattern = new Regex("\\bcommit\\s*=\\s*\"(?[^\"]*)\"", RegexOptions.IgnoreCase); + + var assetsFiles = (ProjectAssetsFiles ?? string.Empty) + .Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) + .Select(file => file.Trim()) + .Where(file => file.Length > 0 && File.Exists(file)) + .Distinct(StringComparer.OrdinalIgnoreCase); + + foreach (var assetsFile in assetsFiles) { - var roots = (NuGetPackageFolders ?? string.Empty) - .Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries) - .Select(root => root.TrimEnd('\\', '/')) - .Where(root => root.Length > 0) - .ToArray(); - - // Same "libraries" shape DiscoverAgentPackageFolders reads; the version is the - // segment after the id, and is reported so a diagnostic can name it. - var json = File.ReadAllText(ProjectAssetsFile); - var libraryPattern = new Regex( - "\"(?[^\"/]+)/(?[^\"]+)\"\\s*:\\s*\\{[^{}]*?\"type\"\\s*:\\s*\"package\"[^{}]*?\"path\"\\s*:\\s*\"(?[^\"]+)\"", - RegexOptions.Singleline - ); - // NuGet writes the repository element on one line; url and commit may appear in - // either order, so match the element then pull each attribute out of it. - var repositoryElementPattern = new Regex("]*/?>", RegexOptions.IgnoreCase); - var urlPattern = new Regex("\\burl\\s*=\\s*\"(?[^\"]*)\"", RegexOptions.IgnoreCase); - var commitPattern = new Regex("\\bcommit\\s*=\\s*\"(?[^\"]*)\"", RegexOptions.IgnoreCase); + var json = File.ReadAllText(assetsFile); foreach (Match match in libraryPattern.Matches(json)) { @@ -445,6 +462,8 @@ var packageId = match.Groups["id"].Value; var packageVersion = match.Groups["version"].Value; + if (!seen.Add(packageId + "/" + packageVersion)) + continue; var normalizedPath = libraryPath.Replace('/', Path.DirectorySeparatorChar); foreach (var root in roots) @@ -531,6 +550,21 @@ } } + VersionConflicts = resolved + .Concat(unresolved) + .GroupBy(item => item.GetMetadata("PackageId"), StringComparer.OrdinalIgnoreCase) + .Where(group => group.Count() > 1) + .Select(group => + { + var conflict = new TaskItem(group.Key); + conflict.SetMetadata( + "Versions", + string.Join(", ", group.Select(item => item.GetMetadata("PackageVersion"))) + ); + return (ITaskItem)conflict; + }) + .ToArray(); + SkillSources = resolved.ToArray(); Unresolvable = unresolved.ToArray(); ]]> @@ -538,6 +572,71 @@ + + + + + + + + + + + + + + + + + + (); + + foreach (Match element in Regex.Matches(SolutionConfiguration ?? string.Empty, "]*>")) + { + // A project the solution configuration does not build is not part of this run. + if (Regex.IsMatch(element.Value, "\\bBuildProjectInSolution\\s*=\\s*\"False\"", RegexOptions.IgnoreCase)) + continue; + + var path = Regex.Match(element.Value, "\\bAbsolutePath\\s*=\\s*\"(?[^\"]+)\""); + if (!path.Success) + continue; + + var fullPath = Path.GetFullPath(WebUtility.HtmlDecode(path.Groups["path"].Value)); + if (File.Exists(fullPath)) + projects.Add(fullPath); + } + + if (projects.Count == 0) + projects.Add(Path.GetFullPath(ProjectPath)); + + Projects = projects + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(project => project, StringComparer.OrdinalIgnoreCase) + .Select(project => (ITaskItem)new TaskItem(project)) + .ToArray(); + ]]> + + + + + + + + + <_PurviewSkillsProjectInfoReply Include="$(MSBuildProjectFullPath)"> + $(ProjectAssetsFile) + $(NuGetPackageFolders) + + + + + + + + + + + + + + + + + <_PurviewSkillsAnsweringProjects>@(_PurviewSkillsProjectInfo) + <_PurviewSkillsLeaderProject>$([System.Text.RegularExpressions.Regex]::Match('$(_PurviewSkillsAnsweringProjects)', '^[^;]+').Value) + <_PurviewSkillsLeaderProject Condition="'$(_PurviewSkillsLeaderProject)' == ''" + >$(MSBuildProjectFullPath) + + + + + <_PurviewSkillsUnrestoredProject + Include="@(_PurviewSkillsProjectInfo)" + Condition="'%(_PurviewSkillsProjectInfo.ProjectAssetsFile)' == '' OR !Exists('%(_PurviewSkillsProjectInfo.ProjectAssetsFile)')" + /> + <_PurviewSkillsRestoredProject + Include="@(_PurviewSkillsProjectInfo)" + Exclude="@(_PurviewSkillsUnrestoredProject)" + /> + + + + + + + + + <_PurviewSkillsLeaderRemoveProperties>TargetFramework;RuntimeIdentifier + + + + + + + + + + + + + + +/// Skills belong to the repository, so a skill target pointed at a solution must do its work once +/// for the whole solution rather than once per project. MSBuild runs a custom target in every project +/// of a solution, so each project hands the work to one elected leader, and MSBuild's result cache runs +/// the leader's target once however many projects asked. +/// +sealed partial class PurviewSkillSolutionTests +{ + const string SolutionFileName = "Skills.slnx"; + + // The dry run prints the command it would have run, once per run of the install target. + [GeneratedRegex("Agent skills \\(dry run\\):")] + private static partial Regex DryRunInstallLine(); + + // MSBuild logs this line when a target actually runs, not when a cached result is handed back. + [GeneratedRegex("is building \"[^\"]+\" \\([^)]+\\) on node \\d+ \\(_ListPurviewSkillsCore")] + private static partial Regex ListCoreRun(); + + [GeneratedRegex("No restored package ships agent skills\\.")] + private static partial Regex NoSkillsLine(); + + [GeneratedRegex("Purview\\.Example 1\\.2\\.3 -> purview-dev/purview-example#")] + private static partial Regex ExampleListedLine(); + + /// + /// Three SDK projects in one solution, each with its own restore graph. The harness's own solution + /// lists projects as files, so this writes one that lists them as projects. + /// + static async Task CreateSolutionAsync(CancellationToken cancellationToken) + { + var first = await ProjectHarness.CreateAsync( + "Alpha", + extraProps: "$(MSBuildProjectDirectory)/fake.assets.json", + cancellationToken: cancellationToken + ); + + var projects = new List { first }; + foreach (var name in new[] { "Bravo", "Charlie" }) + { + projects.Add( + await ProjectHarness + .For(name) + .WithNamespacePrefix("Test") + .AddPropertyRaw( + "$(MSBuildProjectDirectory)/fake.assets.json" + ) + .WithSolutionDirectory(first.SolutionDirectory) + .BuildAsync(cancellationToken) + ); + } + + await File.WriteAllTextAsync( + Path.Combine(first.SolutionDirectory, SolutionFileName), + $""" + + {string.Concat( + projects.Select(project => + $"\t\n" + ) + )} + """, + cancellationToken + ); + + // The repository root resolves from a marker, as it would in a real repository. + await File.WriteAllTextAsync(Path.Combine(first.SolutionDirectory, "AGENTS.md"), "# Test", cancellationToken); + + foreach (var project in projects) + SkillPackageFixtures.WriteAssetsFile(Path.Combine(project.ProjectDirectory, "fake.assets.json")); + + return [.. projects]; + } + + static async Task<(int Code, string Output)> RunOnSolutionAsync( + ProjectHarness harness, + string arguments, + CancellationToken cancellationToken + ) + { + var (code, stdOut, stdErr) = await harness.RunProcessAsync( + "dotnet", + $"msbuild \"{Path.Combine(harness.SolutionDirectory, SolutionFileName)}\" -nologo -v:n {arguments}", + harness.SolutionDirectory, + cancellationToken + ); + + return (code, stdOut + stdErr); + } + + [Test] + [Arguments("-m:1")] + [Arguments("-m")] + public async Task ASkillTargetOnASolution_RunsOnce(string parallelism, CancellationToken cancellationToken) + { + var projects = await CreateSolutionAsync(cancellationToken); + using var owner = projects[0]; + + var (code, output) = await RunOnSolutionAsync( + owner, + $"{parallelism} -t:InstallPurviewSkills -p:PurviewSkillsDryRun=true " + + "-p:PurviewSkillsSource=purview-dev/engineering-playbook -p:PurviewSkillsRunner=npx", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(DryRunInstallLine().Count(output)).IsEqualTo(1).Because(output); + } + + [Test] + public async Task ASolution_ElectsTheSameLeaderFromEveryProject(CancellationToken cancellationToken) + { + var projects = await CreateSolutionAsync(cancellationToken); + using var owner = projects[0]; + + var (code, output) = await RunOnSolutionAsync(owner, "-t:ListPurviewSkills", cancellationToken); + + await Assert.That(code).IsZero().Because(output); + // Each project runs the public target, but only the leader runs the work behind it. + await Assert.That(ListCoreRun().Count(output)).IsEqualTo(1).Because(output); + await Assert.That(NoSkillsLine().Count(output)).IsEqualTo(1); + } + + [Test] + public async Task APackageOnlyOneProjectReferences_IsFoundFromTheSolution(CancellationToken cancellationToken) + { + var projects = await CreateSolutionAsync(cancellationToken); + using var owner = projects[0]; + + var packagesRoot = Path.Combine(owner.SolutionDirectory, "packages"); + SkillPackageFixtures.WritePackage(packagesRoot, "Purview.Example", "1.2.3", ["purview-alpha"]); + // Only the last project - never the leader - references the package, the way a test project + // is often the only one to reference a testing library. + SkillPackageFixtures.WriteAssetsFile( + Path.Combine(projects[2].ProjectDirectory, "fake.assets.json"), + ("Purview.Example", "1.2.3") + ); + + var (code, output) = await RunOnSolutionAsync( + owner, + $"-t:ListPurviewSkills -p:NuGetPackageFolders=\"{packagesRoot}\"", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(ExampleListedLine().Count(output)).IsEqualTo(1).Because(output); + } + + [Test] + public async Task TwoVersionsOfOnePackage_AreReportedAsAConflict(CancellationToken cancellationToken) + { + var projects = await CreateSolutionAsync(cancellationToken); + using var owner = projects[0]; + + var packagesRoot = Path.Combine(owner.SolutionDirectory, "packages"); + SkillPackageFixtures.WritePackage(packagesRoot, "Purview.Example", "1.0.0", ["purview-alpha"]); + SkillPackageFixtures.WritePackage(packagesRoot, "Purview.Example", "2.0.0", ["purview-alpha"]); + SkillPackageFixtures.WriteAssetsFile( + Path.Combine(projects[1].ProjectDirectory, "fake.assets.json"), + ("Purview.Example", "1.0.0") + ); + SkillPackageFixtures.WriteAssetsFile( + Path.Combine(projects[2].ProjectDirectory, "fake.assets.json"), + ("Purview.Example", "2.0.0") + ); + + var (code, output) = await RunOnSolutionAsync( + owner, + $"-t:ListPurviewSkills -p:NuGetPackageFolders=\"{packagesRoot}\"", + cancellationToken + ); + + await Assert.That(code).IsZero().Because(output); + await Assert.That(output).Contains("PRSKL0017"); + await Assert.That(output).Contains("1.0.0, 2.0.0"); + } + + [Test] + public async Task AnUnrestoredProject_IsNamedInPRSKL0001(CancellationToken cancellationToken) + { + var projects = await CreateSolutionAsync(cancellationToken); + using var owner = projects[0]; + + File.Delete(Path.Combine(projects[1].ProjectDirectory, "fake.assets.json")); + + var (code, output) = await RunOnSolutionAsync(owner, "-t:ListPurviewSkills", cancellationToken); + + await Assert.That(code).IsNotEqualTo(0); + await Assert.That(output).Contains("PRSKL0001"); + await Assert.That(output).Contains("Bravo.csproj"); + } +} diff --git a/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs b/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs index 2be3469..8fdb2dd 100644 --- a/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs +++ b/src/tests/BuildSdk.IntegrationTests/PurviewSkillTargetIsolationTests.cs @@ -18,6 +18,12 @@ sealed class PurviewSkillTargetIsolationTests "UpdatePurviewSkills", "FixPurviewSkills", "CheckPurviewSkills", + // The leader-side halves the public targets hand their work to; reaching one directly is + // reaching the work. + "_InstallPurviewSkillsCore", + "_UpdatePurviewSkillsCore", + "_FixPurviewSkillsCore", + "_CheckPurviewSkillsCore", ]; static IEnumerable SdkProjectFiles() =>