From f536d177c7ab38e57dcd3f70d13c95166d85fc63 Mon Sep 17 00:00:00 2001 From: Roland Salardon Date: Fri, 2 Oct 2026 02:05:56 +0200 Subject: [PATCH] ci: enforce branch protection on main and reconcile CI docs - Apply classic branch protection: 9 required status checks, PR required, linear history, no force push/deletion, enforce_admins - ci.yml: CONTRIBUTING lists 9 required checks, not 6; dco-check comment claimed informational-only although it is now a required gate - dependabot-auto-merge.yml: drop dangling plans/136 + plans/137 refs (those plans are an arcdps sidecar and a buff-uptime model) - CONTRIBUTING.md: record that the ruleset is applied and via which API - CHANGELOG.md: document the gap and the smoke-test validation Signed-off-by: Roland Salardon --- .github/workflows/ci.yml | 17 ++++++------- .github/workflows/dependabot-auto-merge.yml | 9 ++++--- CHANGELOG.md | 28 +++++++++++++++++++++ CONTRIBUTING.md | 24 ++++++++++++------ 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31f1f6a4..9c2c0aec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -428,12 +428,12 @@ jobs: # the flag only prevents workflow FAILURE on this job's # failure; the underlying status check still reports ``failure``. # It is currently safe because CONTRIBUTING.md §"Branch - # protection for main" lists 6 required-status-checks (see + # protection for main" lists 9 required-status-checks (see # that section for the current list), and ``Playwright # (visual-regression)`` is NOT among them -- so a failed run # here does NOT block the merge gate. WARNING: a future - # maintainer who adds this job to the required_status_checks - # ruleset MUST also remove ``continue-on-error: true`` (or the + # maintainer who adds this job to ``required_status_checks`` + # MUST also remove ``continue-on-error: true`` (or the # gate will silently start blocking merges). Cite # CONTRIBUTING.md "Branch protection for main" by section # when revisiting. @@ -631,12 +631,11 @@ jobs: # external action dependency — the previous ``crazy-max/ghaction-dco@v1`` # was removed upstream, breaking the check silently. # - # NOT currently added to the branch-protection ``required_status_checks`` - # rule — we ship it informational-only first. Bump to hard-required - # after one real Contributor-signed PR has demonstrated the workflow - # end-to-end. The job will FAIL (not error) on unsigned commits, so the - # first failing PR will surface clearly in the Actions UI without - # blocking the merge until we wire it into required_status_checks. + # Part of ``main``'s ``required_status_checks`` (9 required checks — + # see CONTRIBUTING.md §"Branch protection for main"). The job FAILs + # (not errors) on unsigned commits, and because it is a required gate + # that failure now blocks the merge until the commit is re-signed with + # ``git commit -s``. # --------------------------------------------------------------------------- dco-check: name: DCO check diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index 717b292f..78258f4f 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -4,7 +4,7 @@ # (`paths-ignore`). When the diff does touch `.github/**`, this # workflow does not fire at all, leaving the PR for human review — # the only reliable way to prevent dependabot from auto-merging an -# invalid major tag on a GitHub Action (see plans/136). +# invalid major tag on a GitHub Action (see .github/dependabot.yml). # # Combined with the `ignore:` block on the `github-actions` ecosystem # in .github/dependabot.yml (cap majors for stale actions), this @@ -26,9 +26,10 @@ # (b) enable the repo's "Allow GitHub Actions to create and approve # pull requests" setting in repo Settings -> Actions -> General. # Without either (a) or (b), auto-merge silently fails on every PR and -# this workflow becomes a no-op. Defense-in-depth: combine with the -# branch-protection rule in plans/137 so even if auto-merge fails, -# the workflow still cannot land an invalid tag on `main`. +# this workflow becomes a no-op. Defense-in-depth: the branch protection +# on `main` (9 required status checks — CONTRIBUTING.md §"Branch +# protection for main") means that even if auto-merge fails, the +# workflow still cannot land an invalid tag on `main`. name: Dependabot auto-merge diff --git a/CHANGELOG.md b/CHANGELOG.md index 116ff908..6174e67b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,33 @@ ## [Unreleased] +### Changed -- Branch protection actually enforced on ``main`` +- **Applied classic branch protection to ``main``** via + ``PUT /repos/:owner/:repo/branches/main/protection``: 9 required + status checks (Lint Python, Test Python, Lint Web, Playwright + (chromium), ARQ worker integration, DCO check, Build API image, + Build Web image, Trivy filesystem scan), pull request required + (0 approvals), linear history, force pushes and branch deletion + blocked, ``enforce_admins`` on. +- **The ruleset documented since v0.15.2 was never applied.** + ``CONTRIBUTING.md`` and ``CHANGELOG.md`` both described "the ``main`` + ruleset" as if it existed, while ``required_status_checks`` was + ``null`` — dependabot PRs were therefore auto-merging with red checks + (4 landed on 2026-10-01, including one whose CI had failed). Root + cause: the public-flip cheatsheet's manual "Step 8 — apply the + branch-protection ruleset via the GitHub web UI" was never executed. +- **``dependabot-auto-merge`` is now genuinely gated** — + ``gh pr merge --auto --squash`` waits for the 9 required checks + instead of merging immediately, which is the defense-in-depth its + own header comment called for. +- **Doc drift fixed**: ``ci.yml`` said CONTRIBUTING listed 6 required + checks (it lists 9), the ``dco-check`` job comment still claimed DCO + was informational-only, and ``dependabot-auto-merge.yml`` cited + ``plans/136`` / ``plans/137`` as sources for CI decisions — those + plans are an arcdps sidecar and a buff-uptime model, and neither file + is present in the tree. +- **Validation**: PR #315 (empty commit) reported ``BLOCKED`` on every + poll while checks ran and flipped to ``CLEAN`` at 10/10, then merged. + ### Added -- Webhook subscriptions CRUD UI (PR #69) + API hardening (PR #68) - **New ``/webhooks`` management page** (``web/src/app/webhooks/page.tsx``) renders a full subscription lifecycle alongside the existing DLQ surface: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4b761d7..9bb0512c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -278,24 +278,32 @@ until they configure .env via `cp .env.example .env`. ## Branch protection for ``main`` -Recommended GitHub repository ruleset (Settings → Rules → -Rulesets → New branch ruleset): +**Status: applied** (2026-10-02). Set on `main` through the classic +branch-protection API — ``PUT /repos/:owner/:repo/branches/main/protection`` +— because ``gh ruleset`` is a GitHub-Enterprise CLI and the free plan +only exposes the equivalent Rulesets UI by hand. The table below is the +effective configuration; re-apply it with that endpoint if it is ever +reset (the private-flip appendix at the end of this file walks the +manual UI path). | Setting | Value | |--------------------------------------------------|--------------------------------------| | Target branches | `main` only | -| Restrict creations | Enable (only via PR) | -| Restrict updates | Enable (PR or admin bypass only) | -| Required status checks | Lint Python, Test Python, Lint Web, Playwright (chromium), ARQ worker integration, DCO check, Build API image, Build Web image, Trivy filesystem scan | +| Require a pull request before merging | Yes, 0 required approvals (PR-only) | +| Required status checks (9) | Lint Python, Test Python, Lint Web, Playwright (chromium), ARQ worker integration, DCO check, Build API image, Build Web image, Trivy filesystem scan | | Require linear history | Yes (no merge commits) | | Require deployments before merging | No (no deploys from `main` directly) | -| Block force pushes | **Yes** (admin included) | +| Block force pushes | **Yes** (`enforce_admins`, admin included) | | Block branch deletion | **Yes** | +| Enforce for admins | **Yes** | +| Require status checks to be up to date | No (branches need not be rebased) | CI runs automatically on pushes to `main` and PRs targeting `main`. The required checks are intentionally limited to deterministic merge -gates. Visual regression still runs on PRs but is not required while -the screenshot baselines remain under refresh. +gates and are all reported on `pull_request`, so none of them can leave +a PR permanently blocked by being skipped. Visual regression still runs +on PRs but is not required while the screenshot baselines remain under +refresh. ## Pre-commit / CI mirror