Make the merge the release, and every check the thing that stands in front of it

.github/rulesets/main-build-and-test.json → .github/rulesets/main-every-check-green.json

@@ -1,5 +1,5 @@
 {
-  "name": "main: build and test must pass",
+  "name": "main: every check must pass",
   "target": "branch",
   "enforcement": "active",
   "conditions": {
@@ -23,9 +23,12 @@
     {
       "type": "required_status_checks",
       "parameters": {
-        "strict_required_status_checks_policy": false,
+        "strict_required_status_checks_policy": true,
         "required_status_checks": [
-          { "context": "Build and test" }
+          { "context": "Build and test" },
+          { "context": "Coverage" },
+          { "context": "Playwright" },
+          { "context": "Visual check" }
         ]
       }
     },

.github/workflows/ci.yml

@@ -8,22 +8,20 @@ name: CI
 # against, so the app the browsers see is the app the unit tests passed on, not a rebuild of it.
 
 on:
-  # The merge ref, which is the only thing worth building. `actions/checkout` resolves a
-  # `pull_request` event to `refs/pull/N/merge` — this branch already merged into main — where a
-  # push event checks out the branch tip on its own. `strict_required_status_checks_policy` is
-  # deliberately off (see docs/deployment.md), so a branch can merge without being rebuilt against
-  # a moved main, and this event is what covers that. It also covers a fork's pull request.
+  # The event that reports these four checks in the ordinary course of things, and the merge ref is
+  # the thing worth building: `actions/checkout` resolves a `pull_request` event to
+  # `refs/pull/N/merge`, this branch already merged into main, where a push event would check out
+  # the branch tip on its own. Nothing runs on main afterwards — merging deploys — so what goes
+  # green here is the last word on the commit that reaches the volume. It covers a fork's pull
+  # request too.
   pull_request:
-  # Called by fly-deploy.yml, so the gate in front of a deploy is literally the same build and test
-  # a pull request ran, rather than a copy of it that can drift. The browser jobs below opt out of
-  # this event: they are advisory, and a flaky browser should not be able to hold up a release.
-  workflow_call:
-  # The escape hatch. Runs everything, browsers included — useful for checking a branch that has no
-  # pull request yet, and for the case where a pull request somehow shows no checks at all.
+  # The escape hatch, for a branch with no pull request yet and for the case where a pull request
+  # somehow shows no checks at all. It reports the same four contexts, so it can unlock a merge on
+  # its own — off the branch tip rather than the merge ref. See docs/deployment.md.
   workflow_dispatch:
 
-# Keyed by ref, so a pull request's checks start straight away instead of queueing behind a
-# deploy. A run on main is left to finish — its build is what a deploy is waiting on.
+# Keyed by ref, so pushing again to a pull request cancels that pull request's run and nothing
+# else. A dispatched run is left to finish: it was asked for by hand.
 concurrency:
   group: ci-${{ github.ref }}
   cancel-in-progress: ${{ github.event_name == 'pull_request' }}
@@ -32,8 +30,10 @@ permissions:
   contents: read
 
 jobs:
-  # The required check. `main`'s ruleset names "Build and test" exactly, so renaming this job means
-  # editing .github/rulesets/main-build-and-test.json to match — see docs/deployment.md.
+  # All four jobs here are required checks, and `main`'s ruleset names each of them by the `name:`
+  # it declares — so renaming one means editing .github/rulesets/main-every-check-green.json in the
+  # same change, or the merge button waits forever on a check nothing reports. See
+  # docs/deployment.md.
   build:
     name: Build and test
     runs-on: ubuntu-latest
@@ -112,13 +112,14 @@ jobs:
   # docs/testing.md has the reasoning. The result is written to this run's summary page, so the
   # number is read without opening a log or downloading the report.
   #
-  # Advisory, like the two browser jobs: it goes red under the floor but `main`'s ruleset requires
-  # "Build and test" alone, so it reports rather than blocks. It needs no compiler — the report was
-  # produced by the test run above — so it is node and a git history, and it is over in seconds.
+  # Blocking, like the two browser jobs below: under the floor, the merge button stays disabled. A
+  # change that touches no coverable line — docs, CSS, a workflow — measures nothing and passes, so
+  # requiring this cannot wedge a pull request that had no code in it. It needs no compiler, the
+  # report having come out of the test run above, so it is node and a git history and it is over in
+  # seconds.
   coverage:
     name: Coverage
     needs: build
-    if: github.event_name != 'workflow_call'
     runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
@@ -148,15 +149,12 @@ jobs:
   # What the app does: the public/admin split, the dialogs, the match-day journey, both languages,
   # the phone layout. See docs/testing.md.
   #
-  # Advisory. `main`'s ruleset requires "Build and test" and nothing else, so this reports without
-  # blocking a merge — promoting it once it has a track record on real runners is one line in
-  # .github/rulesets/main-build-and-test.json and nothing here. It also skips the workflow_call
-  # event, which is fly-deploy.yml asking for its gate: a browser should not stand between a merged
-  # pull request and a release.
+  # Blocking. Merging is now the release, and nothing re-tests on main afterwards, so a browser
+  # failure is dealt with here or not at all. The cost is that a flake holds the merge: re-run the
+  # job from the Actions page, because the ruleset grants no bypass to anyone.
   playwright:
     name: Playwright
     needs: build
-    if: github.event_name != 'workflow_call'
     runs-on: ubuntu-latest
     timeout-minutes: 20
     steps:
@@ -229,12 +227,11 @@ jobs:
 
   # Whether it renders at all, and whether a thumb can hit it. This is the only thing holding the
   # Touch / PWA fixes in docs/known_issues.md in place — every one of them is CSS, and a MudBlazor
-  # upgrade or one more global .mud-* rule undoes any of them silently. Advisory, and skipped for
-  # workflow_call, for the same reasons as the job above.
+  # upgrade or one more global .mud-* rule undoes any of them silently. Blocking, for the same
+  # reason as the job above.
   visual:
     name: Visual check
     needs: build
-    if: github.event_name != 'workflow_call'
     runs-on: ubuntu-latest
     timeout-minutes: 20
     steps:

.github/workflows/fly-deploy.yml

@@ -5,7 +5,15 @@ name: Deploy to Fly.io
 on:
   push:
     branches: [main]
+  # The confirmation is what the gate job and the environment reviewer used to be for this route:
+  # both went when merging became the release, and a dispatch has neither a pull request behind it
+  # nor a pause in front of it. A stray "Run workflow" click would otherwise replace the container
+  # that is serving, which on a match day drops the live screen's circuits.
   workflow_dispatch:
+    inputs:
+      confirm:
+        description: Type "deploy" to replace what is serving right now
+        required: true
 
 # One deploy at a time: SQLite lives on a single volume and the app migrates on boot, so two
 # overlapping deploys race over one machine.
@@ -17,27 +25,30 @@ permissions:
   contents: read
 
 jobs:
-  # Gate: nothing reaches the Fly volume that doesn't compile and pass its tests. The app
-  # auto-migrates on startup, so a broken build is a broken production database.
-  build:
-    uses: ./.github/workflows/ci.yml
-
+  # The merge is the decision. Every check on the pull request had to be green before it could land,
+  # and the branch had to be up to date with main to get there, so this starts the moment the merge
+  # commit exists — no gate job, no approval, nobody to wait for. The compile still happens: flyctl
+  # builds the image from the Dockerfile, so a merge that does not build never reaches the volume,
+  # and the smoke check below is what proves the one that did is serving.
   deploy:
-    needs: build
-    # `workflow_dispatch` runs against any branch, and without the ref check would put that branch
-    # on the production volume. The actor check is about *who*: a dispatch can be started by anyone
-    # with write access, and while the environment's reviewer below would stop it anyway, refusing
-    # it here keeps a run that was never going to deploy out of the approval queue.
+    # Three conditions, and only the first applies to a merge. `workflow_dispatch` runs against any
+    # branch, and without the ref check would put that branch on the production volume; the actor
+    # check is about *who*, since a dispatch can be started by anyone with write access; the input
+    # is about *meant to*. A merge is none of these — it arrives already reviewed by four green
+    # checks — so the `push` arm passes on the ref alone and deploys with nothing to fill in.
+    #
+    # On a push `inputs.confirm` is null, which is why the dispatch conditions sit behind the
+    # event_name test rather than beside it.
     if: >-
       github.ref == 'refs/heads/main'
-      && (github.event_name != 'workflow_dispatch' || github.actor == github.repository_owner)
+      && (github.event_name != 'workflow_dispatch'
+          || (github.actor == github.repository_owner && inputs.confirm == 'deploy'))
     runs-on: ubuntu-latest
-    # The gate that decides *whether* a release goes out, and the only place the Fly token exists.
-    # `production` carries a required reviewer, so this job holds at "Waiting" until a human
-    # approves it — merging to main proposes a deploy rather than performing one. It also carries a
-    # deployment branch policy of `main` only, which is what stops a workflow added on a feature
-    # branch from reading FLY_API_TOKEN at all: an environment secret is unreachable from a ref the
-    # environment does not admit. See docs/deployment.md — none of it lives in this file, and the
+    # Not a gate any more — the only place the Fly token exists. `production` carries a deployment
+    # branch policy of `main` only, which is what stops a workflow added on a feature branch from
+    # reading FLY_API_TOKEN at all: an environment secret is unreachable from a ref the environment
+    # does not admit. Its *Required reviewers* box is deliberately empty, which is what makes a
+    # merge deploy on its own. See docs/deployment.md — none of it lives in this file, and the
     # environment has to exist before this workflow runs.
     environment:
       name: production

CLAUDE.md

@@ -42,7 +42,7 @@ dotnet ef database update    --project src/FootballFormation.Core
 src/FootballFormation.Core/   Models, Data (EF Core), Reporting, Services, Result — no UI references
 src/FootballFormation.UI/     Razor Class Library: pages, components, navigation, state, theming
 src/FootballFormation.Web/    Host: Program.cs, App.razor, Routes.razor, wwwroot (CSS, JS, PWA)
-tests/FootballFormation.Core.Tests/   xUnit v3 — the CI gate the deploy depends on
+tests/FootballFormation.Core.Tests/   xUnit v3 — one of the four checks a merge waits for
 docs/                         The detailed documentation (see the index below)
 scripts/                      visual-check.sh + its Playwright driver
 ```
@@ -167,14 +167,14 @@ There are **no component tests** (no bUnit). The UI is checked by driving the re
 browser, in two places, both against a throwaway database:
 
 - **`cd tests/ui && npm test`** — Playwright, ~39 tests, about a minute. Runs on every pull request
-  as a job in `ci.yml`, advisory rather than the merge gate. Behaviour: the public/admin
+  as a job in `ci.yml`, and a red run holds the merge. Behaviour: the public/admin
   split, the squad and match dialogs, the whole match-day journey from dragging a lineup to blowing
   the final whistle, both languages, and the phone layout. Read
   [docs/testing.md](docs/testing.md#ui-tests-testsui) **before adding one** — a Blazor Server page
   renders twice and the prerender is fully clickable and completely inert, so `goto()` waits for
   Blazor's `_bl_*` attributes and `clickFor()` clicks for an outcome. There are no fixed sleeps in
   that directory; do not introduce one.
-- **`scripts/visual-check.sh`** — the `Visual check` job in `ci.yml`, also advisory. Screenshots every
+- **`scripts/visual-check.sh`** — the `Visual check` job in `ci.yml`, also blocking. Screenshots every
   page, then measures every touch target on three phone viewports against the 44px floor. The only check that a page renders at all, and the only
   thing holding the Touch / PWA fixes in `known_issues.md` in place.
 
@@ -184,16 +184,17 @@ admin matrix; it is not part of this repository.)
 ## Workflow
 
 - Work on a feature branch. `main` takes pull requests only, and the merge button stays disabled
-  until **Build and test** is green and every review thread is resolved
-  (`.github/rulesets/main-build-and-test.json`).
+  until **all four** checks are green — **Build and test**, **Coverage**, **Playwright**, **Visual
+  check** — the branch is up to date with `main`, and every review thread is resolved
+  (`.github/rulesets/main-every-check-green.json`, which grants no bypass to anyone).
 - `ci.yml` runs `dotnet build -c Release` + `dotnet test` on every pull request. That test run
-  carries the coverage collector, and an advisory **Coverage** job judges the lines the pull
-  request changed against the 80% floor and writes the numbers to the run's summary page.
-  `fly-deploy.yml` *calls that same workflow* as the gate its deploy job depends on, then
-  smoke-checks `/health` until it reports the commit that was just built.
-- Merging to `main` *proposes* a deploy; it does not perform one. The deploy job runs in the
-  `production` environment, which has a required reviewer, so the run waits at *Waiting* until the
-  maintainer approves it. There is no staging environment — that approval is the last look.
+  carries the coverage collector, and the **Coverage** job judges the lines the pull request changed
+  against the 80% floor and writes the numbers to the run's summary page. A change with no coverable
+  line in it passes rather than wedging.
+- **Merging to `main` releases.** `fly-deploy.yml` starts on the merge commit with no gate job and
+  no approval, then smoke-checks `/health` until it reports the commit that was just built. There is
+  no staging environment and nothing re-runs on `main`, so the four checks on the pull request are
+  the last look — which is why a flaky browser job is re-run rather than merged past.
 - Commit messages are plain imperative sentences describing the intent, not conventional-commit
   prefixes: *"Let a deploy recognise its own release, not just a live one"*, *"Split the games list
   on the scoreline, not the calendar"*.

README.md

@@ -59,7 +59,7 @@ FootballFormation/
 │   ├── FootballFormation.UI/      # Blazor components, pages, helpers, layout
 │   └── FootballFormation.Web/     # Host project, Program.cs, wwwroot
 ├── tests/
-│   └── FootballFormation.Core.Tests/   # Service and domain tests (gate the deploy)
+│   └── FootballFormation.Core.Tests/   # Service and domain tests (gate the merge)
 ├── docs/                          # Architecture & project documentation
 │   ├── project_overview.md
 │   ├── architecture.md
@@ -117,18 +117,18 @@ outside Production *and* only for loopback callers.
 dotnet test
 ```
 
-`.github/workflows/ci.yml` runs `dotnet build -c Release` and `dotnet test`. It runs on every pull
-request, and `fly-deploy.yml` calls that same workflow as the gate its deploy job depends on — so
-the checks in front of production are literally the ones a pull request ran, not a copy that can
-drift. Note that `Directory.Build.props` sets `TreatWarningsAsErrors` in **Release only** — a
-warning that builds fine locally will fail CI. Build Release before pushing. The exception is
-`MSB3568` (a duplicate resource name, which quietly changes what the app says), an error in every
-configuration.
+`.github/workflows/ci.yml` runs `dotnet build -c Release` and `dotnet test` on every pull request,
+alongside the coverage and browser jobs below. Note that `Directory.Build.props` sets
+`TreatWarningsAsErrors` in **Release only** — a warning that builds fine locally will fail CI. Build
+Release before pushing. The exception is `MSB3568` (a duplicate resource name, which quietly changes
+what the app says), an error in every configuration.
 
-A ruleset makes that check binding: `main` takes pull requests only, and the merge button stays
-disabled until `Build and test` is green. It lives in `.github/rulesets/main-build-and-test.json`
-and has to be imported into the repository's settings once — see
-[docs/deployment.md](docs/deployment.md#only-a-green-build-can-be-merged).
+A ruleset makes those checks binding: `main` takes pull requests only, the branch has to be up to
+date with `main`, and the merge button stays disabled until all four — `Build and test`, `Coverage`,
+`Playwright` and `Visual check` — are green. Merging is what releases: nothing runs on `main`
+afterwards, so those four are the last word on what reaches production. The ruleset lives in
+`.github/rulesets/main-every-check-green.json` and has to be imported into the repository's settings
+once — see [docs/deployment.md](docs/deployment.md#only-a-green-build-can-be-merged).
 
 ### UI tests
 
@@ -140,7 +140,7 @@ Playwright, driving the real app in a browser against a database that exists onl
 public/admin split, the squad and match dialogs, the full match-day journey from dragging a lineup
 to blowing the final whistle, both languages, and the phone layout. About a minute, 34 tests. Runs
 on every pull request as the `Playwright` job in `.github/workflows/ci.yml`, against the app that
-workflow published — an advisory check rather than the merge gate. See [docs/testing.md](docs/testing.md#ui-tests-testsui).
+workflow published — one of the four checks the merge waits for. See [docs/testing.md](docs/testing.md#ui-tests-testsui).
 
 ### Visual checks
 
@@ -157,7 +157,7 @@ It then measures rather than looks: the match dialog and its date picker are reo
 and landscape phone sizes and every touch target is checked for the 44px minimum and for dead space
 between it and its neighbours. That is the only thing holding the touch fixes in
 [docs/known_issues.md](docs/known_issues.md) in place, so it runs on every pull request too — the
-`Visual check` job in `ci.yml`, advisory like the one beside it, and it uploads its screenshots
+`Visual check` job in `ci.yml`, blocking like the one beside it, and it uploads its screenshots
 either way.
 See [docs/testing.md](docs/testing.md).