perf: stop recompiling every project on every build (release 1.3.0)

.config/lefthook.yml

@@ -2,6 +2,8 @@ pre-commit:
   jobs:
     - name: csharpier check
       run: just lint-check
+    - name: version pins
+      run: just version-sync-check
 
 commit-msg:
   jobs:

.editorconfig

@@ -1,4 +1,4 @@
-# Purview.BuildSdk 1.0.7 - managed file, do not edit
+# Purview.BuildSdk 1.3.0 - 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

AGENTS.md

@@ -191,26 +191,43 @@ Two rules are worth knowing the reasoning for. A repository with **no** `skills/
 
 Tests live in `src/tests/SkillsTesting.UnitTests/`, with the repository-level ones in `src/tests/BuildSdk.IntegrationTests/RepositorySkillTests.cs`, `SkillRepositoryRootParityTests.cs` and `RepositorySkillsSelfTests.cs`.
 
-### Recommending skills
+### Checking skills
 
-Installation is explicit, but **recommendation runs during an ordinary build** — it is the one skill feature that does. `RecommendPurviewSkillsOnBuild` (`AfterTargets="Build"`) delegates to `RecommendPurviewSkills`, which can also be invoked by name:
+**Nothing about skills runs during a build.** This used to be the opposite — a
+`RecommendPurviewSkillsOnBuild` hook ran on `AfterTargets="Build"` — and it was removed after
+measurement: on a 39-project solution it read and regex-scanned every project's
+`project.assets.json` (≈500 KB, 256 libraries each), made ~10 000 failing `Directory.Exists` calls,
+read the lockfile and walked the repository root, on every build, to print advice that a repository
+managing its own skills had already acted on. Every input is repository-scoped, so the per-project
+cost was pure duplication. The remaining work is now paid only when somebody asks:
 
 ```bash
-dotnet msbuild <project> -t:RecommendPurviewSkills
+dotnet msbuild <project> -t:RecommendPurviewSkills   # report, never fail
+dotnet msbuild <project> -t:VerifyPurviewSkills      # report, and fail on anything unverifiable
+dotnet msbuild <project> -t:InstallPurviewSkills     # install what is missing
+dotnet msbuild <project> -t:ListPurviewSkills        # what the restored packages ship
 ```
 
+`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
+package published without repository metadata is not the consumer's to fix; `PRSKL0013`/`PRSKL0014`
+stay messages, because 0013 is an owner-scoped heuristic and 0014 fires on any clone of a repository
+that installs skills on demand. Failing a build over either would hand people errors they cannot
+clear, which is how a gate gets switched off. The escalation is safe because the target cannot be
+reached by accident: a build that did not ask for it is never broken by skill advice.
+
 `EvaluatePurviewSkillState` (inline task in `Sdk/Sdk.targets`) joins the skills each restored package ships against the CLI's own `skills-lock.json` and classifies every one: satisfied, missing (`PRSKL0010`), ref-drifted (`PRSKL0011`), unverifiable (`PRSKL0012`), orphaned (`PRSKL0013`), or recorded-but-never-restored (`PRSKL0014`). Both inputs are local, so it stays offline and spawns no process.
 
 The lockfile is read directly rather than by parsing `npx 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.
 
 Drift **never** recommends `npx 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.
 
-Four things keep it quiet, and all of them are load-bearing:
+Three things shape the output, and all of them are load-bearing:
 
-- design-time builds (`DesignTimeBuild`), CI builds (`ContinuousIntegrationBuild`/`TF_BUILD`/`GITHUB_ACTIONS`/`CI` unless `PurviewSkillRecommendationsInCI=true`), non-default TFM passes and `EnablePurviewSkillRecommendations=false` are all skipped outright;
-- a fingerprint stamp (`<repo root>/.purview/skill-recommendations.cache`) means a given state is reported once per repository rather than once per project and again on every rebuild — the explicit target ignores the stamp;
-- missing and drifted are warnings (actionable in one command), while unverifiable and orphaned are messages, so no amount of skill advice can break a build using `TreatWarningsAsErrors`; and
+- missing and drifted are warnings (actionable in one command), while unverifiable and orphaned are messages — and `VerifyPurviewSkills` is what turns them into failures; and
 - 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.
 
 Orphan detection is **owner-scoped on purpose**: a lockfile cannot record who asked for an entry, so 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.
 
@@ -279,13 +296,16 @@ Prefer `just` tasks:
 - `just pack`
 - `just skills-validate` — the pack-time check over the authored skills
 - `just skills-add <source>` — install a skill from another repository into `.agents/skills/`
-- `just version-sync` — rewrite the `Purview.BuildSdk` pins in the docs and `templates/global.json`
-  after bumping `package.json`
-
-Nothing packed carries a hand-maintained version. The `.editorconfig` marker and
-`Props/PackedVersion.props` (the `global.json` fallback when the SDK is not in a versioned package
-folder) are stamped at pack time from `$(PackageVersion)`. Only the copy-paste pins in the docs live in
-git, and `PackagedVersionFallbackTests` names each stale one as `file:line`.
+- `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
+
+Nothing packed carries a hand-maintained version. `Props/PackedVersion.props` (the `global.json`
+fallback when the SDK is not in a versioned package folder) is stamped at pack time from
+`$(PackageVersion)`. Only the copy-paste pins live in git — the quick-start snippets, the starter
+template, and this repository's own `.editorconfig` marker, so that reading the tracked file never
+shows a release-old number. `PackagedVersionFallbackTests` names each stale one as `file:line`, and
+`just version-sync-check` fails the commit before it can land.
 
 `dotnet` fallback uses `src/BuildSdk.slnx` and `Release`.
 

CHANGELOG.md

@@ -11,6 +11,95 @@ release summarise what the commits recorded rather than what was written at the
 
 ## [Unreleased]
 
+## [1.3.0] - 2026-11-08
+
+### Changed (breaking)
+
+- **The skill recommendation hook is gone. No skill work happens during a build.** `RecommendPurviewSkillsOnBuild`
+  (`AfterTargets="Build"`) and its `_PurviewSkillRecommendationBuildSetup` helper are removed. On a
+  39-project solution the hook read and regex-scanned every project's `project.assets.json`
+  (≈500 KB, 256 libraries each), made thousands of failing `Directory.Exists` calls, read the lockfile
+  and walked the repository root — on every build, including a no-change rebuild — and every one of its
+  inputs is repository-scoped, so the per-project cost was pure duplication of one repository-wide
+  answer. Removed rather than defaulted off, because a repository that manages its skills deliberately
+  (any repository with a `skills-lock.json`) was paying twice for an answer it had already acted on.
+  Use `-t:RecommendPurviewSkills` to report and `-t:InstallPurviewSkills` to install.
+
+- **Removed properties:** `EnablePurviewSkillRecommendations`, `PurviewSkillRecommendationsInCI` and
+  `PurviewSkillRecommendationStampPath`. All three existed to gate and deduplicate the hook, including
+  the `<repo root>/.purview/skill-recommendations.cache` stamp. A target a person invokes should answer
+  every time it is asked, so no stamp is needed. They are also no longer compiler-visible.
+
+### Added
+
+- **`VerifyPurviewSkills`** — the skill state as a gate rather than advice. It reports the same
+  classifications as `RecommendPurviewSkills` but escalates `PRSKL0010` (missing) and `PRSKL0011`
+  (drifted) to **errors**, which is what a CI job should call. Those two are always factual and always
+  fixed by one `npx skills add` command. `PRSKL0012` is reported as a warning rather than an error,
+  because it also covers a package published without repository metadata — something only that
+  package's author can fix — and `PRSKL0013`/`PRSKL0014` stay messages, because 0013 is an owner-scoped
+  heuristic and 0014 fires on any clone of a repository that installs skills on demand. Safe because the
+  target is only reachable by name: a build that did not ask is never broken by skill advice.
+
+```bash
+dotnet msbuild <project> -t:VerifyPurviewSkills
+```
+
+### Fixed
+
+- **Incremental builds were broken outright.** `GenerateAssemblyInfoClassTarget` and
+  `GenerateAssemblyIdentifierClassTarget` wrote their generated files with `Overwrite="true"` and no
+  `WriteOnlyWhenDifferent`, giving them a fresh timestamp on every build. Both files are compile inputs,
+  so `CoreCompile` — whose `Inputs` are `@(Compile)` — could never be up to date, and a no-change rebuild
+  recompiled every project. Measured on a 39-project solution: a warm rebuild ran **38** `Csc`
+  invocations totalling **111 s** of a 68 s build. `WriteOnlyWhenDifferent="true"` makes an unchanged
+  build compile nothing. This also restores the value of `AccelerateBuildsInVisualStudio=true`, which
+  defaults to on and was being defeated by the same timestamps.
+
+- **`ValidatePurviewStylePolicy` read every `.editorconfig` in the chain twice** — once to find
+  `root = true` and once to merge it. The SDK's own `.editorconfig` is 2,714 lines / 95 KB, so each
+  project read it twice on every compile. The file is now read once and the lines are shared.
+
+- **Both repository-file bootstraps ran during design-time builds.** `EnsureRepositoryEditorConfigTarget`
+  and `EnsureRepositoryGlobalJsonTarget` were not guarded against `DesignTimeBuild`, and an IDE runs a
+  design-time build for every edit, on every project, for as long as a solution is open. Each pass
+  repeated the repository-root probe ladder and two full reads of the 95 KB `.editorconfig`
+  (0.83 ms of I/O per project, per pass) and could never produce anything a design-time build uses.
+  Both are now skipped when `DesignTimeBuild=true`; a real build still bootstraps them, which is the
+  only thing that ever needed to.
+
+- **`BuildSdk.IntegrationTests` had the same unconditional-rewrite defect**, in its own
+  `WriteSdkSourcePaths` target: `Generated/SdkPaths.cs` was written with `Overwrite="true"` and no
+  `WriteOnlyWhenDifferent`, so that project recompiled on every build even after the two SDK writers
+  were fixed. It is fixed the same way, and a `Release` build of this repository with no changes now
+  runs **zero** `Csc` invocations in 0.85 s.
+
+### Tests
+
+- `IncrementalBuildTests` pins both SDK-generated files: building an unchanged project twice must
+  leave `GeneratedAssemblyInfo.cs` and `GeneratedAssemblyIdentifier.cs` with their original
+  timestamps and content. A rewrite is what breaks incrementality, and a rewrite always moves the
+  timestamp, so the test asserts the timestamp rather than counting compiler invocations.
+
+### Repository
+
+- **The tracked `.editorconfig` marker now self-syncs.** It names the SDK version that wrote the file,
+  and it had drifted to `1.0.7` while `package.json` was on `1.2.x` — misleading for anyone reading the
+  repository, though never wrong for a consumer, because packing stamps the version in. The file is now
+  in `scripts/sync-version.ts`, so `just version-sync` rewrites it along with the quick-start snippets,
+  `AdoptionPin_MatchesPackageJson` asserts it, and a new `just version-sync-check` fails the pre-commit
+  hook when it is stale. `CHANGELOG.md` is deliberately excluded: its marker lines are history and keep
+  the version they shipped with.
+
+### Notes
+
+- `docs/wiki/Performance.md` now carries measured numbers for a 39-project solution instead of a list
+  of things that had not been measured. The headline, measured end to end on `changeops` with this
+  version: a rebuild with **no source changes went from 67.7 s to 7.2 s**, and from **38 `Csc`
+  invocations to 0**. A cold build went from 154.4 s to 103.2 s. Note that the *first* warm build after
+  upgrading is slower than that (27.3 s), because the upgrade invalidates analyzers, `.editorconfig`
+  and compiler state at once; steady state is reached on the next no-change build.
+
 ## [1.2.0] - 2026-10-28
 
 ### Added

Justfile

@@ -88,14 +88,23 @@ restore *args:
 current_version:
     echo "==> Current version: {{ GREEN }}{{ current_version }}{{ NORMAL }} (defined in package.json and automatically included in the build output through the Purview.BuildSdk package)"
 
-# Rewrite the Purview.BuildSdk pins in the docs and starter template to the version in package.json
+# Rewrite the Purview.BuildSdk pins in the docs, starter template and .editorconfig marker
 #
 # Run after bumping package.json. Packed files are stamped at pack time and need nothing.
 [group('Build and Test')]
 version-sync:
     echo "==> Syncing version pins to {{ GREEN }}{{ current_version }}{{ NORMAL }}"
     bun scripts/sync-version.ts
 
+# Check the version pins without writing anything; non-zero when a pin is stale
+#
+# What the pre-commit hook runs, so a stale .editorconfig marker or quick-start snippet cannot be
+# committed in the first place.
+[group('Build and Test')]
+version-sync-check:
+    echo "==> Checking version pins against {{ GREEN }}{{ current_version }}{{ NORMAL }}"
+    bun scripts/sync-version.ts --check
+
 # Run tests for a specific project with a filter (e.g., "/*/*/*/*/", or "/*/*/*/*[Category=Unit]" to run just unit tests) and configuration (e.g., "Release")
 [group('Build and Test')]
 test filter="/*/*/*/*/" *args:

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.2.0"
+    "Purview.BuildSdk": "1.3.0"
   }
 }
 ```
@@ -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.2.0 - managed file, do not edit
+# Purview.BuildSdk 1.3.0 - managed file, do not edit
 ```
 
 That line is how the SDK recognises a file **it** wrote. When the SDK version changes, a file carrying
@@ -290,9 +290,13 @@ dotnet msbuild <project> -t:InstallPurviewSkills    # explicit; delegates to the
 
 Compatibility is pinned by commit: a package's nuspec names the commit it was built from, the CLI
 accepts `owner/repo#<commit>` and records it, so a project on an older package version is never
-advised to install skills describing a newer API. Set `EnablePurviewSkillRecommendations=false` to
-turn the build-time reporting off; it is already silent on CI, during design-time builds, and when
-everything is installed and compatible.
+advised to install skills describing a newer API.
+
+**Nothing about skills runs during a build.** `-t:RecommendPurviewSkills` reports the state and
+`-t:VerifyPurviewSkills` reports it and fails when a referenced package's skills are missing or pinned
+to a different ref — the second is what a CI job calls. Reading the restore graph and the lockfile on
+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
@@ -416,9 +420,6 @@ Non-packable projects (including web applications) default `WarnOnPackingNonPack
 
 | Property | Default | Description |
 | -- | -- | -- |
-| `EnablePurviewSkillRecommendations` | `true` | Reports missing or version-drifted agent skills after an ordinary build. Offline; never installs; never fails the build. |
-| `PurviewSkillRecommendationsInCI` | `false` | When `true`, recommendations are reported on CI builds as well. |
-| `PurviewSkillRecommendationStampPath` | `<repo root>/.purview/skill-recommendations.cache` | Overrides the stamp that reports each distinct skill state once per repository. |
 | `PurviewSkillsLockFile` | `<repo root>/skills-lock.json` | Overrides where the skills CLI's lockfile is read from. |
 | `PurviewSkillsSourceFolder` | `<repo root>/skills` | Repository folder holding authored skills. |
 | `PurviewSkillsAgents` | `*` | Agents that `InstallPurviewSkills` installs to, and that recommended commands name. |
@@ -521,9 +522,6 @@ The SDK now exports its properties via `CompilerVisibleProperty`, so analyzers a
 | `PurviewRepoBootstrapCopyFailureAsError` | When `true` (default), a bootstrap write that still fails after every retry is an error. |
 | `PurviewSuppressCopyRetryWarnings` | When `true` (default), built-in copy retry notices (`MSB3026`) are demoted to messages. |
 | `PurviewAutoSdkPack` | When `true`, automatically packs the `Sdk/` folder contents into the NuGet package with the correct root-level paths. |
-| `EnablePurviewSkillRecommendations` | When `true` (default), an ordinary build reports missing or version-drifted agent skills. |
-| `PurviewSkillRecommendationsInCI` | When `true`, skill recommendations are reported on CI builds as well (default `false`). |
-| `PurviewSkillRecommendationStampPath` | Stamp used to report each distinct skill state once (default `<repo root>/.purview/skill-recommendations.cache`). |
 | `PurviewSkillsLockFile` | Where the skills CLI's `skills-lock.json` is read from (default `<repo root>/skills-lock.json`). |
 | `PurviewSkillsSourceFolder` | Repository folder holding authored agent skills (default `<repo root>/skills`). |
 | `PurviewSkillsAgents` | Agents that `InstallPurviewSkills` installs to (default `*`). |