Skip to content

Commit e6d69ce

Browse files
committed
ci: let a red build block a merge (issue #105)
main carried no required status checks, so the whole suite was advisory: a PR could go red and still merge. That matters here because the suite is where the real guarantees live, the exit-code protocol, the env allowlist, the queue and cron assumptions against a live Valkey, and the CONST-MERGE-NEVER-AUTOMATIC grep, and because CONST-PI-VERSION-PINNED's own Statement claims an upgrade is "gated by the upstream contract tests" while nothing mechanical enforced it. Five checks from pi-upgrade-check.yml become required on main, with enforce_admins on and the approving-review requirement dropped to zero. A PR stays mandatory; a green one now merges without --admin, which was previously the only path because a solo author cannot approve their own PR. The blocker was the path filter. A required check that never reports blocks the merge forever, and pi-upgrade-check.yml's pull_request filter meant a docs-only PR reported nothing at all (PR #106: "no checks reported"). Removing the filter from the pull_request trigger alone fixes it; the push trigger keeps its filter. The rejected alternative, a companion workflow with the complementary paths-ignore list emitting same-named stand-in jobs, fires BOTH workflows on a PR touching both docs/ and worker/, and a green stand-in sharing a context name with the real run can mask a failure. A full run is ~90s, which is not worth a mechanism that can hide one. Two PR-reporting checks are deliberately excluded. The host-pi canary is green-on-drift by design, so its red means pi failed to install rather than a defect; deploy-lint is still path-filtered to deploy/**, so requiring it would deadlock every PR that leaves deploy/ alone. Four workflow headers claimed "PR + 1 approving review" and now describe the real gate. CLAUDE.md records the required contexts, why the pull_request trigger must stay unfiltered, and the enforce_admins escape hatch for a frozen main. Specs UNCHANGED, checked: CONST-PI-VERSION-PINNED and REQ-UPSTREAM-CONTRACT-TESTS already assert that CI gates an upgrade; this makes the assertion true rather than restating it. CONST-MERGE-NEVER-AUTOMATIC untouched, its grep job is one of the five now required. Signed-off-by: Rob Boerman <robboerman@live.nl>
1 parent bcfd0c8 commit e6d69ce

6 files changed

Lines changed: 41 additions & 22 deletions

File tree

.github/workflows/image.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Build and publish the pi-dispatch job image to GHCR so operators `docker pull` instead of the slow local
2-
# build (Chromium + Playwright + fonts, minutes). main is branch-protected (PR + 1 approving review), so a
3-
# push here is an already-approved merge — nothing in this workflow merges anything (CONST-MERGE-NEVER-AUTOMATIC).
2+
# build (Chromium + Playwright + fonts, minutes). main is branch-protected (a PR is required and the five
3+
# contract checks in pi-upgrade-check.yml must be green, admins included), so a push here is an already-gated
4+
# merge — nothing in this workflow merges anything (CONST-MERGE-NEVER-AUTOMATIC).
45
#
56
# The pushed image is a snapshot of THIS repo's runner + guardrails at the built commit. Operators who bake
67
# their own toolchain into image/Dockerfile still build locally; the pull is only the fast default path.

.github/workflows/pi-upgrade-check.yml

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,17 @@ on:
2828
- ".github/workflows/pi-upgrade-check.yml"
2929
- ".github/scripts/admin-pi-canary.mjs"
3030
- ".github/scripts/host-pi-canary.mjs"
31+
# DELIBERATELY UNFILTERED, unlike the push trigger above. Five of the jobs below are REQUIRED status
32+
# checks on main (issue #105), and a required check that never reports blocks the merge FOREVER -- a
33+
# docs-only PR under the old path filter reported nothing at all (PR #106: "no checks reported") and
34+
# would now be unmergeable. Re-adding a `paths:` here re-opens that deadlock.
35+
#
36+
# The rejected alternative was a companion workflow carrying the complementary `paths-ignore:` list and
37+
# emitting same-named stand-in jobs: a PR touching BOTH docs/ and worker/ fires both workflows, because
38+
# paths-ignore runs whenever ANY changed file falls outside its list, and two check runs then share one
39+
# context name -- a green stand-in that can mask a red real run. The whole suite is ~90s; buying that
40+
# back is not worth a mechanism that can hide a failure.
3141
pull_request:
32-
paths:
33-
- "image/**"
34-
- "worker/**"
35-
- "receiver/**"
36-
- "admin/**"
37-
- "guardrails/**"
38-
- "package.json"
39-
- "pi-packages.example.json"
40-
- "triggers.example.json"
41-
- ".github/workflows/pi-upgrade-check.yml"
42-
- ".github/scripts/admin-pi-canary.mjs"
43-
- ".github/scripts/host-pi-canary.mjs"
4442
schedule:
4543
# Weekly: pi ships breaking changes between minors and its HEAD moved within 24h of this
4644
# project's design being written. A pin that is never exercised rots silently.

.github/workflows/receiver-image.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Build and publish the receiver image to GHCR so operators `docker compose --profile receiver up` a
2-
# prebuilt multi-arch image instead of building locally. main is branch-protected (PR + 1 approving
3-
# review), so a push here is an already-approved merge — nothing in this workflow merges anything
4-
# (CONST-MERGE-NEVER-AUTOMATIC).
2+
# prebuilt multi-arch image instead of building locally. main is branch-protected (a PR is required and the
3+
# five contract checks in pi-upgrade-check.yml must be green, admins included), so a push here is an
4+
# already-gated merge — nothing in this workflow merges anything (CONST-MERGE-NEVER-AUTOMATIC).
55
#
66
# The pushed image is a snapshot of the receiver AND the worker sources it imports (`@edgehero/pi-dispatch`
77
# is a workspace dependency) at the built commit — hence the worker paths in the trigger: a change to a

.github/workflows/release.yml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
# in an earlier group stops the later ones, which is what you want when the later package depends on the
1111
# earlier one landing.
1212
#
13-
# main is branch-protected (PR + 1 approving review required), so a push here is an already-approved merge —
14-
# the human gate is the merge, not this workflow (consistent with CONST-MERGE-NEVER-AUTOMATIC: nothing here
15-
# merges anything). Publishing is idempotent PER PACKAGE: each group fires only when its workspace's version
13+
# main is branch-protected (a PR is required and the five contract checks in pi-upgrade-check.yml must be
14+
# green, admins included), so a push here is an already-gated merge — the human gate is the merge, not this
15+
# workflow (consistent with CONST-MERGE-NEVER-AUTOMATIC: nothing here merges anything). Publishing is
16+
# idempotent PER PACKAGE: each group fires only when its workspace's version
1617
# is not already on npm, so re-runs, reverts, and pushes that touch only one workspace are no-ops for the
1718
# other two. Bump a workspace's version in your PR to ship that package.
1819
#

.github/workflows/repo-release.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
# `version` in the root package.json in a PR; it is idempotent (skips when the tag already exists), so an
66
# unrelated root package.json edit — a dependency bump, a script tweak — is a no-op.
77
#
8-
# main is branch-protected (PR + 1 approving review), so a push here is an already-approved merge — nothing
9-
# here merges anything (CONST-MERGE-NEVER-AUTOMATIC). No secret is required to tag a release; the optional
8+
# main is branch-protected (a PR is required and the five contract checks in pi-upgrade-check.yml must be
9+
# green, admins included), so a push here is an already-gated merge — nothing here merges anything
10+
# (CONST-MERGE-NEVER-AUTOMATIC). No secret is required to tag a release; the optional
1011
# repo secret ANTHROPIC_API_KEY enables AI-written notes, and without it the release falls back to a plain
1112
# commit list. Optional repo variable RELEASE_MODEL (default: claude-sonnet-5).
1213

CLAUDE.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ Two consequences worth internalising before you design anything:
8686
touched: the workspaces share the triggers schema and the queue.
8787
- CI runs the same suite with `PI_DISPATCH_REQUIRE_{LOADER,WORKER,RECEIVER}_TESTS=1` and a live Valkey, which
8888
is where the integration tests that skip locally actually execute.
89+
- **Five checks are REQUIRED on `main`** (issue #105), all from `pi-upgrade-check.yml`: `pins are exact
90+
(CONST-PI-VERSION-PINNED)`, `no automatic merge (CONST-MERGE-NEVER-AUTOMATIC)`, `pinned assumptions still
91+
hold (offline, no API key)`, `the job image holds its contract`, `the admin extension survives latest pi
92+
(canary)`. `enforce_admins` is **on**, so a red build is unmergeable by the owner too.
93+
- That workflow's **`pull_request` trigger is deliberately unfiltered**. A required check that never
94+
reports blocks a merge forever, and the old path filter meant a docs-only PR reported nothing at all.
95+
Do not add `paths:` back to it. The `push:` filter is unaffected and stays.
96+
- Two PR-reporting checks are deliberately **not** required. `host-pi mirrors survive latest pi (canary)`
97+
is green-on-drift by design, so its red means pi failed to install (upstream flake, not a defect), and
98+
`deploy/ artifacts are syntactically valid` is still path-filtered to `deploy/**`, so requiring it
99+
would deadlock every PR that does not touch `deploy/`.
100+
- If Actions is down or a workflow file breaks, `main` is frozen. Escape hatch: `gh api -X DELETE
101+
repos/edgehero/pi-dispatch/branches/main/protection/enforce_admins`, merge, then `gh api -X POST` the
102+
same path to put it back.
89103
- `admin/dist/` is gitignored and built by `node admin/build.mjs`. Never commit it.
90104
- Two mirrors must stay **byte-identical**, pinned by tests: `worker/.env.example` to the root
91105
`.env.example`, and `worker/deploy/*` to `deploy/*`. Edit both.
@@ -99,6 +113,10 @@ Two consequences worth internalising before you design anything:
99113
- DCO sign-off (`git commit -s`). Branch names are `type/short-slug`.
100114
- One PR per issue where possible; stacked PRs are merged one at a time, parent first, never with
101115
`--delete-branch` until the whole chain has landed.
116+
- **`gh pr merge --admin` is no longer the merge path.** It was, while `main` required an approving review
117+
that a solo author cannot give themselves. `main` now requires a PR and green checks instead of an
118+
approval, so a green PR merges with a plain `gh pr merge`. Reach for `--admin` and you are bypassing the
119+
contract tests, not a paperwork rule.
102120

103121
## Things that look like bugs and are not
104122

0 commit comments

Comments
 (0)