diff --git a/.agents/skills/telemetry-sourcegenerator-design/.gitignore b/.agents/skills/telemetry-sourcegenerator-design/.gitignore deleted file mode 100644 index 2799754..0000000 --- a/.agents/skills/telemetry-sourcegenerator-design/.gitignore +++ /dev/null @@ -1,8 +0,0 @@ -# Ignore all files -* - -# Don't ignore directories, so Git can traverse them -!*/ - -# Keep this file -!.gitignore \ No newline at end of file diff --git a/.agents/skills/telemetry-sourcegenerator-design/SKILL.md b/.agents/skills/telemetry-sourcegenerator-design/SKILL.md new file mode 100644 index 0000000..c73347d --- /dev/null +++ b/.agents/skills/telemetry-sourcegenerator-design/SKILL.md @@ -0,0 +1,96 @@ +--- +name: telemetry-sourcegenerator-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: ISC +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/telemetry-sourcegenerator-design/references/REFERENCE.md b/.agents/skills/telemetry-sourcegenerator-design/references/REFERENCE.md new file mode 100644 index 0000000..b15cfcf --- /dev/null +++ b/.agents/skills/telemetry-sourcegenerator-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/telemetry-sourcegenerator-migration/.gitignore b/.agents/skills/telemetry-sourcegenerator-migration/.gitignore deleted file mode 100644 index 2799754..0000000 --- a/.agents/skills/telemetry-sourcegenerator-migration/.gitignore +++ /dev/null @@ -1,8 +0,0 @@ -# Ignore all files -* - -# Don't ignore directories, so Git can traverse them -!*/ - -# Keep this file -!.gitignore \ No newline at end of file diff --git a/.agents/skills/telemetry-sourcegenerator-migration/SKILL.md b/.agents/skills/telemetry-sourcegenerator-migration/SKILL.md new file mode 100644 index 0000000..387077c --- /dev/null +++ b/.agents/skills/telemetry-sourcegenerator-migration/SKILL.md @@ -0,0 +1,81 @@ +--- +name: telemetry-sourcegenerator-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: ISC +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/telemetry-sourcegenerator-migration/references/REFERENCE.md b/.agents/skills/telemetry-sourcegenerator-migration/references/REFERENCE.md new file mode 100644 index 0000000..d85eb73 --- /dev/null +++ b/.agents/skills/telemetry-sourcegenerator-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/telemetry-sourcegenerator-quickstart/.gitignore b/.agents/skills/telemetry-sourcegenerator-quickstart/.gitignore deleted file mode 100644 index 2799754..0000000 --- a/.agents/skills/telemetry-sourcegenerator-quickstart/.gitignore +++ /dev/null @@ -1,8 +0,0 @@ -# Ignore all files -* - -# Don't ignore directories, so Git can traverse them -!*/ - -# Keep this file -!.gitignore \ No newline at end of file diff --git a/.agents/skills/telemetry-sourcegenerator-quickstart/SKILL.md b/.agents/skills/telemetry-sourcegenerator-quickstart/SKILL.md new file mode 100644 index 0000000..0e1c367 --- /dev/null +++ b/.agents/skills/telemetry-sourcegenerator-quickstart/SKILL.md @@ -0,0 +1,133 @@ +--- +name: telemetry-sourcegenerator-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: ISC +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 also produces a `TelemetryNames` static class containing the meter and activity source names: + +```csharp +builder.AddServiceDefaults(TelemetryNames.MeterNames, TelemetryNames.ActivitySourceNames); +``` + +## 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/.github/workflows/purview-build.yml b/.github/workflows/purview-build.yml index 012256d..086810c 100644 --- a/.github/workflows/purview-build.yml +++ b/.github/workflows/purview-build.yml @@ -10,7 +10,10 @@ on: required: false type: string dotnet-version: - description: .NET SDK used by the consuming repository. + description: >- + .NET SDK used by the consuming repository. Ignored when global.json pins an sdk.version, + because every dotnet command resolves against global.json and installing anything else + fails the job. required: false default: 10.0.x type: string @@ -77,9 +80,36 @@ jobs: fetch-depth: 0 fetch-tags: true + # global.json is the single source of truth for the SDK whenever it pins one. Every dotnet + # command in the job resolves against it, so installing anything else fails the whole job at + # the first one - "A compatible .NET SDK was not found" - no matter what dotnet-version says. + # Deriving it here means a consuming repository states its SDK once, instead of repeating it + # in global.json, pr.yml and release.yml and keeping three copies in step by hand. + # + # When global.json pins nothing, the dotnet-version input applies, which is the behaviour + # every repository had before. + - name: Resolve the .NET SDK version + id: resolve-sdk + shell: bash + run: | + set -euo pipefail + + pinned="" + if [ -f global.json ]; then + pinned="$(jq -r '.sdk.version // empty' global.json)" + fi + + if [ -n "$pinned" ]; then + echo "global.json pins $pinned; installing that." + echo "dotnet-version=$pinned" >> "$GITHUB_OUTPUT" + else + echo "global.json pins no sdk.version; using the dotnet-version input (${{ inputs.dotnet-version }})." + echo "dotnet-version=${{ inputs.dotnet-version }}" >> "$GITHUB_OUTPUT" + fi + - uses: actions/setup-dotnet@v6 with: - dotnet-version: ${{ inputs.dotnet-version }} + dotnet-version: ${{ steps.resolve-sdk.outputs.dotnet-version }} - uses: oven-sh/setup-bun@v2 diff --git a/.github/workflows/purview-release.yml b/.github/workflows/purview-release.yml index b3dbcfb..280d62f 100644 --- a/.github/workflows/purview-release.yml +++ b/.github/workflows/purview-release.yml @@ -10,7 +10,10 @@ on: required: false type: string dotnet-version: - description: .NET SDK used by the consuming repository. + description: >- + .NET SDK used by the consuming repository. Ignored when global.json pins an sdk.version, + because every dotnet command resolves against global.json and installing anything else + fails the job. required: false default: 10.0.x type: string @@ -119,9 +122,30 @@ jobs: fetch-depth: 0 fetch-tags: true + # global.json is the single source of truth for the SDK whenever it pins one; see the matching + # step in purview-build.yml. When it pins nothing, the dotnet-version input applies. + - name: Resolve the .NET SDK version + id: resolve-sdk + shell: bash + run: | + set -euo pipefail + + pinned="" + if [ -f global.json ]; then + pinned="$(jq -r '.sdk.version // empty' global.json)" + fi + + if [ -n "$pinned" ]; then + echo "global.json pins $pinned; installing that." + echo "dotnet-version=$pinned" >> "$GITHUB_OUTPUT" + else + echo "global.json pins no sdk.version; using the dotnet-version input (${{ inputs.dotnet-version }})." + echo "dotnet-version=${{ inputs.dotnet-version }}" >> "$GITHUB_OUTPUT" + fi + - uses: actions/setup-dotnet@v6 with: - dotnet-version: ${{ inputs.dotnet-version }} + dotnet-version: ${{ steps.resolve-sdk.outputs.dotnet-version }} - uses: oven-sh/setup-bun@v2 diff --git a/AGENTS.md b/AGENTS.md index e6e62a0..b52a1c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,3 +83,37 @@ When changing release behaviour, grep the consuming repositories' `docs/wiki/Rel - "The workflow does not publish to NuGet" — `release-mode: NuGet` runs `PublishNuGetModule` and pushes to `NuGet:FeedUrl`. - "The shared pipeline does not mark the GitHub release with the prerelease flag" — `Release:MarkPrerelease` defaults to `true`. - "…and attaches the package artifacts" — assets are attached only when the caller passes `upload-artifacts: true`, which no consumer currently does. + +## 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/build +``` + +| Skill | Answers | +| -- | -- | +| `skills/purview-build-adoption/` | wiring the pipeline into a repository: the workflows, the action, local use, the two version axes | +| `skills/purview-build-configuration/` | where `purview-build.json` is found, precedence, every setting, the secrets | +| `skills/purview-build-pipeline/` | what each module does, its dependencies, and why it was skipped | +| `skills/purview-build-pack-validation/` | the package rules, the target-framework token, and `RequireExplicitContent` | +| `skills/purview-build-release-eligibility/` | the `REL0nn` rules, the built-in policies, and the three release models | + +Every skill carries the `purview-build` 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 994bba8..89d0b09 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -2,8 +2,8 @@ true 5.9.0 - 1.72.16 - 10.0.12 + 1.75.0 + 10.0.13 10.10.0 3.2.8 7.9.0 @@ -12,16 +12,18 @@ - + + + - - + + diff --git a/docs/wiki/Configuration-Reference.md b/docs/wiki/Configuration-Reference.md index 0916233..18ef1cc 100644 --- a/docs/wiki/Configuration-Reference.md +++ b/docs/wiki/Configuration-Reference.md @@ -152,7 +152,7 @@ Malformed JSON in a configuration file fails with the file path and, where the p | `ForbiddenContent` | `{}` | Package-id glob → entry-path globs that must not be present in the `.nupkg` (`"*"` matches every package). Also supports the `$(TFM)` partial token | | `RequireExplicitContent` | `true` | Makes `RequiredContent` exhaustive: every generated package must match a rule, and every non-metadata entry must match a declared glob; undeclared packages/entries are errors | -Content entry paths and package-id keys are matched as globs (case-insensitive), e.g. `tools/**/Foo.dll` or `**/*.pdb`. Entries may also contain the `$(TFM)` partial token (e.g. `lib/$(TFM)/Foo.dll`), which expands to one entry per target framework the package actually ships (short folder name, discovered from the package's own content groups); each expanded entry is checked independently. Required content is satisfied when any package entry matches; forbidden content fails when any entry matches. When `RequireExplicitContent` is `true`, `RequiredContent` becomes exhaustive: every generated package must match a rule, and every non-metadata entry in a matched package must match a declared (and `$(TFM)`-expanded) glob — undeclared packages or entries are errors. PDBs are normally delivered through `.snupkg`, but analyzer-only packages may embed them under `analyzers/dotnet/` in the `.nupkg`; those packages do not require a sibling `.snupkg`. The assembly checks (`RequireSourceLink`, `RequireDeterministic`, `RequiredCompilerFlags`) inspect each `.dll`/`.exe` in the `.nupkg` (PE header) and its sibling portable PDB in the `.snupkg` or analyzer-slot PDB in the `.nupkg`; they only apply to assemblies the package ships symbols for. Determinism is detected via the PE's Reproducible debug directory entry, source link via the PDB's Source Link record, and compiler flags via the PDB's key/value compiler-flags record (matched case-insensitively, e.g. `optimization=release`). +Content entry paths and package-id keys are matched as globs (case-insensitive), e.g. `tools/**/Foo.dll` or `**/*.pdb`. Entries may also contain the `$(TFM)` partial token (e.g. `lib/$(TFM)/Foo.dll`), which expands to one entry per target framework the package actually ships (short folder name, discovered from the package's own content groups); each expanded entry is checked independently. NuGet's framework discovery also reports the pseudo-frameworks `any`, `agnostic`, and `unsupported` — notably the `any` target framework a package's `contentFiles` group contributes — and these are ignored, since they are not real target frameworks. Required content is satisfied when any package entry matches; forbidden content fails when any entry matches. When `RequireExplicitContent` is `true`, `RequiredContent` becomes exhaustive: every generated package must match a rule, and every non-metadata entry in a matched package must match a declared (and `$(TFM)`-expanded) glob — undeclared packages or entries are errors. PDBs are normally delivered through `.snupkg`, but analyzer-only packages may embed them under `analyzers/dotnet/` in the `.nupkg`; those packages do not require a sibling `.snupkg`. The assembly checks (`RequireSourceLink`, `RequireDeterministic`, `RequiredCompilerFlags`) inspect each `.dll`/`.exe` in the `.nupkg` (PE header) and its sibling portable PDB in the `.snupkg` or analyzer-slot PDB in the `.nupkg`; they only apply to assemblies the package ships symbols for. Determinism is detected via the PE's Reproducible debug directory entry, source link via the PDB's Source Link record, and compiler flags via the PDB's key/value compiler-flags record (matched case-insensitively, e.g. `optimization=release`). > **Tool defaults vs code defaults.** The shipped `appsettings.json` sets `RequireSourceLink: false`, `RequireDeterministic: false`, and `RequiredCompilerFlags: []`. The C# property initializers in `PackValidationSettings` default the two booleans to `true`, but `appsettings.json` always loads and wins over a code default, so the effective shipped defaults are the `false`/`false`/`[]` values shown above. > diff --git a/docs/wiki/Pack-Validation.md b/docs/wiki/Pack-Validation.md index 5c73257..984f4cf 100644 --- a/docs/wiki/Pack-Validation.md +++ b/docs/wiki/Pack-Validation.md @@ -27,6 +27,8 @@ Both maps are keyed by package-id glob (case-insensitive; `"*"` matches every pa An entry may contain the literal token `$(TFM)`, e.g. `lib/$(TFM)/Purview.Telemetry.dll`. The token is expanded into one entry per target framework the package actually ships (short folder name, e.g. `net8.0`, `net48`, `netstandard2.0`, discovered via the package's own `lib`/`ref`/`build`/`tools`/`frameworkAssemblies` groups), so the rule must be satisfied independently for every one of the package's target frameworks. If the package has no detectable target frameworks, a `$(TFM)` entry is reported as an error rather than silently skipped. +NuGet's framework discovery also reports the pseudo-frameworks `any`, `agnostic`, and `unsupported` for some layouts — most commonly the `any` target framework a package's `contentFiles` group contributes. They are not target frameworks and are ignored: substituting one would expand the token to an entry such as `lib/any/Foo.dll` that can never match, failing a package that is actually correct. + ### Explicit/exhaustive content (`RequireExplicitContent`) When `RequireExplicitContent` is `true`, `RequiredContent` becomes the precise, exhaustive definition of every package's contents instead of a "must contain at least" list: diff --git a/global.json b/global.json index 5ad4318..32f1a20 100644 --- a/global.json +++ b/global.json @@ -3,7 +3,7 @@ "allowPrerelease": false }, "msbuild-sdks": { - "Purview.BuildSdk": "1.0.5" + "Purview.BuildSdk": "1.0.6" }, "test": { "runner": "Microsoft.Testing.Platform" diff --git a/package.json b/package.json index 37ac3e7..f49053c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "build", - "version": "0.5.3", + "version": "0.5.4", "description": "Shared build/test/release pipeline for the purview-dev organisation, packaged as a pinned .NET tool with a GitHub composite action and reusable workflows.", "license": "MIT", "author": { diff --git a/skills/purview-build-adoption/SKILL.md b/skills/purview-build-adoption/SKILL.md new file mode 100644 index 0000000..a46a3f7 --- /dev/null +++ b/skills/purview-build-adoption/SKILL.md @@ -0,0 +1,146 @@ +--- +name: purview-build-adoption +description: >- + Use when wiring Purview.Build into a repository - choosing the reusable workflow or the composite + action, pinning the workflow ref versus the tool version, running the pipeline locally as a dotnet + tool, and what a consuming repository has to add before anything will run. +metadata: + product: Purview.Build + repository: purview-dev/build +--- + +# Adopting Purview.Build + +`Purview.Build` is a pipeline tool consumed from a repository that owns its own configuration. The +pipeline code lives in `purview-dev/build`; a consumer references a reusable workflow or the +composite action — or runs the tool locally — and configures it with `.config/purview-build.json`. + +A consuming repository needs exactly three things: a `purview-build.json`, a `version` field in the +root `package.json`, and a workflow that calls one of the entry points below. No copied pipeline +project, no package-source credentials. + +## Two independent version axes + +Get this wrong and you run new tool code through old workflow inputs. + +| Axis | What it selects | How | +| --- | --- | --- | +| `@ref` on the workflow or action reference | the **workflow/action code** | `@main` for latest, a release tag (`@v0.2.0`) for reproducibility. There is no `@latest`, and the `@ref` is **required** for a cross-repository reference. | +| the `build-version` input | the installed **tool** | omit it for the latest stable from nuget.org; pin an exact version (`build-version: 0.2.0`) for reproducibility. | + +Pin both, or neither. Pinning one and floating the other is the mismatch. + +## Reusable workflow — the usual choice + +```yaml +# .github/workflows/pr.yml +name: PR +on: + pull_request: + branches: [main] +jobs: + build: + uses: purview-dev/build/.github/workflows/purview-build.yml@main + secrets: inherit +``` + +```yaml +# .github/workflows/release.yml +name: Release +on: + push: + branches: [main] +concurrency: + # Callers own release serialisation; the reusable workflow defines no concurrency group. + group: release-${{ github.ref }} + cancel-in-progress: false +jobs: + release: + uses: purview-dev/build/.github/workflows/purview-release.yml@main + with: + release-mode: NuGet + secrets: inherit +``` + +The release caller's **trigger** is what picks your release model: + +| Model | Trigger | Policy | +| --- | --- | --- | +| A — release on `main` | `push: branches: [main]` | `ReleaseOnMain` (the default) | +| B — main-as-head, release branch | `push: branches: [release]` | `ReleaseOnMain` | +| C — per-line release branches | `push: branches: ['release/**']` plus `workflow_dispatch` | `TrunkReservesMinor` | + +```yaml +jobs: + release: + uses: purview-dev/build/.github/workflows/purview-release.yml@main + with: + release-mode: NuGet + eligibility-policy: TrunkReservesMinor + secrets: inherit +``` + +The reusable release workflow asks the tool to evaluate eligibility and reads the verdict: an +already-released version **skips** and the job finishes green; a policy violation **fails** the job +naming the rule. Merging `main` into `release` therefore releases exactly once. + +## Composite action — when you need surrounding steps + +```yaml +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: purview-dev/build/.github/actions/purview-build@main + env: + Build__TestFilter: "/*/*/*/*[Category=Unit]" +``` + +`build-version` is optional here too. + +## Locally + +```shell +dotnet new tool-manifest # only if there is no .config/dotnet-tools.json +dotnet tool install Purview.Build +dotnet purview-build +``` + +It is an ordinary .NET local tool: the manifest pins the version, and `dotnet tool restore` installs +it. Most repositories wrap it in `just` recipes: + +```just +pipeline-pr: dotnet purview-build +pipeline-pack-validate: dotnet purview-build --Build:RunPack=true --Build:ValidatePack=true --Release:Mode=None +``` + +Before running anything destructive, two commands tell you what *would* happen: + +```shell +dotnet purview-build --help # usage, probe order, and the resolved config path +dotnet purview-build release-explain # the release decision, without running a module +``` + +## Releasing a consuming repository + +Bump the `version` field in the root `package.json`, and merge the validated pull request into the +release head. **The pipeline owns tagging (`v{version}`) and the GitHub release** — never push a +release tag by hand. + +## A setting that will not take effect + +Precedence is **command line > environment variables > `purview-build.json` > defaults**. Because +environment variables outrank the JSON file, an *empty* forwarded variable silently erases a +configured value. That is why the reusable workflows forward every optional input only when the +caller actually provided it, and why `WorkflowParityTests` asserts that no optional input is mapped +straight into `env:` without an `if [ -n … ]` guard. + +If a `purview-build.json` setting appears to be ignored, check for an environment variable of the +same name first. + +## Related skills + +`purview-build-configuration` for the full `purview-build.json` reference and the secrets, +`purview-build-pipeline` for the modules and their order, `purview-build-pack-validation` for the +package rules, and `purview-build-release-eligibility` for the `REL0nn` rules and the three models. diff --git a/skills/purview-build-configuration/SKILL.md b/skills/purview-build-configuration/SKILL.md new file mode 100644 index 0000000..b882d8e --- /dev/null +++ b/skills/purview-build-configuration/SKILL.md @@ -0,0 +1,164 @@ +--- +name: purview-build-configuration +description: >- + Use when configuring Purview.Build - where purview-build.json is found and how it is anchored, + the precedence order, every Build/PackValidation/NuGet/GitHub/Version/Release setting, the secrets + and environment variables, and why a configured value appears to be ignored. +metadata: + product: Purview.Build + repository: purview-dev/build +--- + +# Configuring Purview.Build + +## Where the file lives + +Explicitly: + +| Form | Resolution | +| --- | --- | +| `--config ` / `-c ` | absolute, or relative to the current working directory | +| `PURVIEW_BUILD_CONFIG=` | the same resolution; `--config` wins | + +An explicit path that does not exist is an **error**, never a silent fallback. + +Otherwise the tool probes for `purview-build.json`, conventionally at `.config/purview-build.json`. +Relative paths inside the file are anchored to the repository root, not to the file's own folder, so +moving the file does not change what `src/tests` means. + +`PURVIEW_BUILD_USER_CONFIG=1` additionally reads machine-local user configuration — ignored when not +running locally. + +`MODULAR_PIPELINES_DIRECTORY` is **unrelated**: it is where the tool's own shipped `appsettings.json` +lives, not part of `purview-build.json` discovery. + +## Precedence + +**Command line > environment variables > `purview-build.json` > baked-in defaults.** + +Nested environment keys use `__`: `Release__Mode=NuGet`, `Build__TestFilter=…`. Configuration keys +are case-insensitive. + +The consequence worth internalising: because environment variables outrank the file, an **empty** +forwarded variable silently overrides a configured value. If a setting appears to be ignored, look +for an environment variable of that name before anything else. + +## `Build` + +| Key | Default | Purpose | +| --- | --- | --- | +| `LogLevel` | `Information` | `Information` reports every module's command output and progress; `Warning` keeps CI logs quiet | +| `ProjectType` | `DotNet` | `DotNet`, or `Web` (Bun commands from the root `package.json` scripts) | +| `Solution` | `src/Product.slnx` | solution, project or directory for restore/build/pack | +| `Configuration` | `Release` | .NET configuration | +| `ArtifactsFolder` | `artifacts` | package output directory | +| `CleanArtifacts` | `true` | delete and recreate `ArtifactsFolder` before anything is produced. Ignored when `RunPack` is `false` | +| `RunTests` / `RunLint` / `RunPack` / `ValidatePack` | `true` | the per-stage switches | +| `TestRoot` | `src/tests` | discovery root, relative to the repository root | +| `TestPatterns` | `*Tests.csproj` | comma-separated search patterns under `TestRoot` | +| `TestProjects` | `*` | comma-separated names/globs to actually run | +| `TestFramework` | `TUnit` | `TUnit` (tree-node filter) or `xUnit` (VSTest filter) | +| `TestFilter` | `/*/*/*/*/` | the filter; empty disables it | +| `Web*Command`, `WebBuildOutput` | Bun defaults | only for `ProjectType=Web` | + +## `PackValidation` + +| Key | Default | Purpose | +| --- | --- | --- | +| `RequireSymbolPackage` | `true` | every `.nupkg` needs a matching `.snupkg` and vice versa, except analyzer-only packages embedding PDBs under `analyzers/dotnet/` | +| `RequireSymbolFiles` | `true` | every `.snupkg` holds at least one `.pdb` | +| `RequireSourceLink` | `false` | every shipped assembly's PDB carries a Source Link record | +| `RequireDeterministic` | `false` | every shipped assembly carries the Reproducible debug directory entry | +| `RequiredCompilerFlags` | `[]` | flags that must appear in the PDB compiler-flags record, e.g. `optimization=release` | +| `RequiredContent` | `{}` | package-id glob → entry-path globs that must be present. Supports the `$(TFM)` token | +| `ForbiddenContent` | `{}` | package-id glob → entry-path globs that must not be present | +| `RequireExplicitContent` | `true` | makes `RequiredContent` **exhaustive** | + +`RequireExplicitContent` defaulting to `true` is the one that surprises people: an undeclared +package, or an undeclared entry in a declared package, is an error. See the +`purview-build-pack-validation` skill. + +## `NuGet`, `PublishLocalNuGet`, `GitHub` + +| Key | Default | Purpose | +| --- | --- | --- | +| `NuGet:FeedUrl` | nuget.org v3 | remote source | +| `NuGet:TrustedPublishing` | `false` | push without an API key (OIDC federation) | +| `NuGet:APIKey` / `EnvAPIKey` | unset | secret | +| `PublishLocalNuGet:LocalFeedPath` | unset | **absolute** local source; a drive-relative path such as `p:foo` is rejected with a remediation message | +| `PublishLocalNuGet:OverwriteExistingPackages` / `ShutdownDotnetBuilderServer` / `ClearPackageCache` | `true` | local publish behaviour | +| `GitHub:AccessToken` / `EnvAccessToken` | unset | secret | +| `GitHub:ProductHeader` | `Purview.Build.Pipeline` | API product header | + +## `Version` + +| Key | Default | Purpose | +| --- | --- | --- | +| `Source` | `PackageJson` | the only implemented source: the `version` field of the root `package.json` | +| `Strictness` | `NuGet` | accepts everything `NuGetVersion` does, including four-part (`13.5.3.10`) and one/two-part (`1.0`) versions. `SemVer2` is the stricter opt-in | + +The tag preserves the version **exactly as written**, not its normalised form — normalising would +drop a trailing `.0` revision and desynchronise the tag from the declared version. + +## `Release` + +| Key | Default | Purpose | +| --- | --- | --- | +| `Mode` | `None` | preset: `None`, `LocalNuGet`, `NuGet`, `GitHubRelease`. Derives `Publish` and `GitHubRelease` | +| `Publish` | derived | push packages; setting it explicitly wins over the preset | +| `GitHubRelease` | derived from `Mode`, then the channel | create the tag and release; explicit wins over both | +| `Channel` / `Channels` | `stable` / `{}` | a channel selects the feed and the label policy: `{ FeedUrl, GitHubRelease, MarkPrerelease, LabelPattern }` | +| `DryRun` | `false` | run everything, publish nothing, log what each step would have done | +| `UploadArtifacts` | `false` | upload `ArtifactsFolder` as GitHub release assets | +| `MarkPrerelease` | `true` | a prerelease **version** becomes a GitHub prerelease. GitHub metadata only — NuGet publication is unaffected | + +| `Mode` | `Publish` | `GitHubRelease` | +| --- | --- | --- | +| `None` | false | false | +| `NuGet` | true | true | +| `GitHubRelease` | false | true | +| `LocalNuGet` | false | false | + +`Release:Eligibility` (`Policy`, `Policies`, `Inherits`, `Rules`, the ref sets, `AllowFourPart`) and +`Release:Context` (`Ref`, `ExistingTags`, `PublishedVersions`, for simulated evaluation) are covered +by the `purview-build-release-eligibility` skill. + +## Secrets + +Never committed; supplied at runtime. + +| Secret | Used for | Bound alias | +| --- | --- | --- | +| `NUGET_APIKEY` | NuGet push | `NuGet__NUGET_APIKEY`; also read from process env `NUGET_APIKEY`/`NUGET_API_KEY` | +| `NuGet__APIKey` | NuGet push | `APIKey`; the historical `NUGET__APIKEY` name binds here too, keys being case-insensitive | +| `GITHUB_TOKEN` | the GitHub release | `GitHub__GITHUB_TOKEN`; also read from process env | +| `LOCAL_NUGET_FEED_PATH` | local publishing | `PublishLocalNuGet__LOCAL_NUGET_FEED_PATH`; also read from process env | + +The config binder does not map the plain process variables under their settings sections, which is +why the settings classes read the process environment directly as a fallback. + +## Diagnostics variables + +| Variable | Purpose | +| --- | --- | +| `PURVIEW_BUILD_CONFIG` | which config file to read | +| `PURVIEW_BUILD_USER_CONFIG` | also read machine-local user config; local runs only | +| `PURVIEW_BUILD_STACKTRACE` | include stack traces. Unset, a failure prints only the failing module and its output, then exits 1 | +| `GITHUB_REF` | the ref the eligibility rules evaluate, unless `Release:Context:Ref` overrides it | +| `Build__LogLevel` | pipeline verbosity | + +## Informational commands + +| Option | Effect | +| --- | --- | +| `-v`, `--version` | print the tool version and exit | +| `-h`, `--help`, `-?` | usage, options, the probe order, and the **resolved configuration path** (or "none found") | +| `release-explain` | print the release decision and exit, running no module. `--format=json` emits the stable contract the reusable workflow consumes | + +`--help` printing the resolved path is the fastest answer to "which config is it actually reading?". + +## Related skills + +`purview-build-adoption` for wiring it up, `purview-build-pipeline` for what each module does, +`purview-build-pack-validation` for the content rules, and `purview-build-release-eligibility` for +the `REL0nn` policies. diff --git a/skills/purview-build-pack-validation/SKILL.md b/skills/purview-build-pack-validation/SKILL.md new file mode 100644 index 0000000..d189c9a --- /dev/null +++ b/skills/purview-build-pack-validation/SKILL.md @@ -0,0 +1,115 @@ +--- +name: purview-build-pack-validation +description: >- + Use when a Purview.Build pack validation fails, or when writing the RequiredContent and + ForbiddenContent rules for a package - symbol pairing, PDB placement, the $(TFM) token, exhaustive + content with RequireExplicitContent, source link, determinism and compiler-flag inspection. +metadata: + product: Purview.Build + repository: purview-dev/build +--- + +# Pack validation + +`ValidatePackModule` inspects every `.nupkg` and `.snupkg` in `Build:ArtifactsFolder` and fails the +pipeline when any package has errors. Each package is reported valid or invalid in the summary. + +`CleanArtifactsModule` resets that folder before the run produces anything, so validation only ever +inspects **this run's** packages — a leftover from an earlier or differently-configured build cannot +fail (or pass) validation. `Build:CleanArtifacts=false` keeps existing artifacts. + +## Symbol packages + +- **`RequireSymbolPackage`** — every `.nupkg` must have a matching `.snupkg` (same id and version) + and vice versa. A package without its sibling is an error. +- **`RequireSymbolFiles`** — every `.snupkg` must contain at least one `.pdb`, and must contain no + non-symbol files other than OPC metadata (`[Content_Types].xml`, `_rels/`, + `package/services/metadata/`) and the `.nuspec`. + +## PDB placement + +A `.nupkg` must not contain `.pdb` files **outside `tools/`** — PDBs are delivered through the +`.snupkg`. The exception is a tool package (`PackAsTool`), whose runtime PDBs legitimately live under +`tools/`. `Purview.Build`'s own csproj strips those PDBs from the `.nupkg` after `GenerateNuspec`, so +the `.snupkg` keeps them for source-link validation. + +## Content rules + +`RequiredContent` and `ForbiddenContent` are both keyed by **package-id glob** (case-insensitive; +`"*"` matches every package) and hold entry-path glob lists with forward slashes: + +```json +"RequiredContent": { + "purview.build": [ + "tools/**/Purview.Build.dll", + "tools/**/appsettings.json", + "README.md" + ] +}, +"ForbiddenContent": { "*": ["**/*.pdb"] } +``` + +- **Required** — satisfied when any package entry matches; a missing match is an error. +- **Forbidden** — any matching entry is an error. + +### The `$(TFM)` token + +An entry may contain the literal `$(TFM)`, e.g. `lib/$(TFM)/Purview.Telemetry.dll`. It expands into +one entry **per target framework the package actually ships** — the short folder name (`net8.0`, +`net48`, `netstandard2.0`), discovered from the package's own `lib`/`ref`/`build`/`tools`/ +`frameworkAssemblies` groups — so the rule must be satisfied **independently for every one of them**. + +If the package has no detectable target frameworks, a `$(TFM)` entry is an **error**, not a silent +skip. That is deliberate: a token that quietly matched nothing would turn a required rule into no +rule at all. + +A package that ships a TFM-less folder is the case where `$(TFM)` does not work — the token expands +to include `any`, and the rule could never be met. Write the literal paths instead. + +### `RequireExplicitContent` — exhaustive, and on by default + +When `true` (**the default**), `RequiredContent` stops being a "must contain at least" list and +becomes the precise definition of every package's contents: + +- every produced `.nupkg`'s id must match a `RequiredContent` key — **a generated package with no + matching rule is an error**; +- every entry in a matched package, after `$(TFM)` expansion, must match one of that package's globs + — **any undeclared entry is an error**. + +Standard NuGet/OPC metadata is excluded from that check: `.nuspec`, `[Content_Types].xml`, `_rels/`, +`package/services/metadata/`, `.signature.p7s`. + +`ForbiddenContent` is unaffected and continues to apply as a plain deny-list. + +This is the setting behind most first-run failures after adding a package or an asset: the +manifest is now wrong, not the package. Add the entry rather than turning the check off. + +## Assembly inspection + +Triggered when **any** of `RequireSourceLink`, `RequireDeterministic` or `RequiredCompilerFlags` is +enabled. Each `.dll`/`.exe` in the `.nupkg` **that the package ships symbols for** — a sibling PDB +exists in the `.snupkg` or `.nupkg` — is inspected: + +| Check | What is read | +| --- | --- | +| `RequireDeterministic` | the PE must carry the `Reproducible` debug directory entry. Bundled third-party binaries without symbols are not judged. | +| `RequireSourceLink` | the matching portable PDB must contain a Source Link record (custom debug info GUID `CC110556-A091-4D38-9FEC-25AB9A351A6A`) | +| `RequiredCompilerFlags` | the PDB's compiler-flags record (GUID `B5FEEC05-8CD0-4A83-96DA-466284BB4BD8`, NUL-separated `key=value` pairs) must contain each required flag, matched case-insensitively — e.g. `optimization=release` | + +Unreadable packages and invalid PE/PDB files are errors. + +A practical consequence: an **analyzer-only** package that ships no PDB is never inspected, so these +checks neither pass nor fail for it. An analyzer package that *does* embed a PDB under +`analyzers/dotnet/` is inspected — and if a merge pass strips the compiler-flags record, the +`RequiredCompilerFlags` check fails. Either stop embedding the PDB, or relax the check with a +comment saying why. + +## Filename checks + +Package file names must match the nuspec id and version: `..nupkg` and +`..snupkg`. + +## Related skills + +`purview-build-configuration` for the `PackValidation` keys, and `purview-build-pipeline` for where +`ValidatePackModule` sits in the graph. diff --git a/skills/purview-build-pipeline/SKILL.md b/skills/purview-build-pipeline/SKILL.md new file mode 100644 index 0000000..d8809cc --- /dev/null +++ b/skills/purview-build-pipeline/SKILL.md @@ -0,0 +1,178 @@ +--- +name: purview-build-pipeline +description: >- + Use when reasoning about what a Purview.Build run does or why a module was skipped - the module + graph and its dependency edges, each module's behaviour and skip condition, the DotNet and Web + paths, and how the resolved version flows from VersionModule to pack and release. +metadata: + product: Purview.Build + repository: purview-dev/build +--- + +# The pipeline modules + +A Modular Pipelines orchestration. Modules are registered in `BuildPipeline.cs`; `Program.cs` owns +the CLI — informational options, the version banner, failure reporting. Explicit `[DependsOn]` edges +define ordering, and `ModuleConfiguration` skip conditions gate opt-in behaviour. Categories are +`Build` and `Release`. + +```text +CleanArtifacts → { Version, Restore → Build → Test, Restore → Lint } → Pack → ValidatePack → Publish → GitHub release + └→ ReportEligibility +``` + +**`Lint` and `ReportEligibility` are gates and reports, not inputs.** Nothing depends on either, so +neither feeds `Pack`. + +Every module logs a line when it starts (`Running BuildModule...`) before its command output, so a +long run still reports progress in CI logs where the live progress display is disabled. + +## CleanArtifacts + +Deletes `Build:ArtifactsFolder` when it exists and recreates it empty, **before any other module +runs**. `VersionModule` and `RestoreModule` depend on it, so the reset completes first. + +The folder is shared output — pack writes it, validation inspects every package in it, publishing +moves packages out of it, the release step can upload its contents. A leftover package from an +earlier or differently-configured run would otherwise be validated, published or uploaded as if it +belonged to this run. + +*Skipped when* `Build:CleanArtifacts` is false, **or** `Build:RunPack` is false — nothing will be +packed, so an existing folder (one being inspected ahead of a manual publish, say) is left alone. + +## Version + +Resolves the release units **once**, through `Version:Source`, and exposes them as an immutable +module result. Every downstream module reads that result; **none recomputes a version.** + +The result is an ordered **set**, not a scalar, even though every current source yields exactly one. +Each unit carries its id, version, prerelease flag and label, line (`MAJOR.MINOR`), channel, tag +(`v{version}`) and source. Modelling it as a set means adding a second unit needs no change in +`PackModule` or `CreateGitHubReleaseModule` — both enumerate. + +`PackageJson` reads the `version` field of the repository-root `package.json` and validates it +against `Version:Strictness`. It fails when the file is missing, the field is missing or empty, or +the value does not satisfy the strictness. + +The tag preserves the version **exactly as written**: a normalised tag would drop a trailing `.0` +revision and desynchronise from the declared version. + +## ReportEligibility + +Category `Build`. Depends on `Version`. No skip condition. + +Evaluates the `Release:Eligibility` rules and **reports** the verdict — the policy name, every +rule's outcome and message in evaluation order, and the short-circuit point — to the log and run +summary. **It never acts on the verdict.** + +It is deliberately *not* category `Release`: it observes, and must never decline a release. The +workflow owns the decision (it runs `release-explain`, reads the verdict, sets `Release__Mode`). +This module exists so a real run still records *why* a release was or was not eligible — which a +workflow-only gate never surfaced in the tool's own log. + +Every failure path inside it is a **warning**. A misconfigured policy or an unreadable simulated tag +list must not fail a build that would otherwise succeed. When `Release:Context:*` supplied a +simulated context, the report says so. + +## Restore + +`Build:WebInstallCommand` (default `bun install`) when `Build:ProjectType` is `Web`; otherwise +`dotnet restore` against `Build:Solution`. + +## Build + +Depends on `Restore`. + +- **DotNet** — `dotnet build` against `Build:Solution`, with `Build:Configuration` and + `--no-restore`. +- **Web** — when `WebBuildCommand` is left at its default **and** the repository declares a + `data:sync` script, it runs `bun run data:sync` first, so a fresh checkout can build offline. + Overriding `WebBuildCommand` takes full control, and the automatic data sync is then skipped. + +## Lint + +Depends on `Restore` — Web lint invokes tooling that `bun install` put there. *Skipped when* +`Build:RunLint` is false. + +- **DotNet** — `dotnet tool restore` (retried up to 3 times with a 2-second backoff), then + `dotnet tool run csharpier check `. The root is resolved by walking up to the + nearest `package.json`. +- **Web** — `WebFormatCheckCommand` then `WebLintCommand`, each skipped when the corresponding + script is not declared in the root `package.json`. + +## RunTests + +Depends on `Build`. *Skipped when* `Build:RunTests` is false. + +- **DotNet** — enumerates `*.csproj` recursively under `Build:TestRoot`, matching names against + `Build:TestPatterns` (comma-separated, case-insensitive globs), then narrows to `Build:TestProjects` + (default `*`). No matches logs a **warning** and returns empty. Each project runs with + `dotnet test --no-build --no-restore`, in parallel. + - **TUnit** (default) — passes `--ignore-exit-code 8` (Microsoft.Testing.Platform exits 8 when no + tests are selected; treated as success) and `--treenode-filter ` when `TestFilter` is + non-empty. + - **xUnit** — `--filter ` when non-empty. +- **Web** — `WebTestCommand` from the repository root. + +Per-project timings are logged, ordered by elapsed time. + +## Pack + +Depends on `RunTests` and `Version`. *Skipped when* `Build:RunPack` is false. + +Packs **once per release unit**, so a multi-unit version source needs no change here. + +- **DotNet** — `dotnet pack` with `--output ` and + `-p:PackageVersion= -p:Version=` from the resolved release unit. +- **Web** — zips `Build:WebBuildOutput` into `-.zip`. Logs a warning and + produces nothing when the build output directory does not exist. + +## ValidatePack + +Depends on `Pack`. *Skipped when* `Build:ValidatePack` is false **or** `ProjectType` is `Web` (no +`.nupkg` to inspect). + +Inspects every `.nupkg`/`.snupkg` in `ArtifactsFolder` and fails the run if any package has errors. +Because `CleanArtifacts` cleared the folder, those are exactly the packages this run packed. + +## PublishNuGet + +Category `Release`. Depends on `Pack`, `ValidatePack`, `RunTests`. *Skipped when* `ProjectType` is +`Web`, **or** the resolved `Release:Publish` is false, **or** `NuGet:TrustedPublishing` is false and +no API key resolves. + +Pushes every `*.nupkg` with `--skip-duplicate`, to the channel's `FeedUrl` when `Release:Channel` +declares one and `NuGet:FeedUrl` otherwise. With `TrustedPublishing`, pushes without an API key. +With `Release:DryRun`, logs the push and performs none. + +## PublishLocalNuGet + +Category **`Build`** — it never reaches a remote feed. Depends on `Pack` and `ValidatePack`. +*Skipped when* `ProjectType` is `Web`, **or** the tool is not running locally, **or** `Release:Mode` +is not `LocalNuGet`. The mode is intentionally ignored in CI. + +Validates the local feed path (which must be **absolute**; a drive-relative path such as `p:foo`, +left by a sh-style shell stripping backslashes, is rejected with a remediation message), moves the +packages into the feed, and optionally clears the NuGet caches and shuts down the build server. + +## CreateGitHubRelease + +Category `Release`. Depends on `PublishNuGet`, `ValidatePack`, `Version`. *Skipped unless* the +resolved `Release:GitHubRelease` is true **and** a GitHub token resolves. + +One release per release unit, with that unit's tag and `GenerateReleaseNotes = true`. A prerelease +**version** becomes a GitHub **prerelease** unless `Release:MarkPrerelease` — or the channel's — is +false, so a prerelease build is never presented as the latest stable. That changes GitHub metadata +only: prerelease versions are still published to the feed. + +With `Release:UploadArtifacts`, every file in `ArtifactsFolder` is uploaded as an asset. With +`DryRun`, it logs and creates nothing. + +**The tag must not already exist.** The workflow gates eligibility; the tool evaluates and reports +it but does not skip an existing tag itself. + +## Related skills + +`purview-build-configuration` for every setting named above, `purview-build-pack-validation` for +what `ValidatePack` actually checks, and `purview-build-release-eligibility` for the verdict +`ReportEligibility` prints. diff --git a/skills/purview-build-release-eligibility/SKILL.md b/skills/purview-build-release-eligibility/SKILL.md new file mode 100644 index 0000000..549adf5 --- /dev/null +++ b/skills/purview-build-release-eligibility/SKILL.md @@ -0,0 +1,153 @@ +--- +name: purview-build-release-eligibility +description: >- + Use when a Purview.Build release is skipped or fails a REL0nn rule, or when choosing a release + model - release-on-main, main-as-head with a release branch, or per-line release branches. Covers + the rules, the built-in policies, defining your own, and simulating an evaluation with + Release:Context. +metadata: + product: Purview.Build + repository: purview-dev/build +--- + +# Release eligibility + +Two layers, and keeping them apart is the design: + +- **The workflow owns the decision.** It runs `release-explain`, reads the verdict, and sets + `Release__Mode`. +- **The tool owns evaluation and execution.** `ReportEligibilityModule` evaluates and *reports* the + verdict during a real run — it never declines a release. + +```text +Trigger → resolve version → evaluate REL0nn → workflow reads the verdict → execute → publish, tag, release +``` + +Verdicts: **Release** (exit 0), **Skip** (exit 0 — the job finishes green, nothing published), or +**Fail** (exit 1, naming the rule). + +## Checking before you push + +```shell +dotnet purview-build release-explain +dotnet purview-build release-explain --format=json # the contract the reusable workflow consumes +``` + +## The rules + +They short-circuit **in ID order**. The first non-passing rule decides the verdict. + +| Rule | Passes when | In the default policy | +| --- | --- | --- | +| `REL001` | the version parses for the configured strictness | yes | +| `REL002` | `v{version}` is not already tagged, and the version is not already on the target feed | yes | +| `REL003` | a stable (non-prerelease) version originates from a ref in `StableRefs` | yes | +| `REL004` | a non-zero `PATCH` originates from a ref matching `ServicingRefs` | no | +| `REL005` | the version is strictly greater than the highest existing version on the same line | no | +| `REL006` | a four-part version is permitted — `AllowFourPart`, and the ref matches `FourPartRefs` | no | +| `REL007` | the prerelease label matches the channel's allowed pattern | no | + +`REL002` is the one that produces a **Skip** rather than a failure: an already-released version means +there is nothing to do, so merging `main` into `release` releases exactly once however many times it +happens. + +**Only the rules a policy enables are evaluated.** A disabled rule is skipped entirely rather than +passing vacuously — which is why adopting a stricter policy can surface a problem that was always +there. + +## Built-in policies + +| Policy | Rules | Ref sets | +| --- | --- | --- | +| `ReleaseOnMain` (default) | `REL001`–`REL003` | `StableRefs`: `refs/heads/main`, `refs/heads/release` | +| `TrunkReservesMinor` | `REL001`–`REL005` | `StableRefs`/`ServicingRefs`: `refs/heads/release/*`; `TrunkRefs`: `refs/heads/main` | +| `FourPartServicing` | `TrunkReservesMinor` + `REL006` | as above, plus `AllowFourPart` and `FourPartRefs`: `refs/heads/release/*` | + +## The three models + +### Model A — release on `main` + +Every merge to `main` may release. Trigger `push: branches: [main]`, policy `ReleaseOnMain`. + +```text +PR → build, lint, test → merge to main → release-explain + Release → pack, validate, publish, tag + Skip (already tagged) → job succeeds, nothing published + Fail (policy violation) → job fails, naming the REL0nn rule +``` + +### Model B — main-as-head with a release branch + +Development merges to `main`; merging `main` into `release` performs the release. Trigger +`push: branches: [release]`, policy `ReleaseOnMain`. + +### Model C — per-line release branches + +Merging to `main` ships **nothing**. Work accumulates on trunk, and a release is cut from a +`release/` branch, so a serviced stable line and an in-flight prerelease line advance +independently. + +```text +main (trunk, reserves the minor) + ├─ merge or cherry-pick → release/2.0 → v2.0.1, v2.0.2 (serviced stable line) + └─ branch when the minor opens → release/2.1 → v2.1.0-prerelease.1, .2 +``` + +Trunk itself is guarded: a stable version from `main` fails `REL003` (trunk is not a `StableRef`), +and a non-zero `PATCH` from `main` fails `REL004` (a patch is serviced from the line). + +```yaml +on: + push: + branches: ['release/**'] + workflow_dispatch: + +concurrency: + # Callers own release serialisation; the reusable workflow defines no concurrency group. + group: release-${{ github.ref }} + cancel-in-progress: false + +jobs: + release: + uses: purview-dev/build/.github/workflows/purview-release.yml@main + with: + release-mode: NuGet + eligibility-policy: TrunkReservesMinor + secrets: inherit +``` + +## Defining a policy + +`Release:Eligibility:Policies` is merged **over** the built-ins, so declaring a built-in's name +overrides it. + +| Key | Meaning | +| --- | --- | +| `Inherits` | the policy this one is a diff from. A missing parent is an error naming the unresolved policy | +| `Rules` | either an **absolute** list (`["REL001","REL002"]`) or **deltas** against the parent (`["+REL006","-REL005"]`). **Mixing the two forms is an error.** | +| `StableRefs`, `ServicingRefs`, `TrunkRefs`, `FourPartRefs` | ref patterns. A single trailing `*` is a prefix match (`refs/heads/release/*`). A non-empty child list **replaces** the parent's, so a policy can narrow a ref set | +| `AllowFourPart` | whether four-part versions are permitted at all (`REL006`) | + +## Simulating an evaluation + +`Release:Context` lets you answer "what would happen on that branch?" without being on it: + +| Key | Effect | +| --- | --- | +| `Ref` | the evaluated ref; defaults to `GITHUB_REF`, else the current git branch | +| `ExistingTags` | a path to a newline- or JSON-delimited tag list, used instead of querying git | +| `PublishedVersions` | a path to a version list treated as already on the feed, instead of querying it | + +When a simulated context is in play, `ReportEligibilityModule`'s report says so — a simulated verdict +is never mistaken for a real one. + +## Every failure here is a warning + +Inside `ReportEligibilityModule`, every failure path is a **warning**. A misconfigured policy or an +unreadable simulated tag list must not fail a build that would otherwise succeed: the module +observes, and the workflow decides. + +## Related skills + +`purview-build-adoption` for the caller workflows, `purview-build-configuration` for the +`Release:*` settings, and `purview-build-pipeline` for where the release modules sit. diff --git a/src/Build.slnx b/src/Build.slnx index bd7833f..be559da 100644 --- a/src/Build.slnx +++ b/src/Build.slnx @@ -17,5 +17,6 @@ + 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 +} diff --git a/src/src/Build/Build.csproj b/src/src/Build/Build.csproj index 265aa43..e61aea7 100644 --- a/src/src/Build/Build.csproj +++ b/src/src/Build/Build.csproj @@ -6,7 +6,8 @@ purview-build Shared build/test/release pipeline for purview-dev. A Modular Pipelines based .NET tool (Purview.Build) with a GitHub composite action and reusable workflows; consumers configure, the pipeline runs. - $(NoWarn);CA1873; + $(NoWarn);CA1515;CA1873; + true true @@ -15,11 +16,7 @@ - + @@ -59,11 +56,7 @@ - + <_StrippableToolPackage Include="$(PackageOutputPath)/**/*.nupkg" Exclude="$(PackageOutputPath)/**/*.snupkg" /> @@ -79,6 +72,7 @@ + @@ -91,14 +85,8 @@ It is embedded so validation never depends on the filesystem, and copied to output so the packaged tool ships it for editors and consumers. --> - - + + PreserveNewest diff --git a/src/src/Build/Helpers/PackageInspector.cs b/src/src/Build/Helpers/PackageInspector.cs index 0e7178c..815f178 100644 --- a/src/src/Build/Helpers/PackageInspector.cs +++ b/src/src/Build/Helpers/PackageInspector.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.FileSystemGlobbing; +using NuGet.Frameworks; using NuGet.Packaging; using System.Reflection.Metadata; using System.Reflection.PortableExecutable; @@ -70,6 +71,14 @@ IReadOnlyCollection targetFrameworkFolderNames )]; } + // NuGet's framework discovery also reports the pseudo-frameworks "any", "agnostic" and + // "unsupported" for some package layouts - most commonly the "any" target framework that a + // package's contentFiles group contributes. They are not real target frameworks, so they must + // never be substituted for $(TFM): doing so expands the token to an entry such as + // "lib/any/Foo.dll" that can never match, turning a valid package into a validation failure. + static bool IsTargetFrameworkFolderName(string folderName) => + NuGetFramework.Parse(folderName).IsSpecificFramework; + public static void ValidateContentRules( IReadOnlyList files, string packageId, @@ -80,7 +89,7 @@ public static void ValidateContentRules( bool requireExplicitContent = false ) { - targetFrameworkFolderNames ??= []; + targetFrameworkFolderNames = [.. (targetFrameworkFolderNames ?? []).Where(IsTargetFrameworkFolderName)]; var required = GetContentRule(requiredRules, packageId); if (required is null) diff --git a/src/src/Build/Schema/SchemaDocumentation.cs b/src/src/Build/Schema/SchemaDocumentation.cs index 0d74ba1..aaf0197 100644 --- a/src/src/Build/Schema/SchemaDocumentation.cs +++ b/src/src/Build/Schema/SchemaDocumentation.cs @@ -25,7 +25,7 @@ public static IReadOnlyDictionary Load(Assembly assembly) if (members is null) return Empty; - Dictionary documentation = new(StringComparer.Ordinal); + Dictionary documentation = [with(StringComparer.Ordinal)]; foreach (var member in members) { var name = member.Attribute("name")?.Value; diff --git a/src/src/Build/Settings/PackValidationSettings.cs b/src/src/Build/Settings/PackValidationSettings.cs index afcfe6a..5548c33 100644 --- a/src/src/Build/Settings/PackValidationSettings.cs +++ b/src/src/Build/Settings/PackValidationSettings.cs @@ -53,6 +53,9 @@ public sealed record PackValidationSettings /// "lib/$(TFM)/Foo.dll". The token is expanded into one required entry per target framework /// the package actually supports (short folder name, e.g. "net8.0", "net48", "netstandard2.0"), /// so every one of the package's target frameworks must independently satisfy the entry. + /// Pseudo-frameworks reported by NuGet's package discovery ("any", "agnostic", "unsupported" - + /// notably the "any" target framework a package's contentFiles group contributes) are not + /// target frameworks and are ignored. /// public Dictionary RequiredContent { get; init; } = []; diff --git a/src/tests/Build.UnitTests/GlobTests.cs b/src/tests/Build.UnitTests/GlobTests.cs index 07fd6de..d958467 100644 --- a/src/tests/Build.UnitTests/GlobTests.cs +++ b/src/tests/Build.UnitTests/GlobTests.cs @@ -195,6 +195,30 @@ public async Task TfmToken_NoFrameworksDetected_ReportsError() await Assert.That(errors).Count().IsEqualTo(1); } + // NuGet's framework discovery also reports the "any" pseudo-framework for a package whose + // nuspec declares a contentFiles group, alongside the package's real target frameworks. It is + // not a target framework, so it must never be substituted for $(TFM): doing so expands the + // token to an entry such as "lib/any/Foo.dll" that can never match, and a valid package then + // fails validation. + [Test] + public async Task TfmToken_ContentFilesAnyFramework_IsIgnored() + { + List errors = []; + string[] files = ["lib/net10.0/Foo.dll", "contentFiles/any/any/somefile.json"]; + + PackageInspector.ValidateContentRules( + files, + "purview.build", + new() { ["purview.build"] = ["lib/$(TFM)/Foo.dll", "contentFiles/any/any/somefile.json"] }, + [], + errors, + targetFrameworkFolderNames: ["any", "net10.0"], + requireExplicitContent: true + ); + + await Assert.That(errors).IsEmpty(); + } + [Test] public async Task ExplicitContent_AllDeclared_Passes() { diff --git a/src/tests/Build.UnitTests/PackValidationSettingsTests.cs b/src/tests/Build.UnitTests/PackValidationSettingsTests.cs index ad069ef..664f406 100644 --- a/src/tests/Build.UnitTests/PackValidationSettingsTests.cs +++ b/src/tests/Build.UnitTests/PackValidationSettingsTests.cs @@ -34,6 +34,7 @@ public async Task RequiresAssemblyInspection_GivenTheShippedDefaults_ReturnsFals public async Task RequiredCompilerFlags_GivenAnEmptyArrayInConfiguration_StaysEmpty() { var settings = Bind( + /*lang=json,strict*/ """ { "PackValidation": { @@ -52,7 +53,9 @@ public async Task RequiredCompilerFlags_GivenAnEmptyArrayInConfiguration_StaysEm [Test] public async Task RequiredCompilerFlags_GivenFlagsInConfiguration_BindsThem() { - var settings = Bind("""{ "PackValidation": { "RequiredCompilerFlags": [ "optimization=release" ] } }"""); + var settings = Bind( /*lang=json,strict*/ + """{ "PackValidation": { "RequiredCompilerFlags": [ "optimization=release" ] } }""" + ); await Assert.That(settings.RequiredCompilerFlags).IsEquivalentTo(["optimization=release"]); await Assert.That(PackageInspector.RequiresAssemblyInspection(settings)).IsTrue(); diff --git a/src/tests/Repository.UnitTests/Repository.UnitTests.csproj b/src/tests/Repository.UnitTests/Repository.UnitTests.csproj new file mode 100644 index 0000000..ade4cc0 --- /dev/null +++ b/src/tests/Repository.UnitTests/Repository.UnitTests.csproj @@ -0,0 +1,7 @@ + + + + + + diff --git a/src/tests/Repository.UnitTests/SkillTests.cs b/src/tests/Repository.UnitTests/SkillTests.cs new file mode 100644 index 0000000..27846b7 --- /dev/null +++ b/src/tests/Repository.UnitTests/SkillTests.cs @@ -0,0 +1,60 @@ +using Purview.Skills.Testing; + +namespace Purview.Build.Repository; + +/// +/// 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-build", StringComparison.Ordinal)) + .ToArray(); + + await Assert + .That(unprefixed) + .IsEmpty() + .Because( + "every skill this repository publishes must carry the product prefix: " + string.Join(", ", unprefixed) + ); + } +}