From b2bdfa49b2639ac7d826c859744443317f0da635 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Wed, 14 Oct 2026 23:47:00 +0100 Subject: [PATCH 1/8] build: take .NET SDK 11.0.100-rc.2.26504.105 --- .github/workflows/integration-wsl.yml | 2 +- .github/workflows/pr.yml | 4 ++-- .github/workflows/release.yml | 2 +- global.json | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/integration-wsl.yml b/.github/workflows/integration-wsl.yml index a4a5007..46984b4 100644 --- a/.github/workflows/integration-wsl.yml +++ b/.github/workflows/integration-wsl.yml @@ -40,7 +40,7 @@ jobs: uses: actions/setup-dotnet@v4 with: # Keep in sync with `sdk.version` in global.json (and the other workflows). - dotnet-version: "11.0.100-rc.1.26425.128" + dotnet-version: "11.0.100-rc.2.26504.105" - name: Verify the WSL Containers host shell: pwsh diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 237cd14..5a9245f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -20,7 +20,7 @@ jobs: # the .NET 11 SDK in global.json. Without the matching SDK the runner cannot resolve global.json # ("A compatible .NET SDK was not found") and every step fails. # Keep in sync with `sdk.version` in global.json. - dotnet-version: "11.0.100-rc.1.26425.128" + dotnet-version: "11.0.100-rc.2.26504.105" # Mirrors Build:TestFilter in purview-build.json: the SDK categorises unit test projects as Unit, # so the WSLC integration suites are never selected and the runner needs no WSLC host. test-filter: "/*/*/*/*[Category=Unit]" @@ -42,7 +42,7 @@ jobs: - uses: actions/setup-dotnet@v4 with: # Keep in sync with `sdk.version` in global.json. - dotnet-version: "11.0.100-rc.1.26425.128" + dotnet-version: "11.0.100-rc.2.26504.105" - name: Docker backend run: dotnet test src/tests/Docker.IntegrationTests/Docker.IntegrationTests.csproj --configuration Release --treenode-filter '/*/*/*/*' - name: Service modules on Docker diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c797ef7..7f071b2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ jobs: # The shared workflow defaults to the 10.0.x SDK, but this repository targets net11.0 and pins # the .NET 11 SDK in global.json; without it the release job cannot resolve global.json. # Keep in sync with `sdk.version` in global.json (and with .github/workflows/pr.yml). - dotnet-version: "11.0.100-rc.1.26425.128" + dotnet-version: "11.0.100-rc.2.26504.105" release-mode: NuGet release-branch: main secrets: inherit diff --git a/global.json b/global.json index 791bc0e..528a2ac 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "11.0.100-rc.1.26425.128", + "version": "11.0.100-rc.2.26504.105", "allowPrerelease": true }, "msbuild-sdks": { From bca70a963b79af78b5bbc86a32225fc85e4117c8 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Thu, 15 Oct 2026 16:12:53 +0100 Subject: [PATCH 2/8] feat: migrate agent skills to the root skills/ folder and the skills CLI Purview.BuildSdk 1.0.6 removes skill packaging and the build-time .agents/ mirror, so the skills CLI cloning the git repository is the only delivery route. Skills are authored at skills//SKILL.md and nothing packs them. - rename every skill to a purview--* prefix; skills install into one flat directory per agent, so an unprefixed name collides with somebody else's and the loser is whichever installed second - validate them on every test run with Purview.Skills.Testing 1.0.6, through a Repository.UnitTests project whose [InheritsTests] attribute is load-bearing - raise the TUnit floor to 1.75.0, which Purview.Skills.Testing depends on and central transitive pinning would otherwise pin back down --- AGENTS.md | 37 +++- Directory.Packages.props | 9 +- global.json | 2 +- .../SKILL.md | 128 +++++++++++++ skills/purview-containers-lifecycle/SKILL.md | 144 ++++++++++++++ skills/purview-containers-quickstart/SKILL.md | 181 ++++++++++++++++++ skills/purview-containers-testing/SKILL.md | 126 ++++++++++++ src/Containers.slnx | 1 + .../Repository.UnitTests.csproj | 22 +++ src/tests/Repository.UnitTests/SkillTests.cs | 60 ++++++ 10 files changed, 704 insertions(+), 6 deletions(-) create mode 100644 skills/purview-containers-consumer-requirements/SKILL.md create mode 100644 skills/purview-containers-lifecycle/SKILL.md create mode 100644 skills/purview-containers-quickstart/SKILL.md create mode 100644 skills/purview-containers-testing/SKILL.md create mode 100644 src/tests/Repository.UnitTests/Repository.UnitTests.csproj create mode 100644 src/tests/Repository.UnitTests/SkillTests.cs diff --git a/AGENTS.md b/AGENTS.md index 9f9603f..8d94a75 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,8 +36,8 @@ whenever a target framework, a backend package or the `buildTransitive` assets c | `src/Directory.Build.props` / `src/Directory.Build.targets` | Solution-wide SDK import, package metadata and the shared package icon | | `purview-build.json` | Shared `Purview.Build` pipeline configuration: solution, test discovery/filter and the **exhaustive** pack-validation manifest | | `Justfile` | Supported local workflow commands | -| `.agents` | Package-delivered skills, agents and prompts (read-only here) | -| `.purview/agent-sync.cache` | SDK-managed manifest recording which `.agents` files are already mirrored | +| `skills` | The agent skills this repository authors and publishes | +| `.agents/skills` | Skills installed from other repositories by the skills CLI (gitignored; `skills-lock.json` is the tracked record) | ## Standard workflow @@ -203,3 +203,36 @@ Before handing work back: - Run the appropriate build, test, formatting and pack checks in proportion to risk. - State exactly what validation ran. If a check was skipped or blocked, give the concrete reason and the remaining risk. + +## Agent skills + +Authored skills live in the repository-root `skills/` folder. **Nothing packs them into a NuGet +package** - `Purview.BuildSdk` 1.0.6 removed skill packaging and the build-time `.agents/` mirror +outright. The skills CLI clones the *git* repository and only discovers a root `skills/` folder, so +that is the one delivery route. + +`.agents/skills/` is the CLI's install location for skills from **other** repositories, and is +gitignored. This repository's own skills are never installed into it - that would leave two copies to +drift apart. Read them at their authored path, `skills//SKILL.md`. + +```bash +npx skills add purview-dev/containers +``` + +| Skill | Answers | +| -- | -- | +| `skills/purview-containers-quickstart/` | which package, how a backend is chosen, a first container, a typed module, connection strings | +| `skills/purview-containers-lifecycle/` | start/stop/dispose semantics, cleanup guarantees, wait strategies, WSLC networking | +| `skills/purview-containers-testing/` | the test tiers, why modules run serially, pinning a backend, the CI shape | +| `skills/purview-containers-consumer-requirements/` | the target-framework contract, `PCC0001`/`PCC0002` and the other consumer build errors | + +Every skill carries the `purview-containers` prefix. Skills install into one flat directory per +agent, shared with every skill a developer has installed from anywhere, so an unprefixed name +collides and the loser is whichever installed second. + +`src/tests/Repository.UnitTests/SkillTests.cs` validates them on every test run through +`Purview.Skills.Testing`. The `[InheritsTests]` attribute there is load-bearing - TUnit does not walk +base types during discovery, so without it the run is green and nothing is checked. + +A relative link from one `SKILL.md` into another skill's folder is a rule violation: only a skill's +own folder is installed with it. Name the other skill in prose instead. diff --git a/Directory.Packages.props b/Directory.Packages.props index deddac5..4e534a3 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -8,9 +8,12 @@ - - - + + + + + + diff --git a/global.json b/global.json index 528a2ac..9883acd 100644 --- a/global.json +++ b/global.json @@ -4,7 +4,7 @@ "allowPrerelease": true }, "msbuild-sdks": { - "Purview.BuildSdk": "1.0.5" + "Purview.BuildSdk": "1.0.6" }, "test": { "runner": "Microsoft.Testing.Platform" diff --git a/skills/purview-containers-consumer-requirements/SKILL.md b/skills/purview-containers-consumer-requirements/SKILL.md new file mode 100644 index 0000000..28a7ee4 --- /dev/null +++ b/skills/purview-containers-consumer-requirements/SKILL.md @@ -0,0 +1,128 @@ +--- +name: purview-containers-consumer-requirements +description: >- + Use when a project consuming Purview.Containers will not build - PCC0001 or PCC0002, CS1705 for + Microsoft.Windows.SDK.NET, CS0234/CS0246 for Microsoft.WSL types, NETSDK1100 on a Linux build agent + - or when choosing the target framework and platform for a consumer of the WSL Containers backend. +metadata: + product: Purview.Containers + repository: purview-dev/containers +--- + +# Consumer requirements + +## Which package requires what + +| Package | Target frameworks | Notes | +| --- | --- | --- | +| `Purview.Containers` | `net10.0`, any platform | the umbrella: `Core` plus both backends | +| `Purview.Containers.Core` | `net10.0`, any platform | backend-neutral abstractions | +| `Purview.Containers.` | `net10.0`, any platform | service modules; restore anywhere, need a backend to run | +| `Purview.Containers.Wsl` | `net10.0` **and** `net10.0-windows10.0.19041.0` | a portable facade plus the Windows implementation. **Everything below is its contract.** | +| `Purview.Containers.Docker` | `net10.0`, any platform | needs a reachable Docker daemon, not a Windows target framework | + +A consumer of any backend other than WSL needs only a portable `net10.0` project. + +## Most projects need nothing + +```xml +net10.0 +``` + +That binds the portable facade and gets automatic backend selection — WSLC on a Windows host, Docker +everywhere else. It is the recommended choice, and it is why a Windows developer and a Linux CI +runner can share one test project. + +A Windows-targeting project binds the implementation directly, with no run-time load: + +```xml + + net10.0-windows10.0.19041.0 + x64 + 10.0.26100.80 + +``` + +The last two lines are optional — the package ships MSBuild defaults — but stating them makes the +requirement visible to the next person reading the project file, and the defaults only fill in a +value you have not chosen. + +## The five requirements + +| # | Requirement | Why | +| --- | --- | --- | +| 1 | **.NET 10 or later** | the package ships `lib/net10.0/` (facade) and `lib/net10.0-windows10.0.19041/` (implementation) | +| 2 | **A Windows target framework that names the OS version** — *only* if you want the implementation bound directly | the `Microsoft.WSL.Containers` projection ships Windows assets only | +| 3 | **64-bit** — `PlatformTarget` `x64`/`arm64`, or a matching `RuntimeIdentifier` | the native `wslcsdk.dll` ships for `win-x64` and `win-arm64` only | +| 4 | **`WindowsSdkPackageVersion` ≥ `10.0.26100.80`** | the projection is compiled against `Microsoft.Windows.SDK.NET` `10.0.26100.79`, and the pack the SDK resolves for a `10.0.19041.0` target framework is older | +| 5 | **A Windows 10/11 host with WSL Containers**, to actually run anything | the library drives WSLC; it never installs or updates it for you | + +### Always spell out the Windows version + +```xml +net10.0-windows10.0.19041.0 +net10.0-windows +``` + +`net10.0-windows` resolves its platform version to **7.0**, older than the `10.0.19041.0` the +implementation was built against, so NuGet selects no compile assets at all. (The +`Microsoft.WSL.Containers` package is itself `net8.0-windows10.0.19041.0`, which a +`net10.0-windows10.0.19041.0` project consumes without issue.) + +## What the package does for you + +`Purview.Containers.Wsl` ships two `buildTransitive` files, imported by NuGet for **direct and +transitive** references: + +| File | Effect | +| --- | --- | +| `…Wsl.props` | defaults `WindowsSdkPackageVersion` to `10.0.26100.80` when unset, and registers the `wsl` backend | +| `…Wsl.targets` | **Windows consumer:** defaults `PlatformTarget` to `x64` when unset or `AnyCPU` and no `RuntimeIdentifier` is selected, and fails with `PCC0001`/`PCC0002` when the target framework or platform cannot be supported. **Platform-neutral consumer:** copies the Windows implementation payload — implementation, WSLC projection, its Windows SDK dependencies, the native SDK — into `wslc/` beside the output on a Windows build host, so the facade can load it at run time. | + +An explicit `PlatformTarget`, `RuntimeIdentifier` or `WindowsSdkPackageVersion` in the consumer +always wins. + +A **service module** is backend-neutral and brings no backend. Reference the module *and* a backend +package, and use `PURVIEW_CONTAINERS_BACKEND` to pin which one runs. + +> Because `buildTransitive` assets are framework-agnostic, NuGet no longer reports `NU1202` for an +> incompatible target framework: restore succeeds and the **guard target** produces the failure +> instead. That is deliberate. `PCC0001` names the required target framework, whereas an empty +> compile-asset set only produces `CS0246` errors in your own code — which blame the wrong thing. + +## Error reference + +| Error | Raised by | Means | Fix | +| --- | --- | --- | --- | +| `PCC0001` | the shipped guard target | the target framework is neither .NET 10+ on Windows `10.0.19041.0`+ nor a platform-neutral .NET 10+ framework | retarget as above | +| `PCC0002` | the shipped guard target | the consumer is not 64-bit | remove `PlatformTarget`, set it to `x64`/`arm64`, or choose a matching `RuntimeIdentifier` | +| `CS1705` naming `Microsoft.Windows.SDK.NET` | the compiler | the resolved Windows SDK targeting pack is older than the projection's | pin `WindowsSdkPackageVersion` to `10.0.26100.80` | +| `CS0234`/`CS0246` for `Microsoft.WSL`/`Purview` types | the compiler | the package contributed no compile assets — an unsupported target framework, or an unverified escape hatch | fix the target framework; `PCC0001` says the same thing more clearly | +| `NETSDK1100` | the SDK | building a Windows-targeted project on a non-Windows agent | set `EnableWindowsTargeting=true` | + +### `EnableWindowsTargeting` on a non-Windows agent + +It lets a Linux build agent *compile* a Windows-targeted project. It does **not** make the +implementation runnable there — the facade still reports `wsl` unavailable, and Docker is selected. +Use it to keep one solution building in CI, not as a way to run WSLC off Windows. + +## Verifying all of this + +The repository proves every row above rather than asserting it: + +- `Wsl.UnitTests/ConsumerRequirementsTests.cs` guards the **shape** inside the normal unit run — the + Windows build targets `.NETCoreApp,Version=v10.0`, targets `Windows10.0.19041.0`, and declares only + that OS platform. If the target framework drifts, that test fails, and this page, the package + READMEs and the `buildTransitive` defaults all have to move with it. +- `just verify-consumers` proves the **behaviour**: it packs, then builds throwaway consumer projects + for every documented outcome, including the guard errors (`PlatformTarget=x86` → `PCC0002`; + `net8.0-windows` → `PCC0001`; `net11.0-windows` without an OS version → `PCC0001`). It sits outside + the `[Category=Unit]` filter because it restores from nuget.org. + +That split is the point: **a library's own build is not a consumer**, so a claim about consumers is +only true if something builds one. + +## Related skills + +`purview-containers-quickstart` for packages and backend selection, and +`purview-containers-testing` for the test tiers and CI shape. diff --git a/skills/purview-containers-lifecycle/SKILL.md b/skills/purview-containers-lifecycle/SKILL.md new file mode 100644 index 0000000..01b84ed --- /dev/null +++ b/skills/purview-containers-lifecycle/SKILL.md @@ -0,0 +1,144 @@ +--- +name: purview-containers-lifecycle +description: >- + Use when reasoning about a Purview.Containers container's lifecycle or readiness - StartAsync, + StopAsync and DisposeAsync semantics, cleanup guarantees and why there is no restart, the Wait + strategy family and started-versus-ready, and WSL Containers networking, port mapping and the + absence of inter-container DNS. +metadata: + product: Purview.Containers + repository: purview-dev/containers +--- + +# Lifecycle, readiness and networking + +## The public surface + +```csharp +public interface IContainer : IAsyncDisposable +{ + Task StartAsync(CancellationToken ct = default); + Task StopAsync(CancellationToken ct = default); + ValueTask DisposeAsync(); + // restart is intentionally NOT offered +} +``` + +**There is no `RestartAsync`.** WSLC has no restart operation, and stop-then-start is not equivalent, +so offering one would be a method that quietly means something different per backend. + +WSLC's `ContainerState` runs `Invalid → Created → Running → Exited → Deleted`. + +| Operation | Behaviour | +| --- | --- | +| `StartAsync` | creates the session if needed, ensures the image per the pull policy, creates the container, starts it, then waits. A missing init binary throws `ArgumentException 0x80070057` with the OCI message, surfaced as `WslContainerStartupException`. | +| `StopAsync` | `Stop(SIGTERM, grace)`, then `Stop(SIGKILL, …)` if still running. **Idempotent** — WSLC accepts stopping an already-stopped container. | +| `DeleteAsync` | `Delete(Force)`. A double delete throws `0x80010108` (`RPC_E_DISCONNECTED`) and is swallowed. | +| `DisposeAsync` | stop if running → delete → release the WinRT object. **Idempotent.** | + +## Cleanup guarantees + +These are the ones that matter when a test throws: + +- `DisposeAsync` runs in a `finally` by callers, **and** the container object is registered for + cleanup by the runtime, so disposal happens even if a test throws after `StartAsync`. +- `EnableAutoRemove` defaults to **`false`**: the library owns deletion. WSLC auto-removal would + delete a one-shot container before its state or output could be inspected — precisely when you need + them. Opt in with `.WithAutoRemove()`. +- A **partial startup failure** — image pull, create, start or wait — tears down through the same + `DisposeAsync` path. So does a **cancelled** `StartAsync` or wait. +- A **container crash** fires `InitProcess.Exited`; the container is then `Exited` and `DisposeAsync` + deletes it. +- A **session crash** marks the runtime session dead, and subsequent operations throw + `WslContainerSessionTerminatedException` rather than failing obscurely. +- **Test-process termination** disposes the shared session through a process-exit hook, freeing the + name. A *crashed* process leaves an orphaned session that only reserves its name; sweep it with + `wslc --session system session terminate`. + +## Secrets and names + +Container names are generated uniquely as `{service}-{processId}-{random8}` and **never contain +credentials**; session names are `wslc-{pid}-{random8}`. Diagnostics redact secrets through a +`Secret` wrapper whose `ToString()` returns ``. + +## Wait strategies + +**A started container is not a ready service.** `Container.Start()` returns once the init process is +running; readiness is a separate check, and it is the one your test actually depends on. + +```text +IWaitStrategy + TimeSpan? Timeout // null -> the container's StartupTimeout + TimeSpan Interval // default 250 ms + int? Retries // optional max failed checks + Task UntilAsync(WaitContext context, CancellationToken ct) + +WaitContext { IContainer Container; string? NetworkIp; int? GetHostPort(ushort); int? FirstHostPort; } +``` + +| Factory | Checks | +| --- | --- | +| `Wait.ForContainerRunning()` | `State == Running`. **Started, not ready.** | +| `Wait.ForTcpPort(8080)` | connects to `127.0.0.1:`. IPv4 only. | +| `Wait.ForHttp("/health")` | `.ForPort`, `.ForScheme`, `.ForStatusCode`, `.ForStatusPredicate`, `.ForHeader`, `.AllowInsecureTls` | +| `Wait.ForLogMessage(string \| Regex)` | case-insensitive substring, or regex, against **accumulated** init-process output | +| `Wait.ForCommand("pg_isready", "-U", "postgres")` | runs it via `ExecAsync`; ready at the expected exit code (default 0, `.ForExitCode(…)`) | +| `Wait.ForCustom((context, ct) => …)` | anything else | +| `Wait.ForAll(…)` / `Wait.ForAny(…)` | every strategy ready on the same check / at least one | + +Fluent modifiers: `.WithTimeout(…)`, `.WithInterval(…)`, `.WithRetries(…)`. + +Details worth knowing: + +- Waits run **inside** `StartAsync()`, after the container starts. `StartAsync` only returns when + every strategy is ready, or throws `ContainerTimeoutException`. +- The default timeout is the container's `StartupTimeout` — 5 minutes, overridable with + `.WithStartupTimeout(…)` or per-strategy. +- A timeout diagnostic includes the container name, image, state, mapped ports, the strategy type, + the last check error, and a bounded, secret-redacted tail of stdout/stderr. +- The container buffers output from process start, so a log message emitted **before** the waiter's + first check still matches. +- HTTP TLS validation is **on** by default. `AllowInsecureTls()` disables it for that request handler + only, never globally. +- Transient exec failures during startup count as not-ready, not as failure. + +Modules pick their own readiness check: PostgreSQL `pg_isready`, Redis `redis-cli ping`, RabbitMQ +`rabbitmq-diagnostics ping`, SQL Server a host-side connection. + +## Networking + +| `ContainerNetworkingMode` | Effect | Port mappings | +| --- | --- | --- | +| `null` (raw default) | Docker `NetworkMode: "none"` — no network, no IP | `CreateContainer` throws `0x80070057` | +| `None` | the same | no | +| `Bridged` | `172.17.0.x` on the session bridge; host→container mapped ports work | yes | + +`ContainerBuilder` defaults to **`Bridged`**, because a container with no network is not useful. + +Verified WSLC behaviour, and the parts that surprise people: + +- The host bind address is **IPv4 loopback only** (`HostIp: 127.0.0.1`). IPv6 `::1` does not connect. +- **Clients must use `127.0.0.1`, not `localhost`.** `localhost` resolves to `::1` first on .NET. + Npgsql and StackExchange.Redis fall back to IPv4; `Microsoft.Data.SqlClient` **hangs** without + falling back. +- Containers in the same session **can** reach each other by IP under `Bridged`. +- There is **no name or DNS resolution between containers**. `http://:8080/` and + `http://:8080/` fail with "bad address"; `/etc/hosts` holds only localhost and the + container's own `IP `. +- There are **no managed network objects** — `Microsoft.WSL.Containers` has no `network + create/connect`, unlike the CLI. + +How the library models it: `GetMappedPublicPort(containerPort)` reads +`Inspect().Ports["/tcp"][0].HostPort`; `GetNetworkIp()` reads +`Inspect().NetworkSettings.Networks.bridge.IPAddress` (Bridged only); TCP and HTTP waits connect to +`127.0.0.1:`. + +**What not to model.** Do not invent Docker-style `Network` objects — the semantics are just "bridged +with IPs". Do not claim container-name resolution works; it does not. Multi-container environments +are deliberately deferred, and would wire containers together by passing each other's IPs from +`GetNetworkIp()` into configuration or environment — which is why the core API does not preclude it. + +## Related skills + +`purview-containers-quickstart` for packages, backends and connection strings; +`purview-containers-testing` for how a suite is organised and why it runs serially. diff --git a/skills/purview-containers-quickstart/SKILL.md b/skills/purview-containers-quickstart/SKILL.md new file mode 100644 index 0000000..c90b6c1 --- /dev/null +++ b/skills/purview-containers-quickstart/SKILL.md @@ -0,0 +1,181 @@ +--- +name: purview-containers-quickstart +description: >- + Use when adding throwaway containers to a .NET test suite with Purview.Containers - which package to + reference, how the WSL Containers and Docker backends are selected automatically, running a generic + container with ContainerBuilder, using a typed service module, and getting a connection string. +metadata: + product: Purview.Containers + repository: purview-dev/containers +--- + +# Purview.Containers quickstart + +Throwaway Linux containers for .NET tests, behind one API, over either **WSL Containers** (no Docker +installation on Windows) or **Docker** (anywhere). The same test code runs on both. + +> **Experimental and prerelease.** The API, defaults and packaging rules can change between releases. +> Pin the version you adopt. + +## Which package + +| Package | Use it when | +| --- | --- | +| `Purview.Containers` | the umbrella: `Core` plus both backends. One reference, zero config — WSLC on Windows, Docker elsewhere. **The default.** | +| `Purview.Containers.Wsl` | WSL Containers only — a Windows machine with no Docker. | +| `Purview.Containers.Docker` | Docker only — any platform, CI, Linux/macOS. A Linux-only job can skip the WSL backend's ~19 MB payload with `Core` + `.Docker`. | + +A service module is backend-neutral, so it needs a backend (or the umbrella) alongside it: + +```bash +dotnet add package Purview.Containers.PostgreSql +dotnet add package Purview.Containers +``` + +## The zero-config shape + +```xml + + + net10.0 + + + + + + + +``` + +Three things make it work, and nothing else is required: + +- **A platform-neutral target framework** (`net10.0`). `Purview.Containers.Wsl` is multi-targeted; a + `net10.0` project binds its **portable facade**, which runs the WSLC implementation on Windows and + reports `wsl` unavailable everywhere else. A Windows target framework binds the implementation + directly — and then cannot run on a Linux CI runner. +- **The umbrella package**, bringing `Core` and both backends. +- **No registration line and no environment variable.** Each backend package ships `buildTransitive` + assets that generate a module initializer in your assembly, so the process discovers the registered + backends itself. Those assets flow transitively through the umbrella. + +## Backend selection + +| Host | Selected | Why | +| --- | --- | --- | +| Windows with WSL Containers | `wsl` | the facade loads the WSLC implementation, and WSLC is preferred | +| Windows with Docker only | `docker` | the facade reports `wsl` unavailable, so selection falls through | +| Linux / macOS | `docker` | the facade only ever activates on Windows | + +Selection probes the registered backends in priority order (`wsl` is `0`, `docker` is `100`) and +returns the first that is both **available** and **compatible**. When none is usable, the exception +lists each backend's availability, version and missing components. + +```csharp +foreach (var backend in await ContainerBackends.ProbeAllAsync()) + Console.WriteLine($"{backend.Name}: usable={backend.IsUsable} version={backend.Version}"); +``` + +```bash +PURVIEW_CONTAINERS_BACKEND=docker # or wsl - fail loudly instead of falling back +``` + +A **named** backend never silently falls back: if it cannot run, the test fails with that backend's own +diagnostics. That is usually what you want in CI. + +The library never installs a runtime. `WslContainerRuntime.GetInfoAsync()` and +`DockerContainerBackend.GetInfoAsync()` report what is missing. Verify a host with `wsl --version` and +`wslc version`, or `docker info`. + +## A generic container + +```csharp +using Purview.Containers; +using Purview.Containers.Waiting; + +await using var container = new ContainerBuilder() + .WithImage("docker.io/library/redis:latest") + .WithPortBinding(6379, assignRandomHostPort: true) + .WithWaitStrategy(Wait.ForTcpPort(6379)) + .Build(); + +await container.StartAsync(); + +ushort port = container.GetMappedPublicPort(6379); +``` + +`Build()` validates the accumulated configuration and resolves the backend when the container starts. +`StartAsync()` creates the container, starts it, and **only returns once every configured wait +strategy is satisfied**. `DisposeAsync()` stops and deletes the container — and never terminates a +shared WSLC session. + +## A typed module + +```csharp +using Npgsql; +using Purview.Containers.PostgreSql; + +await using var postgres = new PostgreSqlBuilder() + .WithDatabase("tests") + .WithUsername("postgres") + .WithPassword("postgres") + .Build(); + +await postgres.StartAsync(); // returns once pg_isready succeeds + +await using var connection = new NpgsqlConnection(postgres.GetConnectionString()); +await connection.OpenAsync(); +``` + +No image names, no ports, no `docker`, no `wslc`. The default image, the random host port mapping and +the readiness check all come from the module. + +Modules: PostgreSQL, Redis, SQL Server, MySQL, RabbitMQ, Azurite, NATS and the Azure Key Vault +Emulator. Each ships a README inside its own package. + +## Connection strings + +Every container implements `IConnectionStringProvider`, so you can ask any container without knowing +its module type: + +```csharp +string connectionString = container.GetConnectionString(); // ConnectionMode.Host +string same = container.GetConnectionString(ConnectionMode.Host); +``` + +| Mode | Meaning | Support | +| --- | --- | --- | +| `Host` | test host → container, via the mapped host port (`127.0.0.1:{port}`) | both backends | +| `Container` | container → container, via the container network | **not supported** — throws `ConnectionStringModeNotSupportedException` | + +Container-to-container is deliberately deferred: WSLC has no managed networks or inter-container DNS, +and the Docker backend does not attach containers to a shared network. + +**Use `127.0.0.1`, never `localhost`.** `localhost` resolves to `::1` first on .NET, and while most +clients fall back to IPv4, `Microsoft.Data.SqlClient` **hangs** on `::1` without falling back — which +is why the SQL Server module's connection strings are explicitly `127.0.0.1,{port}`. + +### A custom provider + +```csharp +sealed class ReadOnlyPostgreSql : ContainerConnectionStringProvider +{ + protected override string GetHostConnectionString() => + $"{Container.GetConnectionString()};ApplicationName=readonly"; +} + +var postgres = new PostgreSqlBuilder() + .WithConnectionStringProvider(new ReadOnlyPostgreSql()) + .Build(); +``` + +`Configure` runs once, **after** the container has started, so runtime-assigned ports are available. A +provider producing an empty string throws `ConnectionStringNotAvailableException`; one used before +`Configure` throws `ConnectionStringProviderNotConfiguredException`. The named overload +(`GetConnectionString(name, mode)`) exists for modules with several endpoints and otherwise throws +`ConnectionStringNameNotSupportedException`. + +## Related skills + +`purview-containers-lifecycle` for start/stop/dispose semantics, wait strategies and networking; +`purview-containers-testing` for suite layout, serial execution and CI; and +`purview-containers-consumer-requirements` for the target-framework contract and the `PCC` guards. diff --git a/skills/purview-containers-testing/SKILL.md b/skills/purview-containers-testing/SKILL.md new file mode 100644 index 0000000..5d8be98 --- /dev/null +++ b/skills/purview-containers-testing/SKILL.md @@ -0,0 +1,126 @@ +--- +name: purview-containers-testing +description: >- + Use when organising or running a test suite that uses Purview.Containers - the Unit and Integration + tiers, why test modules must run serially under WSL Containers, pinning or skipping a backend, + running the Docker suites in CI and the WSL suites on a self-hosted runner, and verifying the + consumer contract. +metadata: + product: Purview.Containers + repository: purview-dev/containers +--- + +# Testing with containers + +## Tiers + +The SDK stamps every test assembly with a TUnit category derived from the **project name**, so the +tier is a property of the project rather than of a hand-written attribute: + +| Project | Category | Runtime needed | +| --- | --- | --- | +| `*.UnitTests` | `Unit` | none — logic, parsing, configuration, wait-strategy units, in-process fakes | +| `Wsl.*` and module `*.IntegrationTests` | `Integration` | a **WSLC host** | +| `Docker.IntegrationTests`, `Modules.DockerIntegrationTests` | `Integration` | any **Docker daemon**; these target `net10.0`, so they run on Linux too | + +The shared pipeline filters to `/*/*/*/*[Category=Unit]`, so **the standard pipeline never starts a +container**. The Docker suites are the exception: the PR workflow adds an `integration-docker` job on +`ubuntu-latest`. That job is the standing proof that the library and every service module work off +Windows and WSL — the same modules a developer runs on WSLC locally. + +```powershell +just test # every discovered project +just test '/*/*/*/*[Category=Unit]' # unit only +just test '/*/*/*/*[Category=Integration]' --max-parallel-test-modules 1 +just test src/tests/Docker.IntegrationTests/Docker.IntegrationTests.csproj +``` + +Filtering uses TUnit's `--treenode-filter`, not `--filter`: this runs on Microsoft.Testing.Platform, +where `dotnet test --filter` reports "Zero tests ran". + +## Why test modules run serially + +A WSLC session takes an **exclusive lock on its `storage.vhdx`**, and takes it lazily on first store +access rather than at session start. Running test assemblies in parallel therefore makes the losers +fail: + +```text +0x80070020: The process cannot access the file because it is being used by another process. +``` + +`just test` passes `--max-parallel-test-modules 1` for exactly that reason. The runtime verifies the +store once and transparently falls back to an isolated per-process store, but serial modules keep the +**warm shared image cache** — no per-process re-pull — and are the fastest option. + +In Visual Studio, untick **Run Tests in Parallel** (or set *Maximum Parallel Test Projects* to 1) +before running the WSLC integration suites. + +## Choosing a backend in a suite + +Automatic detection prefers WSLC and falls back to Docker, so a WSLC host runs the WSLC suites and a +Docker-only host runs the Docker ones. Pin one to **fail loudly instead of falling back** — +usually what you want in CI: + +```bash +PURVIEW_CONTAINERS_BACKEND=wsl # or docker +``` + +Both halves skip themselves when their runtime is unreachable: the Docker suites when no daemon +responds, the WSLC suites when the host lacks the WSL Containers components. That is what lets a +machine without WSLC still run the unit tier — and it is also why a mis-provisioned CI runner reports +*successes with skips* rather than real coverage. Print `wsl --version` and `wslc version` as the +job's first step so that failure is visible immediately. + +## CI shape + +- **Docker half** — runs on every pull request, on `ubuntu-latest`. +- **WSL half** — cannot: it needs a Windows host with WSL Containers, which no GitHub-hosted runner + provides. It is a **manual** workflow on a self-hosted runner: + + ```bash + gh workflow run "Integration (WSL Containers)" --ref main + gh run watch + ``` + + It runs `Wsl.IntegrationTests` then each service-module suite **one project at a time**, so the + shared WSLC image store is never contended. Inputs: `ref` and `filter` (a treenode filter). + +Self-hosted runner prerequisites: a **Windows x64** runner labelled `self-hosted` and `wslc`, with +WSL Containers installed (`wsl --install --no-distribution`) and the pinned .NET SDK. Tell +`actionlint` about the custom label in `.github/actionlint.yaml`. + +> The shared pipeline runs on `ubuntu-latest`, so it builds the portable `net10.0` projects **and** +> the Windows-targeted test projects on Linux. That works only because `src/Directory.Build.props` +> sets `EnableWindowsTargeting=true`; without it the SDK reports `NETSDK1100`. + +## Expectations + +`Wsl.IntegrationTests` runs many real containers in one session and can take several minutes, because +WSLC serialises container operations. Slow-test warnings while it runs are expected, not a signal. + +## Verifying the consumer contract + +Two layers, and the split matters. + +**Shape, inside the unit run.** `Wsl.UnitTests/ConsumerRequirementsTests.cs` asserts that the Windows +build targets `.NETCoreApp,Version=v10.0`, targets `Windows10.0.19041.0`, and declares only that OS +platform. If the target framework drifts, that test fails — and the consumer-requirements page, the +package READMEs and the shipped `buildTransitive` defaults all have to move with it. + +**Behaviour, outside it.** `just verify-consumers` packs the packages and builds throwaway consumer +projects for every documented outcome — the happy paths *and* the documented guard errors. It is +deliberately outside the `[Category=Unit]` filter because it packs and restores from nuget.org: + +```powershell +just verify-consumers # 21 consumer projects, all assertions +just verify-consumers -Keep # keep the generated projects for inspection +``` + +This is the general rule worth copying: **a library's own build is not a consumer**, so a claim about +consumers is only true if something builds one. + +## Related skills + +`purview-containers-quickstart` for the packages and backend selection, +`purview-containers-lifecycle` for readiness and cleanup, and +`purview-containers-consumer-requirements` for the target-framework contract and `PCC` guards. diff --git a/src/Containers.slnx b/src/Containers.slnx index f9cabb9..184155c 100644 --- a/src/Containers.slnx +++ b/src/Containers.slnx @@ -51,6 +51,7 @@ + diff --git a/src/tests/Repository.UnitTests/Repository.UnitTests.csproj b/src/tests/Repository.UnitTests/Repository.UnitTests.csproj new file mode 100644 index 0000000..a53ba97 --- /dev/null +++ b/src/tests/Repository.UnitTests/Repository.UnitTests.csproj @@ -0,0 +1,22 @@ + + + + net10.0 + false + + + + + + + + + + + + diff --git a/src/tests/Repository.UnitTests/SkillTests.cs b/src/tests/Repository.UnitTests/SkillTests.cs new file mode 100644 index 0000000..8db0473 --- /dev/null +++ b/src/tests/Repository.UnitTests/SkillTests.cs @@ -0,0 +1,60 @@ +using Purview.Skills.Testing; + +namespace Purview.Containers; + +/// +/// Runs the Agent Skills conformance rule set against this repository's authored skills in +/// skills/. +/// +/// +/// +/// [InheritsTests] is load-bearing. TUnit does not walk base types when it discovers +/// tests, so without it this class compiles, the run is green, and nothing is validated. +/// +/// +/// There is no RepositoryRoot override: the default is the SDK's repository-root marker walk, +/// the same resolution InstallPurviewSkills uses, so an ordinary test run exercises it. +/// is what catches a walk that landed elsewhere - +/// every inherited rule passes vacuously against a catalogue that was never found. +/// +/// +[InheritsTests] +public sealed class SkillTests : PurviewSkillTests +{ + [Test] + public async Task SkillsFolder_ResolvesToThisRepository() + { + var catalogue = Evaluator.Catalogue; + + await Assert + .That(catalogue.FolderExists) + .IsTrue() + .Because($"the marker walk resolved '{catalogue.RepositoryRoot}', whose skills folder does not exist"); + + await Assert + .That(File.Exists(Path.Combine(catalogue.RepositoryRoot, "AGENTS.md"))) + .IsTrue() + .Because($"'{catalogue.RepositoryRoot}' does not look like this repository"); + + await Assert.That(catalogue.Skills).IsNotEmpty(); + } + + [Test] + public async Task EverySkill_IsNamedForThisProduct() + { + // Skills install into one flat directory per agent, shared with every skill a developer has + // installed from anywhere. An unprefixed name collides with somebody else's, and the loser is + // whichever installed second. + var unprefixed = Evaluator + .Catalogue.Skills.Select(skill => skill.FolderName) + .Where(name => !name.StartsWith("purview-containers", StringComparison.Ordinal)) + .ToArray(); + + await Assert + .That(unprefixed) + .IsEmpty() + .Because( + "every skill this repository publishes must carry the product prefix: " + string.Join(", ", unprefixed) + ); + } +} From 2f3c1a2989f8b82f6093502ff4bfabb267d801bf Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Thu, 15 Oct 2026 16:17:46 +0100 Subject: [PATCH 3/8] chore: run CSharpier on save in Visual Studio A tracked solution-level setting, so format-on-save matches the lint check the pipeline runs rather than being set per developer. --- src/settings.VisualStudio.json | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 src/settings.VisualStudio.json diff --git a/src/settings.VisualStudio.json b/src/settings.VisualStudio.json new file mode 100644 index 0000000..194ba35 --- /dev/null +++ b/src/settings.VisualStudio.json @@ -0,0 +1,4 @@ +/* Visual Studio Settings File */ +{ + "csharpier.general.runonsave": true +} From f5e16058ccf1a4153fa5288d762934070f76f097 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Thu, 15 Oct 2026 22:41:34 +0100 Subject: [PATCH 4/8] chore: bumped nuget versions --- .config/dotnet-tools.json | 6 +++--- Directory.Packages.props | 18 +++++++++--------- package.json | 2 +- src/tests/Repository.UnitTests/SkillTests.cs | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 94a32bb..e57a0aa 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -10,18 +10,18 @@ "rollForward": false }, "dotnet-inspect": { - "version": "0.25.0", + "version": "0.26.0", "commands": [ "dotnet-inspect" ], "rollForward": false }, "purview.build": { - "version": "0.5.3", + "version": "0.5.4", "commands": [ "purview-build" ], "rollForward": false } } -} +} \ No newline at end of file diff --git a/Directory.Packages.props b/Directory.Packages.props index 4e534a3..ab32424 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,18 +1,17 @@ true + true - - - - - - + + + + + - @@ -21,9 +20,10 @@ - + - + + diff --git a/package.json b/package.json index ffbacfd..95d5913 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "containers", - "version": "1.0.0-prerelease.6", + "version": "1.0.0-prerelease.7", "description": "Testcontainers-style library for .NET that runs throwaway Linux containers for integration testing, on Microsoft WSL Containers or Docker.", "license": "MIT", "author": { diff --git a/src/tests/Repository.UnitTests/SkillTests.cs b/src/tests/Repository.UnitTests/SkillTests.cs index 8db0473..6efe0a7 100644 --- a/src/tests/Repository.UnitTests/SkillTests.cs +++ b/src/tests/Repository.UnitTests/SkillTests.cs @@ -1,6 +1,6 @@ using Purview.Skills.Testing; -namespace Purview.Containers; +namespace Purview.Containers.Repository; /// /// Runs the Agent Skills conformance rule set against this repository's authored skills in From c7bdc329cda187416a42d1cc9c3d600aa0029a2c Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Fri, 16 Oct 2026 13:05:11 +0100 Subject: [PATCH 5/8] chore: updated packages and added skills --- .../purview-architecture-testing/SKILL.md | 183 +++++++++++++ .agents/skills/purview-architecture/SKILL.md | 146 ++++++++++ .../references/project-context-discovery.md | 115 ++++++++ .../references/project-responsibilities.md | 107 ++++++++ .../references/proportional-architecture.md | 92 +++++++ .../skills/purview-buildsdk-adoption/SKILL.md | 114 ++++++++ .../purview-buildsdk-configuration/SKILL.md | 110 ++++++++ .../references/properties.md | 146 ++++++++++ .../SKILL.md | 134 +++++++++ .../references/detection.md | 79 ++++++ .../references/test-types.md | 35 +++ .../purview-buildsdk-project-layout/SKILL.md | 136 ++++++++++ .../SKILL.md | 111 ++++++++ .../SKILL.md | 166 ++++++++++++ .../references/repository-root.md | 55 ++++ .../purview-buildsdk-skill-tests/SKILL.md | 131 +++++++++ .../references/authoring.md | 79 ++++++ .../skills/purview-buildsdk-testing/SKILL.md | 121 +++++++++ .../references/treenode-filter.md | 107 ++++++++ .../purview-engineering-principles/SKILL.md | 171 ++++++++++++ .agents/skills/purview-persistence/SKILL.md | 106 ++++++++ .../references/guid-generation.md | 93 +++++++ .../references/postgresql.md | 98 +++++++ .../references/sqlserver.md | 101 +++++++ .../skills/purview-telemetry-design/SKILL.md | 96 +++++++ .../references/REFERENCE.md | 101 +++++++ .../purview-telemetry-diagnostics/SKILL.md | 168 ++++++++++++ .../purview-telemetry-migration/SKILL.md | 81 ++++++ .../references/REFERENCE.md | 181 +++++++++++++ .../SKILL.md | 224 +++++++++++++++ .../purview-telemetry-quickstart/SKILL.md | 140 ++++++++++ .agents/skills/purview-telemetry/SKILL.md | 123 +++++++++ .../references/observability-levels.md | 93 +++++++ .../purview-testing-conventions/SKILL.md | 255 ++++++++++++++++++ .config/purview-build.json | 6 +- Directory.Packages.props | 6 +- global.json | 2 +- skills-lock.json | 119 ++++++++ 38 files changed, 4326 insertions(+), 5 deletions(-) create mode 100644 .agents/skills/purview-architecture-testing/SKILL.md create mode 100644 .agents/skills/purview-architecture/SKILL.md create mode 100644 .agents/skills/purview-architecture/references/project-context-discovery.md create mode 100644 .agents/skills/purview-architecture/references/project-responsibilities.md create mode 100644 .agents/skills/purview-architecture/references/proportional-architecture.md create mode 100644 .agents/skills/purview-buildsdk-adoption/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-configuration/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-configuration/references/properties.md create mode 100644 .agents/skills/purview-buildsdk-project-detection/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-project-detection/references/detection.md create mode 100644 .agents/skills/purview-buildsdk-project-detection/references/test-types.md create mode 100644 .agents/skills/purview-buildsdk-project-layout/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-repository-review/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-skill-installation/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-skill-installation/references/repository-root.md create mode 100644 .agents/skills/purview-buildsdk-skill-tests/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-skill-tests/references/authoring.md create mode 100644 .agents/skills/purview-buildsdk-testing/SKILL.md create mode 100644 .agents/skills/purview-buildsdk-testing/references/treenode-filter.md create mode 100644 .agents/skills/purview-engineering-principles/SKILL.md create mode 100644 .agents/skills/purview-persistence/SKILL.md create mode 100644 .agents/skills/purview-persistence/references/guid-generation.md create mode 100644 .agents/skills/purview-persistence/references/postgresql.md create mode 100644 .agents/skills/purview-persistence/references/sqlserver.md create mode 100644 .agents/skills/purview-telemetry-design/SKILL.md create mode 100644 .agents/skills/purview-telemetry-design/references/REFERENCE.md create mode 100644 .agents/skills/purview-telemetry-diagnostics/SKILL.md create mode 100644 .agents/skills/purview-telemetry-migration/SKILL.md create mode 100644 .agents/skills/purview-telemetry-migration/references/REFERENCE.md create mode 100644 .agents/skills/purview-telemetry-multi-targeting/SKILL.md create mode 100644 .agents/skills/purview-telemetry-quickstart/SKILL.md create mode 100644 .agents/skills/purview-telemetry/SKILL.md create mode 100644 .agents/skills/purview-telemetry/references/observability-levels.md create mode 100644 .agents/skills/purview-testing-conventions/SKILL.md create mode 100644 skills-lock.json diff --git a/.agents/skills/purview-architecture-testing/SKILL.md b/.agents/skills/purview-architecture-testing/SKILL.md new file mode 100644 index 0000000..de41062 --- /dev/null +++ b/.agents/skills/purview-architecture-testing/SKILL.md @@ -0,0 +1,183 @@ +--- +name: purview-architecture-testing +description: >- + Use when adding, changing or debugging architecture tests in a .NET solution - dependency-direction + rules, forbidden framework references, project naming checks, cycle detection - or when a boundary + keeps being broken in review and should fail the build instead. Adopts Purview.Architecture.Testing + and a single policy file rather than hand-maintained lists. +license: MIT +metadata: + author: Purview-Dev + version: "2.0" +--- + +# Purview architecture testing + +A boundary that only review enforces is a boundary that erodes. The job of this skill is to turn a +rule a reviewer keeps repeating into a test that fails the build, **without** producing a wall of +brittle assertions that everyone disables six months later. + +Prerequisite: know what the projects are *for*. See `purview-architecture`. + +## 1. Only test a rule that is real + +Before writing anything, answer: **has this rule been broken, or is it certain to be?** + +A test for a dependency nobody would plausibly add is cost with no benefit. Good candidates: + +- A direction that has already been violated once. +- A boundary that is load-bearing - Core staying free of EF Core, Composition being referenced by + nothing. +- A rule a newcomer cannot infer from the code. + +Bad candidates: anything the compiler already enforces, and style preferences dressed up as +architecture. + +## 2. Adopt the package; define the policy once + +The failure mode of architecture tests is a dependency matrix written twice - once in a wiki table and +once in assertions - which then disagree. The prose is the copy that drifts, because nothing fails +when it is wrong. + +So do not write the rule engine. `Purview.Architecture.Testing` ships it, so the matrix, naming, +sibling-independence, composition, forbidden-package, cycle, test-reference and waiver rules are +defined once and shared. Adoption is two files. + +**A policy** at `.config/architecture-policy.json`, pointing at the published schema so an editor +validates it as you type: + +```json +{ + "$schema": "https://raw.githubusercontent.com/purview-dev/build-sdk/main/architecture-policy.schema.json", + "version": 1, + "sourceRoots": ["src"], + "responsibilities": [ + { + "name": "Core", + "aliases": ["Domain"], + "category": "domain", + "purpose": "The domain model, its invariants and its rules.", + "mayDependOn": ["Shared"], + "mustNotDependOn": ["Application", "Persistence", "Composition"], + "forbiddenPackagePrefixes": ["Microsoft.EntityFrameworkCore", "Microsoft.AspNetCore"] + } + ], + "waivers": [] +} +``` + +**One derived test class:** + +```csharp +[InheritsTests] +public sealed class ArchitectureTests : PurviewArchitectureTests; +``` + +That is the whole of the adoption. Each rule is a separate test, so a failure names the rule rather +than collapsing the set into one assertion, and the package validates the policy against its schema +first - a policy that does not match the schema may be silently missing a rule. + +The build-sdk ships a skill for the package itself, `purview-buildsdk-architecture-tests`, which is +authoritative on its API and rule ids. Read it rather than this page when you are extending or +debugging the package. + +Then add a test that the documentation matches the policy, so the table in the wiki is falsifiable +too. Full worked example: +[Project Responsibilities](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Project-Responsibilities.md). + +## 3. Read project files, and assemblies + +Two sources, because they catch different things: + +| Source | Catches | Misses | +| -- | -- | -- | +| `.csproj` `ProjectReference` | The reference a reviewer actually added | A dependency arriving transitively | +| Compiled assembly references | A dependency that arrived indirectly | Intent — it cannot tell direct from transitive | + +The package's rules read the **project files**, because a declared `ProjectReference` is what a +reviewer adds and what the policy governs. Add a small hand-written test that reads the **compiled +assemblies** for the leak a project file cannot show - a namespace from the wrong layer reaching a +public type. + +Classify a project by the **first segment** of its name, so `Persistence.SqlServer` and `Persistence` +are both Persistence - qualification is a naming choice, never a different rule. The package does +this for you; a hand-written assembly test must do it too. + +## 4. The rules worth having + +| Rule | What it catches | +| -- | -- | +| Dependency matrix | Core referencing Persistence; Application referencing a concrete provider | +| Nothing references Composition | A circular implementation choice | +| Sibling independence | `Providers.GitHub` reaching into `Providers.AzureDevOps` | +| Forbidden package prefixes | EF Core in Core; ASP.NET hosting in a domain project | +| Cycle detection | A graph that MSBuild rejects with an unreadable message | +| Catch-all names | `Infrastructure`, `Services`, `Helpers` appearing as project names | +| No production → test reference | A test helper leaking into shipped code | + +The package provides all of these. Do not re-implement one; change the policy instead. + +## 5. Write failures someone can act on + +This is what decides whether the suite survives. A failure must name: + +1. The offending project. +2. The rule violated. +3. The specific reference that broke it. +4. The direction the dependency *should* run. +5. The likely fix. + +```text +'Application' (Application) references 'Persistence.SqlServer' (Persistence), but Application +must not depend on Persistence. Invert it: Persistence may depend on Application, or introduce +an abstraction that Application owns and Persistence implements. +``` + +Not `Expected collection to be empty`. Collect every violation and report them as a numbered list - +one test run should show all of them, not the first. The package already reports this way; keep the +same standard in any rule you add. + +## 6. Waivers, not suppressions + +A repository that predates the policy will violate it. Deleting the test or adding a blanket +`#pragma` makes the policy a lie. Record the departure in the policy instead: + +```json +{ + "project": "Infrastructure", + "mapsTo": "Adapters", + "rules": ["naming"], + "reason": "Holds one adapter, predates the catalogue; renaming churns the wiki and docs for no gain.", + "reviewedOn": "2026-10-15" +} +``` + +The package tests the waivers themselves: each must name a project that still exists, a rule, and a +substantive reason. A waiver for a deleted project is dead weight that hides the next one. + +Critically, a waiver excuses **the narrowest thing possible** - usually the name. `mapsTo` keeps the +project classified so every dependency rule still applies to it. Prefer a waiver to +`SkipRule`, which removes a rule for the whole repository with no record. + +## 7. Conventions + +- TUnit, matching the repository's other tests, adopted through `PurviewArchitectureTests` with + `[InheritsTests]`. +- Arrange/Act/Assert, one behaviour per test. +- Let the package find the repository root - it probes upward for the policy and stops at the `.git` + boundary. Override `RepositoryRoot` only when the policy does not sit above the test binary. +- Keep the policy file free of anything a rule cannot read; a field no rule consults is a rule that + looks enforced and is not. + +## Checks before you answer + +- Is every rule you added one that has been, or will be, broken? +- Is the matrix defined once, in the policy, with the documentation checked against it? +- Does each failure name the fix, not just the fact? +- Did you record a waiver rather than weaken a rule for existing code? + +## See also + +- `purview-architecture` — what the projects are for, and how they are named +- `purview-buildsdk-architecture-tests` — the package's own skill, authoritative on its API +- [Architecture as Tests](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Architecture-As-Tests.md) diff --git a/.agents/skills/purview-architecture/SKILL.md b/.agents/skills/purview-architecture/SKILL.md new file mode 100644 index 0000000..8d4b5a8 --- /dev/null +++ b/.agents/skills/purview-architecture/SKILL.md @@ -0,0 +1,146 @@ +--- +name: purview-architecture +description: >- + Use when deciding the shape of a .NET solution that uses Purview libraries — how many projects, + whether to add a repository or CQRS layer, where validation lives — or before generating substantial + implementation code. Establishes project context first, then applies DRY, KISS and architecture + proportional to the actual requirements rather than to an enterprise reference diagram. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview architecture + +The job of this skill is to stop two failures that cost far more than they look: + +- **Architecture bought in advance.** Four projects, a repository interface and a mapping layer for an + application that has one table and one caller. +- **A recommendation made without context.** Advice about database-side GUID generation given to a + stateless CLI, or a Clean Architecture split proposed for a 300-line utility. + +So: establish context, then choose proportionally. Both steps are mandatory, and the first one is the +one that gets skipped. + +## 1. Establish context before deciding + +Before recommending a structure, adding a project, or generating substantial code, you must know four +things. **Read them out of the repository first** — asking a user a question the code already answers +is the most common way this skill is applied badly. + +| Dimension | Read it from | Ask only if | +| -- | -- | -- | +| Complexity (small / medium / large) | Number of functional areas, integrations and external consumers — not file count | The requirements are genuinely not written down anywhere | +| Project type | SDK attribute, `OutputType`, `Microsoft.NET.Sdk.Web`, an AppHost project, a `Main` | The repository is empty | +| Persistence | `PackageReference` to an EF Core provider, `DbContext`, a connection string, a `Purview.Containers` resource | Persistence is being introduced now | +| Telemetry | `Purview.Telemetry.SourceGenerator`, `ILogger` injection, `ActivitySource`, an OTLP exporter | Nothing is configured and you are about to add it | + +Identifier generation is a **conditional** dimension: resolve it only when persistence exists *and* +GUID keys are in play. See `purview-persistence`. + +Judge complexity on requirements, not size. A 2,000-line CLI with one output format is small. A +900-line service that reconciles three upstream systems and owns money movement is not. + +**Do not re-ask.** A dimension answered by repository configuration, existing source, a project file, +an earlier turn in the conversation, or a recorded architectural decision is answered. Ask concise, +targeted questions only for the unknowns that change your recommendation — and if nothing changes, +ask nothing. + +Full decision procedure: `references/project-context-discovery.md`. + +## 2. Choose proportionally + +| Scale | Shape | What you still do not add | +| -- | -- | -- | +| Small | One project, or two when a test project needs a non-trivial subject. Direct implementations. | Repository over `DbContext`, CQRS, a mapping layer, speculative interfaces | +| Medium | Explicit boundaries where complexity already hurts — usually a domain project and a host | Event sourcing, a generic infrastructure framework, a second infrastructure implementation nobody asked for | +| Large | Modularity, independent infrastructure implementations, enforceable boundaries (see the playbook's Architecture-As-Tests page) | Patterns adopted for symmetry rather than for a named problem | + +Architecture is allowed to arrive late. A boundary added when the second caller appears is cheap; a +boundary added in anticipation of a second caller that never appears is permanent. + +**Never add a project for symmetry.** "Domain, Application, Infrastructure, API" is a response to a +problem, not a starting position. If you cannot name the failure a layer prevents in this codebase, +do not add the layer. + +Full guidance and the per-pattern justification tests: `references/proportional-architecture.md`. + +## 3. Name what you do create + +A project name states a **responsibility**, optionally qualified by an implementation: + +```text +[.] +``` + +The responsibility decides the rules; the qualifier never changes them. Three rules people usually +get wrong: + +- **Do not qualify a single implementation.** `Persistence` is correct even when it uses SQL Server. + Rename when a second provider arrives, not in anticipation. +- **Do not create an empty parent.** `Persistence.SqlServer` + `Persistence.PostgreSql` with nothing + shared need no `Persistence` project. +- **Multiple implementations do not require multiple assemblies.** Two small adapters with the same + dependencies belong in one project. + +Never use a *category* as a project name — `Infrastructure`, `Services`, `Managers`, `Models`, +`Helpers`, `Common`. Nothing can be excluded from a category, so it grows without limit. And never +rename an established project just to match canonical terminology; `Domain`, `UseCases` and +`Integrations` are recognised aliases. + +Responsibility catalogue, dependency matrix and the naming decision tree: +`references/project-responsibilities.md`. + +## 4. Check the direction + +`A → B` means **A depends on B at compile time** — not a runtime call, a DI registration, an HTTP +request or an Aspire orchestration edge. Confusing those is the most common source of a wrong answer +about whether a dependency is allowed. + +The rules that matter most: + +- Core depends on nothing but `Shared`. +- Application never references a concrete `Persistence`, `Providers` or `Adapters` project. +- Composition references implementations; **nothing references Composition** except a host. +- Sibling implementations (`Providers.GitHub`, `Providers.AzureDevOps`) stay independent. +- Persistence owns migration *definitions*; `Migrator` owns their *execution*. Production API startup + does not apply schema migrations. + +When a boundary is worth having, make it fail the build rather than the review — see +`purview-architecture-testing`. + +## 5. DRY, applied where it pays + +Reuse the Purview libraries when they remove a real source of truth, not for coverage: + +| Concern | Library | Use it when | +| -- | -- | -- | +| Operation outcomes without exceptions for control flow | `Purview.Results` | Failures are expected and the caller must handle them | +| Primitive obsession, validated identity types | `Purview.ValueObjects` | A value has rules that would otherwise be re-checked at each boundary | +| Schema shared with a TypeScript client | `Purview.ZodSharp` | A contract is duplicated between C# and TypeScript today | +| Local dependencies for dev and integration tests | `Purview.Containers` | Tests need a real database, queue or emulator | +| Logging, tracing and metrics from one declaration | `Purview.Telemetry.SourceGenerator` | Always — see `purview-telemetry` | + +Duplication that matters: validation repeated per entry point, a domain rule restated in the UI, +configuration bound in two places, two types that both claim to be the truth. Duplication that does +not: two similar-looking methods that answer to different owners. + +## 6. Telemetry is not an advanced topic + +Recommend instrumentation from the first commit, at every project size. A small application gets +small telemetry — it does not get none. See `purview-telemetry`. + +## Checks before you answer + +- Did you read the four context dimensions rather than ask for them? +- Can you name, for each project and each abstraction you are proposing, the failure it prevents here? +- Is anything in your proposal there only because a reference architecture has it? +- Did you include telemetry? + +## See also + +- `purview-telemetry` — instrumentation at every scale +- `purview-persistence` — storage and GUID-key guidance, only when persistence is real +- [Engineering Principles](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Engineering-Principles.md) +- [Adopting This At Your Scale](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Adopting-This-At-Your-Scale.md) diff --git a/.agents/skills/purview-architecture/references/project-context-discovery.md b/.agents/skills/purview-architecture/references/project-context-discovery.md new file mode 100644 index 0000000..d40569a --- /dev/null +++ b/.agents/skills/purview-architecture/references/project-context-discovery.md @@ -0,0 +1,115 @@ +# Project context discovery + +The procedure for establishing enough context to make an architectural recommendation, and — just as +importantly — for knowing when to stop asking. + +## The principle + +Every question you ask a user is a question they have usually already answered in the repository. +Reading is free and silent; asking is slow and, when the answer was already in a project file, it +reads as not having looked. So the order is fixed: + +1. Read the repository. +2. Read the conversation so far. +3. Ask only about what remains *and* changes your answer. + +A dimension whose answer would not change your recommendation is not worth a question. If a project +is a stateless CLI, the database GUID strategy is not an unknown — it is irrelevant, and raising it +is noise. + +## Dimension 1 — complexity + +Assess from requirements, never from line or project count. + +| Signal | Suggests | +| -- | -- | +| One functional area, one consumer, no integrations | Small | +| A handful of features, one or two integrations, a single team | Medium | +| Several bounded areas, multiple services or teams, operational obligations (SLOs, auditability, money) | Large | + +Where to look: `README.md`, the wiki, issue or ticket references, the solution file's project list, +`PackageReference` entries naming external systems, the number of distinct hosts. + +Traps: + +- **A big repository is not a complex one.** A monorepo of ten tiny tools is ten small projects. +- **A small repository can be complex.** Few files, hard invariants. +- **Generated code inflates everything.** Discount it. + +## Dimension 2 — project type + +Read it; do not ask it. + +| Evidence | Type | +| -- | -- | +| `Microsoft.NET.Sdk.Web` | Web application or HTTP API — distinguish by whether views/pages or endpoints dominate | +| `OutputType` of `Exe` plus an argument parser | CLI | +| `IsPackable`, no entry point | Reusable library | +| `IHostedService` / `BackgroundService`, `Worker` SDK | Background worker | +| An AppHost project, `Aspire.Hosting` | Distributed application | +| WPF/WinForms/Avalonia/MAUI SDK or `UseWPF` | Desktop application | + +A reusable library carries the strongest constraint on the rest of your advice: it must not configure +global telemetry providers, must not read configuration from the ambient environment, and must not +decide a persistence technology for its consumer. + +## Dimension 3 — persistence + +| Question | Read it from | +| -- | -- | +| Is persistence required at all? | Any data-access `PackageReference`, a `DbContext`, a connection string, a repository type, an Aspire resource | +| Which technology? | The EF Core provider package (`Microsoft.EntityFrameworkCore.SqlServer`, `Npgsql.EntityFrameworkCore.PostgreSQL`), the client package, or the `Purview.Containers` resource in the AppHost | +| Is more than one implementation required? | Two providers referenced, a provider abstraction already present, or an explicitly stated requirement | +| EF Core or something else? | Presence of `Microsoft.EntityFrameworkCore` versus Dapper, a raw client, or a document store SDK | + +"No persistence" is a complete and common answer. When it is the answer, every storage-shaped +recommendation — repositories, migrations, GUID strategies, connection resilience — is out of scope. + +## Dimension 4 — telemetry + +| Question | Read it from | +| -- | -- | +| Is telemetry configured? | `ILogger` injection, `ActivitySource`/`Meter` construction, `AddOpenTelemetry`, a `ServiceDefaults` project | +| Is Purview Telemetry available? | A `Purview.Telemetry.SourceGenerator` `PackageReference`, or interfaces attributed `[Logger]`, `[ActivitySource]`, `[Meter]` | +| Which signals are relevant? | Logs always; traces once a call crosses a process or a dependency; metrics once someone would alert on a rate or a duration | +| Is there an exporter? | An OTLP endpoint in configuration, a console exporter, an Aspire dashboard, an APM package | + +Existing instrumentation is a constraint, not a starting point to replace. Preserve it, extend it, +and never register the same source twice. + +## Conditional dimension — identifier generation + +Resolve **only** when persistence exists and GUIDs are, or are about to be, primary keys. + +- Are GUIDs primary keys, or just surface identifiers? +- Is the value generated in the application or by the database? +- Which provider and version? (This changes the answer — see `purview-persistence`.) +- Does index locality matter at the expected write volume? +- Must the identifier exist before the entity is persisted (outbox, correlation, client-side graph)? + +The last question usually decides it: if the application needs the value before `SaveChanges`, the +database cannot be the generator. + +## Asking well + +When you must ask, ask few and ask concretely. Batch the unknowns into one short set, offer the +likely answers, and state what you will do if the user does not care. + +Good: + +> Two things I can't tell from the repository: is this service expected to own its own database, or +> read from the existing one? And should failures be returned to the caller or thrown? I'll assume +> its own database and `Purview.Results` for expected failures unless you say otherwise. + +Bad: + +> What is the project complexity? What is the project type? Is persistence required? Which database? +> Is telemetry configured? Do you use GUID primary keys? + +The second asks six questions, five of which the repository answers, and one of which is irrelevant. + +## Recording the answer + +When a dimension is resolved by conversation rather than by the repository, that is a decision the +next agent will not be able to read. Write it down — in `AGENTS.md`, a wiki page, or an ADR — so the +question is asked once and never again. diff --git a/.agents/skills/purview-architecture/references/project-responsibilities.md b/.agents/skills/purview-architecture/references/project-responsibilities.md new file mode 100644 index 0000000..1bf0a1a --- /dev/null +++ b/.agents/skills/purview-architecture/references/project-responsibilities.md @@ -0,0 +1,107 @@ +# Project responsibilities, naming and dependency direction + +The catalogue in brief. The authoritative definition is +[`.config/architecture-policy.json`](https://github.com/purview-dev/engineering-playbook/blob/main/.config/architecture-policy.json), +and the full page — with diagrams — is +[Project Responsibilities](https://github.com/purview-dev/engineering-playbook/blob/main/docs/wiki/Project-Responsibilities.md). +Do not restate either here or in another skill; link to them. + +## The catalogue + +`A → B` means A depends on B **at compile time**. + +| Responsibility | Also known as | Owns | May depend on | +| -- | -- | -- | -- | +| `Core` | `Domain` | Entities, value objects, invariants, domain events and errors | Shared | +| `Abstractions` | — | Cross-boundary capability contracts | Core, Shared | +| `Application` | `UseCases` | Use cases, workflows, orchestration | Core, Abstractions, Contracts, Shared | +| `Contracts` | — | Externally exposed request/response types | Core, Shared | +| `Persistence` | — | DbContext, mappings, migration *definitions* | Core, Abstractions, Application, Shared | +| `Providers` | `Integrations` | Integration with a *named external platform* | Core, Abstractions, Application, Shared | +| `Adapters` | — | An *application-defined capability* via one technology | Core, Abstractions, Application, Shared | +| `Composition` | — | DI registration and implementation selection | Everything above | +| `API` | — | HTTP host | Composition, Contracts, ServiceDefaults, … | +| `Worker` | — | Background-processing host | Composition, ServiceDefaults, … | +| `Migrator` | — | *Executes* migrations and controlled seeding | Persistence, Composition, ServiceDefaults | +| `AppHost` | — | Aspire orchestration | The hosts it orchestrates | +| `ServiceDefaults` | — | Telemetry, health, resilience, discovery defaults | Shared | +| `WebApp` | — | Frontend | Contracts, ServiceDefaults, Shared | +| `Shared` | — | Small domain-independent primitives | *nothing* | + +**None of these is mandatory.** A small application with `Core` and one host has a complete and +correct architecture. The catalogue names responsibilities so you can tell when splitting one out +buys something — it is not a list of projects to create. + +## When to split + +| Responsibility | Split it out when | Leave it combined when | +| -- | -- | -- | +| `Core` | A domain model exists | The app is a thin utility | +| `Abstractions` | Two implementations, or the consumer must not see the implementation | One implementation — keep the interface beside it | +| `Application` | Workflows are reused by more than one host | It would only forward CRUD | +| `Contracts` | A client compiles against it without taking the server | The API host is the only consumer | +| `Persistence` | Persistence exists at all | A single-project utility writing a file | +| `Providers` | Two platforms with different SDKs or lifecycles | One platform | +| `Adapters` | One capability, several technologies | One capability, one technology | +| `Composition` | Two hosts compose the same implementations | One host — compose in the host | +| `Migrator` | Schema changes are a deliberate deployment step | Development-only startup migration | +| `Shared` | A primitive is used by two or more responsibilities | Almost always | + +## Naming + +```text +[.] +``` + +| Situation | Name | +| -- | -- | +| One implementation | `Persistence` — do **not** qualify it just because it is SQL Server | +| Several, same dependencies and lifecycle | `Persistence` — one project, several classes | +| Several, genuinely different | `Persistence.SqlServer`, `Persistence.PostgreSql` | +| Several, with shared code | Add `Persistence` alongside the qualified projects | +| Several, nothing shared | Qualified projects only — no empty parent | + +Discouraged as project names, because each names a category rather than a responsibility: +`Infrastructure`, `Services`, `Managers`, `Models`, `Helpers`, `Common`, `Utils`, `Utilities`. + +`Infrastructure` is the common case. It is a useful *category* — covering Persistence, Providers and +Adapters — and a poor *project*, because every technical class qualifies for it. + +**Do not rename an established project** merely to match canonical terminology. A rename costs more +than the consistency is worth, and `Domain`/`UseCases`/`Integrations` are recognised aliases. + +## Providers versus Adapters + +Not a vendor distinction: + +- **Provider** — the requirement names the platform: "sync issues from GitHub". Swapping it changes + the product. +- **Adapter** — the requirement names the capability: "send an email". Swapping the technology + changes nothing the user sees. + +Azure Blob behind `IFileStore` is an Adapter. The Azure DevOps work-item API in a product that +integrates Azure DevOps is a Provider. + +## Dependency rules that catch people out + +1. **Composition is referenced by nothing** except a host. It selects implementations, so it depends + on them; the reverse would make the choice circular. +2. **Application never references a concrete infrastructure project.** It depends on an abstraction + and receives an implementation it cannot name. +3. **Sibling implementations stay independent.** If `Providers.GitHub` needs something from + `Providers.AzureDevOps`, that shared thing belongs in the unqualified project or an abstraction. +4. **Persistence owns migration definitions; Migrator executes them.** Production API startup does + not mutate a schema. +5. **Runtime is not compile time.** DI registration, an HTTP call and Aspire orchestration are not + assembly references. A Worker and an API can share Application without referencing each other. + +## Recommending this to a repository that predates it + +Do not propose a mass rename. Suggest, in order: + +1. Fix dependency *directions* that are actually wrong — those are bugs. +2. Split a project only where a named problem justifies it. +3. Record a waiver for an established name rather than renaming it, so the rule stays enforceable + without churn. + +A waiver excuses the name only; the project still classifies, so every dependency rule still applies. diff --git a/.agents/skills/purview-architecture/references/proportional-architecture.md b/.agents/skills/purview-architecture/references/proportional-architecture.md new file mode 100644 index 0000000..3ed7f55 --- /dev/null +++ b/.agents/skills/purview-architecture/references/proportional-architecture.md @@ -0,0 +1,92 @@ +# Proportional architecture + +Structure is a cost paid now against a failure expected later. Proportional architecture is the +practice of only paying when the failure is named, present, and more expensive than the structure. + +## The justification test + +Before adding a project, an interface, a layer or a pattern, answer in one sentence: + +> **What failure does this prevent in *this* codebase, and has it happened or is it certain to?** + +If the sentence needs the words "in case", "later", "when we scale", or "it's good practice", the +structure is not yet justified. Write the simpler thing. The simpler thing is also the thing that is +cheapest to change when the failure finally does arrive. + +This is not an argument against structure. It is an argument against *unfalsifiable* structure. + +## By scale + +### Small — a utility, CLI, prototype, simple API, limited-scope application + +- One project. A second only when there is a test project, or when a genuinely reusable library + falls out. +- Call the database, the API or the file system directly from the code that needs it. +- Validate at the edge, once. +- Logging from the first commit; traces and metrics when there is something to correlate. + +Do not add: a repository interface over `DbContext`, a service layer that forwards, a mapping layer +between two near-identical shapes, an interface with one implementation and no test double need +(`Purview.Containers` and TUnit.Mocks remove most of that need), a `Domain`/`Application`/ +`Infrastructure` split. + +### Medium — several functional areas, some integrations, moderate domain complexity + +- Introduce a boundary where you can name what it separates. Usually that is domain rules from the + host, so a domain project and an application host. +- An abstraction over an external dependency earns its place when the dependency is genuinely + swappable, or when testing against it is otherwise painful. "Swappable in principle" is not + swappable. +- Dependency instrumentation, trace propagation across the integrations, metrics on the operations + someone would be paged about. + +Do not add: CQRS unless reads and writes have actually diverged in shape, lifetime or scaling; +event sourcing unless the history *is* the product; a second infrastructure implementation that has +no second consumer. + +### Large — complex domain, multiple services or teams, substantial operational requirements + +- Modularity along ownership lines, because the real constraint is people, not code. +- Independent infrastructure implementations where they are genuinely independent. +- Boundaries made enforceable rather than documented — a dependency-direction rule that a wrong + `ProjectReference` fails the build. See the playbook's Architecture-As-Tests page. +- End-to-end tracing, standardised resource attributes, SLIs, retention and sampling policy. + +Even here, the justification test holds per decision. "Large" licenses the *option* to add +structure, not the obligation to add all of it on day one. + +## Patterns and the conditions that earn them + +| Pattern | Earned when | Not earned by | +| -- | -- | -- | +| Repository over EF Core | You must swap the store, or the `DbContext` API genuinely leaks into domain decisions | Testability — EF Core's in-memory and container-backed testing already covers it | +| CQRS | Read and write models have diverged in shape or scaling needs | A desire for symmetry with a reference architecture | +| Event sourcing | The sequence of changes is the product, or audit is a hard requirement | "We might want history" | +| Mediator / in-process bus | Many handlers, genuinely decoupled registration | Three call sites that could be method calls | +| Mapping layer | Two shapes that must diverge and are owned separately | Two shapes that are identical today | +| Extra DI indirection | A real second implementation, or a composition root that must vary | Inversion for its own sake | +| Generic infrastructure framework | Three or more concrete uses exist now | One use and an expectation of more | + +## Evolving incrementally + +The honest sequence is: write the direct implementation; let the second caller arrive; extract the +boundary the second caller revealed. Each step is small, reversible and informed. + +The dishonest sequence is: predict the second caller; build the boundary; never get the second +caller; maintain the boundary forever. The cost is invisible because it is spread across every +future change. + +When you do extract, extract along the seam the code showed you — not the seam the diagram +predicted. + +## What this does not license + +Proportional is not an excuse for: + +- Skipping tests, or skipping telemetry. +- Leaving validation duplicated across entry points. +- Letting domain rules live in the UI or in the database. +- Two sources of truth for the same fact. + +Those are not structure bought in advance; they are defects. Simplicity means fewer moving parts, +not fewer guarantees. diff --git a/.agents/skills/purview-buildsdk-adoption/SKILL.md b/.agents/skills/purview-buildsdk-adoption/SKILL.md new file mode 100644 index 0000000..b1f222c --- /dev/null +++ b/.agents/skills/purview-buildsdk-adoption/SKILL.md @@ -0,0 +1,114 @@ +--- +name: purview-buildsdk-adoption +description: Adopt Purview.BuildSdk in a repository, or troubleshoot an adoption that is not behaving. Use when wiring the SDK into Directory.Build.props for the first time, when a property seems to have no effect, when the resolved version or namespace is wrong, or when a consuming repository builds but the SDK's automatic behaviour is not appearing. Covers the import chain, pre-import property ordering, and version detection from package.json. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Adopting Purview.BuildSdk + +The goal is a working adoption that does not break existing build behaviour. Prefer minimal, +targeted property changes over rewriting a repository's `Directory.Build.props`. + +For the full property list use `purview-buildsdk-configuration`; for auditing an existing +repository's shape, `purview-buildsdk-repository-review`. + +## 1. The import chain + +```xml + + + + + Acme.Sales + + + + +``` + +```xml + + + + +``` + +Both halves are required. A repository with only `Sdk.props` imported gets the evaluation-time +defaults — naming, identity, framework selection — but none of the targets, so validation, +packaging behaviour and the generated files never run. That combination usually presents as "the +properties look right but nothing happens". + +The SDK version comes from `global.json`, which the SDK itself bootstraps. + +## 2. Pre-import ordering is the first thing to check + +**A property that must affect `Sdk.props` evaluation has to be set before the import.** Setting it +in a `.csproj`, or below the import, is too late and silently does nothing — no error, no warning. +This is the single most common adoption failure. + +Set before the import: + +- `NamespacePrefix` +- `UsePackageJsonVersion`, `RootPackageJson` +- `EnableAssemblyNameGeneration` +- `AutoIncludeUsings` +- `PurviewSharedTestingOutputType`, `PurviewTestContextNoWarn` +- any repo-wide testing-framework selection you want every project to inherit + +Everything else is read in `Sdk.targets` and can be set in a project file. + +When a property appears to have no effect, confirm what MSBuild actually resolved before changing +anything else: + +```bash +dotnet msbuild -getProperty:NamespacePrefix -getProperty:RootNamespace +``` + +## 3. Version detection + +Versions come from `package.json`. Resolution order: + +1. An explicit `RootPackageJson`. +2. CI workspace variables. +3. The `.git` root. +4. A nearby `package.json`. + +On success both `Version` and `PackageVersion` are set from the `version` field. + +If the version looks wrong, do not guess at the discovery — make it fail loudly instead: + +```xml +Strict +``` + +`Strict` fails the build when discovery or resolution cannot succeed, rather than silently skipping +it and leaving the default `1.0.0`. Set `VersionDetectionLogEnabled=true` to see what it found. + +## 4. Naming and placement + +Most of the SDK's value is inferred from project names and locations, so an adoption that keeps a +bespoke structure gets correspondingly little. Confirm the repository follows the conventions rather +than introducing new structure to work around them — see `purview-buildsdk-project-layout`, and +`purview-buildsdk-repository-review` for auditing an existing layout. + +The highest-value single check: every test project is named `.Tests`. **The dot is +required.** A project named plainly `UnitTests` is not detected as a test project at all, and +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`. + +## Constraints + +- Prefer minimal, targeted property changes over broad `Directory.Build.props` rewrites. +- Do not disable `PurviewAutoSdkPack` unless the consumer explicitly asks to opt out. +- Do not restate an SDK default in a project file. Hand-configuration that merely repeats a default + is pure maintenance cost, and it hides the fact that the default later changed. +- Re-run the build — or the repository's canonical build command — after each change, and confirm + the fix before moving to the next one. diff --git a/.agents/skills/purview-buildsdk-configuration/SKILL.md b/.agents/skills/purview-buildsdk-configuration/SKILL.md new file mode 100644 index 0000000..f0ee715 --- /dev/null +++ b/.agents/skills/purview-buildsdk-configuration/SKILL.md @@ -0,0 +1,110 @@ +--- +name: purview-buildsdk-configuration +description: Configure Purview.BuildSdk from Directory.Build.props or a .csproj. Use when setting NamespacePrefix, choosing a testing or mocking framework, turning telemetry or SourceLink off, controlling version detection from package.json, tuning the repository bootstrap, or working out whether a property must be set before the SDK import. Also use when the SDK "did something automatically" and you need the property that controls it. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Purview.BuildSdk configuration + +Use this skill when the question is "what property controls this, and where do I set it?" + +For *why* the SDK classified a project a particular way, use `purview-buildsdk-project-detection` +instead. For where projects and tests should live, use `purview-buildsdk-project-layout`. + +## First rule: know where a property must be set + +A property that changes `Sdk.props` evaluation must be set **before** the SDK import — in +`Directory.Build.props`, above the import. Setting it in a `.csproj` is too late and silently has no +effect. + +Pre-import properties include: + +- `NamespacePrefix` +- `UsePackageJsonVersion`, `RootPackageJson` +- `EnableAssemblyNameGeneration` +- `PurviewSharedTestingOutputType`, `PurviewTestContextNoWarn` +- `AutoIncludeUsings` +- repo-wide testing-framework selection you want every project to inherit + +Everything else is read in `Sdk.targets` and can be set in a project file. Prefer repo-wide defaults +in `Directory.Build.props` unless the setting is deliberately project-specific. + +## The properties you will reach for most + +| Property | Default | What it does | +| -- | -- | -- | +| `NamespacePrefix` | *(required)* | The root identity. Required unless `DisableNamespacePrefixCheck=true` | +| `TestingFramework` | `TUnit` | `TUnit`, `Xunit` or `None` | +| `SubstituteFramework` | `TUnitMocks` | `TUnitMocks`, `NSubstitute` or `None` | +| `TestDataFramework` | `Bogus` | `Bogus` or `None` | +| `TargetFramework` | `net10.0` | `netstandard2.0` when `IsRoslynComponent=true` | +| `IsPackable` | `false` | Opts a project into the packable defaults | +| `UsePackageJsonVersion` | `true` | `true`, `false` or `Strict` — version from `package.json` | +| `ExcludePurviewTelemetry` | `false` | Drops the telemetry source generator | +| `DisableSourceLink` | `false` | Drops SourceLink from packable projects | +| `EnableAssemblyNameGeneration` | `true` | `AssemblyName`/`PackageId` follow the resolved `RootNamespace` | + +The full set, grouped by area and with the behaviour each one changes, is in +[references/properties.md](references/properties.md). Read it rather than guessing a property name — +several of these have near-miss names that do nothing. + +## Derived properties to inspect, not re-derive + +When explaining what the SDK did, read the value it computed rather than reasoning from the folder +name: + +`PurviewLogicalProjectName`, `PurviewNamespacePrefix`, `PurviewProjectShortName`, `PurviewTestType`, +`RootNamespace`, `AssemblyName`, `PackageVersion`, `TestingType`, `TargetProjectName`, `RepoRoot`, +`RootPackageJson`. + +Ask MSBuild for one directly: + +```bash +dotnet msbuild -getProperty:RootNamespace +dotnet msbuild -getProperty:TestingType -getProperty:TargetProjectName +``` + +## Compiler-visible properties + +The SDK exports its properties to analyzers and generators as `build_property.`. When +authoring an analyzer, read the exported property instead of re-deriving SDK behaviour — the export +is the SDK's answer, and it stays correct when the rules change. The exported set is listed in +[references/properties.md](references/properties.md). + +## Version detection + +1. If `RootPackageJson` is set, that file is used. +2. Otherwise the repo root is discovered from CI variables, `.git`, or a nearby `package.json`. +3. On success, both `Version` and `PackageVersion` come from the `version` field. +4. `UsePackageJsonVersion=Strict` turns a failed discovery into a build failure instead of a silent + fallback. + +## Repository bootstrap + +The SDK bootstraps two repository-wide files from the package: `.editorconfig` and `global.json`. + +- `PurviewRepoBootstrapMode` — `IfMissing` (default), `Always`, `WarnOnDrift`, `Never` +- `DisableAutoCopySdkFiles` — the master off switch +- `PurviewRepoBootstrapCopyRetries` / `...RetryDelayMilliseconds` / `...FailureAsError` + +Every project in a solution writes those same destinations, so the write is race-tolerant by +design: an existing file is never overwritten unless asked, each write is staged into a temporary +file and renamed into place, retries are quiet, and only a failure that survives every retry is +reported. + +**The SDK writes nothing into an agent directory.** It used to mirror a package's `.agents/**` into +the repository; that is gone, along with `EnableAgentFolderInPackage`, `AgentPackDestinationFolder` +and the `PurviewAgentFolder*` properties. Skills have exactly one installer, the skills CLI — see +`purview-buildsdk-skill-installation`. + +## Guidance when changing SDK configuration + +1. Preserve existing defaults unless the task explicitly changes product behaviour. +2. Keep `README.md`, the property declaration, its validation and the shipped skills aligned — a + property documented in one place and absent from another is the failure mode this repository + keeps hitting. +3. A new property that affects import-time behaviour must say so in its documentation. +4. Add the property to the `CompilerVisibleProperty` list if an analyzer would benefit from it. diff --git a/.agents/skills/purview-buildsdk-configuration/references/properties.md b/.agents/skills/purview-buildsdk-configuration/references/properties.md new file mode 100644 index 0000000..93a0d9b --- /dev/null +++ b/.agents/skills/purview-buildsdk-configuration/references/properties.md @@ -0,0 +1,146 @@ +# Purview.BuildSdk property reference + +Every user-facing property, grouped by what it changes. Defaults are what the SDK applies when the +consuming project supplies nothing. + +## Identity and naming + +| Property | Default | Behaviour | +| -- | -- | -- | +| `NamespacePrefix` | *(required)* | The root identity source. Required unless the check is disabled. Set before the SDK import | +| `DisableNamespacePrefixCheck` | `false` | Stops the build failing when `NamespacePrefix` is unset | +| `EnableAssemblyNameGeneration` | `true` | `AssemblyName` and the default `PackageId` follow the fully evaluated `RootNamespace`, or the full logical project name when suffix-stripping removed a segment (`Shared`, `ServiceDefaults`). Set `false` before the SDK import for standard project-name behaviour | +| `AssemblyIdentifierClassName` | *(derived)* | Renames the generated marker class | +| `DisableGenerateAssemblyIdentifierClass` | `false` (forced `true` for test and shared-testing projects) | Drops the empty marker class (`Assembly`, global namespace) that architecture tests use to pin an assembly by `typeof` rather than by name | +| `DisableGenerateAssemblyInfoClass` | `false` | Drops the generated `AssemblyInfo` | +| `DisableAutoInternalsVisibleTo` | `false` | Drops automatic friend-assembly generation | +| `DisableProjectFileNamingConventionCheck` | `false` | Stops validating that the `.csproj` filename matches its directory | +| `AutoIncludeUsings` | `true` | Controls the SDK-added global usings. Set before the SDK import | + +## Target framework and language + +| Property | Default | Behaviour | +| -- | -- | -- | +| `TargetFramework` | `net10.0` | Applied when neither `TargetFramework` nor `TargetFrameworks` is set. A project that explicitly declares `IsRoslynComponent=true` defaults to `netstandard2.0` | +| `IsRoslynComponent` | *(unset)* | When explicitly `true`: a single `netstandard2.0` target, `LangVersion=latest`, `Nullable=enable`, `TreatWarningsAsErrors=true`, `Deterministic=true`, extended analyzer rules, SourceLink with `EmbedUntrackedSources=true`, no dependency file, telemetry exclusion, and `PrivateAssets=all` on `Microsoft.CodeAnalysis.*` references | +| `IsRoslynComponentOnly` | `true` for Roslyn components | Analyzer-only package: `IncludeBuildOutput=false`, `IncludeSymbols=false`, portable PDB packed beside the analyzer under `analyzers/dotnet/cs/`. Set `false` for a dual-role component using normal library symbol packaging | +| `DisableRoslynCompilerDefaultsValidation` | `false` | Opts out of `ValidateRoslynComponentCompilerSettings`, which fails the pack of a packable Roslyn component missing `LangVersion`, `Nullable`, `TreatWarningsAsErrors` or `EnforceExtendedAnalyzerRules` | +| `PackProjectReferencedSourceGenerators` | `true` | Packable projects include analyzer `ProjectReference` outputs under `analyzers/dotnet/cs/`. Use `Pack="false"` on one reference to opt that one out | + +## Style policy + +| Property | Default | Behaviour | +| -- | -- | -- | +| `DisablePurviewStylePolicyValidation` | `false` | Stops the build failing (`PRSGD0006`–`PRSGD0009`) when the repository `.editorconfig` overrides the modifier policy — `dotnet_style_require_accessibility_modifiers` other than `omit_if_default`, hiding `IDE0040`/`IDE1006`, disabling the Style category in bulk, weakening the `_camelCase` private-instance-field rule, or adding the accessibility rules to `NoWarn`. Entries the SDK injects itself are ignored | +| `PurviewTestContextNoWarn` | `CA1002;CA1012;CA1034;CA1047;CA1050;CA1051;CA1062;CA1064;CA1515;CA1707` | The production API-surface rules exempted in test and shared-testing projects. Test projects keep the strict style contract (`IDE0040`, field naming, formatting, `IDE1006`) but are context aware: public test classes, `Method_Scenario_Expectation` names, exposed fields and unvalidated helper parameters are allowed. Set before the SDK import | +| `DisablePurviewTestContextRuleSet` | `false` | Makes test and shared-testing projects enforce the production API-surface rules too | +| `PurviewPolicyExemptNoWarn` | *(derived)* | The SDK-injected `NoWarn` entries `ValidatePurviewStylePolicy` accepts | + +## Testing + +| Property | Default | Behaviour | +| -- | -- | -- | +| `TestingFramework` | `TUnit` | `TUnit`, `Xunit`, `None` | +| `SubstituteFramework` | `TUnitMocks` | `TUnitMocks`, `NSubstitute`, `None` | +| `TestDataFramework` | `Bogus` | `Bogus`, `None` | +| `CollectCoverage` | `true` for detected test projects | | +| `EnableStaticNativeInstrumentation` | `false` for test projects | | +| `EnableDynamicNativeInstrumentation` | `false` for test projects | | +| `TestingPlatformDotnetTestSupport`, `UseMicrosoftTestingPlatformRunner`, `EnableMicrosoftTestingPlatform` | enabled for TUnit test projects | Microsoft.Testing.Platform wiring | +| `PurviewSharedTestingOutputType` | `Library` | Forced onto `IsSharedTestingProject` projects, with `IsTestProject`/`IsTestingPlatformApplication` cleared, because the test packages otherwise flip them into an executable test host. Set `Exe` before the SDK import to keep the package-driven shape | + +## Packaging + +| Property | Default | Behaviour | +| -- | -- | -- | +| `IsPackable` | `false` | | +| `PurviewAutoSdkPack` | `true` | Packs `Sdk/**/*` into the package at the right root-level paths | +| `PurviewSkillsSourceFolder` | `/skills` | Where authored skills live | + +Packable-project defaults, applied only when the project supplied nothing: +`GenerateDocumentationFile=true`, `IncludeSymbols=true`, `SymbolPackageFormat=snupkg`, +`PublishRepositoryUrl=true`, `EmbedUntrackedSources=true`, `DebugType=portable`. Portable PDBs ship +in the `.snupkg`; the `.nupkg` gets none unless the project opts in. Roslyn-component-only packages +instead default `IncludeSymbols=false` and ship their PDB inside `analyzers/dotnet/cs/`. + +When the repository root is discoverable, the root `README.md` is packed automatically and +registered as `PackageReadmeFile`, unless one is configured already. + +Non-packable projects, including web apps, default `WarnOnPackingNonPackableProject=false` so a +solution-wide pack skips them silently. + +`ContinuousIntegrationBuild` is set only by real CI environment variables. Packability alone never +forces SourceLink's CI-mode dirty-repository checks. + +## Telemetry and SourceLink + +| Property | Default | Behaviour | +| -- | -- | -- | +| `ExcludePurviewTelemetry` | `false` | Removes `Purview.Telemetry.SourceGenerator` | +| `ExcludeMSTelemetryExtension` | `false` | Removes `Microsoft.Extensions.Telemetry.Abstractions`. Only relevant when `ExcludePurviewTelemetry` is also `false` — the whole group is skipped otherwise | +| `SourceLinkPackageName` | `Microsoft.SourceLink.GitHub` | | +| `DisableSourceLink` | `false` | | + +## Version detection + +| Property | Default | Behaviour | +| -- | -- | -- | +| `UsePackageJsonVersion` | `true` | `true`, `false`, `Strict` | +| `RootPackageJson` | *(discovered)* | Explicit path to the `package.json` to read | +| `EnableVersionDetectionCache` | `true` | | +| `VersionDetectionCacheFile` | *(derived)* | | +| `VersionDetectionLogEnabled` | `false` | Logs the detected version | + +## Repository bootstrap + +| Property | Default | Behaviour | +| -- | -- | -- | +| `DisableAutoCopySdkFiles` | `false` | Master switch for SDK-managed repository file writes | +| `PurviewRepoBootstrapMode` | `IfMissing` | Also `Always`, `WarnOnDrift`, `Never` | +| `BootstrapEditorConfigToRepoRoot` | `true` | | +| `RepositoryEditorConfigFilePath` | *(derived)* | Overrides the destination `.editorconfig` | +| `BootstrapGlobalJsonToRepoRoot` | `true` | | +| `RepositoryGlobalJsonFilePath` | *(derived)* | Overrides the destination `global.json` | +| `PurviewBuildSdkVersionForGlobalJson` | detected SDK version, else `1.0.0` | | +| `PurviewRepoBootstrapCopyRetries` | `3` | | +| `PurviewRepoBootstrapCopyRetryDelayMilliseconds` | `500` | | +| `PurviewRepoBootstrapCopyFailureAsError` | `true` | `false` downgrades a persistent write failure to a warning | +| `PurviewSuppressCopyRetryWarnings` | `true` | Demotes the built-in copy task's `MSB3026` retry notice to a message | +| `PurviewRepositoryRootMarkers` | `.git/;.github/;.gitattributes;LICENSE.md;AGENTS.md;.agents/;CLAUDE.md;.claude/;package.json` | Ordered markers identifying a repository root, most authoritative first | + +There is no agent-folder mirror. `EnableAgentFolderInPackage`, `AgentPackDestinationFolder`, +`PurviewAgentFolderSourcePath`, `PurviewAgentFolderCopy*`, `PurviewAgentSyncManifestPath` and +`DisableMirroredFileTrackingCheck` were removed: skills are installed by the skills CLI, and +nothing else was left for the mirror to deliver. + +## Skills + +Covered in full by `purview-buildsdk-skill-installation`: `PurviewSkillsRunner`, +`PurviewSkillsRunnerCandidates`, `PurviewSkillsCommand`, `PurviewSkillsPackage`, +`PurviewSkillsAgents`, `PurviewSkillsSource`, `PurviewSkillsInstallRoot`, `PurviewSkillsRepoRoot`, +`EnablePurviewSkillRecommendations`, `PurviewSkillRecommendationsInCI`. + +## Compiler-visible properties + +Exported to analyzers and generators as `build_property.`: + +- `UsePackageJsonVersion`, `RootPackageJson`, `RepoRoot`, `Version`, `PackageVersion` +- `NamespacePrefix`, `DisableNamespacePrefixCheck` +- `TestingFramework`, `SubstituteFramework`, `TestDataFramework` +- `ExcludePurviewTelemetry`, `ExcludeMSTelemetryExtension` +- `EnableAssemblyNameGeneration`, `DisableAutoInternalsVisibleTo`, `DisableGenerateAssemblyInfoClass`, + `DisableGenerateAssemblyIdentifierClass`, `AssemblyIdentifierClassName` +- `IsCSharpProject`, `IsTestProject`, `IsSharedTestingProject`, `IsSharedProject` +- `TestingType`, `TargetProjectName` +- `IsContainerProject`, `IsSdkProject`, `SdkProjectName`, `IsWebProject`, `IsWebSdkProject`, + `IsWorkerSdkProject`, `IsAspireHostProject`, `IsCLIProject` +- `EditorConfigFilePath`, `RepositoryEditorConfigFilePath`, `BootstrapEditorConfigToRepoRoot` +- `RepositoryGlobalJsonFilePath`, `BootstrapGlobalJsonToRepoRoot`, `DisableAutoCopySdkFiles` +- `PurviewRepoBootstrapMode`, `PurviewRepoBootstrapCopyRetries`, + `PurviewRepoBootstrapCopyRetryDelayMilliseconds`, `PurviewRepoBootstrapCopyFailureAsError` +- `PurviewSuppressCopyRetryWarnings` +- `PurviewSkillsSourceFolder`, + `PurviewSkillsAgents`, `PurviewSkillsInstallRoot`, `PurviewRepositoryRootMarkers`, + `PurviewSkillsRunner` +- `PurviewBuildSdkVersionForGlobalJson`, `CurrentYear`, `AutoGeneratedAssemblyInfoFile`, + `AutoGeneratedAssemblyIdentifierFile` diff --git a/.agents/skills/purview-buildsdk-project-detection/SKILL.md b/.agents/skills/purview-buildsdk-project-detection/SKILL.md new file mode 100644 index 0000000..2d0d986 --- /dev/null +++ b/.agents/skills/purview-buildsdk-project-detection/SKILL.md @@ -0,0 +1,134 @@ +--- +name: purview-buildsdk-project-detection +description: Explain why Purview.BuildSdk classified a project as test, shared, shared-testing, CLI, web, worker, Aspire host, container or Roslyn component, and why it added a package, a project reference, a namespace or an analyzer automatically. Use when SDK behaviour looks surprising, when a test project gets no runner or no InternalsVisibleTo, or when reasoning about RootNamespace, AssemblyName and TargetProjectName. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Why Purview.BuildSdk did that + +Use this skill when the question is **why** the SDK applied something automatically. + +For where a project should live, use `purview-buildsdk-project-layout`. For the property that turns +a behaviour off, use `purview-buildsdk-configuration`. + +## Diagnose from computed properties, never from the folder name + +Read what the SDK actually computed: + +```bash +dotnet msbuild -getProperty:PurviewLogicalProjectName -getProperty:RootNamespace \ + -getProperty:TestingType -getProperty:TargetProjectName -getProperty:IsTestProject +``` + +The values worth inspecting first: `MSBuildProjectName`, `NamespacePrefix`, +`PurviewLogicalProjectName`, `RootNamespace`, `TestingType`, `TargetProjectName`, `SdkProjectName`, +`IsTestProject`, `IsSharedProject`, `IsSharedTestingProject`, `IsContainerProject`, `IsCLIProject`, +`IsWebSdkProject`, `IsAspireHostProject`. + +## Detection rules + +### Test projects + +Name matches `^.*\.([\w|_]\w*)Tests?$` — a target name, a dot, then any word before `Test`/`Tests`. +The captured word becomes `TestingType`. + +This is a pattern, **not** a lookup. `MyProject.GoldenTests` is detected exactly as readily as +`MyProject.UnitTests`, with `TestingType=Golden` and `Category=Golden`, even though `Golden` is not a +name the SDK ships. + +**The dot is required.** A project named plainly `UnitTests` does not match, and silently gets no +runner, no coverage, no `InternalsVisibleTo` and no category. This is the single most common +surprise. + +Derived: `IsTestProject=true`, `TestingType=`, `PurviewTestType=Tests`, +`TargetProjectName=`. + +The SDK's explicit `TestType` item group governs **`InternalsVisibleTo` generation**, not detection: +a non-test project grants access to `{AssemblyName}.{TestType}Tests` for each name in the group. A +suffix outside it still builds, runs and gets a category — it just cannot see `internal` members. +The 53 names are in [references/test-types.md](references/test-types.md). + +### Shared and shared-testing projects + +Recognised by **exact name**, never by substring. The two lists are in +[references/detection.md](references/detection.md). Derived: `IsSharedProject`, +`IsSharedTestingProject`. + +### SDK and content based + +| Signal | Result | +| -- | -- | +| the project's `Sdk="..."` declaration | `IsSdkProject`, `SdkProjectName` | +| `Microsoft.NET.Sdk.Web` | `IsWebSdkProject=true` | +| `Microsoft.NET.Sdk.Worker` | `IsWorkerSdkProject=true` | +| SDK starts with `Aspire.Sdk.Host` or `Aspire.AppHost.Sdk` | `IsAspireHostProject=true` | +| `Dockerfile`, `dockerfile` or `Dockerfile.dev` in the project directory | `IsContainerProject=true` | +| name ends with `CLI`, `Console`, `CommandLine`, `QuickStart`, `QuickStarts` | `IsCLIProject=true` | + +## Namespace and identity + +1. `PurviewLogicalProjectName` is `NamespacePrefix` plus the project name, de-duplicated when the + project name already starts with the prefix's tail. +2. `RootNamespace` defaults to `PurviewLogicalProjectName`. +3. Known suffixes are stripped from `RootNamespace` — shared and shared-testing names, and segments + such as `Core`, `EF`, `Shared`, `ClientShared`, `ServiceDefaults`. +4. Test suffixes are stripped too, so `Acme.Api.UnitTests` maps back to `Acme.Api`. +5. `AssemblyName` and `PackageId` default to the fully evaluated `RootNamespace` — except where + stripping removed a segment, where they use the full `PurviewLogicalProjectName` so the artifact + stays distinct from its parent. Test and shared-testing projects keep their detected suffix. + Explicit `AssemblyName`/`PackageId`/`RootNamespace` always win. +6. The naming defaults are applied during `Sdk.props`, before the Microsoft SDK computes + `TargetName`, so the compiled output name always matches `AssemblyName`. + +## Automatic packages and references + +- **Non-test C# projects**: SourceLink (unless `DisableSourceLink`), the Purview telemetry packages + (unless `ExcludePurviewTelemetry`), `GenerateDocumentationFile=true`, and `InternalsVisibleTo` + generation (unless `DisableAutoInternalsVisibleTo`). +- **Test and shared-testing projects**: test-friendly `NoWarn` defaults, not packable or + publishable, and the packages selected by `TestingFramework`, `SubstituteFramework` and + `TestDataFramework`. TUnit test projects get Microsoft.Testing.Platform wiring automatically. + Shared-testing projects skip the runnable test package. +- **Packable projects**, **Roslyn components** and **special project types** (CLI, container, web, + Aspire host): see [references/detection.md](references/detection.md). + +Project-reference probing is layout-driven and covered by `purview-buildsdk-project-layout`. + +## The shipped analyzers + +The SDK adds `Purview.BuildSdk.Analyzers` as an `` item to every C# project, so its rules +surface in command-line builds and in Visual Studio. The code-fix assembly ships beside it. + +| Rule | What it enforces | +| -- | -- | +| `PDS0002` | Extensions classes live under `Extensions//` | +| `PDS0003` | Explicit types with target-typed `new()` | +| `PDS0004` | Correct acronym capitalisation | + +`PDS0004` follows .NET naming guidance: well-known framework spellings (`Sql`, `Guid`, `Uuid`, +`Url`, `Dns`, `Tcp`, `Http`, `Xml`, `Db`, …) are exempt — `Db` among them, so `DbContext`, +`DbConnection` and `DbSet` are never flagged. A small set is still uppercased: `Api`, `Ai`, `Ui`, +`Io`, `Os`, `Cpu`, `Gpu`, `Cli`, `Gui`, `Ram`, `Ssh`. Members mandated by a contract — interface +implementations and base-class overrides — are never renamed. + +Customise through `dotnet_analyzer_configuration.pds0004.allowed_words` (exempt segments), +`.acronym_map` (segment renames, e.g. `Sql:SQL`, or `Db:DB` to re-enable it) and +`.allowed_identifiers` (brand names, exact or word-boundary prefix, first match wins — `CosmosDb` +covers `CosmosDbServer`). All three merge with and override the shipped defaults. + +The shipped `.editorconfig` marks generated content (`Migrations/`, `*.g.cs`, `Generated/`, +`*.Designer.cs`, `obj/`, `bin/`) as `generated_code = true`, and suppresses namespace-conflict +diagnostics under `Extensions/`, so no `#pragma` suppressions are needed there. + +Two Visual Studio refactorings ship with them, both described in +[references/detection.md](references/detection.md). + +## When changing a detection rule + +1. Keep the `.csproj` filename equal to its directory name. +2. Update `README.md` and the shipped skills in the same change — a rule true in code and false in + prose is this repository's recurring failure. +3. Add or update the integration test in `src/tests/BuildSdk.IntegrationTests/` that pins it. diff --git a/.agents/skills/purview-buildsdk-project-detection/references/detection.md b/.agents/skills/purview-buildsdk-project-detection/references/detection.md new file mode 100644 index 0000000..bf75823 --- /dev/null +++ b/.agents/skills/purview-buildsdk-project-detection/references/detection.md @@ -0,0 +1,79 @@ +# Detection detail + +## Shared project names (exact) + +`Shared`, `SharedFramework`, `SharedInfrastructure`, `SharedInfra`, `SharedUtilities`, +`SharedUtils`, `SharedLibrary`, `SharedLib`, `SharedHelpers` + +## Shared testing project names (exact) + +`SharedTestingFramework`, `SharedTestingInfrastructure`, `SharedTestingInfra`, +`SharedTestingUtilities`, `SharedTestingUtils`, `SharedTestingLibrary`, `SharedTestingLib`, +`SharedTestingHelpers` + +These are matched whole, not as substrings. `SharedModels` is not a shared project. + +A shared-testing project is forced to `PurviewSharedTestingOutputType` (default `Library`), with +`IsTestProject` and `IsTestingPlatformApplication` cleared, because the test packages otherwise flip +it into an executable test host. Set `PurviewSharedTestingOutputType=Exe` before the SDK import to +keep that package-driven shape. + +## Packable projects + +Defaults applied only where the project supplied nothing: `GenerateDocumentationFile`, +`IncludeSymbols`, `SymbolPackageFormat=snupkg`, `PublishRepositoryUrl`, `EmbedUntrackedSources`, +`IncludeSource`, `DebugType=portable`. + +- Portable PDBs ship in the `.snupkg`. The `.nupkg` receives none unless the project opts in. +- The repository-root `README.md` is packed and registered as `PackageReadmeFile` when it exists and + nothing else is already being packed as one. +- Non-packable projects, web apps included, default `WarnOnPackingNonPackableProject=false`, so a + solution-wide `dotnet pack` skips them silently. +- `ContinuousIntegrationBuild` is set only by real CI environment variables. Packability alone never + forces SourceLink's CI-mode dirty-repository checks. + +## Roslyn components + +A project that explicitly declares `IsRoslynComponent=true` gets: + +- a single `netstandard2.0` target, `LangVersion=latest`, `Nullable=enable`, + `TreatWarningsAsErrors=true`, `Deterministic=true`, extended analyzer rules +- SourceLink with `EmbedUntrackedSources=true`, no dependency file, telemetry excluded +- compiler-generated output under the framework-specific intermediate directory +- `PrivateAssets=all` on `Microsoft.CodeAnalysis.*` and `Microsoft.CodeAnalysis.Analyzers`, so they + never leak into the packed nuspec + +`IsRoslynComponentOnly` defaults to `true`: `IncludeBuildOutput=false`, `IncludeSymbols=false`, no +`.symbols.nupkg` or `.snupkg`, and the analyzer PDB shipped inside the main `.nupkg` under +`analyzers/dotnet/cs/` beside the assembly (`PurviewPackAnalyzerPdb=true`). Set it to `false` for a +dual-role component that wants normal library symbol packaging. + +`ValidateRoslynComponentCompilerSettings` fails the pack of a packable Roslyn component missing +`LangVersion`, `Nullable`, `TreatWarningsAsErrors` or `EnforceExtendedAnalyzerRules`. Opt out with +`DisableRoslynCompilerDefaultsValidation=true`. + +`PackProjectReferencedSourceGenerators` (default `true`) makes a packable project include analyzer +`ProjectReference` outputs and their runtime dependencies under `analyzers/dotnet/cs/`. Set it +`false` globally, or `Pack="false"` on one analyzer reference. + +## Special project types + +| Type | Applied | +| -- | -- | +| CLI | `OutputType=Exe`, `appsettings*.json` included as content | +| Container | `InvariantGlobalization`, `PublishAot`, Linux Docker defaults, container tooling packages | +| Web SDK | `Microsoft.AspNetCore.OpenApi.Generated` added to `InterceptorsNamespaces`, unless marked as a separate web-project mode | +| Aspire host | `OutputType=Exe` | + +## The Visual Studio refactorings + +Two ship with the code-fix assembly, both serving the `PDS0002` convention: + +- **Split extensions class into one class per receiver type** — offered on a static extensions class + targeting several receiver types. It produces one `Extensions` class per receiver + (a generic `this TBuilder where TBuilder : IHostApplicationBuilder` receiver becomes + `HostApplicationBuilderExtensions`) and places each under `Extensions//`. +- **Move extensions class to conventional location** — offered on a misplaced single-receiver + extensions class. It re-paths the file to `Extensions//`, fixes the namespace + to the receiver's, and updates `using` directives in other referencing documents so the move + compiles. diff --git a/.agents/skills/purview-buildsdk-project-detection/references/test-types.md b/.agents/skills/purview-buildsdk-project-detection/references/test-types.md new file mode 100644 index 0000000..dace44f --- /dev/null +++ b/.agents/skills/purview-buildsdk-project-detection/references/test-types.md @@ -0,0 +1,35 @@ +# The shipped `TestType` group + +These 53 names drive **`InternalsVisibleTo` generation**, not test detection. A non-test project +grants friend access to `{AssemblyName}.{TestType}Tests` for each name here. + +Detection is the pattern `^.*\.([\w|_]\w*)Tests?$`, so a suffix outside this group is still detected, +still runs and still gets its category — it simply cannot reach the target project's `internal` +members. Prefer a name from this group unless the test type genuinely needs its own word. + +```text +Unit Integration E2E EndToEnd Acceptance +Functional Performance Load Smoke Stress +Regression Security Chaos Scenario System +Threat BlackBox WhiteBox GrayBox GreyBox +Accessibility Usability Visual Interactive Localization +Globalization Environment Configuration Deployment Architecture +Contract Component Sanity Property Fuzz +Mutation Snapshot Approval Benchmark Capacity +Concurrency Endurance Soak Spike Failover +Recovery Resilience Compatibility Interoperability Migration +Upgrade Penetration Compliance +``` + +## The five to reach for first + +`Unit`, `Integration`, `E2E`, `Functional`, `Contract`. Everything else is available, but a reader +has to ask what it means — use one only when the test type itself carries operational weight. + +## What a suffix buys you + +| | detected as a test project | gets a category | gets a runner and coverage | sees `internal` members | +| -- | -- | -- | -- | -- | +| `Api.UnitTests` | yes | `Unit` | yes | yes | +| `Api.GoldenTests` | yes | `Golden` | yes | **no** | +| `UnitTests` *(no dot)* | **no** | no | **no** | no | diff --git a/.agents/skills/purview-buildsdk-project-layout/SKILL.md b/.agents/skills/purview-buildsdk-project-layout/SKILL.md new file mode 100644 index 0000000..55a019c --- /dev/null +++ b/.agents/skills/purview-buildsdk-project-layout/SKILL.md @@ -0,0 +1,136 @@ +--- +name: purview-buildsdk-project-layout +description: Decide where a project goes and what to call it in a repository that uses Purview.BuildSdk. Use when creating, moving, splitting or renaming a project or test project, choosing a test suffix, aligning namespaces and assembly identity with NamespacePrefix, or relying on the SDK's automatic project references. Also use when rationalising a repository's shape so the SDK's conventions work without overrides. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Project layout and naming for Purview.BuildSdk + +Use this skill when physically adding, moving, splitting or renaming a project. + +For *why* the SDK classified an existing project a particular way, use +`purview-buildsdk-project-detection`. For the property that changes a behaviour, use +`purview-buildsdk-configuration`. + +## First principle + +**Treat naming and placement as configuration, not decoration.** + +The SDK infers namespaces, assembly identity, test categories, package wiring and project references +from a small set of conventions. The closer a repository sits to them, the fewer overrides it needs. + +The corollary matters just as much: **preserve the host repository's existing layout first.** Only +introduce new structure when no established pattern exists. + +## Canonical layout + +```text +/ + package.json version source + .editorconfig bootstrapped by the SDK + skills/ authored agent skills, if any + src/ + .slnx + src//.csproj + tests/.Tests/.Tests.csproj +``` + +- The `.csproj` filename must equal its containing directory name, unless + `DisableProjectFileNamingConventionCheck=true`. +- Source projects under `src/`, test projects under `tests/`. +- Place a new project beside similar ones — same language, layer and test type. +- One test type per project by default. + +A repository that already uses a different shape keeps it; add to it rather than converting it. + +## Naming and identity + +`NamespacePrefix` is the root identity source. Keep project names short and let the SDK apply the +prefix: + +| `NamespacePrefix` | Project | `RootNamespace` | +| -- | -- | -- | +| `Aspire` | `Hosting` | `Aspire.Hosting` | +| `Acme.Sales.RegionalPipeline` | `Identity.API` | `Acme.Sales.RegionalPipeline.Identity.API` | +| `Acme.Sales.RegionalPipeline` | `Identity.Core` | `Acme.Sales.RegionalPipeline.Identity` | + +`AssemblyName` and `PackageId` follow the resolved `RootNamespace` — except where suffix-stripping +removed a segment (`Core`, `Shared`, `ServiceDefaults`), in which case they keep the fuller logical +name so the artifacts stay distinct from their parent. Explicit values always win, and +`EnableAssemblyNameGeneration=false` (before the SDK import) restores standard behaviour. + +Do not hand-author an alternate namespace scheme when one is already in use. + +## Test project naming + +Test detection is a **pattern**, not a list: `^.*\.([\w|_]\w*)Tests?$`. A target name, a dot, then +any word before `Test` or `Tests`. The captured word becomes `TestingType` and the baseline category. + +**The dot is required.** A project named plainly `UnitTests`, with no target in front of it, is not +detected as a test project at all — no runner, no coverage, no `InternalsVisibleTo`, no category. +Always name a test project `.Tests`. + +Prefer these by default: `UnitTests`, `IntegrationTests`, `E2ETests`, `FunctionalTests`, +`ContractTests`. Use another suffix only when the test type itself carries operational meaning. + +A suffix outside the SDK's shipped `TestType` group still builds, runs and gets its category — it +just cannot reach the target project's `internal` members, because that group drives +`InternalsVisibleTo` generation. The full list is in `purview-buildsdk-project-detection`. + +## Shared projects: exact names only + +The SDK recognises these names exactly — a near miss gets none of the behaviour. + +Shared: `Shared`, `SharedFramework`, `SharedInfrastructure`, `SharedInfra`, `SharedUtilities`, +`SharedUtils`, `SharedLibrary`, `SharedLib`, `SharedHelpers`. + +Shared testing: `SharedTestingFramework`, `SharedTestingInfrastructure`, `SharedTestingInfra`, +`SharedTestingUtilities`, `SharedTestingUtils`, `SharedTestingLibrary`, `SharedTestingLib`, +`SharedTestingHelpers`. + +## Automatic project references to preserve + +Layout is what makes these work. Move a project off the convention and you own the reference. + +Test projects probe for their target, in order: + +- `../$(TargetProjectName)/$(TargetProjectName).csproj` +- `../../$(TargetProjectName)/$(TargetProjectName).csproj` +- `../src/$(TargetProjectName)/$(TargetProjectName).csproj` +- `../../src/$(TargetProjectName)/$(TargetProjectName).csproj` + +Non-test projects pick up sibling shared projects via `../Shared*/Shared*.csproj`; test projects +pick up `../SharedTesting*/SharedTesting*.csproj`. + +## Creating a new project + +1. Match the SDK and style of sibling projects. +2. Use Central Package Management if the repository has it. +3. Add only the dependencies the project's scope needs — the SDK supplies the testing stack, + SourceLink and telemetry by default. +4. Add the project to the solution. +5. Keep target frameworks, nullability, analyzers and warning settings consistent with neighbours. + +## Moving or splitting + +1. Create the destination using the established layout. +2. Move the files. +3. Update namespaces, usings and references for the destination. +4. Verify the destination still produces the intended `TestingType`, `TargetProjectName` and + `RootNamespace` — ask MSBuild rather than assuming: + `dotnet msbuild -getProperty:RootNamespace -getProperty:TargetProjectName`. +5. Remove dependencies the source no longer needs. +6. Update solution membership and project references. +7. Build, then run the affected tests. + +## Guardrails + +- Prefer minimal, targeted diffs; avoid cross-cutting renames unrelated to the move. +- Keep test intent unchanged while relocating. +- When the structure is ambiguous, infer from the nearest sibling projects and say which assumption + you made. +- When in doubt, preserve compatibility with the SDK's automatic naming, namespace and + project-reference behaviour. diff --git a/.agents/skills/purview-buildsdk-repository-review/SKILL.md b/.agents/skills/purview-buildsdk-repository-review/SKILL.md new file mode 100644 index 0000000..ce1fd29 --- /dev/null +++ b/.agents/skills/purview-buildsdk-repository-review/SKILL.md @@ -0,0 +1,111 @@ +--- +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. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Reviewing a repository against the SDK's conventions + +This is an audit, not a refactor. The deliverable is a prioritised list of findings and the cost of +each, so somebody else can decide what to change. + +For the conventions themselves use `purview-buildsdk-project-layout`; for why one project was +classified as it was, `purview-buildsdk-project-detection`. + +## The governing judgement + +**Deviation is only a finding when it costs the repository something.** The SDK infers behaviour +from naming and placement, so a deviation is interesting precisely to the extent that it turns an +automatic behaviour off. A layout that differs from the canonical one but still gets every automatic +behaviour is not debt, and reporting it as such wastes the reader's attention. + +Sort every finding into one of three buckets, and say which: + +| Bucket | Meaning | +| -- | -- | +| **Acceptable variance** | Differs from the canonical layout, costs nothing. Preserve it. | +| **Structural debt** | Blocks an automatic behaviour the repository would benefit from. | +| **Incremental opportunity** | Low-risk tidying worth doing when the area is touched anyway. | + +## 1. Read before concluding + +Inspect, in this order, before forming any opinion: + +- `Directory.Build.props` and `Directory.Build.targets` — where the SDK is imported, and what is set + before the import +- the solution entry point, for what is actually in the build +- `Directory.Packages.props`, for Central Package Management +- the project layout on disk + +## 2. Check the structural signals + +- Source projects under `src/`, test projects under `tests/`. +- `.csproj` filename equals its containing directory name — unless + `DisableProjectFileNamingConventionCheck=true`, which is itself a finding worth surfacing. +- Short project names, with `NamespacePrefix` carrying the repository identity. +- Test project names match `^.*\.([\w|_]\w*)Tests?$`. **The dot is required**; a project named + plainly `UnitTests` is not detected at all and silently gets no runner, no coverage, no + `InternalsVisibleTo` and no category. This is the single highest-value thing to check. +- Test suffixes come from the SDK's shipped `TestType` group where `internal` access is wanted — a + suffix outside it still runs, it just cannot see `internal` members. +- Shared and shared-testing projects use the **exact** recognised names. A near miss gets none of + the behaviour. + +## 3. Verify with MSBuild, do not infer from folder names + +A review that reasons from directory names will be wrong about at least one project. Ask: + +```bash +dotnet msbuild -getProperty:RootNamespace -getProperty:AssemblyName \ + -getProperty:IsTestProject -getProperty:TestingType -getProperty:TargetProjectName +``` + +Quote the computed values in the findings. "`Api.Tests` resolves `TestingType=` empty, so it has no +runner" is actionable; "`Api.Tests` looks like it might not be detected" is not. + +## 4. Review test readability + +- Subject-based suites: `{SubjectName}Tests`, with + `{SubjectOrMemberUnderTest}_{Scenario}_{Expectation}` methods. +- Non-subject-based suites — build, generator, workflow, package, full-system — named clearly for + their broader role. A forced `{Subject}Tests` on one of these is worse, not better. +- TUnit categories and display names used where a suite is large enough to need navigating. + +See `purview-buildsdk-testing`. + +## 5. Check the test dependencies are earned + +The SDK already supplies `TUnit`, `TUnit.Mocks`, `Bogus` and the Microsoft.Testing.Platform wiring +to every detected test project. Anything beyond that was a deliberate choice, and deliberate choices +go stale: + +- `TUnit.Aspire` and `Purview.Containers` are **not** universal defaults. Confirm each reference is + genuinely needed by that project rather than copied from a sibling. +- Where a project references `Testcontainers` directly, check whether a `Purview.Containers` typed + module now covers it — the module set grows, and a direct reference that was once necessary often + 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 + +For each finding, name the specific automatic behaviour that is lost: `RootNamespace`, +`AssemblyName`, `PackageId`, `TargetProjectName`, the automatic target-project reference, the +sibling `Shared*` reference, `InternalsVisibleTo` generation, the test runner and coverage wiring. + +"Project is not under `src/`" is not a finding. "Project is not under `src/`, so the test project's +four probe paths all miss and the `ProjectReference` has to be maintained by hand" is. + +## Reporting the result + +1. One sentence on whether the repository broadly fits the SDK's conventions. +2. The findings, ordered by impact, each with its bucket and the behaviour it costs. +3. The low-risk rationalisation steps, as concrete renames or moves. +4. An explicit split of what is **already automatic** versus what this repository has configured by + hand — that list is usually the most useful part of the review, because hand-configuration that + merely restates a default is pure maintenance cost. + +Do not propose a cross-cutting rename as a first step. Name the one change with the best +cost-to-benefit ratio and say what it unlocks. diff --git a/.agents/skills/purview-buildsdk-skill-installation/SKILL.md b/.agents/skills/purview-buildsdk-skill-installation/SKILL.md new file mode 100644 index 0000000..66c20a3 --- /dev/null +++ b/.agents/skills/purview-buildsdk-skill-installation/SKILL.md @@ -0,0 +1,166 @@ +--- +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. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Installing the skills a package ships + +For *authoring* skills, use `purview-buildsdk-skill-tests`. + +## The one thing to understand first + +**The SDK writes nothing into an agent directory.** A package *ships* `.agents/skills//`, but +as metadata — it is what tells the SDK which skills the package offers and at which commit. Nothing +copies it out. + +Skills are installed by the skills CLI and by nothing else. That is a deliberate single-writer rule. +Earlier versions of this SDK mirrored a package's whole `.agents/**` folder into the consuming +repository, and the CLI's own universal install location is `.agents/skills/` — the same paths. The +mirror would silently overwrite a pinned install on the next build, while `PRSKL0011` compared a +lockfile ref against content the mirror had already replaced. The mirror was removed outright rather +than fenced off, because the CLI covers the whole job. + +Installing is therefore a separate, deliberately **explicit** step. It is never a side effect of a +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 +``` + +Neither target has `BeforeTargets` or `AfterTargets`. If skills are appearing or changing on their +own, something has been wired up that should not be. + +## How a skill is resolved, and why the version is always right + +There is deliberately **no custom manifest**. A package that ships skills already carries both +halves of the mapping, and `ResolvePurviewSkillSources` reads them: + +- `.agents/skills//` in the restored package says *which* skills it ships. +- The nuspec's ``, written by SourceLink for every package this + SDK builds, says which git repository and which **exact commit** produced them. + +So the resolved ref is the commit the *restored package version* was built from. An older +`PackageReference` gets that version's skills, not whatever is on the default branch. Compatibility +is exact by construction rather than by convention. + +## Installing from an explicit source + +For a skill you want but have no `PackageReference` for: + +```bash +dotnet msbuild -t:InstallPurviewSkills -p:PurviewSkillsSource=purview-dev/build-sdk +``` + +The caller owns the ref here — append `#` to pin it. This path needs no restore, so +`PRSKL0001` does not apply. + +## Diagnostics + +| Code | Meaning | Fix | +| -- | -- | -- | +| `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` | +| `PRSKL0005` | An authored skill is malformed | See `purview-buildsdk-skill-tests` | +| `PRSKL0010` | A skill a referenced package ships is missing | Warning. `npx 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 | + +## Recommendations run during an ordinary build + +This is the **one** skill feature that does. `RecommendPurviewSkillsOnBuild` runs after `Build` and +delegates to `RecommendPurviewSkills`, which can also be invoked by name: + +```bash +dotnet msbuild -t:RecommendPurviewSkills +``` + +`EvaluatePurviewSkillState` joins the skills each restored package ships against the CLI's own +`skills-lock.json`. Both inputs are local files, so it stays offline and spawns no process. + +Four things keep it quiet, and all of them are load-bearing: + +- Design-time builds, CI builds, non-default TFM passes and + `EnablePurviewSkillRecommendations=false` are skipped outright. + `PurviewSkillRecommendationsInCI=true` re-enables CI. +- A fingerprint stamp at `/.purview/skill-recommendations.cache` means a given state is + reported once per repository, not once per project and again on every rebuild. The explicit target + ignores the stamp. +- Missing and drifted are **warnings** (fixable in one command); unverifiable and orphaned are + **messages**. No amount of skill advice can break a build using `TreatWarningsAsErrors`. +- A correctly installed set produces no output at all. + +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. + +### 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. + +## 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. + +| Runner | Invocation | +| -- | -- | +| `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` | + +An unknown runner falls through to ` $(PurviewSkillsPackage)`, so it still works. + +`DetectPurviewSkillsCli` 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, and it +cannot hang on a cold download. **The runner actually found wins over the configured preference.** +`PurviewSkillsCommand` overrides the launcher entirely and skips detection. + +`ListPurviewSkills` reports runner availability but **never** errors on its absence — resolving and +reporting needs no Node tooling at all. + +## Always the repository root + +`skills-lock.json` and the agent directories belong to the repository, not to a project, so every +skill target runs from one resolved root: + +1. `PurviewSkillsInstallRoot` — an explicit override +2. `PurviewSkillsRepoRoot` — only when the **user** set it +3. the `PurviewRepositoryRootMarkers` walk +4. `PurviewSkillsRepoRoot` / `RepoRoot` as the SDK guessed them +5. CI workspace variables +6. SourceLink's git `SourceRoot` + +There is deliberately **no project-directory fallback**. That would scatter a lockfile and an agent +directory through the tree, one per project. An unresolved root is `PRSKL0003` on install, and +silence for recommendations — a build never fails over advice. + +The marker walk is described in [references/repository-root.md](references/repository-root.md); its +two-pass behaviour and its home-directory bound are both load-bearing. + +## Why the CLI rather than a reimplementation + +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 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. diff --git a/.agents/skills/purview-buildsdk-skill-installation/references/repository-root.md b/.agents/skills/purview-buildsdk-skill-installation/references/repository-root.md new file mode 100644 index 0000000..e90b695 --- /dev/null +++ b/.agents/skills/purview-buildsdk-skill-installation/references/repository-root.md @@ -0,0 +1,55 @@ +# The repository-root marker walk + +`FindPurviewRepositoryRoot` is an inline MSBuild task driven by `PurviewRepositoryRootMarkers`: + +```text +.git/;.github/;.gitattributes;LICENSE.md;AGENTS.md;.agents/;CLAUDE.md;.claude/;package.json +``` + +A trailing slash marks a directory. A directory marker still matches a **file** of that name, +because `.git` is a directory in a clone but a file in a worktree or a submodule, and both identify +a repository root. + +## Three things that are load-bearing + +### It cannot be an MSBuild intrinsic + +`GetDirectoryNameOfFileAbove` matches **files only** and returns nothing for `.git/`, `.github/`, +`.agents/` or `.claude/` — four of the nine markers. Verified, not assumed. + +### Two passes + +- **Pass 1** — the first marker in the list is *definitive* and is searched all the way up. `.git/` + *is* the repository root, so a monorepo sub-package with its own `package.json` does not win. +- **Pass 2** — every other marker resolves nearest-ancestor-first, with list order breaking ties + inside a single directory. Searching them all to the top would let a stray `.claude/` in a + developer's home directory outrank the real root. + +### The home directory is never a candidate + +The home directory and everything above it are excluded. Without that bound, a project outside any +repository resolves to `~` and installs into it — and a `.claude/` or a `.github/` very commonly +sits there. A repository *below* the home directory is unaffected, because it is below it. + +## Why the marker walk outranks the evaluation-time guess + +`PurviewSkillsRepoRoot` is computed during evaluation from `RepoRoot` and an `AGENTS.md` walk-up. +An `AGENTS.md` can legitimately sit in a subdirectory, and resolving to it would put a lockfile and +an agent directory somewhere that is not the repository. + +`_PurviewSkillsRepoRootWasSpecified` is captured in `Sdk.props` *before* defaulting, so a value the +**user** set still outranks the walk while a value the SDK guessed does not. + +## A target's Condition is evaluated before its DependsOnTargets + +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. + +## The managed port + +`Purview.Skills.Testing` ships `SkillRepositoryRoot`, a managed port of the same walk with the same +default markers, so a test that validates a repository's skills resolves the same root the build +installs them into. The two are kept in step by `SkillRepositoryRootTests`, including an assertion +on the default marker list itself — both are defaults nobody sets, so nothing else would notice them +drifting apart. diff --git a/.agents/skills/purview-buildsdk-skill-tests/SKILL.md b/.agents/skills/purview-buildsdk-skill-tests/SKILL.md new file mode 100644 index 0000000..4043ac4 --- /dev/null +++ b/.agents/skills/purview-buildsdk-skill-tests/SKILL.md @@ -0,0 +1,131 @@ +--- +name: purview-buildsdk-skill-tests +description: Author agent skills in a repository that uses Purview.BuildSdk, and validate them with Purview.Skills.Testing or the SDK's pack-time ValidatePurviewSkills target. Use when adding or renaming a skill, writing SKILL.md frontmatter, splitting content into references/ or scripts/, fixing a PRSKL0005 pack failure or a skill-name, skill-description or skill-bundled-resources test failure, or deciding where skills belong in a repository. +license: MIT +metadata: + package: Purview.Skills.Testing + repository: purview-dev/build-sdk +--- + +# Authoring and validating agent skills + +For installing skills a package ships, use `purview-buildsdk-skill-installation`. + +## Where skills live + +**At the repository root, in `skills//SKILL.md`.** Always. + +The skills CLI only discovers skills at a root `skills/` folder, and it reaches a repository by +The skills CLI only discovers skills at a root `skills/` folder, and it reaches a repository by +cloning it, so nothing else is visible to it. Nothing is packed into a NuGet package — a package +carries .NET artifacts, and skills are delivered by the CLI from git. + +```text +skills/ + my-skill/ + SKILL.md required + references/ loaded on demand + scripts/ executable, run on demand + assets/ templates and static resources +``` + +Link to a skill by its **authored** path, `./skills//SKILL.md`. A link into `.agents/skills/` +points at a gitignored file that does not exist in a fresh clone. + +## `SKILL.md` format + +```markdown +--- +name: my-skill +description: What it does, and when to use it. Include the words a task would contain. +license: MIT +metadata: + package: My.Package +--- + +# My skill + +Instructions. +``` + +| Field | Required | Constraint | +| -- | -- | -- | +| `name` | yes | 1–64 chars, lowercase `a-z0-9` and single hyphens, no leading, trailing or consecutive hyphen, **must equal the folder name** | +| `description` | yes | 1–1024 chars, non-empty | +| `license` | no | A licence name, or the name of a bundled licence file | +| `compatibility` | no | ≤500 chars, only when the skill has real environment requirements | +| `metadata` | no | A map of string to string. Anything not defined above belongs here | +| `allowed-tools` | no | Space-separated pre-approved tools. Experimental | + +**There are no other top-level fields.** `category`, `roles` and `tags` are common inventions and a +strict client rejects them; put them under `metadata`. + +The full specification is at . +[references/authoring.md](references/authoring.md) covers description writing, naming for a shared +namespace, and progressive disclosure. + +## Progressive disclosure + +An agent loads a skill in three stages: the `name` and `description` at startup, the whole +`SKILL.md` body once the skill activates, and `references/`, `scripts/` or `assets/` only when the +task reaches them. + +So: keep `SKILL.md` **under 500 lines**, and move detail into `references/`. The body is a cost paid +on every activation; a reference file is paid only when it is needed. + +Keep file references one level deep from `SKILL.md`, and relative to the skill root. A path that +climbs out of the skill folder is broken everywhere except the authoring repository, because only +the skill's own folder is installed. + +## Validating + +### In tests — `Purview.Skills.Testing` + +```csharp +[InheritsTests] +public sealed class SkillTests : PurviewSkillTests; +``` + +**`[InheritsTests]` is load-bearing.** TUnit does not walk base types during discovery, so without +it the class compiles, the run is green and nothing is validated. TUnit reports `TUnit0030`; treat +it as an error. + +| Rule id | What it checks | +| -- | -- | +| `skills-discovery` | The folder, when present, holds at least one skill | +| `skill-structure` | Every folder has a `SKILL.md` whose frontmatter parses | +| `skill-name` | Spec-legal, unique, and equal to the folder name | +| `skill-description` | Present, non-empty, within 1024 characters | +| `skill-optional-fields` | Only spec-defined fields, each within its limit | +| `skill-bundled-resources` | Every linked file exists, inside the skill folder | +| `skill-body-size` | `SKILL.md` stays under the recommended 500 lines | + +A repository with **no** `skills/` folder passes every rule — authoring skills is optional. An +**empty** folder fails `skills-discovery`, because every other rule is vacuous against an empty +catalogue. + +Overrides: `RepositoryRoot` (default: the SDK's repository-root marker walk), +`SkillsFolderName` (default `skills`), and `SkipRule(string)`. + +The evaluator takes a `SkillCatalogue`, so a rule can be exercised against a fixture folder rather +than a real repository — the same three-layer shape this repository uses throughout: a reader, an +evaluator over an in-memory model, and a thin test base class. + +### At pack time — `ValidatePurviewSkills` + +It runs `BeforeTargets="Pack"`, so a malformed skill fails in the repository that owns it rather +than reaching the CLI, which would silently skip it. + +```bash +dotnet msbuild -t:ValidatePurviewSkills +``` + +Failures are reported as `PRSKL0005`, one per problem. Every authored skill is checked. + +The two checks overlap deliberately: the pack-time one runs without a test host, the test one +covers more of the specification. + +## What is deliberately not checked + +Prose quality, and whether an agent actually obeys the skill. A build can only check that a skill is +well formed. Treat a green run as "installable", not "good". diff --git a/.agents/skills/purview-buildsdk-skill-tests/references/authoring.md b/.agents/skills/purview-buildsdk-skill-tests/references/authoring.md new file mode 100644 index 0000000..69d4ee4 --- /dev/null +++ b/.agents/skills/purview-buildsdk-skill-tests/references/authoring.md @@ -0,0 +1,79 @@ +# Authoring a skill that fires, and keeps firing + +## The description is the whole activation decision + +`name` and `description` are the only things loaded at startup. Everything else — the body, the +references, the scripts — is invisible until an agent has already decided to activate the skill on +the strength of those two fields. + +So write the description for a *matcher*, not for a reader: + +- Say **what it does** and **when to use it**, in that order. +- Include the words a real task would contain: the tool names, the file names, the error codes, the + property names, the phrases a person would type. +- Name the thing the skill is *not*, when a neighbouring skill could be confused with it. + +| | | +| -- | -- | +| Poor | `Helps with the build SDK.` | +| Better | `Configure Purview.BuildSdk from Directory.Build.props or a .csproj. Use when setting NamespacePrefix, choosing a testing framework, or working out whether a property must be set before the SDK import.` | + +The limit is 1024 characters. Most of that budget is better spent on trigger words than on prose. + +## Naming for a shared namespace + +Skills install into one flat directory per agent, shared with every other skill a developer has +installed from anywhere. A generic name is a collision waiting to happen, and the loser is whichever +one installed second. + +- Prefix with the vendor and the product: `purview-buildsdk-configuration`, not `sdk-configuration`. +- `sdk-*`, `project-*`, `test-*`, `build-*` are all too common to use unprefixed. +- The name must be 1–64 characters, lowercase `a-z0-9` with single hyphens, no leading, trailing or + consecutive hyphen — and it must equal the folder name, because the CLI installs by the + frontmatter name while a human reads the folder. + +Renaming a skill means renaming the folder and the frontmatter together, and re-running the install. + +## Progressive disclosure in practice + +| Stage | Loaded | Budget | +| -- | -- | -- | +| Metadata | `name`, `description` | ~100 tokens, always | +| Instructions | the whole `SKILL.md` body | under 5000 tokens recommended; keep the file under 500 lines | +| Resources | `references/`, `scripts/`, `assets/` | only when the task reaches them | + +What belongs where: + +- **`SKILL.md`** — the decisions, the rules, the things that are wrong by default, and a pointer to + everything else. If a reader needs it to make the *first* correct move, it goes here. +- **`references/`** — long tables, full option lists, grammars, enumerations. Keep each file + focused; an agent loads the whole file. +- **`scripts/`** — executable code. Self-contained, or documenting its dependencies clearly, with + helpful error messages. +- **`assets/`** — templates, schemas, images, lookup data. + +Keep references one level deep. Avoid a chain where one reference sends an agent to another. + +## Links + +Use relative paths from the skill root: + +```markdown +See [the reference](references/REFERENCE.md) for detail. +Run `scripts/extract.py`. +``` + +Only the skill's own folder is installed, so a link that climbs out of it (`../other-skill/...`) is +broken on every machine except the one that authored it. To point at another skill, name it in prose +and let the agent find it by name. + +## Keeping a skill true + +A skill that describes a product goes stale the moment the product changes, and nothing about a +stale skill looks broken — it just gives confidently wrong advice. + +- Change the skill in the **same commit** as the behaviour it documents. +- Prefer stating the property name, the diagnostic code or the target name over paraphrasing the + behaviour; those are greppable when the code changes. +- Where a number or a list is authoritative somewhere else — a default, an enumeration — say where + it comes from, so a reader can check it. diff --git a/.agents/skills/purview-buildsdk-testing/SKILL.md b/.agents/skills/purview-buildsdk-testing/SKILL.md new file mode 100644 index 0000000..5a5a7d5 --- /dev/null +++ b/.agents/skills/purview-buildsdk-testing/SKILL.md @@ -0,0 +1,121 @@ +--- +name: purview-buildsdk-testing +description: Write and run tests in a repository that uses Purview.BuildSdk. Use when authoring or refactoring TUnit tests (AAA structure, Subject_Scenario_Expectation naming, awaited assertions, CancellationToken), when choosing a test project's framework packages, and when running or filtering tests - the SDK wires Microsoft.Testing.Platform, so `dotnet test --filter` reports "Zero tests ran" and `--treenode-filter` is the one that works. +license: MIT +metadata: + package: Purview.BuildSdk + repository: purview-dev/build-sdk +--- + +# Testing in a Purview.BuildSdk repository + +Two things to know before anything else: + +1. A test project is detected by **name**, and detection is what wires the runner. See + `purview-buildsdk-project-layout` if the project is new or being moved. +2. The runner is **Microsoft.Testing.Platform (MTP)**, not VSTest. `--filter` does nothing useful. + +## What a test project gets automatically + +With the defaults (`TestingFramework=TUnit`, `SubstituteFramework=TUnitMocks`, +`TestDataFramework=Bogus`), a detected test project receives: + +- `TUnit`, `TUnit.Mocks`, `Bogus` +- Microsoft.Testing.Platform integration +- `CollectCoverage=true` +- test-friendly `NoWarn` defaults (`PurviewTestContextNoWarn`), while keeping the strict style + contract — `IDE0040`, field naming, formatting, `IDE1006` +- `IsPackable=false`, `IsPublishable=false` + +Add nothing else unless the project's purpose demands it. Two that stay explicit: + +- `TUnit.Aspire` — Aspire lifecycle and AppHost-backed integration tests +- `Purview.Containers` — container-backed integration tests. One API over Microsoft WSL Containers + (no Docker install needed on Windows) or Docker, with typed modules for SQL Server, PostgreSQL, + Redis, MySQL, RabbitMQ, Azurite and NATS. Experimental and prerelease, so pin the version. + Reference `Testcontainers` directly only where no typed module exists yet. + +## Authoring rules + +- Use TUnit (`TUnit.Core`, `TUnit.Assertions`). Do not introduce xUnit, NUnit, MSTest or + FluentAssertions unless explicitly asked. +- Every test method carries `[Test]`. +- **Every assertion is awaited**: `await Assert.That(actual).IsEqualTo(expected);`. An un-awaited + assertion silently passes. +- Arrange / Act / Assert, with the `// Arrange`, `// Act`, `// Assert` comments present. +- Test classes are `{SubjectName}Tests`, in the **same namespace** as the subject. +- Test methods are `{SubjectOrMemberUnderTest}_{Scenario}_{Expectation}`. Use `ctor` as the subject + for constructor tests. Methods, constructors, properties, operators, conversions and validation + hooks are all valid subjects. +- When any method called in the test body accepts a `CancellationToken`, take + `CancellationToken cancellationToken` as the **last parameter** and pass it through. + +```csharp +[Test] +public async Task ProcessAsync_GivenAnEmptyBatch_ReturnsZero(CancellationToken cancellationToken) +{ + // Arrange + var sut = new BatchProcessor(); + + // Act + var result = await sut.ProcessAsync([], cancellationToken); + + // Assert + await Assert.That(result).IsEqualTo(0); +} +``` + +For a suite that is not subject-based — a build, generator, workflow, package or full-system suite — +a broader, more truthful class name is better than a forced `{Subject}Tests`. Use TUnit display +names and categories to keep it navigable. + +When refactoring existing tests, bring them into compliance with all of the above while preserving +test intent. + +## Categories + +The detected test type becomes the baseline category automatically — `Api.IntegrationTests` yields +`Category=Integration`. Add further categories when they make a large suite easier to navigate; +they are what `--treenode-filter` property predicates select on. + +## Running tests + +```bash +dotnet test # the whole suite +dotnet test --treenode-filter "/*/*/LoginTests/*" # one class +dotnet test --treenode-filter "/*/*/*/*[Category=Unit]" +``` + +Prefer the repository's own task (`just test`, or equivalent) for a full run. + +The filter path is `////`, with `*` wildcards, `=`/`!=` property +predicates, `&`/`|` inside a single bracket group, and `**` only at the end. The full grammar, the +`--` separator rules across SDK versions, and the zero-tests checklist are in +[references/treenode-filter.md](references/treenode-filter.md). + +### "Zero tests ran" + +In order: + +1. Are you using `--treenode-filter` and not `--filter`? This is the cause most of the time. +2. Try `dotnet test --treenode-filter "..."`; on an older SDK use `dotnet test -- --treenode-filter "..."`. +3. Broaden to `/*/*/*/*`, then narrow one segment at a time. +4. Does the project actually contain `[Test]` methods? +5. Is the project **detected** as a test project? A name without the dot — `UnitTests` rather than + `Api.UnitTests` — gets no runner at all. Check with + `dotnet msbuild -getProperty:IsTestProject`. +6. Are all property predicates inside one bracket group, `[(A=1)&(B=2)]`? + +## Cross-platform requirement + +CI runs the full suite on Linux, so a test that only passes on Windows is a failing change. + +- Never hardcode a platform-specific path — no `C:\...`, no backslash-only separators, no + case-insensitive path assumptions. +- Build paths with `Path.Combine`, `Path.GetTempPath()`, `Path.DirectorySeparatorChar`, + `Path.GetFullPath()`. +- Feed production path-math code only native-format paths. Normalise a fixed Windows-style literal + to the current platform before passing it in. +- Do not rely on a case-insensitive filesystem, a `C:` drive, or trailing-separator behaviour. +- Keep `SyntaxTree` file paths and `build_property.*` values consistent and native when writing + analyzer tests. diff --git a/.agents/skills/purview-buildsdk-testing/references/treenode-filter.md b/.agents/skills/purview-buildsdk-testing/references/treenode-filter.md new file mode 100644 index 0000000..35d032b --- /dev/null +++ b/.agents/skills/purview-buildsdk-testing/references/treenode-filter.md @@ -0,0 +1,107 @@ +# `--treenode-filter` reference + +TUnit runs on Microsoft.Testing.Platform, not VSTest, so the VSTest `--filter` syntax does not +apply. Narrowing is done with `--treenode-filter`. + +## The path + +```text +//// +``` + +Each segment maps to one level of the tree: + +| Position | Matches | +| -- | -- | +| 1 | Assembly | +| 2 | Namespace | +| 3 | Class | +| 4 | Test name | + +```text +/*/*/LoginTests/* every test in LoginTests +/*/*/*/AcceptCookiesTest one test by name +/*/MyProject.Tests.Integration/*/* every test in a namespace +/*/*/*/* everything +``` + +## Operators + +### `*` — wildcard + +Matches a whole segment or part of one. + +```text +/*/*/LoginTests*/* +/*/*/MyProject.Tests.Api*/* +``` + +### `=` and `!=` — property predicates + +In the last segment, inside `[...]`. + +```text +/*/*/*/*[Category=Unit] +/*/*/*/*[Category!=Slow] +/*/*/*/*[Owner=*Team-Backend*] +``` + +### `&` and `|` — combining + +Inside a **single** bracket group, or a single parenthesised segment. + +```text +/*/*/*/*[(Category=Unit)&(Priority=High)] +/**[(Category=Unit)|(Priority=High)] +/*/*/(LoginTests)|(SignupTests)/* +``` + +### `**` — match all + +Any depth, but **only at the end** of the path. + +```text +/** +/MyAssembly/** +``` + +`/**/Path` is not valid. + +## Rules that catch people out + +- Only **one** property group `[...]` per path segment. `[Category=Smoke]|[Priority=High]` is not + valid; write `[(Category=Smoke)|(Priority=High)]`. +- `**` must be last. +- A segment left empty is not a wildcard — write `*`. + +## The `--` separator + +```bash +dotnet test --treenode-filter "/*/*/*/*[Category=Unit]" # preferred +dotnet test -- --treenode-filter "/*/*/*/*[Category=Unit]" # older SDKs +``` + +Newer .NET SDKs forward unrecognised arguments to the MTP runner directly. On an older SDK the `--` +separator is required, and omitting it produces an argument error rather than a filter. + +## Common selections + +| Intent | Filter | +| -- | -- | +| Everything | `/*/*/*/*` | +| All unit tests | `/*/*/*/*[Category=Unit]` | +| High-priority unit tests | `/*/*/*/*[(Category=Unit)&(Priority=High)]` | +| One class | `/*/*/MyFeatureTests/*` | +| A name pattern | `/*/*/*/MyScenario*` | +| Critical integration tests | `/*/MyProject.Tests.Integration/*/*[Priority=Critical]` | + +## Zero tests ran + +1. `--treenode-filter`, not `--filter`. +2. Try without the `--` separator first, then with it. +3. Broaden to `/*/*/*/*`, then narrow one segment at a time. +4. Confirm the project has `[Test]` methods. +5. Confirm the project is **detected** as a test project — + `dotnet msbuild -getProperty:IsTestProject`. A name without the dot, such as + `UnitTests` rather than `Api.UnitTests`, is not detected and gets no runner. +6. Confirm every property predicate sits in one bracket group. diff --git a/.agents/skills/purview-engineering-principles/SKILL.md b/.agents/skills/purview-engineering-principles/SKILL.md new file mode 100644 index 0000000..ba4dd9b --- /dev/null +++ b/.agents/skills/purview-engineering-principles/SKILL.md @@ -0,0 +1,171 @@ +--- +name: purview-engineering-principles +description: >- + Use when setting up, reviewing or rationalising a .NET repository against the Purview engineering + conventions - convention over configuration, derived identity, architecture as tests, warnings as + errors, proving consumer-facing claims, documentation ownership, test data builders, agent + legibility, and tiering verification by risk. Each rule names the failure it prevents and whether a + build or a reviewer enforces it. +metadata: + product: Purview engineering playbook + repository: purview-dev/engineering-playbook +--- + +# Purview engineering principles + +Ten rules, each with the failure it prevents and the mechanism that makes it true. The distinction +that matters throughout: **a convention a build can check is checked by the build; a convention only +a reviewer can check is labelled a review obligation and never described as enforced.** A rule that +claims enforcement it does not have is worse than no rule, because people stop looking. + +## 1. Convention over configuration + +A repository declares as little as possible. One MSBuild SDK supplies the defaults, and a project +file is short because the SDK already knows what a project of that shape needs. + +Configuration copied between repositories diverges. When the rule lives in one package, fixing it +once fixes it everywhere; when it lives in eleven project files, it is fixed eleven times — or, more +often, once. + +*Enforced by:* `` in a single +`Directory.Build.props`. The SDK detects project type from the filename and the `Sdk` attribute. + +*At your scale:* a single-project repository still benefits. The import is two lines and removes the +target framework, language version, nullable context, analyzer set and package metadata. + +## 2. Identity is derived, never repeated + +`RootNamespace` comes from `$(NamespacePrefix).$(ProjectName)`, and `AssemblyName` follows the +resolved `RootNamespace`. A name appears once — in the folder and file name — and everything else is +computed. + +A hand-typed namespace drifts from the assembly it lives in, and the drift is invisible until +something reflects over types: an architecture test, a serializer, a DI scan. + +*Enforced by:* the SDK during `.props` evaluation, plus a build-time check +(`PurviewProjectFileNameMismatch`) when a `.csproj` filename does not match its parent directory. + +*At your scale:* always. It costs nothing and removes a class of bug. + +## 3. Architecture is a test, not a diagram + +The dependencies a layer may have are asserted by tests that read the compiled assemblies. A +`ProjectReference` pointing the wrong way fails the build. + +A diagram is a claim about the past. Dependency direction erodes one convenient reference at a time, +and every such reference looks reasonable in isolation. + +*Enforced by:* a test project that references every layer and inspects their metadata references. + +*At your scale:* small repositories can skip it. Adopt it the moment a **second layer** exists — +that is when the wrong direction first becomes possible. + +## 4. The build is the only authority + +Style is enforced by analyzers and `EnforceCodeStyleInBuild`, and the SDK's own style policy is +re-validated on every compile, so a repository cannot quietly downgrade a rule. + +*At your scale:* always. If a rule matters and cannot be checked, say so plainly. + +## 5. Warnings are not acceptable output + +A clean build has no warnings. One that will never be fixed is either fixed, suppressed *where it +occurs* with a stated justification, or turned off deliberately. + +Warnings are a budget. A build that emits twenty is a build where the twenty-first is invisible — and +the twenty-first is the one that mattered. + +*Enforced by:* severity, partly. The SDK's shipped `.editorconfig` sets many rules to `error`. The +rest is review, backed by the pipeline running `dotnet build` with the repository's configuration. + +*At your scale:* a small repository can tolerate `warning`; a large one cannot. Decide deliberately +rather than by accumulation. + +## 6. Every claim about a consumer is proved by a consumer + +When a package documents how it must be used — a target framework, an asset, a guard, a workaround — +something in the repository builds that documented consumer and asserts the documented outcome. + +Consumer-facing documentation is the part of a library least likely to be exercised by its own tests, +because **the library's own build is not a consumer**. It drifts silently and fails in somebody +else's repository. + +*Enforced by:* a verification script that packs, then builds throwaway consumer projects against the +produced packages, asserting the happy path *and* the documented guard errors. `purview-dev/containers` +is the worked example (`just verify-consumers`), deliberately excluded from the default unit filter +because it restores from nuget.org. + +*At your scale:* only once a package is consumed outside its own repository. Before that, the +repository's own tests already are the consumer. + +## 7. Documentation is a build artefact of the repository + +The wiki lives in the repository it describes, in `docs/wiki`, and the repository owns its +`mkdocs.yml`, its navigation and its `edit_uri`. + +Documentation that lives elsewhere is updated elsewhere. A page with no owner and no review path is +wrong within one release. + +*Enforced by:* review, plus the navigation file — a page absent from `_Sidebar.md` is not published. + +*At your scale:* always. Even a repository with one page should own that page. + +## 8. Everything a test needs is built by one builder + +Domain content in a test comes from `TestBuilder`. A test never constructs a domain object inline, +and it overrides **only** the value it is proving. + +Inline construction encodes a type's shape into every test that touches it, so one constructor change +becomes a repository-wide edit. It also hides which values the test actually depends on, because +every value looks deliberate. + +*Enforced by:* review. + +*At your scale:* adopt from the first test. Retrofitting is a mechanical edit across every test file, +and exactly the kind of edit that is postponed forever. + +## 9. An agent is a reader with no memory + +A repository states its purpose, its layout and its conventions in `AGENTS.md`. **Nothing important +is discoverable only by reading the code.** + +A coding agent has the repository and nothing else — no colleague to ask, no institutional memory, no +intuition about which of two plausible patterns is intended. It will confidently generalise from +whatever it finds first. That is also every new-joiner failure mode; the agent just makes the cost +visible sooner, because an agent does not ask. + +*Enforced by:* review, plus publishing skills so agent-facing guidance tracks the library version +rather than a repository's memory of it. + +*At your scale:* always. `AGENTS.md` is cheaper than the questions it prevents. + +## 10. A change is verified in proportion to its risk + +Checks are tiered, and a repository declares which tiers its pipeline runs. A pull request runs the +fast tier; the slow tier runs deliberately, locally or on a dedicated runner. + +A pipeline that runs everything on every pull request gets made optional. One that runs too little +gets ignored. Tiering is what keeps the gate credible. + +*Enforced by:* the pipeline's `TestFilter` and the test project's own **name** — the SDK categorises +a `*UnitTests` project as `Unit`, so the tier is a property of the project rather than of a +hand-written attribute. + +*At your scale:* one tier is fine until the slow tests exist. Then split by project name, not by +attribute. + +## Applying these in review + +Three questions catch most of it: + +1. **Is this configuration, or is it a default the SDK should own?** If two repositories would copy + it, it belongs in the SDK. +2. **Is this rule enforced, or merely stated?** If stated, say so where it is stated. +3. **Could an agent — or a new joiner — find this without being told?** If not, it goes in + `AGENTS.md` or the wiki. + +## Related skills + +`purview-architecture` for project-shape decisions, `purview-architecture-testing` for turning a +boundary into a failing build, `purview-telemetry` and `purview-persistence` for the per-concern +guidance. diff --git a/.agents/skills/purview-persistence/SKILL.md b/.agents/skills/purview-persistence/SKILL.md new file mode 100644 index 0000000..4667fa2 --- /dev/null +++ b/.agents/skills/purview-persistence/SKILL.md @@ -0,0 +1,106 @@ +--- +name: purview-persistence +description: >- + Use when a .NET project actually persists data and you are choosing a storage approach, deciding + whether to abstract EF Core, or selecting a GUID primary-key strategy. Gives provider-specific + guidance — SQL Server and PostgreSQL differ and must not be treated alike — and stays silent for + projects that do not persist anything. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview persistence + +**Activation condition:** this skill applies only when persistence is real. If the project is a +stateless CLI, a pure library, or a service that calls an API and stores nothing, stop here — none of +this is relevant, and raising a database GUID strategy with that project is noise. + +Confirm persistence from the repository before proceeding (see `purview-architecture`): a data-access +`PackageReference`, a `DbContext`, a connection string, or an explicit requirement to store state. + +## 1. Shape the data access proportionally + +| Scale | Shape | +| -- | -- | +| Small | `DbContext` used directly from the code that needs it. No repository, no unit-of-work wrapper — `DbContext` already is both. | +| Medium | A boundary when domain rules must not know about EF Core, or when a query is complex enough to deserve a name. Often that is a few focused query types, not a generic `IRepository`. | +| Large | Independent infrastructure implementations where there genuinely are several, with the dependency direction enforced by a test. | + +**Do not wrap EF Core in a repository for testability.** `Purview.Containers` gives you a real +database in tests, which is both more faithful and less code than an abstraction built to be mocked. +A repository earns its place when you must swap the store or when `DbContext` is genuinely leaking +into domain decisions — not before. + +## 2. Local dependencies + +`Purview.Containers` provides Aspire resource integrations for the dependencies you need in +development and integration tests — including `MsSql`, `PostgreSql`, `MySql`, `Redis`, `Nats`, +`RabbitMq`, `Azurite` and an Azure Key Vault emulator. Prefer a real container over an in-memory +provider: the in-memory provider does not enforce the constraints that break you in production. + +## 3. GUID keys — only if GUIDs are keys + +Resolve these before recommending anything; most are answered by the repository: + +1. Are GUIDs primary keys, or merely external identifiers over an integer key? +2. Is the value generated by the application or by the database? +3. Which provider, and which version? +4. Does index locality matter at the expected write volume? +5. Must the identifier exist before the row is persisted? + +Question 5 usually decides it. If the application needs the identifier before `SaveChanges` — for an +outbox row, a correlation value, or to build an object graph client-side — the database cannot be the +generator, and the remaining questions are about *which* client-side strategy. + +### The rule that matters most + +**SQL Server and PostgreSQL do not benefit from the same sequential GUID implementation.** Their +index structures and their GUID comparison semantics differ, and a "sequential GUID" that helps one +can be no better than random on the other. Never carry a strategy from one to the other without +re-deriving it. + +- SQL Server specifics: `references/sqlserver.md` +- PostgreSQL specifics: `references/postgresql.md` +- Choosing between them, and when a custom generator is justified: `references/guid-generation.md` + +### Prefer built-in capability + +Use the framework or database capability when it is sufficient — `Guid.CreateVersion7()` on .NET 9+, +`NEWSEQUENTIALID()`, EF Core's own value generators, a provider's native UUIDv7 function. A custom +generator is a maintenance obligation and a correctness risk; introduce one only against a measured +requirement. + +**There is no Purview GUID generation package.** Do not reference one, and do not create one without +first establishing a concrete, measured need that the built-in options cannot meet. If such a +capability is added later, this skill is where the guidance for recommending it belongs. + +## 4. Value objects as keys + +`Purview.ValueObjects` gives strongly-typed identifiers that stop an `OrderId` being passed where a +`CustomerId` belongs. They need an EF Core value converter; configure it once in the model rather +than per property. + +This is about type safety at the boundary, not about the generation strategy — the two decisions are +independent. + +## 5. Migrations and schema + +- Migrations are code: reviewed, tested against a real container, and applied by a deliberate step — + not by the application at startup in production. +- Expect the schema to be ahead of or behind the code during a deployment; design changes to be + compatible in both directions. +- Keep the connection string out of source. Keep it out of telemetry too. + +## Checks before you answer + +- Have you confirmed persistence is actually required? +- If you are recommending a GUID strategy, do you know the provider *and* its version? +- Are you proposing an abstraction you can justify by something other than testability? +- Have you preferred a built-in capability over a custom generator? + +## See also + +- `purview-architecture` — context discovery and proportional structure +- `purview-telemetry` — instrument the data access too; it is where the latency is diff --git a/.agents/skills/purview-persistence/references/guid-generation.md b/.agents/skills/purview-persistence/references/guid-generation.md new file mode 100644 index 0000000..432053a --- /dev/null +++ b/.agents/skills/purview-persistence/references/guid-generation.md @@ -0,0 +1,93 @@ +# Choosing a GUID generation strategy + +The cross-provider decision procedure. Provider detail lives in `sqlserver.md` and `postgresql.md`. + +## Step 0 — is this question in scope? + +Ask nothing and recommend nothing unless **both** are true: + +- The project persists data, and +- GUIDs are (or are about to be) primary keys. + +A stateless CLI, a pure library, a service with an integer key, or a project using GUIDs only as +external correlation identifiers does not need this decision. Raising it anyway is the failure this +page is written to prevent. + +## Step 1 — must the identifier exist before persistence? + +This question decides more than any other. + +**Yes** — the application needs the value before `SaveChanges` for an outbox row, a correlation +identifier, an object graph built client-side, or an identifier returned before commit. The database +cannot be the generator. Go to step 2 with client-side options only. + +**No** — the database may generate it, and database generation is available to you. + +## Step 2 — which provider, and which version? + +Do not skip the version. It changes the available options materially: + +| Provider | Version matters because | +| -- | -- | +| SQL Server | `NEWSEQUENTIALID()` is long-standing; the comparison semantics that make byte-ordered UUIDs useless have never changed | +| PostgreSQL | Native `uuidv7()` exists only from 18; `gen_random_uuid()` is built in from 13 | +| .NET | `Guid.CreateVersion7()` exists from .NET 9 | + +**The two databases do not share an answer.** SQL Server compares `uniqueidentifier` with the last +six bytes most significant, so a byte-ordered UUIDv7 is effectively random to its clustered index. +PostgreSQL compares `uuid` in byte order, so UUIDv7 is genuinely sequential there. A strategy that is +optimal on one is unremarkable or wrong on the other. + +## Step 3 — does locality actually matter? + +Sequential keys exist to reduce index fragmentation and page splits under write pressure. If the +table takes tens of inserts a minute, this is not your problem and a random GUID is the simpler, +more private choice. + +Reach for a sequential strategy when you can point at write volume, measured fragmentation, page +splits, or buffer-pool pressure. "It's best practice" is not a measurement. + +Note also that on SQL Server the problem is often better solved by making the clustered index +something other than the GUID key — see `sqlserver.md`. + +## Step 4 — is unpredictability required? + +Sequential and time-ordered identifiers are guessable, and UUIDv7 discloses creation time to the +millisecond. Where an identifier appears in a URL, is handed to a third party, or acts as a +capability, use a random UUID — or keep the ordered value internal and expose a random surrogate. + +Never use `NEWSEQUENTIALID()` output as a secret. + +## Decision summary + +| Provider | Needs value before insert | Locality matters | Recommendation | +| -- | -- | -- | -- | +| SQL Server | No | Yes | `NEWSEQUENTIALID()` (EF Core's default for `Guid` keys) | +| SQL Server | Yes | Yes | EF Core `SequentialGuidValueGenerator` | +| SQL Server | Either | No | `Guid.NewGuid()` | +| PostgreSQL | Yes | Yes | `Guid.CreateVersion7()` with `ValueGeneratedNever()` | +| PostgreSQL | No | Yes | `uuidv7()` on 18+, otherwise generate v7 in the application | +| PostgreSQL | Either | No | `Guid.NewGuid()` or `gen_random_uuid()` | +| Any | Either | Either, but must be unpredictable | Random v4 | + +## Prefer built-in capability + +In order of preference: + +1. A framework capability — `Guid.CreateVersion7()`, EF Core's value generators. +2. A database capability — `NEWSEQUENTIALID()`, `uuidv7()`, `gen_random_uuid()`. +3. A reviewed, tested implementation of a published standard, when neither of the above is available + on your target versions. +4. A bespoke generator — only against a measured requirement the first three cannot meet. + +A hand-written sequential GUID generator is easy to get subtly wrong — endianness, clock regression, +counter rollover within a millisecond, and thread safety are all live hazards, and the failure mode +is duplicate or badly-ordered keys found in production. + +## There is no Purview GUID package + +No Purview library provides GUID generation today. Do not reference one and do not invent one. + +If a concrete need emerges that the built-in options cannot meet — established by measurement, not by +anticipation — that is the point at which creating one is worth discussing. Until then this guidance +is the deliverable, and this page is where a recommendation for such a package would be documented. diff --git a/.agents/skills/purview-persistence/references/postgresql.md b/.agents/skills/purview-persistence/references/postgresql.md new file mode 100644 index 0000000..cd557ec --- /dev/null +++ b/.agents/skills/purview-persistence/references/postgresql.md @@ -0,0 +1,98 @@ +# PostgreSQL UUID keys + +Read this only when the provider is PostgreSQL and UUIDs are primary keys. The SQL Server answer is +different — see `sqlserver.md`. + +## Why PostgreSQL differs from SQL Server + +Two differences drive everything on this page: + +1. **PostgreSQL has no clustered index.** Tables are heaps; the primary key is a separate B-tree. A + random UUID therefore does not cause row-level page splits the way it does on a clustered SQL + Server table. The cost is in the index: random inserts touch scattered index pages, which hurts + cache locality and WAL volume, but it is a smaller and differently-shaped problem. +2. **`uuid` compares in byte order.** Unlike SQL Server, PostgreSQL orders UUIDs by their bytes. A + UUID that is sequential in byte layout *is* sequential to the index. + +The consequence: **UUIDv7 works here.** It is the natural answer on PostgreSQL and the wrong answer +on SQL Server. Never carry a sequential-GUID choice across the two. + +## The options + +### `Guid.CreateVersion7()` — .NET 9+, application-generated + +The default recommendation for new PostgreSQL schemas with UUID keys. + +```csharp +public sealed class Order +{ + public Guid Id { get; init; } = Guid.CreateVersion7(); +} +``` + +- Time-ordered in byte layout, so index locality is good. +- The value exists before persistence, so outbox rows, correlation and client-side graphs all work. +- Standard (RFC 9562), so it is portable and readable by other systems. +- Embeds a millisecond timestamp — which is also its disclosure caveat. + +Tell EF Core the application supplies it: + +```csharp +builder.Property(o => o.Id).ValueGeneratedNever(); +``` + +Without that, EF Core may treat the key as store-generated and skip your value. + +### Database-generated UUIDv7 — PostgreSQL 18+ + +PostgreSQL 18 added a native `uuidv7()` function. Earlier versions do not have it; on 13–17 you have +`gen_random_uuid()` (v4, from the built-in pgcrypto-derived function) and nothing native for v7. + +**Check the server version before recommending `uuidv7()`.** It is the single most common way this +guidance goes wrong — the function simply does not exist on the version most deployments are running. + +```sql +-- PostgreSQL 18 and later only +ALTER TABLE orders ALTER COLUMN id SET DEFAULT uuidv7(); +``` + +Prefer application-side generation anyway unless rows are inserted by paths that bypass the +application, because the value-before-persistence property is worth more than the marginal +difference. + +### `gen_random_uuid()` — database-generated v4 + +Available without an extension from PostgreSQL 13. Random, so index locality is poor, but +unpredictable and simple. + +### `Guid.NewGuid()` — application-generated v4 + +Random and unpredictable. Fine at low to moderate write volume, which covers most applications. + +## Npgsql and EF Core notes + +- `Guid` maps to `uuid` natively. Do not store UUIDs as `text` or `varchar` — it costs space, loses + index efficiency and defeats comparison. +- Npgsql's `uuid` mapping is byte-order faithful, so a v7 value sorts in the index as you expect. +- If the application generates the key, set `ValueGeneratedNever()` so EF Core does not fetch a + store-generated value that does not exist. +- A legacy database may have UUIDs in `text` columns. Migrating to `uuid` is usually worth it, but it + is a data migration, not a model change. + +## Choosing + +| Situation | Use | +| -- | -- | +| New schema, .NET 9+, UUID keys | `Guid.CreateVersion7()`, `ValueGeneratedNever()` | +| .NET 8 or earlier, high write volume | A reviewed UUIDv7 implementation, or accept v4 after measuring | +| Rows inserted outside the application, PostgreSQL 18+ | `uuidv7()` as a column default | +| Rows inserted outside the application, PostgreSQL 13–17 | `gen_random_uuid()` | +| Key must be unpredictable | `Guid.NewGuid()` or `gen_random_uuid()` — v7 leaks creation time | +| Low or moderate volume | v4 — the locality difference will not be your bottleneck | + +## The timestamp caveat + +A UUIDv7 exposes when the row was created, to the millisecond. Where identifiers are public, that can +leak signup times, order rates or activity patterns — and it makes identifiers partially guessable by +time window. Where that matters, use a random UUID for the public identifier and keep the v7 value +internal, or use a surrogate. diff --git a/.agents/skills/purview-persistence/references/sqlserver.md b/.agents/skills/purview-persistence/references/sqlserver.md new file mode 100644 index 0000000..c728295 --- /dev/null +++ b/.agents/skills/purview-persistence/references/sqlserver.md @@ -0,0 +1,101 @@ +# SQL Server GUID keys + +Read this only when the provider is SQL Server and GUIDs are primary keys. The PostgreSQL answer is +different — see `postgresql.md`. + +## Why SQL Server is a special case + +A clustered index determines physical row order. A `uniqueidentifier` primary key is clustered by +default, so random GUIDs insert into the middle of the index, causing page splits, fragmentation and +a larger buffer-pool working set. On a write-heavy table this is measurable. + +The complication is that **SQL Server does not compare `uniqueidentifier` values the way the bytes +are laid out.** Its ordering weights the last six bytes (node field) most significantly, then earlier +groups. So a GUID that is sequential in big-endian byte order — including a standard UUIDv7 — is +**not** sequential under SQL Server's comparison. This is precisely why the SQL Server and PostgreSQL +answers diverge. + +## The options + +### `NEWSEQUENTIALID()` — database-generated + +A column default that produces values ascending in SQL Server's own ordering. Best index locality +available. + +```sql +ALTER TABLE [Orders] ADD CONSTRAINT [DF_Orders_Id] + DEFAULT NEWSEQUENTIALID() FOR [Id]; +``` + +EF Core, for a `Guid` key on SQL Server, configures this by default — `ValueGeneratedOnAdd` maps to +`NEWSEQUENTIALID()` through the default value-generation strategy. + +Trade-offs: + +- The value does not exist until the row is inserted. EF Core retrieves it, but you cannot build an + object graph around the key beforehand. +- Values are **not** unpredictable: the sequence restarts on service restart but is otherwise + guessable. Never use one as a security token or capability URL. +- It reveals rough insert ordering, which can be an information-disclosure concern on a public + surface. + +### EF Core's `SequentialGuidValueGenerator` — client-generated + +EF Core ships a value generator that produces GUIDs ordered for SQL Server's comparison semantics, +generated in the application. + +```csharp +builder.Property(o => o.Id) + .HasValueGenerator() + .ValueGeneratedOnAdd(); +``` + +Use this when the identifier must exist before `SaveChanges`. It keeps most of the index locality +while letting the application own the value. + +### `Guid.NewGuid()` — random + +Correct, portable and unpredictable; poor index locality. + +Appropriate when write volume is low (the fragmentation never matters), when the key is +non-clustered, or when unpredictability is a requirement. Do not dismiss it by default — for most +applications it is fine, and "most applications" includes more than people expect. + +### `Guid.CreateVersion7()` — .NET 9+ + +Ordered in byte layout, but **not** ordered under SQL Server's comparison. On SQL Server it behaves +roughly like a random GUID for clustered-index purposes. It is the right answer on PostgreSQL and the +wrong answer here, which is the trap this page exists to prevent. + +Use it on SQL Server only when you want the embedded timestamp for application reasons and accept +random-like insert locality. + +## Decoupling the key from the index + +Often the best answer is not a different GUID but a different index. Keep the GUID as the primary key +and make the clustered index something naturally ascending: + +```csharp +builder.HasKey(o => o.Id).IsClustered(false); +builder.HasIndex(o => o.CreatedAt).IsClustered(); +``` + +This removes the page-split problem entirely and leaves the key unpredictable. Consider it before +reaching for a sequential generator. + +## Choosing + +| Situation | Use | +| -- | -- | +| High insert volume, key may be database-generated | `NEWSEQUENTIALID()` | +| High insert volume, identifier needed before persisting | EF Core `SequentialGuidValueGenerator` | +| Key must be unpredictable (public surface, security boundary) | `Guid.NewGuid()`, with a non-clustered PK | +| Low or moderate volume | `Guid.NewGuid()` — do not optimise what does not hurt | +| You want a sortable timestamp in the key and accept the locality cost | `Guid.CreateVersion7()` | + +## Measure before optimising + +Index fragmentation from random GUIDs is real but frequently irrelevant. Before changing a key +strategy, confirm it matters: check page splits, fragmentation and buffer-pool pressure on the actual +table at the actual volume. Changing a primary key strategy on a live table is expensive; do it +against evidence. diff --git a/.agents/skills/purview-telemetry-design/SKILL.md b/.agents/skills/purview-telemetry-design/SKILL.md new file mode 100644 index 0000000..c8df5f8 --- /dev/null +++ b/.agents/skills/purview-telemetry-design/SKILL.md @@ -0,0 +1,96 @@ +--- +name: purview-telemetry-design +description: Design best practices for Purview.Telemetry.SourceGenerator interfaces. Use when deciding between a single telemetry interface and per-area interfaces, choosing telemetry names, designing multi-target methods, or reviewing telemetry structure in a C# project. +license: MIT +compatibility: C# projects using Purview.Telemetry.SourceGenerator. +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Design best practices for Purview Telemetry + +This skill guides the design of generated telemetry interfaces. + +## Single combined interface vs. per-area interfaces + +The generator lets you put all telemetry types on one interface, or split them by concern. + +### Combined interface + +Use one interface when: + +- A single service owns all telemetry and the operations naturally overlap. +- You want a single injection point and fewer constructor parameters. +- Multi-target methods emit Activity + Log + Metric from one call. + +Naming convention: + +```csharp +[ActivitySource] +[Logger] +[Meter] +public interface IOrderServiceTelemetry +``` + +### Per-area interfaces + +Split into separate interfaces when: + +- Different consumers need only one telemetry type (e.g., tests mock only logs). +- The team prefers clear separation between tracing, logging, and metrics. +- The service is large and each telemetry type has many operations. + +Naming convention: + +```csharp +[ActivitySource] public interface IOrderServiceTracing { ... } +[Logger] public interface IOrderServiceLogs { ... } +[Meter] public interface IOrderServiceMetrics { ... } +``` + +Default recommendation: start with one combined interface per service. Split only when a clear boundary emerges. + +## Naming conventions + +Default to the OpenTelemetry naming convention (`NamingConvention.OpenTelemetry`). + +- ActivitySource names: preserve assembly casing (e.g., `MyApp`). +- Activity names: descriptive, sentence-case-ish, no spaces: `placing-order`. +- Log message templates: human-readable, with `{PascalCase}` placeholders. +- Metric instrument names: hierarchical, dot-separated, lowercase: `myapp.orders.placed`. +- Tag/baggage keys: `snake_case`. + +Only use `NamingConvention.Legacy` for exact v3 backward compatibility. + +## Multi-target method design + +Combine attributes on one method when a single operation emits multiple telemetry types: + +```csharp +[Activity] +[Info] +[AutoCounter] +Activity? PlacingOrder(int orderId); +``` + +Rules: + +- Methods that return `Activity?` create an Activity. +- Methods that return `void` typically add events, log, or record metrics. +- Pass the `Activity?` from the starting method to event/context methods. + +## Parameter attributes + +| Attribute | Use | +|---|---| +| `[Baggage]` | Adds the parameter to Activity baggage; also included in logs. | +| `[Tag]` | Adds the parameter as an Activity tag. | +| `[ExcludeTargets(Targets.Activities)]` | Excludes the parameter from Activity generation only. | +| `[ExpandEnumerable(maximumValueCount: N)]` | Expands an array/IEnumerable into indexed log properties. | + +## Testing + +Inject the interface in your service and mock it in tests. The generated interface has no implementation code in the interface itself, so standard mocking frameworks (NSubstitute, Moq, TUnitMocks) work directly. + +See [references/REFERENCE.md](references/REFERENCE.md) for a decision matrix and naming examples. diff --git a/.agents/skills/purview-telemetry-design/references/REFERENCE.md b/.agents/skills/purview-telemetry-design/references/REFERENCE.md new file mode 100644 index 0000000..b15cfcf --- /dev/null +++ b/.agents/skills/purview-telemetry-design/references/REFERENCE.md @@ -0,0 +1,101 @@ +# Design reference + +## Interface shape decision matrix + +| Situation | Recommendation | +|---|---| +| Small service with < 10 telemetry operations | One combined `ITelemetry` | +| Service only logs | `ILogs` | +| Service only traces | `ITracing` | +| Service only emits metrics | `IMetrics` | +| Large service with distinct teams owning logs vs. metrics | Split into per-area interfaces | +| Need to mock only one telemetry type in many tests | Split into per-area interfaces | + +## Naming examples + +### OpenTelemetry mode (default) + +```csharp +[ActivitySource] // name = "MyApp" +[Logger] +[Meter] +public interface IOrderServiceTelemetry +{ + [Activity] + Activity? PlacingOrder(int orderId); // activity name = "placing-order" + + [Info("Placing order {OrderId}")] + void PlacingOrderInfo(int orderId); // log + + [AutoCounter] + void OrderPlaced(); // instrument = "myapp.orders.order-placed" +} +``` + +### Legacy mode + +```csharp +[assembly: TelemetryGeneration(NamingConvention = NamingConvention.Legacy)] +``` + +Use only when exact v3 names must be preserved. + +## Multi-target example + +```csharp +[ActivitySource] +[Logger] +[Meter] +public interface IOrderServiceTelemetry +{ + // Starts Activity, logs Info, increments Counter + [Activity(ActivityKind.Internal)] + [Info("Placing order {OrderId} for {CustomerName}")] + [AutoCounter] + Activity? PlacingOrder(int orderId, string customerName, [Baggage] string region); + + // Adds event to the Activity and logs Trace + [Event] + [Trace] + void OrderValidated(Activity? activity, int orderId); + + // Records histogram + [Histogram] + void OrderProcessingTime(double latencyMs); +} +``` + +## Parameter attribute examples + +```csharp +[Activity] +[Info] +Activity? FetchingOrder( + int orderId, + [Baggage] string region, // carried in Activity baggage + [Tag] string tenantId); // Activity tag only + +[Event] +void OrderLoaded( + Activity? activity, + int orderId, + [ExcludeTargets(Targets.Activities)] string[] rawLines); // not in Activity +``` + +## Testing example + +```csharp +public class OrderServiceTests +{ + [Test] + public void PlaceOrder_EmitsTelemetry() + { + var telemetry = Substitute.For(); + var service = new OrderService(telemetry); + + service.PlaceOrder(42, "Alice", "EMEA"); + + telemetry.Received().PlacingOrder(42, "Alice", "EMEA"); + } +} +``` diff --git a/.agents/skills/purview-telemetry-diagnostics/SKILL.md b/.agents/skills/purview-telemetry-diagnostics/SKILL.md new file mode 100644 index 0000000..fec24d2 --- /dev/null +++ b/.agents/skills/purview-telemetry-diagnostics/SKILL.md @@ -0,0 +1,168 @@ +--- +name: purview-telemetry-diagnostics +description: >- + Use when a Purview.Telemetry.SourceGenerator build reports a TSG diagnostic - TSG1xxx general, + TSG2xxx logging, TSG3xxx activities, TSG4xxx metrics - to find what triggered it and how to fix it, + or when deciding whether a rule can be suppressed. +license: MIT +compatibility: C# projects using Purview.Telemetry.SourceGenerator. +metadata: + author: Purview-Dev + product: Purview.Telemetry.SourceGenerator + repository: purview-dev/telemetry-sourcegenerator +--- + +# Purview.Telemetry.SourceGenerator diagnostics + +One Roslyn analyzer, `TelemetryDiagnosticAnalyzer`, validates telemetry interfaces and raises `TSG` +diagnostics in four families: + +| Family | Covers | +| --- | --- | +| `TSG1xxx` | general — interface and method shape, targets, framework support | +| `TSG2xxx` | logging | +| `TSG3xxx` | activities | +| `TSG4xxx` | metrics | + +Every rule is tracked in `AnalyzerReleases.Shipped.md` / `AnalyzerReleases.Unshipped.md` with its ID, +category and severity, validated by the Roslyn release-tracking analyzers on every build. A new or +changed diagnostic must be recorded there in the same change. + +## General — `TSG1xxx` + +| ID | Severity | Reported when | +| --- | --- | --- | +| `TSG1000` | Error | An unexpected exception escaped the generation pipeline. | +| `TSG1001` | Error | A method on a multi-target interface has no explicit generation attribute — inference is not supported there. | +| `TSG1002` | Error | Two attributes from the same target family on one method. Only one Activity, Logging or Metrics attribute per method. | +| `TSG1003` | Error | Two methods on the interface share a name. | +| `TSG1004` | Error | Generic interfaces are not supported. | +| `TSG1005` | Error | Generic methods are not supported. | +| `TSG1006` | Warning | `ExcludeTargets` names a target not present on the method. | +| `TSG1007` | Warning | `ExcludeTargets` leaves a target with an empty or invalid parameter set. | +| `TSG1008` | Warning | An `Activity`-typed parameter on a method with no `[Activity]`/`[Event]`/`[Context]`. | +| `TSG1010` | Error | A method carries an attribute for a target family the interface does not register. | +| `TSG1011` | Error | The compilation targets neither .NET 8+ nor .NET Framework 4.8+. Define `PURVIEW_TELEMETRY_NON_NULLABLE` to opt out. | + +## Logging — `TSG2xxx` + +| ID | Severity | Reported when | +| --- | --- | --- | +| `TSG2000` | Error | A non-scoped log method has more than one `Exception` parameter. | +| `TSG2001` | Error | A v1-mode log method has more than 6 non-exception parameters. | +| `TSG2002` | Info | v1 generation inferred `Error` from a lone `Exception` parameter with no explicit level. | +| `TSG2003` | Warning | No reference to `Microsoft.Extensions.Logging.ILogger`; log generation is skipped. | +| `TSG2004` | Error | Ordinal and named placeholders mixed in one message template. | +| `TSG2005` | Error | The maximum ordinal placeholder exceeds the parameter count. | +| `TSG2006` | Error | `[LogProperties]` and `[ExpandEnumerable]` on the same parameter. | +| `TSG2007` | Warning | A scoped log declares a `LogLevel`; it is ignored. | +| `TSG2008` | Warning | `[ExpandEnumerable]` has a `MaximumValueCount` above the recommended default of 5. | +| `TSG2021` | Error | A log method returns something other than `void` or `IDisposable`. (`Activity` is allowed when the method is also an Activity method.) | + +## Activities — `TSG3xxx` + +| ID | Severity | Reported when | +| --- | --- | --- | +| `TSG3000` | Warning | A baggage parameter is not a string; `ToString()` will be called. | +| `TSG3001` | Warning | No activity source name available anywhere; generation defaults to `purview`. | +| `TSG3002` | Error | An Activity/Event method returns something other than `void` or `Activity`. | +| `TSG3003` | Error | More than one parameter maps to the same reserved destination. | +| `TSG3004` | Error | An `Activity`-destination parameter on an Activity method — only valid on `[Event]`. | +| `TSG3005` | Error | A `timestamp` parameter on a method that is not `[Event]`. | +| `TSG3006` | Error | A start-time parameter on a Create-activity or Event method. | +| `TSG3007` | Error | A parent-context or parent-id parameter on an event. | +| `TSG3008` | Error | Activity-links parameters on an event or context method. | +| `TSG3009` | Error | An activity-tags parameter on a context method. | +| `TSG3010` | Error | An escaped parameter is not a boolean. | +| `TSG3011` | Error | Escaped parameters on an Activity or Context method — Events only. | +| `TSG3012` | Info | No Activity methods defined; `Activity.Current` is assumed. | +| `TSG3013` | Warning | An Activity method does not return the created `Activity`. | +| `TSG3014` | Warning | An Event/Context method has no `Activity` parameter. Controlled by `ActivitySourceGeneration.GenerateDiagnosticsForMissingActivity` (default `true`). | +| `TSG3015` | Info | `Activity` should be the first parameter. Same opt-in as `TSG3014`. | +| `TSG3016` | Error | A status-description parameter is not a string. | +| `TSG3017` | Error | Status-description parameters on an Activity or Context method — Events only. | +| `TSG3021` | Info | An `[Event]` records an exception under the OpenTelemetry rules but is not named `"exception"`. | +| `TSG3022` | Warning | An Activity method returns non-nullable `Activity`. | + +## Metrics — `TSG4xxx` + +| ID | Severity | Reported when | +| --- | --- | --- | +| `TSG4000` | Error | A method on a Metrics interface has no instrument attribute and is not excluded. | +| `TSG4001` | Error | A metrics-owned method returns something other than `void` or `bool`. | +| `TSG4002` | Error | An auto-increment instrument also declares a measurement parameter. | +| `TSG4003` | Error | More than one measurement value. | +| `TSG4004` | Error | A non-auto-increment instrument method has no measurement parameter. | +| `TSG4005` | Error | An observable instrument is not declared as `Func`. | +| `TSG4006` | Error | The measurement type is not `byte`, `short`, `int`, `long`, `double`, `float`, `decimal`, `Measurement` or `IEnumerable>`. | +| `TSG4007` | Error | An observable metric returns `bool`. | +| `TSG4008` | Error | An `AutoCounter` does not return `void`. | +| `TSG4009` | Warning | The instrument name matches the instrument type name. Name what is measured, not the instrument. | + +## The ones people actually hit + +### `TSG1001` — inference is off on a multi-target interface + +A method on an interface carrying `[ActivitySource]` + `[Logger]` + `[Meter]` has no explicit +attribute. Add one per target the method should emit, or mark it `[Exclude]`: + +```csharp +[ActivitySource("MyApp")] +[Logger] +[Meter] +interface IMyTelemetry +{ + [Info] // explicit target + void ProcessItem(int id); +} +``` + +### `TSG3013` / `TSG3014` — the Activity went missing + +Return the created `Activity` (as `Activity?`, per `TSG3022` — it is null when no listeners are +active) and pass it into the Event and Context methods that should attach to it. Both are +best-practice rules controlled by `ActivitySourceGeneration.GenerateDiagnosticsForMissingActivity`. + +### `TSG3021` — exception event name + +Name the event `exception` so the OpenTelemetry exception tags (`exception.type`, +`exception.message`, `exception.stacktrace`, `exception.escaped`) attach to *that* event rather than +a separate one: + +```csharp +[Event(Name = "exception")] // the event carries the exception tags +void FailedToRetrieve(Activity? activity, Exception exception); +``` + +A `Name` on a **logging** attribute does not name the event — it renames the log entry: + +```csharp +[Event] // event name is still the method name +[Error(Name = "exception")] // this renames the log entry +void FailedToRetrieve(Activity? activity, Exception exception); +``` + +The rule is not raised when the exception is not recorded under the OpenTelemetry rules: +`[Event(UseRecordExceptionRules = false)]`, a `[Baggage]` exception parameter, or an exception +parameter excluded from the Activities target (`[ExcludeTargets(Targets.Activities)]`). + +### `TSG2008` — unbounded enumeration + +`[ExpandEnumerable]` with `MaximumValueCount` above 5. Bring it back to the default unless you have +measured the cost. + +### `TSG4002` — auto-increment counter with a measurement + +`[AutoCounter]` — equivalently `[Counter(AutoIncrement = true)]` — must declare no measurement +parameter. The measurement is fixed at 1. + +### `TSG1003` — duplicate method names + +The method name becomes a member on the generated implementation class, so two methods cannot share +one. Rename. + +## Related skills + +`purview-telemetry-quickstart` to get an interface wired up, `purview-telemetry-design` for +structuring interfaces, and `purview-telemetry-migration` for converting existing `ILogger`, +`ActivitySource` or instrument code. diff --git a/.agents/skills/purview-telemetry-migration/SKILL.md b/.agents/skills/purview-telemetry-migration/SKILL.md new file mode 100644 index 0000000..b9a01fa --- /dev/null +++ b/.agents/skills/purview-telemetry-migration/SKILL.md @@ -0,0 +1,81 @@ +--- +name: purview-telemetry-migration +description: Migrate existing ILogger, ActivitySource, or System.Diagnostics.Metrics instrument code to Purview.Telemetry.SourceGenerator interfaces. Use when converting hand-written telemetry to generated telemetry interfaces, or when a user wants to replace ILogger, ActivitySource, or Counter/Histogram/UpDownCounter usage with Purview telemetry. +license: MIT +compatibility: C# projects using Purview.Telemetry.SourceGenerator. +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Migrate to Purview Telemetry Source Generator + +Convert existing telemetry code to generated Purview telemetry interfaces. + +## Primary path: use the IDE refactorings + +The source generator ships with Roslyn code refactorings. Right-click a class containing hand-written telemetry and choose: + +- **Convert ILogger to ILogs** — replaces `ILogger` fields and constructor parameters with a generated logging interface. +- **Convert ActivitySource to ITracing** — replaces `ActivitySource` fields and `StartActivity` calls with a generated tracing interface. +- **Convert Metrics to IMetrics** — replaces `Counter`, `Histogram`, and `UpDownCounter` fields and calls with a generated metrics interface. +- **Convert all telemetry to ITelemetry** — combines all three into one interface when the class uses multiple telemetry types. + +Each refactoring offers scopes: **class**, **document**, **project**, and **solution**. + +## What the refactorings produce + +The generated interface uses attributes from the `Purview.Telemetry` namespace: + +```csharp +using Purview.Telemetry; + +[Logger] +public interface IOrderServiceLogs +{ + [Info] + void OrderPlaced(int orderId, string customerName); +} +``` + +The original class is rewritten to use the interface: + +```csharp +public class OrderService(IOrderServiceLogs logger) +{ + public void PlaceOrder(int orderId, string customerName) + { + logger.OrderPlaced(orderId, customerName); + } +} +``` + +## Manual fallback + +If the refactoring does not cover a call pattern, convert it manually: + +1. Identify the telemetry type (Logging, Activity, Metric). +2. Create a new interface with the matching class-level attribute (`[Logger]`, `[ActivitySource]`, `[Meter]`). +3. Add a method for each distinct operation, using the method-level attribute from the mapping below. +4. Replace the hand-written call with the interface method. +5. Register the interface in DI with `services.Add{InterfaceNameWithoutI}()`. + +## Attribute mapping + +| Hand-written telemetry | Generated attribute | +|---|---| +| `ILogger.LogInformation(...)` | `[Info]` | +| `ILogger.LogDebug(...)` | `[Debug]` | +| `ILogger.LogTrace(...)` | `[Trace]` | +| `ILogger.LogWarning(...)` | `[Warning]` | +| `ILogger.LogError(...)` | `[Error]` | +| `ILogger.LogCritical(...)` | `[Critical]` | +| `ActivitySource.StartActivity(...)` | `[Activity]` | +| `activity.AddEvent(...)` | `[Event]` | +| `activity.AddBaggage(...)` / `SetBaggage(...)` | `[Context]` with `[Baggage]` parameter | +| `activity.SetTag(...)` | `[Context]` with `[Tag]` parameter | +| `Counter.Add(...)` | `[Counter]` or `[AutoCounter]` for `Add(1)` | +| `Histogram.Record(...)` | `[Histogram]` | +| `UpDownCounter.Add(...)` | `[UpDownCounter]` | + +See [references/REFERENCE.md](references/REFERENCE.md) for detailed before/after examples and common conversions. diff --git a/.agents/skills/purview-telemetry-migration/references/REFERENCE.md b/.agents/skills/purview-telemetry-migration/references/REFERENCE.md new file mode 100644 index 0000000..d85eb73 --- /dev/null +++ b/.agents/skills/purview-telemetry-migration/references/REFERENCE.md @@ -0,0 +1,181 @@ +# Migration reference + +Detailed before/after examples for migrating hand-written telemetry to Purview telemetry interfaces. + +## ILogger to generated logging interface + +### Before + +```csharp +using Microsoft.Extensions.Logging; + +public class OrderService(ILogger logger) +{ + public void PlaceOrder(int orderId, string customerName) + { + logger.LogInformation("Placing order {OrderId} for {CustomerName}", orderId, customerName); + } + + public void CancelOrder(int orderId, Exception ex) + { + logger.LogError(ex, "Order {OrderId} cancelled", orderId); + } +} +``` + +### After + +```csharp +using Purview.Telemetry; + +[Logger] +public interface IOrderServiceLogs +{ + [Info("Placing order {OrderId} for {CustomerName}")] + void OrderPlaced(int orderId, string customerName); + + [Error("Order {OrderId} cancelled")] + void OrderCancelled(Exception ex, int orderId); +} + +public class OrderService(IOrderServiceLogs logger) +{ + public void PlaceOrder(int orderId, string customerName) + { + logger.OrderPlaced(orderId, customerName); + } + + public void CancelOrder(int orderId, Exception ex) + { + logger.OrderCancelled(ex, orderId); + } +} +``` + +## ActivitySource to generated tracing interface + +### Before + +```csharp +using System.Diagnostics; + +public class OrderService +{ + static readonly ActivitySource _source = new("OrderService"); + + public void PlaceOrder(int orderId) + { + using var activity = _source.StartActivity("PlaceOrder", ActivityKind.Internal); + activity?.AddEvent(new ActivityEvent("Validated")); + } +} +``` + +### After + +```csharp +using System.Diagnostics; +using Purview.Telemetry; + +[ActivitySource] +public interface IOrderServiceTracing +{ + [Activity] + Activity? PlaceOrder(int orderId); + + [Event] + void Validated(Activity? activity); +} + +public class OrderService(IOrderServiceTracing tracing) +{ + public void PlaceOrder(int orderId) + { + using var activity = tracing.PlaceOrder(orderId); + tracing.Validated(activity); + } +} +``` + +## Metrics to generated metrics interface + +### Before + +```csharp +using System.Diagnostics.Metrics; + +public class OrderService(IMeterFactory meterFactory) +{ + readonly Counter _orders = meterFactory.Create("OrderService").CreateCounter("orders-placed"); + readonly Histogram _latency = meterFactory.Create("OrderService").CreateHistogram("processing-latency-ms"); + + public void PlaceOrder(int orderId, double latencyMs) + { + _orders.Add(1); + _latency.Record(latencyMs); + } +} +``` + +### After + +```csharp +using Purview.Telemetry; + +[Meter] +public interface IOrderServiceMetrics +{ + [AutoCounter] + void OrderPlaced(); + + [Histogram] + void OrderProcessingTime(double latencyMs); +} + +public class OrderService(IOrderServiceMetrics metrics) +{ + public void PlaceOrder(int orderId, double latencyMs) + { + metrics.OrderPlaced(); + metrics.OrderProcessingTime(latencyMs); + } +} +``` + +## Combined telemetry interface + +When a class uses multiple telemetry types, prefer the combined refactoring: + +```csharp +using System.Diagnostics; +using Purview.Telemetry; + +[ActivitySource] +[Logger] +[Meter] +public interface IOrderServiceTelemetry +{ + [Activity] + [Info] + [AutoCounter] + Activity? PlacingOrder(int orderId); + + [Event] + void OrderShipped(Activity? activity, int orderId); + + [Histogram] + void OrderProcessingTime(double latencyMs); +} +``` + +## Common conversions + +| Original code | Generated method signature | +|---|---| +| `logger.LogError(ex, "Failed {OrderId}", orderId)` | `[Error("Failed {OrderId}")] void Failed(Exception exception, int orderId);` | +| `activity.AddEvent(new ActivityEvent("Loaded"))` | `[Event] void Loaded(Activity? activity);` | +| `activity.SetBaggage("tenant", tenantId)` | `[Context] void SetTenant(Activity? activity, [Baggage] string tenantId);` | +| `activity.SetTag("tenant", tenantId)` | `[Context] void SetTenant(Activity? activity, [Tag] string tenantId);` | +| `counter.Add(1, new KeyValuePair("region", region))` | `[AutoCounter] void RequestCompleted([Tag] string region);` | +| `histogram.Record(value, new KeyValuePair("region", region))` | `[Histogram] void RequestDuration(long value, [Tag] string region);` | +| `upDownCounter.Add(1)` | `[UpDownCounter] void Increment();` | diff --git a/.agents/skills/purview-telemetry-multi-targeting/SKILL.md b/.agents/skills/purview-telemetry-multi-targeting/SKILL.md new file mode 100644 index 0000000..d9654ab --- /dev/null +++ b/.agents/skills/purview-telemetry-multi-targeting/SKILL.md @@ -0,0 +1,224 @@ +--- +name: purview-telemetry-multi-targeting +description: >- + Use when one Purview.Telemetry.SourceGenerator method should emit several kinds of telemetry at + once - an Activity, a log entry and a metric from a single call. Covers combining [ActivitySource], + [Logger] and [Meter] on an interface, the supported per-method attribute combinations, excluding a + parameter from one target with [ExcludeTargets], return types, and why inference turns off. +license: MIT +compatibility: C# projects using Purview.Telemetry.SourceGenerator. +metadata: + author: Purview-Dev + product: Purview.Telemetry.SourceGenerator + repository: purview-dev/telemetry-sourcegenerator +--- + +# Multi-targeting + +One interface — and one *method* — can generate Activities, logs and metrics together. A single call +then emits all of them, from one set of parameters. + +```csharp +using Purview.Telemetry; + +[ActivitySource("OrderService")] +[Logger] +[Meter] +interface IOrderTelemetry +{ + // Activity + Info log + counter increment, from one call. + [Activity] + [Info] + [AutoCounter] + Activity? ProcessingOrder([Baggage] int orderId, [Tag] string customerName); + + // ActivityEvent + Debug log. + [Event] + [Debug] + void OrderValidated(Activity? activity, decimal amount); + + // Log only. + [Warning] + void OrderRejected(int orderId, string reason); + + // Metric only. + [Histogram] + void OrderProcessingDuration([InstrumentMeasurement] int milliseconds); +} +``` + +```csharp +using var activity = telemetry.ProcessingOrder(123, "Acme Corp"); +// Activity created and started, Info log written, counter incremented by 1. +``` + +## What you may combine + +| Combination | Example | +| --- | --- | +| Activity + Log | `[Activity]` + `[Info]` | +| Activity + Metric | `[Activity]` + `[AutoCounter]` | +| Log + Metric | `[Info]` + `[Histogram]` | +| Activity + Log + Metric | `[Activity]` + `[Info]` + `[AutoCounter]` | +| Event + Log | `[Event]` + `[Debug]` | +| Context + Log | `[Context]` + `[Trace]` | + +**One attribute per family, per method** — more is `TSG1002`: + +- Activities: one of `[Activity]`, `[Event]`, `[Context]`. +- Logging: one of `[Log]`, `[Trace]`, `[Debug]`, `[Info]`, `[Warning]`, `[Error]`, `[Critical]`. +- Metrics: one of `[Counter]`, `[AutoCounter]`, `[Histogram]`, `[UpDownCounter]`, or an observable + variant. + +```csharp +[Activity] +[Event] // TSG1002 +void InvalidMethod(Activity? activity); + +[Counter] +[Histogram] // TSG1002 +void InvalidMethod([InstrumentMeasurement] int value); +``` + +## Inference turns off — and that is the point + +An interface with **more than one** class-level attribute (`[ActivitySource]`, `[Logger]`, `[Meter]`) +disables inference. Every method must then declare its targets explicitly, or be `[Exclude]`d: + +```csharp +[ActivitySource("MyApp")] +[Logger] +[Meter] +interface IMyTelemetry +{ + void ProcessItem(int id); // TSG1001 - no explicit attribute + + [Info] + void ProcessItemLogged(int id); // fine + + [Exclude] + void NotTelemetry(int id); // fine +} +``` + +A single-target interface keeps inference. The alternative — guessing which of three families an +unannotated method meant — would silently emit the wrong telemetry, so the generator refuses. + +## Excluding a parameter from one target + +`[ExcludeTargets(Targets.X)]` on a parameter keeps it out of specific families. Use it when a value +is useful in one place and harmful in another — a verbose message is wasted as a metric tag, and an +internal reference may not belong in Activity baggage: + +```csharp +[ActivitySource("PaymentService")] +[Logger] +[Meter] +interface IPaymentTelemetry +{ + [Activity] + [Info] + [Counter] + Activity? ProcessingPayment( + [Baggage] Guid paymentId, + + [ExcludeTargets(Targets.Metrics)] + string processingMessage, + + [InstrumentMeasurement] + decimal amount, + + [ExcludeTargets(Targets.Activities)] + [Tag] // still in logs and metrics + string internalReference + ); +} +``` + +```csharp +[Flags] +public enum Targets +{ + None = 0, + Activities = 1, + Logging = 2, + Metrics = 4, + All = Activities | Logging | Metrics +} +``` + +Flags combine, so `[ExcludeTargets(Targets.Activities | Targets.Metrics)]` leaves a parameter to +logging alone. + +## Return types + +| Shape | Return | +| --- | --- | +| Activity + anything else | `Activity?` — so the caller can dispose it and pass it to Event/Context methods | +| Log + Metric, no Activity | `void`, or `IDisposable?` for a scoped log | +| Event/Context + Log | `void` | + +```csharp +[Activity] +[Info] +[AutoCounter] +Activity? ProcessingOrder(int orderId); + +[Info] +[AutoCounter] +IDisposable? ProcessingBatch(int batchId); // scoped log + counter + +[Event] +[Debug] +void OrderCompleted(Activity? activity, decimal total); +``` + +Return `Activity?`, not `Activity`: it is null when no listeners are active (`TSG3022`). + +## Patterns worth copying + +Full observability on one call: + +```csharp +[Activity] +[Info] +[AutoCounter] +Activity? ProcessingRequest( + [Baggage] string requestId, + [Tag] string endpoint, + [Tag] string method +); +``` + +Selective parameter routing: + +```csharp +[Activity] +[Info] +[Counter] +Activity? ApiCall( + [Baggage] string traceId, // Activity baggage only + [ExcludeTargets(Targets.Metrics)] string verboseMessage, // Activity + log + [InstrumentMeasurement] + [ExcludeTargets(Targets.Activities | Targets.Logging)] + int callCount, // metrics only + [Tag] string endpoint // all three +); +``` + +## Why bother + +One method definition instead of three call sites; the same parameters feed every family, so they +cannot drift; everything is emitted together; a change lands once; and it is one call rather than +several. + +## Diagnostics you will meet here + +| Diagnostic | Fix | +| --- | --- | +| `TSG1001` | Add an explicit target attribute, or `[Exclude]`. | +| `TSG1002` | One Activity, Logging and Metrics attribute per method. | +| `TSG1006` | `[ExcludeTargets]` names a target the method does not have — remove it, or add the target attribute. | +| `TSG1007` | Exclusions left a target with no valid parameters — adjust them. | + +The `purview-telemetry-diagnostics` skill carries the full `TSG` catalogue. diff --git a/.agents/skills/purview-telemetry-quickstart/SKILL.md b/.agents/skills/purview-telemetry-quickstart/SKILL.md new file mode 100644 index 0000000..1743ffe --- /dev/null +++ b/.agents/skills/purview-telemetry-quickstart/SKILL.md @@ -0,0 +1,140 @@ +--- +name: purview-telemetry-quickstart +description: Get started with Purview.Telemetry.SourceGenerator in a C# project. Use when adding the package, creating a first telemetry interface, registering it in DI, or asking how to begin with Purview telemetry. +license: MIT +compatibility: C# projects using Purview.Telemetry.SourceGenerator. +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Quickstart: Purview Telemetry Source Generator + +This skill helps you bootstrap telemetry generation in a C# project using `Purview.Telemetry.SourceGenerator`. + +## Installation + +Add the analyzer package to your project: + +```xml + + all + analyzers + +``` + +Also add runtime dependencies for the telemetry types you use: + +- `System.Diagnostics.DiagnosticSource` for Activities +- `Microsoft.Extensions.Logging.Abstractions` for ILogger +- `System.Diagnostics.Metrics` for metrics (built-in on .NET 6+) + +## Define a telemetry interface + +Create a `public interface` and decorate it with class-level attributes. The generator creates the implementation and a DI registration extension. + +### Single-target examples + +```csharp +using Purview.Telemetry; + +[Logger] +public interface IOrderServiceLogs +{ + [Info] + void OrderPlaced(int orderId, string customerName); + + [Warning] + void OrderNotFound(int orderId); +} +``` + +```csharp +[ActivitySource] +public interface IOrderServiceTracing +{ + [Activity] + Activity? PlacingOrder(int orderId); + + [Event] + void OrderValidated(Activity? activity, int orderId); +} +``` + +```csharp +[Meter] +public interface IOrderServiceMetrics +{ + [Counter] + void OrderPlaced(int itemsInOrder); + + [Histogram] + void OrderProcessingTime(long milliseconds); +} +``` + +### Multi-target example + +One interface can generate Activities, Logging, and Metrics from the same methods: + +```csharp +[ActivitySource] +[Logger] +[Meter] +public interface IOrderServiceTelemetry +{ + [Activity] + [Info] + [AutoCounter] + Activity? PlacingOrder(int orderId, [Baggage] string region); + + [Event] + [Trace] + void OrderProcessed(Activity? activity, long durationMs); + + [Warning] + void OrderFailed(int orderId, Exception exception); +} +``` + +## Register with DI + +The generator creates an extension method named `Add{InterfaceNameWithoutI}()`: + +```csharp +services.AddOrderServiceTelemetry(); +``` + +Inject the interface into your service: + +```csharp +public class OrderService(IOrderServiceTelemetry telemetry) +{ + public void PlaceOrder(int orderId) + { + using var activity = telemetry.PlacingOrder(orderId, "EMEA"); + // ... + telemetry.OrderProcessed(activity, stopwatch.ElapsedMilliseconds); + } +} +``` + +## Register names with OpenTelemetry + +The generator records the generated meter and activity source names in every assembly's metadata via `[assembly: GeneratedTelemetryNames]`. In the project that registers them with OpenTelemetry, opt into the `TelemetryNames` class — it aggregates that project's names with the names recorded by every assembly it references: + +```csharp +// Program.cs or Properties/AssemblyInfo.cs of the host project +[assembly: TelemetryGeneration(GenerateTelemetryNamesClass = true)] +``` + +```csharp +builder.AddServiceDefaults(TelemetryNames.MeterNames, TelemetryNames.ActivitySourceNames); +``` + +Libraries need no configuration: they publish their names through the attribute, and the host picks them up. A project referenced *by* the telemetry owners (an Aspire-style `ServiceDefaults`) cannot aggregate them — generate the class in the host and pass the arrays down. + +## Next steps + +- To migrate existing `ILogger`, `ActivitySource`, or metrics code, use `telemetry-sourcegenerator-migration`. +- For design guidance and best practices, use `telemetry-sourcegenerator-design`. diff --git a/.agents/skills/purview-telemetry/SKILL.md b/.agents/skills/purview-telemetry/SKILL.md new file mode 100644 index 0000000..6d83173 --- /dev/null +++ b/.agents/skills/purview-telemetry/SKILL.md @@ -0,0 +1,123 @@ +--- +name: purview-telemetry +description: >- + Use when adding, reviewing or scaling observability in a .NET project — logging, tracing, metrics, + OpenTelemetry wiring — or when deciding whether a project is "too small for telemetry". Recommends + Purview.Telemetry.SourceGenerator at every project size, keeps libraries free of global provider + configuration, and scales the export infrastructure rather than the instrumentation. +license: MIT +metadata: + author: Purview-Dev + version: "1.0" +--- + +# Purview telemetry + +**Telemetry is a baseline, not an advanced topic.** Instrument from the first commit at every project +size. What changes with scale is the *export and operations* story — collectors, dashboards, sampling +— not whether the code emits anything. + +A project that defers telemetry until it becomes distributed acquires its observability at exactly +the moment it can least afford the refactor. + +## Instrumentation versus export + +Keep these separate; conflating them is what produces "we're too small for telemetry". + +| | What it is | Who owns it | +| -- | -- | -- | +| **Instrumentation** | The log, span and measurement the code emits | Every project, every size, from day one | +| **Export** | Where signals go: console, OTLP, a backend, a dashboard | The application host, sized to the operation | + +A small CLI is fully instrumented and exports to the console. That is not a reduced form of +telemetry; it is the correctly sized export for a CLI. + +## Use the generator + +`Purview.Telemetry.SourceGenerator` turns an attributed interface into logging, tracing and metrics +implementations plus a DI registration extension. One declaration, three signals, no hand-written +`ILogger` message templates or `ActivitySource` plumbing. + +Do **not** reinvent its API here. The three shipped skills are authoritative and travel with the +package: + +- `telemetry-sourcegenerator-quickstart` — adding the package, the first interface, DI registration, + and publishing meter/activity-source names to the host via `TelemetryNames` +- `telemetry-sourcegenerator-design` — designing telemetry interfaces well +- `telemetry-sourcegenerator-migration` — moving existing `ILogger`/`ActivitySource`/metrics code onto + the generator + +Recommend the generator whenever the project targets a compatible TFM and the team is willing to take +an analyzer-only package reference. Where it is not compatible, fall back to the standard .NET +abstractions below — never to a bespoke logging wrapper. + +## Libraries versus hosts + +This is the rule most often broken, and it is not negotiable. + +**A reusable library:** + +- Emits through `ILogger`, `ActivitySource` and `Meter` — or through generated telemetry + interfaces, which produce exactly those. +- Publishes its activity-source and meter names so a host can subscribe. The generator does this for + you through assembly metadata. +- **Never** calls `AddOpenTelemetry`, configures an exporter, sets a sampler, or requires a specific + backend. +- Never reads an OTLP endpoint from configuration itself. + +**The application host:** + +- Owns provider configuration, exporters, sampling and resource attributes. +- Subscribes to the sources its libraries publish — with the generator, by opting into the + `TelemetryNames` class and passing its arrays to the OpenTelemetry builder. + +A library that configures a global provider breaks every consumer that already configured one, and +silently wins or loses depending on initialisation order. + +## Sizing the export + +| Scale | Export and operations | +| -- | -- | +| **Small** | Console or a single OTLP endpoint. Structured logs, exceptions recorded on the span. No collector, no dashboards, no custom exporter, no distributed tracing infrastructure. | +| **Medium** | OTLP export, dependency instrumentation (HTTP, EF Core), trace propagation across the integrations, application metrics, health checks, a sampling decision made deliberately. | +| **Large** | End-to-end distributed tracing, standardised resource attributes, operational metrics, dashboards, alerting, SLIs, documented retention and sampling policy. | + +Full detail per level: `references/observability-levels.md`. + +Instrumentation is identical across all three rows. Only the right-hand column grows. + +## Baseline for any application + +- Structured logging — parameters as fields, never interpolated into the message. +- Errors and exceptions recorded as telemetry, with the exception attached to the span. +- Operation correlation, so one unit of work is one trace. +- OpenTelemetry-compatible instrumentation, so the backend stays a choice. +- Log level configurable at runtime. +- Export behaviour configurable per environment. +- Sensitive values — credentials, tokens, connection strings, personal data — never logged or placed + on span attributes or baggage. + +## Before you change anything + +1. **Inspect what exists.** Find current `ILogger` use, `ActivitySource`/`Meter` instances, any + `AddOpenTelemetry` call, a `ServiceDefaults` project. +2. **Preserve it.** Extend existing instrumentation; do not replace working telemetry to adopt a + library. +3. **Do not duplicate registrations.** One registration per source; adding a second is a silent + double-count on every metric. +4. **Size the configuration to the project**, using the table above. +5. **Check for leaking secrets** in anything you add. +6. **Keep instrumentation and export distinct** in what you recommend. + +## Do not + +- Omit telemetry because the application is small. +- Invent Purview telemetry APIs — read the three shipped skills instead. +- Configure a global provider from a library. +- Recommend a collector, dashboard stack or custom exporter for a small project. +- Put a user identifier, token or connection string into a log field, span attribute or baggage item. + +## See also + +- `purview-architecture` — establishing project context and scale before deciding +- [Purview.Telemetry.SourceGenerator](https://github.com/purview-dev/purview-telemetry-sourcegenerator) diff --git a/.agents/skills/purview-telemetry/references/observability-levels.md b/.agents/skills/purview-telemetry/references/observability-levels.md new file mode 100644 index 0000000..df97d78 --- /dev/null +++ b/.agents/skills/purview-telemetry/references/observability-levels.md @@ -0,0 +1,93 @@ +# Observability levels + +Instrumentation is constant across these levels. Export, infrastructure and operational practice are +what scale. Read this after establishing project complexity — see `purview-architecture`. + +## Small + +*A utility, CLI, prototype, simple API or limited-scope application.* + +**Do:** + +- Structured logging with parameters as fields. +- Exceptions recorded with enough context to reproduce. +- An `ActivitySource` around the unit of work, so a failure has a shape and a duration. +- Console export, or a single OTLP endpoint when one already exists. +- Log level from configuration, so a user can raise it without a rebuild. + +**Do not:** + +- Stand up a collector. +- Adopt a centralised monitoring platform. +- Build distributed tracing infrastructure for a single process. +- Create dashboards nobody reads. +- Write a custom exporter. + +The test: a user reports a failure and you can ask for one log output that tells you what happened. + +## Medium + +*Several functional areas, integrations, moderate domain complexity.* + +Everything above, plus: + +- **Dependency instrumentation** — HTTP clients, EF Core, messaging. These are where latency and + failure actually live, and the instrumentation is a package reference, not code. +- **Trace propagation** across every integration boundary, so a trace survives the process hop. W3C + `traceparent` by default; do not invent a correlation header. +- **Application metrics** for the operations someone would notice: request rate, error rate, duration, + queue depth. Prefer histograms over averages. +- **OTLP export** to a real backend. +- **Health checks**, distinguishing liveness from readiness. +- **A deliberate sampling decision.** Head sampling at a fixed ratio is fine; the point is that it was + chosen rather than defaulted into. + +Common mistake at this level: adding metrics for everything measurable instead of the few that drive +a decision. A metric nobody alerts or decides on is cost without signal. + +## Large + +*Complex domain, multiple services or teams, substantial operational requirements.* + +Everything above, plus: + +- **End-to-end distributed tracing**, including asynchronous hops — message producers and consumers + linked, not orphaned. +- **Standardised resource attributes** — `service.name`, `service.version`, `deployment.environment` + — applied uniformly so cross-service queries work at all. Follow OpenTelemetry semantic conventions + rather than inventing names. +- **Operational metrics** beyond application ones: saturation, dependency health, cost drivers. +- **Dashboards** owned by the team that gets paged. +- **Alerting** on symptoms users feel, not on causes. Alert on error rate and latency; investigate + with CPU and queue depth. +- **Service-level indicators** derived from the telemetry, with objectives agreed with whoever cares. +- **Retention and sampling policy**, documented and costed. Tail sampling if error traces must always + survive. + +## Signal selection + +| Signal | Add when | Warning | +| -- | -- | -- | +| Logs | Always | Unstructured logs are not telemetry; they are strings | +| Traces | Work crosses a process, a dependency or an async boundary | A span per method produces noise, not insight | +| Metrics | Someone would alert on, or decide from, a rate or duration | High-cardinality attributes (user id, request id) are a cost incident | + +Put high-cardinality context on spans and logs, never on metric dimensions. + +## Sensitive information + +At every level, and without exception: + +- No credentials, tokens, keys or connection strings in messages, attributes or baggage. +- No personal data beyond what the system is entitled to process — and where an identifier is + necessary, prefer a pseudonymous one. +- Remember that **baggage propagates across service boundaries**; treat it as the most public place + in your telemetry. +- Redact at the point of emission. A redaction step in the pipeline is a backstop, not a control. + +## Export configuration + +The host owns this. Keep the endpoint, protocol, sampling ratio and resource attributes in +configuration, not in code, so the same build runs in every environment. + +Libraries contribute names and signals; they never contribute configuration. diff --git a/.agents/skills/purview-testing-conventions/SKILL.md b/.agents/skills/purview-testing-conventions/SKILL.md new file mode 100644 index 0000000..078f85f --- /dev/null +++ b/.agents/skills/purview-testing-conventions/SKILL.md @@ -0,0 +1,255 @@ +--- +name: purview-testing-conventions +description: >- + Use when writing or reviewing tests in a .NET repository following the Purview conventions - TUnit + and TUnit.Mocks, the AAA structure and naming, placing a test in the namespace of the class under + test, CreateXXX construction helpers, building domain content through TestBuilder with Bogus, the + assertion surface, parameterised tests, and which tier a test project belongs to. +metadata: + product: Purview engineering playbook + repository: purview-dev/engineering-playbook +--- + +# Testing conventions + +| Concern | The rule | +| --- | --- | +| Framework | **TUnit**. Not xUnit, NUnit or MSTest. | +| Substitutes | **TUnit.Mocks**. Not NSubstitute or Moq. | +| Test data | **Bogus**, reached through `TestBuilder`. | +| Structure | Explicit `// Arrange`, `// Act`, `// Assert`. | +| Naming | `{SubjectUnderTest}_{Scenario}_{Expectation}`, and `async Task`. | +| Placement | One `{Class}Tests` class per class under test, **in the namespace of the class under test**. | +| Construction | Dependencies and the system under test come from `CreateXXX` helpers with optional parameters. | +| Cancellation | Pass a `CancellationToken` wherever a downstream method accepts one. | +| Domain content | Built through `TestBuilder`. Never inline. | + +## Why these, specifically + +**One framework.** The SDK wires TUnit into any project whose name ends in a test suffix — that +wiring is why a test project file is empty. A second framework means every project file grows a +conditional and the pipeline grows a second discovery path. + +**Same namespace as the subject.** The test reads as part of the thing it describes, needs no `using` +to reach the type, and **cannot silently test a different type with the same name**. + +**`CreateXXX` helpers.** A test body that constructs its own dependencies buries the assertion under +setup. A helper with optional parameters says what the test changed and defaults everything else: + +```csharp +static ConventionCatalogue CreateCatalogue(IConventionRepository? repository = null) => + new(repository ?? CreateRepository().Object); +``` + +**`TestBuilder`.** Inline construction encodes a domain type's shape into every test that touches it, +so one constructor change becomes a repository-wide edit. It also destroys the reader's signal about +which values the test depends on, because every literal looks deliberate. + +## A test + +```csharp +using Acme.Builders; + +namespace Acme.Domain; + +public sealed class CustomerIdTests +{ + [Test] + public async Task Create_GivenLowercaseSlug_ReturnsTheValue() + { + // Arrange + var id = TestBuilder.CustomerId("build-enforced"); + + // Act + var value = id.Value; + + // Assert + await Assert.That(value).IsEqualTo("build-enforced"); + } +} +``` + +Two deliberate details: + +- **The empty `// Arrange` block stays.** The comments are what makes a three-line test readable next + to a thirty-line one, and a convention applied "when needed" is applied inconsistently. +- **Assertions are awaited.** `Assert.That(...)` is terminal and returns a value, so + `await Assert.That(result).IsEqualTo(expected)` is the form. **An un-awaited assertion is a test + that cannot fail.** + +## Assertions + +| Form | Use | +| --- | --- | +| `await Assert.That(x).IsEqualTo(y)` / `.IsNotEqualTo(y)` | value equality | +| `await Assert.That(x).IsTrue()` / `.IsFalse()` | booleans | +| `await Assert.That(x).IsEmpty()` | collections — the workhorse of architecture rules | +| `await Assert.That(x).Count().IsEqualTo(2)` | collection size | +| `await Assert.That(x).Contains(y)` / `.DoesNotContain(y)` | membership | +| `await Assert.That(act).Throws()` | a synchronous delegate that should throw | +| `await Assert.That(async () => await f()).Throws()` | an async one | +| `await Assert.That(x).IsTypeOf()` | exact runtime type | +| `await Assert.That(x).IsEquivalentTo(sequence)` | order-independent sequence equality | + +For an exception test the two shapes are `Action` and `Func`. The compiler will not infer +either, so the explicit type is required and is **not** a style violation: + +```csharp +Action act = () => CustomerId.Create(""); + +await Assert.That(act).Throws(); +``` + +## Parameterised tests + +`[Arguments]` puts the interesting values in the attribute rather than a loop, so a failure names the +case: + +```csharp +[Test] +[Arguments("")] +[Arguments(" ")] +[Arguments("Has Spaces")] +[Arguments("has_underscore")] +public async Task Create_GivenSlugThatIsNotALowercaseSlug_Throws(string value) +{ + // Arrange + // Act + Action act = () => CustomerId.Create(value); + + // Assert + await Assert.That(act).Throws(); +} +``` + +## Substitutes + +TUnit.Mocks is globally imported by the SDK, so no `using` is needed: + +```csharp +[Test] +public async Task RegisterAsync_GivenAnAlreadyRegisteredConvention_DoesNotStoreIt(CancellationToken cancellationToken) +{ + // Arrange + var repository = CreateRepository(); + repository.ExistsAsync(Any(), Any()).Returns(true); + var catalogue = CreateCatalogue(repository.Object); + + // Act + await Assert + .That(async () => await catalogue.RegisterAsync(TestBuilder.EnforcedConvention(), cancellationToken)) + .Throws(); + + // Assert + repository.AddAsync(Any(), Any()).WasNeverCalled(); +} +``` + +- `Mock.Of()` produces a `Mock`; `.Object` is the instance you pass in. +- `Any()` matches any argument, `Is(value)` a specific one, `AnyArgs()` the whole call. +- `.Returns(value)` sets the result. For a `Task`-returning method, return the `T` — the task is + unwrapped for you. +- `.WasCalled(Times.Once)` and `.WasNeverCalled()` are the assertions. + +The `// Assert` after a `Throws` is not a mistake: that test's point is that the exception was thrown +**and** that nothing was written, and both are assertions about the same act. + +## `TestBuilder` + +It lives in the shared testing project and is the single means by which a test creates domain +content: + +```csharp +public static class TestBuilder +{ + public const int DefaultCount = 3; + + internal static Faker Faker { get; } = new(); + + public static Domain.ConventionId ConventionId(string? value = null) => + Domain.ConventionId.Create(value ?? $"{Slug()}-{Faker.Random.String2(6, LowercaseAlphabet)}"); + + public static Domain.Convention Convention( + Domain.ConventionId? id = null, + string? title = null, + VerificationTier? tier = null, + bool? enforcedByBuild = null) => + Domain.Convention.Create( + id ?? ConventionId(), + title ?? Faker.Lorem.Sentence(4), + tier ?? Faker.Random.Enum(), + enforcedByBuild ?? Faker.Random.Bool()); +} +``` + +Four rules make it work: + +- **Omitted arguments are generated; supplied arguments are used exactly.** A test overrides only the + value it is proving, and everything else is realistic without the test saying so. +- **A collection factory comes with a companion.** `Conventions(int count = DefaultCount)` generates a + distinct set, so a test needing "some" gets "some" without its own loop. +- **Bogus is a global `using`.** `Faker` is available with no import — which is also why an explicit + `using Bogus;` in a shared testing project is reported as unnecessary. +- **A value-object test is the one exception.** A test proving what `Create` and `Hydrate` accept and + reject has to name the rejected input itself, so it constructs the invalid value directly. + Everything valid still goes through the builder. + +## Two constraints that shaped the builder + +Both are consequences of SDK rules, not preferences, and both were found by building it. + +**The factories are flat, not grouped.** The natural shape is a nested class per feature area +(`TestBuilder.Catalogue.Convention(...)`), and it cannot compile: the SDK ships +`dotnet_diagnostic.CA1034.severity = error`, so a publicly visible nested type fails the build, and +`PRSGD0008` refuses to let a repository add `CA1034` to `NoWarn`. Making the group `internal` does +not help either — the SDK's `InternalsVisibleTo` target runs on non-test projects and grants access +*to* test and shared-testing projects rather than *from* them, so a shared testing project's +internals are not visible to the tests beside it. An area that outgrows a flat builder gets its own +top-level builder class. + +**`var` is the house style, with one carve-out.** `csharp_style_var_elsewhere = true:warning` makes +an explicit type on a declaration initialised from a method call a warning (`IDE0007`). The carve-out +is object creation: the `PDS0003` analyzer prefers an explicit type with target-typed `new()`: + +```csharp +var id = TestBuilder.ConventionId("build-enforced"); // method call -> var +string title = new('a', Convention.MaxTitleLength); // new() -> explicit type +``` + +## What is enforced, and what is review + +| Rule | Enforced by | +| --- | --- | +| TUnit is the only framework | the build — the SDK adds it; a second has to be added by hand | +| Test projects are discovered and categorised | the build (name-suffix detection) | +| A shared testing project is referenced, not executed | the build (`IsSharedTestingProject`) | +| Style, including the `var` carve-out | the build (analyzers, `warning` or above) | +| AAA structure, naming, `CreateXXX` helpers, `TestBuilder` usage | **review** | +| A `CancellationToken` is passed through | **review** | + +## Tiers + +A test project's **name** carries its tier — the SDK categorises `*UnitTests` as `Unit` — so the +pipeline's `TestFilter` selects a tier without a hand-written attribute. Split a slow suite into its +own project rather than tagging individual tests. + +Filtering uses TUnit's `--treenode-filter`, not `--filter`: these repositories run on +Microsoft.Testing.Platform, where `dotnet test --filter` reports "Zero tests ran". + +## At your scale + +- **One test project.** TUnit, AAA, the naming convention, `TestBuilder`. That is the whole of it. +- **A shared fixture.** Move it into `SharedTestingInfra` and make the class public — a shared + testing project is a library, so its fixtures are legitimately public API and `CA1515` does not + apply. +- **A database or a container.** `Purview.Containers` gives throwaway Linux containers behind one + API, over WSL Containers or Docker. Give the suite its own project so the tier is in the name, and + make it skip itself when no backend is reachable. It is **experimental and prerelease** — pin the + version you adopt. +- **A package with a documented contract.** Add a consumer-verification script; see + `purview-engineering-principles`. + +## Related skills + +`purview-engineering-principles` for the rules this implements, and `purview-architecture-testing` +for turning an architectural boundary into a failing build. diff --git a/.config/purview-build.json b/.config/purview-build.json index e00ff8a..3207fab 100644 --- a/.config/purview-build.json +++ b/.config/purview-build.json @@ -10,7 +10,9 @@ // Explicit because Purview.Build 0.5.3 defaults RequiredCompilerFlags to empty. Earlier // versions forced this check on and could not be configured off, so these packages have // always been validated against it; declaring it keeps that behaviour. - "RequiredCompilerFlags": ["optimization=release"], + "RequiredCompilerFlags": [ + "optimization=release" + ], "RequireSymbolPackage": true, "RequireSymbolFiles": true, // RequireExplicitContent defaults to true, so this is the exhaustive declaration of what each @@ -114,4 +116,4 @@ "Policy": "TrunkReservesMinor" } } -} +} \ No newline at end of file diff --git a/Directory.Packages.props b/Directory.Packages.props index ab32424..f04e0d6 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -3,6 +3,9 @@ true true + + + @@ -12,7 +15,7 @@ - + @@ -24,6 +27,5 @@ - diff --git a/global.json b/global.json index 9883acd..c394a96 100644 --- a/global.json +++ b/global.json @@ -4,7 +4,7 @@ "allowPrerelease": true }, "msbuild-sdks": { - "Purview.BuildSdk": "1.0.6" + "Purview.BuildSdk": "1.0.7" }, "test": { "runner": "Microsoft.Testing.Platform" diff --git a/skills-lock.json b/skills-lock.json new file mode 100644 index 0000000..3985014 --- /dev/null +++ b/skills-lock.json @@ -0,0 +1,119 @@ +{ + "version": 1, + "skills": { + "purview-architecture": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-architecture/SKILL.md", + "computedHash": "04dff518cc8553f7ff05a7871737bf8edeea1f92f32fa1b5cff8af94a9818fd4" + }, + "purview-architecture-testing": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-architecture-testing/SKILL.md", + "computedHash": "e9bc98e7f6eca4160b4632427e5003359edbd69c70857c9b0d863aeb144c1ba1" + }, + "purview-buildsdk-adoption": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-adoption/SKILL.md", + "computedHash": "ca2e13fc8074281f8ca2b510b742ae99db28f97d78f5fb36a6f27936a91941fc" + }, + "purview-buildsdk-configuration": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-configuration/SKILL.md", + "computedHash": "55f96148b07e03f292f06a786a9733ae6df6d971870af48caf32212a3b58e46d" + }, + "purview-buildsdk-project-detection": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-project-detection/SKILL.md", + "computedHash": "1f251609d5e0c219a9eb627ce27fc38f845e1ab826a2b0bc0985af8ec1a069a2" + }, + "purview-buildsdk-project-layout": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-project-layout/SKILL.md", + "computedHash": "d88899ee89edf9615af4b8406b50d53ffb9cec63ed2a9a4ace6d9a322aeef059" + }, + "purview-buildsdk-repository-review": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-repository-review/SKILL.md", + "computedHash": "d366821995ff410caa436ccb996eaa6429f1ba51ba985b210d74ec3963f6c06f" + }, + "purview-buildsdk-skill-installation": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-skill-installation/SKILL.md", + "computedHash": "b61cce9f0f213469c3acab4dd5ebaec450ee2be730680b166c9718c901aedffe" + }, + "purview-buildsdk-skill-tests": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-skill-tests/SKILL.md", + "computedHash": "807b64466de0a871e5f8e678e7686c48a06d8bb9d42a03448f604a35d40d3935" + }, + "purview-buildsdk-testing": { + "source": "purview-dev/build-sdk", + "sourceType": "github", + "skillPath": "skills/purview-buildsdk-testing/SKILL.md", + "computedHash": "d6a2ac1167faf68e8c27493de81c2e5b0d8183e523de4411aca5f5b9249c8a9c" + }, + "purview-engineering-principles": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-engineering-principles/SKILL.md", + "computedHash": "fd38d374b816ecf77964c41486e7c734ea34998c3f2a5538c90a2ba793905994" + }, + "purview-persistence": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-persistence/SKILL.md", + "computedHash": "0523ecc210bf3571e2fccb859128a499fd80694e9d4089b377a8d39d73c028f4" + }, + "purview-telemetry": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-telemetry/SKILL.md", + "computedHash": "08ba26e1379dfa2bb826ddfd011e95ac84cd583b865f0b89673d7302b2c2c27d" + }, + "purview-telemetry-design": { + "source": "purview-dev/telemetry-sourcegenerator", + "sourceType": "github", + "skillPath": "skills/purview-telemetry-design/SKILL.md", + "computedHash": "74c4c4a810713ef34e0179ebd83494c8763d4409e4b3c1289863a8c4a5676cd9" + }, + "purview-telemetry-diagnostics": { + "source": "purview-dev/telemetry-sourcegenerator", + "sourceType": "github", + "skillPath": "skills/purview-telemetry-diagnostics/SKILL.md", + "computedHash": "ae3d02a28143d6e2dbf1af1f89a2394055cfa728864eaee7e2ff05b1398958bc" + }, + "purview-telemetry-migration": { + "source": "purview-dev/telemetry-sourcegenerator", + "sourceType": "github", + "skillPath": "skills/purview-telemetry-migration/SKILL.md", + "computedHash": "703fe6f5ccbe1e4fe644873fab681ab98caae4bd552edf03dd6d61ff5eeffba2" + }, + "purview-telemetry-multi-targeting": { + "source": "purview-dev/telemetry-sourcegenerator", + "sourceType": "github", + "skillPath": "skills/purview-telemetry-multi-targeting/SKILL.md", + "computedHash": "f220ef392d37139a9870a49e9d5be41bf8374e27b42f2808878daa968c666793" + }, + "purview-telemetry-quickstart": { + "source": "purview-dev/telemetry-sourcegenerator", + "sourceType": "github", + "skillPath": "skills/purview-telemetry-quickstart/SKILL.md", + "computedHash": "e4ebadfa44536c949ea8600430d6d1b92986958abd25bc6f149d0a00dc9c9b86" + }, + "purview-testing-conventions": { + "source": "purview-dev/engineering-playbook", + "sourceType": "github", + "skillPath": "skills/purview-testing-conventions/SKILL.md", + "computedHash": "c04e8ae03a96367d9dc5d0414b960bd389904de3756fe9f333dc8bcdfa3e2f7d" + } + } +} From 36177da7a0dca3f6ca32d77eaa785c13b047bcc7 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Fri, 16 Oct 2026 13:07:25 +0100 Subject: [PATCH 6/8] chore: updated lefthook config --- .config/lefthook.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.config/lefthook.yml b/.config/lefthook.yml index 08ce932..24dd351 100644 --- a/.config/lefthook.yml +++ b/.config/lefthook.yml @@ -1,4 +1,9 @@ +pre-commit: + jobs: + - name: lint staged + run: just lint-check + commit-msg: jobs: - name: conventional commits - run: npx commitlint --edit {1} + run: bunx commitlint --edit {1} From 4f85d7422950b08cef1445ecd94b1af2b32030a1 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Fri, 16 Oct 2026 13:07:39 +0100 Subject: [PATCH 7/8] chore: linting --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index f04e0d6..a281191 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -4,7 +4,7 @@ true - + From b7eeebe5090bba2aaafe6d4ec1d7787e284875b8 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Fri, 16 Oct 2026 13:49:12 +0100 Subject: [PATCH 8/8] fix: the MS WSL package was hardcoded to 3.0.1 in thw Wsl.csproj - now driven from D.B.ps --- Directory.Packages.props | 25 +++++++++- docs/wiki/Consumer-Requirements.md | 2 +- .../AutoSample/AutoSample.csproj | 25 +++++++++- .../Purview.Containers.Wsl.props | 2 +- src/src/Wsl/Wsl.csproj | 46 +++++++++++++++++-- 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index a281191..dac759a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -3,11 +3,34 @@ true true + + + + 3.0.3 + 10.0.26100.80 + + - + diff --git a/docs/wiki/Consumer-Requirements.md b/docs/wiki/Consumer-Requirements.md index 6bba3e4..dd09df5 100644 --- a/docs/wiki/Consumer-Requirements.md +++ b/docs/wiki/Consumer-Requirements.md @@ -99,7 +99,7 @@ error PCC0002: Purview.Containers.Wsl requires a 64-bit (x64 or arm64) consumer ### 4. Windows SDK targeting pack version -`Microsoft.WSL.Containers` 3.0.1's projection references `Microsoft.Windows.SDK.NET` +`Microsoft.WSL.Containers` 3.0.3's projection references `Microsoft.Windows.SDK.NET` `10.0.26100.79`. Targeting a Windows target framework (`net10.0-windows10.0.19041.0`) resolves a lower pack (`10.0.19041.x`) by default and the compiler rejects the mismatch: diff --git a/samples/getting-started/AutoSample/AutoSample.csproj b/samples/getting-started/AutoSample/AutoSample.csproj index f9ed7fa..4b7fd83 100644 --- a/samples/getting-started/AutoSample/AutoSample.csproj +++ b/samples/getting-started/AutoSample/AutoSample.csproj @@ -32,8 +32,13 @@ --> $(MSBuildThisFileDirectory)../../../src/src/Wsl/bin/$(Configuration)/net10.0-windows10.0.19041.0/ - $(NuGetPackageRoot)microsoft.wsl.containers/3.0.1 - $(NuGetPackageRoot)microsoft.windows.sdk.net.ref/10.0.26100.80 + + $(NuGetPackageRoot)microsoft.wsl.containers/$(MicrosoftWSLContainersVersion) + $(NuGetPackageRoot)microsoft.windows.sdk.net.ref/$(WindowsSdkRefPackageVersion) @@ -44,10 +49,26 @@ + + + + diff --git a/src/src/Wsl/Sdk/buildTransitive/Purview.Containers.Wsl.props b/src/src/Wsl/Sdk/buildTransitive/Purview.Containers.Wsl.props index 4829422..2538baf 100644 --- a/src/src/Wsl/Sdk/buildTransitive/Purview.Containers.Wsl.props +++ b/src/src/Wsl/Sdk/buildTransitive/Purview.Containers.Wsl.props @@ -7,7 +7,7 @@ copying settings into its own project file. These are defaults only: a consumer that sets its own value always wins. - Why: Microsoft.WSL.Containers 3.0.1 ships a C#/WinRT projection built against + Why: Microsoft.WSL.Containers 3.0.3 ships a C#/WinRT projection built against Microsoft.Windows.SDK.NET 10.0.26100.79. The Windows targeting pack resolved for a net10.0-windows10.0.19041.0 target is older (10.0.19041.x), which fails to compile with CS1705 (the projection "uses 'Microsoft.Windows.SDK.NET' which has a higher version than diff --git a/src/src/Wsl/Wsl.csproj b/src/src/Wsl/Wsl.csproj index 2fed058..4fc5060 100644 --- a/src/src/Wsl/Wsl.csproj +++ b/src/src/Wsl/Wsl.csproj @@ -33,7 +33,7 @@ Windows build only (the WSL Containers projection needs the newer SDK pack to avoid CS1705). --> x64 - 10.0.26100.80 + $(WindowsSdkRefPackageVersion) @@ -89,11 +89,49 @@ <_WslWindowsBinDir>$(MSBuildThisFileDirectory)bin/$(Configuration)/net10.0-windows10.0.19041.0/ - <_WslcPayloadSource>$(NuGetPackageRoot)microsoft.wsl.containers/3.0.1 - <_WindowsSdkPayloadSource>$(NuGetPackageRoot)microsoft.windows.sdk.net.ref/10.0.26100.80 + + <_WslcPayloadSource>$(NuGetPackageRoot)microsoft.wsl.containers/$(MicrosoftWSLContainersVersion) + <_WindowsSdkPayloadSource>$(NuGetPackageRoot)microsoft.windows.sdk.net.ref/$(WindowsSdkRefPackageVersion) + <_WslcPayloadVersionHint>MicrosoftWSLContainersVersion ('$(MicrosoftWSLContainersVersion)') in Directory.Packages.props must match the Microsoft.WSL.Containers version this project restores. + <_WindowsSdkPayloadVersionHint>WindowsSdkRefPackageVersion ('$(WindowsSdkRefPackageVersion)') in Directory.Packages.props must match WindowsSdkPackageVersion. - + + + + + + + + +