diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml new file mode 100644 index 00000000..7c8e76a5 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -0,0 +1,69 @@ +name: Bug +description: Something compiles but does less than it should, throws when it shouldn't, or doesn't refuse when it should. +labels: [bug] +body: + - type: markdown + attributes: + value: | + The bugs that matter most here don't crash. A value object builds but was never generated, a + failed `Result` goes unnoticed, an analyzer stops reporting, a column reads back a value the + type would have refused. If that's what you found, please say so below. It decides how soon + the fix ships. + - type: dropdown + id: package + attributes: + label: Package + multiple: true + options: + - CodoMetis.TypeKit + - CodoMetis.TypeKit.Analyzers + - CodoMetis.TypeKit.Generators + - CodoMetis.TypeKit.EntityFrameworkCore + - CodoMetis.TypeKit.AspNetCore + - CodoMetis.TypeKit.Serilog + - CodoMetis.TypeKit.Testing + validations: + required: true + - type: input + id: version + attributes: + label: Version + placeholder: "2.2.0" + validations: + required: true + - type: dropdown + id: failure + attributes: + label: How does it fail? + options: + - Silently. It compiles or answers, and does less than it appears to + - Loudly. It throws, fails the build or reports a diagnostic where it should work + - I'm not sure + validations: + required: true + - type: textarea + id: repro + attributes: + label: What you ran + description: The smallest code that shows it. For EF Core, include the query and the SQL it translated to; for JSON or OpenAPI, the payload or the schema. + render: csharp + validations: + required: true + - type: textarea + id: actual + attributes: + label: What you got + validations: + required: true + - type: textarea + id: expected + attributes: + label: What you expected + description: If a docs page or a README says what should happen, link it. + validations: + required: true + - type: input + id: environment + attributes: + label: Environment + description: .NET SDK version, OS, whether you publish with Native AOT, and the database provider if EF Core is involved. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..d99c7094 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: false +contact_links: + - name: Security report + url: https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/security/advisories/new + about: Report a vulnerability privately, as SECURITY.md describes. Please don't open a public issue for it. diff --git a/.github/ISSUE_TEMPLATE/enhancement.yml b/.github/ISSUE_TEMPLATE/enhancement.yml new file mode 100644 index 00000000..eb3cb593 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/enhancement.yml @@ -0,0 +1,28 @@ +name: Enhancement +description: A new member, generated feature, analyzer rule, satellite or option. +labels: [enhancement] +body: + - type: textarea + id: problem + attributes: + label: What you're trying to do + description: The task, not the API. What do you write today, and why doesn't it work? + validations: + required: true + - type: textarea + id: proposal + attributes: + label: What you'd like + description: A sketch of the code you'd write with it. + render: csharp + - type: dropdown + id: breaking + attributes: + label: Does it change something that compiles or works today? + description: A change to an existing signature, a generated member, a wire form or a diagnostic's severity needs a major version, even if the new behaviour is better. + options: + - No, it only adds + - Yes, existing code would compile or behave differently + - I'm not sure + validations: + required: true diff --git a/.github/ISSUE_TEMPLATE/epic.yml b/.github/ISSUE_TEMPLATE/epic.yml new file mode 100644 index 00000000..b408153d --- /dev/null +++ b/.github/ISSUE_TEMPLATE/epic.yml @@ -0,0 +1,28 @@ +name: Epic +description: A release or a theme. Its sub-issues are the work. (Maintainers.) +title: "Epic: " +labels: [epic] +body: + - type: textarea + id: goal + attributes: + label: Goal + description: What is true when this is done, in one or two sentences. For a release, link its section of design/roadmap.md. + validations: + required: true + - type: textarea + id: scope + attributes: + label: In scope and out of scope + validations: + required: true + - type: textarea + id: done + attributes: + label: Done when + description: Every sub-issue is closed, plus anything that isn't an issue (a tag, a published package, a doc). + validations: + required: true + - type: markdown + attributes: + value: Add the work as sub-issues once this exists. For a release, set the milestone on the epic and on every sub-issue. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..1b0c17ac --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,22 @@ +Closes # + + + +## Problem + + + +## Change + + + +## Tests + + diff --git a/.github/workflows/pr-hygiene.yml b/.github/workflows/pr-hygiene.yml new file mode 100644 index 00000000..421ec67c --- /dev/null +++ b/.github/workflows/pr-hygiene.yml @@ -0,0 +1,51 @@ +# Every pull request into main or a release branch carries one type label, a severity if it is a bug, +# the milestone of the release it goes into, a branch named / that agrees with its type, and a +# closing reference to the issue it resolves; a breaking one goes into a major (CONTRIBUTING.md, "Issues and +# labels"; design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md). The rules +# live in tools/PrHygiene, a tool project like the others (design/adr/0092), which reads the event payload +# alone: no token, nothing of the pull request's code run, and runnable locally on a saved payload. +# +# Triggered on label, milestone and description edits as well as pushes, so adding what is missing turns +# the check green without a new commit. `pull_request`, not `pull_request_target`: a pull request from a +# fork runs with a read-only token and never sees a secret, and builds the tool from its own checkout, +# which can only weaken the check on that pull request's own run, where a reviewer reads the diff. +# +# Actions are pinned to commit SHAs, as in dotnet.yml. + +name: PR hygiene + +on: + pull_request: + branches: [ "main", "release/**" ] + types: [ opened, edited, reopened, synchronize, labeled, unlabeled, milestoned, demilestoned ] + +permissions: + contents: read + +concurrency: + group: pr-hygiene-${{ github.event.pull_request.number }} + cancel-in-progress: true + +env: + DOTNET_NOLOGO: true + DOTNET_CLI_TELEMETRY_OPTOUT: true + DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true + +jobs: + check: + name: Labels and issue link + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + + - name: Setup .NET + uses: actions/setup-dotnet@26b0ec14cb23fa6904739307f278c14f94c95bf1 # v5.4.0 + with: + global-json-file: global.json + + - name: Build the check + run: dotnet build tools/PrHygiene -c Release + + - name: Check labels, milestone, branch and issue link + run: dotnet tools/PrHygiene/bin/Release/net10.0/pr-hygiene.dll "$GITHUB_EVENT_PATH" diff --git a/AGENTS.md b/AGENTS.md index 06300ff6..1e17bca7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,6 +62,7 @@ dotnet test --project test/CodoMetis.TypeKit.Conventions.Tests --filter-method " dotnet build spikes/FabricSpike/FabricSpike.slnx # the fabric spike, standalone dotnet tools/ConsumerSmokeTest/bin/Release/net10.0/consumer-smoke-test.dll # the packages, installed into throwaway consumers, one published with Native AOT (needs the platform linker); build it with -c Release first dotnet tools/BuildVerdict/bin/Release/net10.0/build-verdict.dll test [--project P] [--filter-class C] [--filter-method M] [-- flags] # an agent's build and test: a verdict, not the transcript +dotnet tools/PrHygiene/bin/Release/net10.0/pr-hygiene.dll event.json # a PR's labels, milestone, branch and issue link, as CI's PR hygiene check reads them from the payload ``` The probing tool, prober (`tools/Prober`, started as its built dll like docs-query). These need no @@ -221,6 +222,28 @@ the whole page. A ranked miss proves nothing. analyzer that stops firing, or a converter that materialises `default` are this repo's characteristic failures. Prefer a diagnostic or an exception to a plausible result. +## Issues +Work is tracked as GitHub issues, not as lists in docs or memory +([decision 109](design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md)). +Open the issue before starting the work: a bug, an enhancement, a follow-up, a finding from a review or a +probe. +- **Labels, on issues and PRs alike** (ValueRanges' scheme; the legend is CONTRIBUTING's "Issues and + labels", and `gh label list` the source): exactly one type (`bug`, `enhancement`, `documentation`, + `tests`, `chore`), exactly one `severity/*` on a bug, the `package/*` and `area/*` it touches, and + `silent` (a bug that does less than it appears to), `breaking`, `found-by/prober` or `stacked` where they + apply. Never invent a label; if none fits, say so. +- **Epics:** a release is an `epic` issue with a milestone of its version (`2.3.0`), its work attached as + native sub-issues with the same milestone (`gh issue create --parent N --milestone M`). A theme that spans + releases is an `epic` without a milestone; a finding belongs to the release it ships in. The plan stays in + `design/roadmap.md`; there is no roadmap issue. +- **PRs** carry their issue's labels and milestone, open with `Closes #n`, and follow + `.github/pull_request_template.md` (Problem, Change, Tests). `pr-hygiene.yml` runs `tools/PrHygiene` on + every PR into `main` or a release branch and fails a missing type, severity or closing reference, a branch + whose `/` disagrees with its type label, a PR into `release/` without that milestone, and + `breaking` into anything but a major. A topic merges into its release branch, where `Closes` closes + nothing: close its issue by hand on merge, with a comment linking the PR. +- When the work changes what an issue says (its scope, its state, a measured fact), edit the issue. + ## Workflow 1. Read `design/plan.md`, the roadmap, the decision records and the design note for the area you touch, and the relevant spike README, before starting. A decision is changed by a new record. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f94af09..00f13127 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -88,6 +88,56 @@ string, an assembly-name prefix or a type-name list will be asked to replace it. **Prefer a loud failure to a silent one.** Where the package cannot do something correctly, it should fail the build or throw with an actionable message. +## Issues and labels + +Use the **Bug** or **Enhancement** form. The bug form asks whether the failure is silent or loud, because a +silent one, code that compiles and does less than it appears to, is the failure this repository exists to +prevent. + +Each release is an `epic` issue with a milestone of its version (`2.3.0`), and its work is attached as +sub-issues with the same milestone; a theme that spans releases is an `epic` without one. The plan stays in +[design/roadmap.md](design/roadmap.md), which an epic links rather than copies +([decision 109](design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md)). +A maintainer sets the labels when triaging, and a pull request gets the same labels and milestone as the +issue it closes: + +| Label | Means | +|---|---| +| `bug`, `enhancement`, `documentation`, `tests`, `chore` | The type. Exactly one | +| `severity/critical` | Data loss, or silently wrong results, in common use | +| `severity/high` | Wrong results, a crash or a failed build in a realistic scenario | +| `severity/medium` | An inconsistency or a failure in a narrow scenario | +| `severity/low` | A cosmetic, message or documentation-level difference | +| `silent` | A bug that compiles or answers and does less than it appears to | +| `breaking` | Changes a signature, a generated member, a wire form or a diagnostic's severity: needs a major | +| `found-by/prober` | Found by the prober; the fix's commit carries `Found-By: PR` | +| `stacked` | Based on another open pull request: merge in order | +| `epic` | A release or a theme; its sub-issues are the work | +| `package/core`, `package/analyzers`, `package/generators`, `package/efcore`, `package/aspnetcore`, `package/serilog`, `package/testing` | The package it affects | +| `area/option-result` | `Option`, `Result` and their extensions | +| `area/value-objects` | The value-object contracts and what the aspects generate for them | +| `area/json` | System.Text.Json wire forms, values and dictionary keys | +| `area/aot` | Native AOT and trimming | +| `area/parsing` | `Parse`, `TryParse`, formatting and the type converter | +| `area/release` | Versioning, changelogs, packaging and release wiring | +| `area/tooling` | The repository's tools: the prober, the verdict, the guard, docs-query, the smoke test | + +A bug carries exactly one severity. CI's "PR hygiene" check reads each pull request into `main` or a release +branch and fails it with no type label, a bug with no severity, `silent` on anything but a bug, no closing +reference, a branch whose `/` disagrees with its type label (`feat` is `enhancement`, `fix` is `bug`, +`docs` is `documentation`, `test` is `tests`, the rest `chore`), a pull request into `release/` +without that milestone, or `breaking` on a release that is not a major. It turns green when you add what is +missing, without a new commit. To run it on a saved payload: + +```bash +dotnet build tools/PrHygiene -c Release +dotnet tools/PrHygiene/bin/Release/net10.0/pr-hygiene.dll path/to/event.json +``` + +`Closes #12` closes an issue only when the pull request merges into `main`, and topics merge into a release +branch, so close the issue by hand when your topic merges, with a comment linking the pull request. The +release's pull request into `main` closes its epic. + ## Pull requests Work for the next version goes into its release branch, `release/` (`release/2.2.0`), not @@ -96,7 +146,9 @@ Branch from it as `/`, where the type is your main commit's (`feat/ `build/…`), and open the PR against it. PRs are merged with a merge commit, never squashed or rebased: the design records cite commits by hash. -- One concern per PR, with the reasoning in the description rather than only in the diff. +- One concern per PR, closing one issue, with its labels and milestone ([above](#issues-and-labels)). + The description follows the template: `Closes #n`, then Problem, Change and Tests, with the reasoning + there rather than only in the diff. - Full suite green. - User-visible changes get a changelog entry. - Match the surrounding style. Comments explain *why*, especially where behaviour is load-bearing @@ -121,7 +173,8 @@ The packages share one version and are released together, only through 2. Move the rules in `src/CodoMetis.TypeKit.Analyzers/AnalyzerReleases.Unshipped.md` to `AnalyzerReleases.Shipped.md`, under `## Release `. Their ids and severities are public contract from then on. `ChangelogTests` fails until steps 1 and 2 agree. -3. Commit on the release branch, open its PR into `main`, and merge it with a merge commit. Tag that +3. Commit on the release branch, open its PR into `main` (`chore` and `area/release`, the version's + milestone, `Closes #`), and merge it with a merge commit. Tag that merge commit and push the tag: `git tag v1.0.0 && git push origin v1.0.0`. The workflow checks the tag against `Version` and the dated heading, runs the suite and the smoke test, and waits for approval in the `nuget` environment before pushing. Delete the release branch once the tag is pushed. @@ -129,7 +182,7 @@ The packages share one version and are released together, only through the version just released, so ApiCompat guards the public surface from then on, and open a new `## — Unreleased` section. `ChangelogTests` fails while the baseline trails the last version the changelog dates. Once that is on `main`, cut the next `release/` from it and - push it. + push it, and open the version's milestone and its `epic` issue. `workflow_dispatch` with `dry_run` (the default) rehearses everything but the push. diff --git a/CodoMetis.TypeKit.slnx b/CodoMetis.TypeKit.slnx index a53f69b2..ee74d407 100644 --- a/CodoMetis.TypeKit.slnx +++ b/CodoMetis.TypeKit.slnx @@ -20,6 +20,7 @@ + @@ -31,6 +32,7 @@ + diff --git a/design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md b/design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md new file mode 100644 index 00000000..2844b560 --- /dev/null +++ b/design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md @@ -0,0 +1,88 @@ +# 0109. Work is tracked as labelled issues under a release epic, and CI checks each pull request's labels, milestone, branch and issue link + +Date: 2026-10-18 +Status: accepted + +Context: 2026-10-18, after the release of 2.2.0. The repository had 66 pull requests and not one issue: none +of the 66 descriptions closed an issue, and the repository had no milestone and no label beyond GitHub's +defaults and Dependabot's two. Work was planned in [the roadmap](../roadmap.md) and recorded in the commits, +the decision records and [the fix history](../fix-history.md), so what was open, what a pull request was for +and which release it belonged to could be read only from files and branch names. CodoMetis.ValueRanges, the +sibling library, tracks its work as labelled issues and has a CI check that holds each pull request to its +labels and its issue; this repository adopts the same scheme, fitted to its own branches (decision 94) and +its own tooling (decision 92). + +## Decision + +**Work is tracked as issues.** A bug, an enhancement, a follow-up, a finding from a review or a probe: each is +an issue, opened before the work starts, through one of the forms in `.github/ISSUE_TEMPLATE/` (Bug, +Enhancement, Epic; no blank issues, and a security report goes to the private advisory form). + +**A release is an epic.** Each version has an `epic` issue and a milestone of the same name (`2.3.0`); its work +is attached as native sub-issues and carries the milestone. A theme that spans releases is an `epic` without a +milestone. [The roadmap](../roadmap.md) stays the plan and the decision records the reasons: an epic links its +section of the roadmap rather than copying it, and there is no roadmap issue. + +**Labels, on issues and pull requests alike**, as ValueRanges has them: + +| Label | Means | +|---|---| +| `bug`, `enhancement`, `documentation`, `tests`, `chore` | The type, exactly one | +| `severity/critical`, `severity/high`, `severity/medium`, `severity/low` | How bad a bug is, exactly one on a bug | +| `silent` | A bug that compiles or answers and does less than it appears to | +| `breaking` | Changes a signature, a generated member, a wire form or a diagnostic's severity: needs a major | +| `found-by/prober` | Found by the prober; the fix's commit carries `Found-By: PR` | +| `stacked` | Based on another open pull request | +| `epic` | A release or a theme; its sub-issues are the work | +| `package/core`, `package/analyzers`, `package/generators`, `package/efcore`, `package/aspnetcore`, `package/serilog`, `package/testing` | The package affected, one per package | +| `area/option-result`, `area/value-objects`, `area/json`, `area/aot`, `area/parsing`, `area/release`, `area/tooling` | The cross-cutting area | + +ValueRanges' `found-by/oracle` is `found-by/prober` here, its oracles' counterpart; its `roadmap` label has no +counterpart, there being no roadmap issue. The legend a contributor reads is CONTRIBUTING's "Issues and labels". + +**A pull request carries its issue's labels and milestone** and opens with a closing keyword (`Closes #12`), +from the template in `.github/pull_request_template.md`: Problem, Change, Tests. Since `Closes` closes an issue +only on a merge into `main`, and a topic merges into its release branch, the issue is closed by hand when its +topic merges, with a comment linking the pull request; the release's pull request into `main` closes its epic. + +**Held by CI.** `.github/workflows/pr-hygiene.yml` runs on every pull request into `main` or a release branch, +and again when its labels, milestone or description change. It runs `tools/PrHygiene`, a tool project under +decision 92 (BCL only, tested in `test/CodoMetis.TypeKit.PrHygiene.Tests`), on the event payload alone, and +fails a pull request that has + +- not exactly one type label, or a bug without exactly one severity, or `silent` on anything but a bug; +- no closing reference in its description; +- a head branch not named `/` (decision 94), or whose type disagrees with its type label: `feat` is + `enhancement`, `fix` is `bug`, `docs` is `documentation`, `test` is `tests`, and `build`, `refactor`, `spike` + and `release` are `chore`; +- a base `release/`, or a head `release/` into `main`, without the milestone ``; +- the label `breaking` and a release branch that is not a major (`x.0.0`). + +ValueRanges' script checks the first two; the branch and milestone rules hold decision 94's naming and the +release a pull request belongs to, which here are visible in the payload. Its rule for a `silent` fix in a minor +has no counterpart, since this repository ships no "changed answers" section; its rule that a breaking change +waits for a major is the one above. Dependabot's pull requests are not checked. + +Its job, "Labels and issue link", is a required status check: on `main` in its branch protection, beside +the test, pack and smoke-test jobs, and on `release/**` through the ruleset "Release branches: PR hygiene", +which does not apply to creating a branch, so a release branch is still cut by a push. + +## Consequences + +- What is open, and in which release, is a GitHub query rather than a reading of the roadmap. +- The labels count what the commit trailers count from the other side: `found-by/prober` beside `Found-By: PR`, + and the severity a trailer does not record. +- A pull request that misses a label fails a check that turns green when the label is added, without a commit. +- A pull request from a fork builds the check from its own checkout, so it can weaken the check on its own run + only, where the reviewer reads the diff; the workflow uses `pull_request`, never `pull_request_target`, and has + a read-only token. + +## Evidence + +- Counted on 2026-10-18 with `gh`: 66 pull requests, 0 issues, 0 milestones; 0 of the 66 descriptions match + GitHub's closing keywords. +- CodoMetis.ValueRanges at 1da4fa8: `.github/ISSUE_TEMPLATE/{bug,enhancement,epic,config}.yml`, + `.github/pull_request_template.md`, `.github/workflows/pr-hygiene.yml` and `.github/scripts/pr-hygiene.sh`, + and its 35 labels. +- The check's rules were each proved by deleting the rule from `tools/PrHygiene` and watching its cases fail + (12 seeds, each caught). diff --git a/design/adr/README.md b/design/adr/README.md index b4692bc5..9037fb86 100644 --- a/design/adr/README.md +++ b/design/adr/README.md @@ -115,3 +115,4 @@ per area ([design](../plan.md)) link to the records they rest on. | [0106](0106-a-parameter-of-a-value-object-over-a-described-type-is-inlined.md) | A value object over a type the JSON options describe is written inline as a route, query, header or form parameter and as an element of a query or header array, where value objects are components: the component is its JSON positions'; every other value object keeps its reference; the schema below a parameter is asked for with the parameter's source and type alone | 2026-10-17 | accepted | | [0107](0107-a-cited-fact-is-executable-and-seeds-run-by-footprint.md) | A fact that prunes or excuses a row is executable: `docs-query check` fails `fact-not-executable-cited` where a refused entry, a not-promised entry, an obligation's declaration or the harness cites a fact without its test; a fact's measured answer is `KBNNN.fixture.json` beside its test, named by `fixture:`, read by a BCL-only reader the harness will use; `prober seeds` runs only the probes whose seeds identity or product footprint changed since every seed was caught, and a release runs `--all` | 2026-10-17 | accepted | | [0108](0108-a-probe-check-earns-credit-against-a-baseline-and-ws2-closes.md) | A probe row runs a check per obligation, each against a baseline, and the ledger credits checks; an excuse is a closed citation, applicability is declared on the obligation, the http, json and text harness compares three sides by exception type and stage, decision 99's refused document among them, and the twins and the context are generated with the shapes; `NoteKeys` keeps its pass keys and `framework-refuses`; `prober fix`, a covering array that places no blocked pair, and `gaps` listing the pairs no passing check read both axes of; WS2 closes, and WS3 to WS6 are dropped | 2026-10-17 | accepted; the gate passed on release/2.2.0 on 2026-10-18 (#62, with the fix #63 for the finding F4 the regenerated C21 rows raised) | +| [0109](0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md) | Work is tracked as issues through the forms in `.github/ISSUE_TEMPLATE/`; a release is an `epic` with a milestone of its version and its work as sub-issues, beside the roadmap; issues and pull requests carry ValueRanges' labels (one type, a severity on a bug, `package/*`, `area/*`, `silent`, `breaking`, `found-by/prober`, `stacked`); a pull request opens with `Closes #n` and carries its issue's labels and milestone; `pr-hygiene.yml` runs `tools/PrHygiene` on the payload and fails a missing type, severity or closing reference, a branch whose `/` disagrees with its type label, a pull request into `release/` without that milestone, and a `breaking` one into anything but a major | 2026-10-18 | accepted | diff --git a/test/CodoMetis.TypeKit.PrHygiene.Tests/CodoMetis.TypeKit.PrHygiene.Tests.csproj b/test/CodoMetis.TypeKit.PrHygiene.Tests/CodoMetis.TypeKit.PrHygiene.Tests.csproj new file mode 100644 index 00000000..a0a4c5c4 --- /dev/null +++ b/test/CodoMetis.TypeKit.PrHygiene.Tests/CodoMetis.TypeKit.PrHygiene.Tests.csproj @@ -0,0 +1,18 @@ + + + + + + + <_Parameter1>ToolOutput + <_Parameter2>$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)../../tools/PrHygiene/bin/$(Configuration)/$(TargetFramework)/')) + + + + + + + diff --git a/test/CodoMetis.TypeKit.PrHygiene.Tests/HygieneTests.cs b/test/CodoMetis.TypeKit.PrHygiene.Tests/HygieneTests.cs new file mode 100644 index 00000000..eb0fac60 --- /dev/null +++ b/test/CodoMetis.TypeKit.PrHygiene.Tests/HygieneTests.cs @@ -0,0 +1,179 @@ +using System.Text.Json; + +namespace CodoMetis.TypeKit.PrHygiene.Tests; + +/// +/// The pull requests the check must pass and the ones it must fail, one rule at a time. Each case writes the +/// event payload GitHub would and runs the built tool on it as its own process, the way the workflow does. +/// Each failing case starts from , which passes, and breaks one thing, so a case that +/// fails proves the rule it names; each rule was also proved by deleting it from the tool and watching its +/// cases fail. +/// +public sealed class HygieneTests(HygieneTests.Sandbox sandbox) : IClassFixture +{ + public sealed class Sandbox : IDisposable + { + public ToolSandbox Tool { get; } = new("PrHygiene", "pr-hygiene"); + + public void Dispose() => Tool.Dispose(); + } + + private sealed record Pr( + string[] Labels, + string? Body = "Closes #12", + string Head = "fix/serilog-max-length", + string Base = "release/2.3.0", + string? Milestone = "2.3.0", + string Author = "CaffeinatedCoder"); + + private static readonly Pr InOrder = new(["bug", "severity/high", "package/serilog"]); + + private (int Code, string Output) Check(Pr pr) + { + var payload = new + { + action = "opened", + pull_request = new + { + user = new { login = pr.Author }, + labels = pr.Labels.Select(name => new { name }).ToArray(), + body = pr.Body, + head = new { @ref = pr.Head }, + @base = new { @ref = pr.Base }, + milestone = pr.Milestone is null ? null : new { title = pr.Milestone }, + }, + }; + var file = Path.Combine(sandbox.Tool.Work, $"{Guid.NewGuid():N}.json"); + File.WriteAllText(file, JsonSerializer.Serialize(payload)); + var (code, output, _) = sandbox.Tool.Run(arguments: file); + return (code, output); + } + + private void Passes(Pr pr) + { + var (code, output) = Check(pr); + code.ShouldBe(0, output); + output.ShouldNotContain("::error::"); + } + + private void Fails(Pr pr, string problem) + { + var (code, output) = Check(pr); + code.ShouldBe(1, output); + output.ShouldContain($"::error::{problem}", customMessage: output); + } + + [Fact] + public void A_pull_request_in_order_passes() => Passes(InOrder); + + [Theory] + [InlineData("feat/cause-on-fault", "enhancement")] + [InlineData("docs/plain-voice", "documentation")] + [InlineData("test/variant-catalogue", "tests")] + [InlineData("build/pr-hygiene", "chore")] + [InlineData("refactor/split-plan", "chore")] + [InlineData("spike/mutation-base", "chore")] + public void Each_branch_type_passes_with_its_label(string head, string type) => Passes(InOrder with { Head = head, Labels = [type] }); + + [Fact] + public void A_release_into_main_passes_with_its_milestone() => + Passes(InOrder with { Head = "release/2.3.0", Base = "main", Labels = ["chore", "area/release"], Body = "Closes #70" }); + + [Fact] + public void Dependabot_is_not_checked() => + Passes(new Pr([], Body: null, Head: "dependabot/github_actions/actions-abc", Base: "main", Milestone: null, Author: "dependabot[bot]")); + + [Fact] + public void No_type_label_fails() => Fails(InOrder with { Labels = ["package/serilog"] }, "Needs exactly one type label"); + + [Fact] + public void Two_type_labels_fail() => Fails(InOrder with { Labels = ["bug", "chore", "severity/low"] }, "Needs exactly one type label (bug, enhancement, documentation, tests, chore); it has 2: bug, chore."); + + [Fact] + public void A_bug_without_a_severity_fails() => Fails(InOrder with { Labels = ["bug"] }, "A bug needs exactly one severity/* label"); + + [Fact] + public void A_bug_with_two_severities_fails() => Fails(InOrder with { Labels = ["bug", "severity/high", "severity/low"] }, "A bug needs exactly one severity/* label (critical, high, medium, low); it has 2."); + + [Fact] + public void Silent_on_no_bug_fails() => Fails(InOrder with { Head = "feat/x", Labels = ["enhancement", "silent"] }, "Only a bug is silent"); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("Refs #12")] + [InlineData("Closes the gap in #12")] + [InlineData("Closes #")] + [InlineData("Discloses #12")] + public void A_description_without_a_closing_reference_fails(string? body) => + Fails(InOrder with { Body = body }, "The description needs a closing reference to its issue"); + + [Theory] + [InlineData("closes #12")] + [InlineData("Fixes: #3")] + [InlineData("Problem first.\n\nResolved CaffeinatedCoder/CodoMetis.TypeKit#4")] + [InlineData("FIXED #5, close #6")] + public void Each_closing_keyword_passes(string body) => Passes(InOrder with { Body = body }); + + [Theory] + [InlineData("claude/quirky-panini")] + [InlineData("pr-hygiene")] + [InlineData("fix/")] + [InlineData("/x")] + public void A_branch_not_named_type_slash_slug_fails(string head) => Fails(InOrder with { Head = head }, $"The branch '{head}' is not named /"); + + [Fact] + public void A_branch_whose_type_disagrees_with_the_label_fails() => + Fails(InOrder with { Head = "feat/cause-on-fault" }, "The branch 'feat/cause-on-fault' is a feat branch, which is labelled enhancement, not bug."); + + [Fact] + public void A_pull_request_into_a_release_without_a_milestone_fails() => + Fails(InOrder with { Milestone = null }, "A pull request into 2.3.0 has the milestone 2.3.0; it has none."); + + [Fact] + public void A_pull_request_into_a_release_with_another_milestone_fails() => + Fails(InOrder with { Milestone = "2.4.0" }, "A pull request into 2.3.0 has the milestone 2.3.0; it has '2.4.0'."); + + [Fact] + public void A_release_into_main_without_its_milestone_fails() => + Fails(InOrder with { Head = "release/2.3.0", Base = "main", Labels = ["chore"], Milestone = null }, "A pull request into 2.3.0 has the milestone 2.3.0; it has none."); + + [Fact] + public void A_topic_into_main_needs_no_milestone() => Passes(InOrder with { Base = "main", Milestone = null }); + + [Theory] + [InlineData("2.3.0")] + [InlineData("2.2.1")] + public void A_breaking_change_into_a_minor_or_a_patch_fails(string version) => + Fails(InOrder with { Base = $"release/{version}", Milestone = version, Labels = ["bug", "severity/high", "breaking"] }, + $"A breaking change needs a major, and {version} is not one"); + + [Fact] + public void A_breaking_change_into_a_major_passes() => + Passes(InOrder with { Base = "release/3.0.0", Milestone = "3.0.0", Labels = ["bug", "severity/high", "breaking"] }); + + [Fact] + public void Every_problem_is_reported_at_once() + { + var (code, output) = Check(new Pr([], Body: null, Head: "claude/x", Milestone: null)); + + code.ShouldBe(1); + output.Split('\n').Count(line => line.StartsWith("::error::", StringComparison.Ordinal)).ShouldBe(4, output); + } + + [Fact] + public void A_payload_without_a_pull_request_is_a_usage_error() + { + var file = Path.Combine(sandbox.Tool.Work, "push.json"); + File.WriteAllText(file, """{"ref":"refs/heads/main"}"""); + + sandbox.Tool.Run(arguments: file).Code.ShouldBe(2); + } + + [Fact] + public void A_missing_payload_is_a_usage_error() + { + sandbox.Tool.Run(arguments: Path.Combine(sandbox.Tool.Work, "absent.json")).Code.ShouldBe(2); + sandbox.Tool.Run().Code.ShouldBe(2); + } +} diff --git a/test/CodoMetis.TypeKit.PrHygiene.Tests/StaleToolTests.cs b/test/CodoMetis.TypeKit.PrHygiene.Tests/StaleToolTests.cs new file mode 100644 index 00000000..9b542b78 --- /dev/null +++ b/test/CodoMetis.TypeKit.PrHygiene.Tests/StaleToolTests.cs @@ -0,0 +1,30 @@ +namespace CodoMetis.TypeKit.PrHygiene.Tests; + +/// +/// A check built from other sources than the checkout holds says so and checks nothing: exit 3, the +/// difference, and the command that rebuilds it. A stale check would pass or fail pull requests by rules +/// nobody can read in the checkout. +/// +public sealed class StaleToolTests +{ + [Fact] + public void A_tool_built_from_its_sources_runs() + { + using var sandbox = new ToolSandbox("PrHygiene", "pr-hygiene"); + + sandbox.Run().Code.ShouldBe(2, "a usage error, so the tool ran"); + } + + [Fact] + public void A_changed_source_stops_it() + { + using var sandbox = new ToolSandbox("PrHygiene", "pr-hygiene"); + File.AppendAllText(Path.Combine(sandbox.Repo, "tools", "PrHygiene", "Program.cs"), "\n// edited\n"); + + var (code, output, _) = sandbox.Run(); + + code.ShouldBe(3); + output.ShouldStartWith("TOOL STALE: tools/PrHygiene was built from other sources than the checkout holds (tools/PrHygiene/Program.cs differs)"); + output.ShouldContain("rebuild it: dotnet build tools/PrHygiene -c "); + } +} diff --git a/tools/AgentTools.slnf b/tools/AgentTools.slnf index be0719e0..d3b8e907 100644 --- a/tools/AgentTools.slnf +++ b/tools/AgentTools.slnf @@ -5,6 +5,7 @@ "tools/BuildVerdict/BuildVerdict.csproj", "tools/ConsumerSmokeTest/ConsumerSmokeTest.csproj", "tools/DocsQuery/DocsQuery.csproj", + "tools/PrHygiene/PrHygiene.csproj", "tools/Prober/Prober.csproj" ] } diff --git a/tools/PrHygiene/PrHygiene.csproj b/tools/PrHygiene/PrHygiene.csproj new file mode 100644 index 00000000..bf50b846 --- /dev/null +++ b/tools/PrHygiene/PrHygiene.csproj @@ -0,0 +1,15 @@ + + + + + Exe + pr-hygiene + PrHygiene + + + diff --git a/tools/PrHygiene/Program.cs b/tools/PrHygiene/Program.cs new file mode 100644 index 00000000..fa0f74a3 --- /dev/null +++ b/tools/PrHygiene/Program.cs @@ -0,0 +1,157 @@ +// Checks a pull_request event payload against CONTRIBUTING's "Issues and labels" +// (design/adr/0109-work-is-tracked-as-labelled-issues-and-ci-checks-each-pull-request.md): +// - exactly one type label: bug, enhancement, documentation, tests or chore; +// - a bug carries exactly one severity/* label, and `silent` is only ever a bug; +// - the description closes its issue with one of GitHub's closing keywords ("Closes #12"); +// - the head branch is named / (decision 94), and its type agrees with the type label; +// - a pull request into release/, or of release/ into main, has the milestone ; +// - a `breaking` pull request goes into a major (x.0.0): ApiCompat holds every other version to the +// surface of the one before. +// It reads the payload alone: no token, no network, nothing of the pull request's code run. Dependabot's +// pull requests carry their own labels and close no issue, so they are not checked. +// +// Usage: +// dotnet tools/PrHygiene/bin/Release/net10.0/pr-hygiene.dll +// +// Each problem is printed as a GitHub annotation (::error::), so the run's page lists them. Exit 0 is a +// pass, 1 a pull request with a problem, 2 a usage error or a payload without a pull request, 3 a tool built +// from other sources than the checkout holds, which names the command that rebuilds it. +using System.Text.Json; +using System.Text.RegularExpressions; + +Console.Out.NewLine = "\n"; +Console.Error.NewLine = "\n"; + +var stale = SourceFingerprint.Difference(SourceFingerprint.FindRoot()); +if (stale is not null) +{ + Console.WriteLine($"TOOL STALE: {SourceFingerprint.Project} was built from other sources than the checkout holds ({stale})"); + Console.WriteLine($" rebuild it: {SourceFingerprint.RebuildCommand}"); + return SourceFingerprint.StaleExitCode; +} + +if (args.Length != 1) +{ + Console.Error.WriteLine("Usage: dotnet tools/PrHygiene/bin/Release/net10.0/pr-hygiene.dll "); + return 2; +} + +PullRequest pr; +try +{ + using var document = JsonDocument.Parse(File.ReadAllText(args[0])); + if (!document.RootElement.TryGetProperty("pull_request", out var element) || element.ValueKind != JsonValueKind.Object) + { + Console.Error.WriteLine($"{args[0]} is no pull_request event: it has no pull_request object."); + return 2; + } + pr = PullRequest.Read(element); +} +catch (Exception e) when (e is IOException or UnauthorizedAccessException or JsonException) +{ + Console.Error.WriteLine($"{args[0]} cannot be read as an event payload: {e.Message}"); + return 2; +} + +if (pr.Author == "dependabot[bot]") +{ + Console.WriteLine("Dependabot pull request, not checked."); + return 0; +} + +var problems = Hygiene.Problems(pr); +if (problems.Count == 0) +{ + Console.WriteLine("Labels, milestone, branch and issue link are in order."); + return 0; +} +foreach (var problem in problems) Console.WriteLine($"::error::{problem}"); +Console.WriteLine("See CONTRIBUTING.md, \"Issues and labels\"."); +return 1; + +sealed record PullRequest(string Author, IReadOnlyList Labels, string Body, string Head, string Base, string? Milestone) +{ + public static PullRequest Read(JsonElement pr) => new( + Author: Text(pr, "user", "login") ?? "", + Labels: pr.TryGetProperty("labels", out var labels) && labels.ValueKind == JsonValueKind.Array + ? [.. labels.EnumerateArray().Select(label => Text(label, "name")).OfType()] + : [], + Body: Text(pr, "body") ?? "", + Head: Text(pr, "head", "ref") ?? "", + Base: Text(pr, "base", "ref") ?? "", + Milestone: Text(pr, "milestone", "title")); + + // The string at the path, or null where a step is missing or null (GitHub writes "body": null for an empty description). + static string? Text(JsonElement element, params string[] path) + { + foreach (var name in path) + { + if (element.ValueKind != JsonValueKind.Object || !element.TryGetProperty(name, out element)) return null; + } + return element.ValueKind == JsonValueKind.String ? element.GetString() : null; + } +} + +static partial class Hygiene +{ + static readonly string[] Types = ["bug", "enhancement", "documentation", "tests", "chore"]; + + // A branch's , as decision 94 names it after the main commit's conventional type, and the type label that goes with it. + static readonly Dictionary TypeOfBranch = new(StringComparer.Ordinal) + { + ["feat"] = "enhancement", + ["fix"] = "bug", + ["docs"] = "documentation", + ["test"] = "tests", + ["build"] = "chore", + ["refactor"] = "chore", + ["spike"] = "chore", + ["release"] = "chore", + }; + + // GitHub's closing keywords, before #n or owner/repo#n. Only these link a pull request to the issue it closes. + [GeneratedRegex(@"\b(close[sd]?|fix(e[sd])?|resolve[sd]?):? +([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#[0-9]+\b", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)] + private static partial Regex Closing(); + + [GeneratedRegex(@"^release/(?(?[0-9]+)\.(?[0-9]+)\.(?[0-9]+))$", RegexOptions.CultureInvariant)] + private static partial Regex ReleaseBranch(); + + public static List Problems(PullRequest pr) + { + var problems = new List(); + var labels = pr.Labels.ToHashSet(StringComparer.Ordinal); + + var types = Types.Where(labels.Contains).ToList(); + if (types.Count != 1) + problems.Add($"Needs exactly one type label ({string.Join(", ", Types)}); it has {types.Count}{(types.Count > 1 ? $": {string.Join(", ", types)}" : "")}."); + + var severities = pr.Labels.Count(label => label.StartsWith("severity/", StringComparison.Ordinal)); + if (labels.Contains("bug") && severities != 1) + problems.Add($"A bug needs exactly one severity/* label (critical, high, medium, low); it has {severities}."); + if (labels.Contains("silent") && !labels.Contains("bug")) + problems.Add("Only a bug is silent: label it bug, or take silent off."); + + if (!Closing().IsMatch(pr.Body)) + problems.Add("The description needs a closing reference to its issue, such as 'Closes #12'."); + + var slash = pr.Head.IndexOf('/', StringComparison.Ordinal); + var branchType = slash > 0 ? pr.Head[..slash] : null; + if (branchType is null || slash == pr.Head.Length - 1 || !TypeOfBranch.TryGetValue(branchType, out var expected)) + problems.Add($"The branch '{pr.Head}' is not named /, where the type is one of {string.Join(", ", TypeOfBranch.Keys)} (decision 94)."); + else if (types.Count == 1 && types[0] != expected) + problems.Add($"The branch '{pr.Head}' is a {branchType} branch, which is labelled {expected}, not {types[0]}."); + + var release = ReleaseBranch().Match(pr.Base); + if (!release.Success && pr.Base == "main") release = ReleaseBranch().Match(pr.Head); + if (release.Success) + { + var version = release.Groups["version"].Value; + if (pr.Milestone != version) + problems.Add($"A pull request into {version} has the milestone {version}; it has {(pr.Milestone is null ? "none" : $"'{pr.Milestone}'")}."); + if (labels.Contains("breaking") && (release.Groups["minor"].Value != "0" || release.Groups["patch"].Value != "0")) + problems.Add($"A breaking change needs a major, and {version} is not one: it waits for the next x.0.0."); + } + + return problems; + } +}