From 5105f6b19b591c48d5d7527ce65b47d81cf74b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 10:35:59 +0200 Subject: [PATCH 1/2] docs(contributing): do not cancel the CI run of the PR being merged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cancelled job is neither a pass nor a failure, and two protections go quiet together: pr-gate never reports (so the required context is absent rather than red, which is what invites the bypass), and the changelog fragment check — a step inside lint, conditioned on pull_request — is skipped silently, so the omission stays invisible until release notes are cut. Both were observed on the same day. #9169 merged with lint failing and five jobs cancelled, breaking method dispatch and property lookup on main for four and a half hours (#9247). #9215, #9230 and #9235 each merged with lint CANCELLED; all three touched crates/, none carried a fragment, and the work is absent from its release notes. States explicitly that the gate is correct and should not be changed: gate in test.yml runs if: always() and treats cancelled as failure, exactly so a cancelled dependency cannot read as green. Every incident has been a bypass of a working gate. Docs only. --- CONTRIBUTING.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 19507a19eb..d32ecb0f8d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -135,6 +135,38 @@ on Node 22 while the suite grew Node 24/26 features and hid 14 tests that way. Any PR touching `crates/` also needs a `changelog.d/-.md` fragment; see [changelog.d/README.md](changelog.d/README.md). +**Never cancel the CI run of the PR you are about to merge.** Cancelling other +runs to free capacity is fine and expected on a 20-slot org. Cancelling the run +of the PR being merged is not, because a cancelled job is neither a pass nor a +failure, and two separate protections go quiet at once: + +* **`pr-gate` never reports**, so the single required context is absent rather + than red. That is what invites the admin bypass, and the bypass is what lands + the change. +* **The changelog fragment stops being required.** That check is a step inside + the `lint` job, conditioned on `github.event_name == 'pull_request'`, so a + cancelled `lint` skips it silently — the PR merges with no fragment, and the + omission is invisible until the release notes are cut weeks later and the + change simply is not in them. + +The gate logic itself is correct and does not need changing: `gate` in +`.github/workflows/test.yml` runs `if: always()` and treats `failure` **and** +`cancelled` as failures, precisely so a cancelled dependency cannot read as +green. Every incident here has been a bypass of a working gate, so resist the +urge to "fix" the gate. + +Both failure modes were observed on the same day. #9169 merged with `lint` +failing and five jobs cancelled; its gap shards never ran and it broke method +dispatch and property lookup on `main` for four and a half hours (#9247). +Separately, #9215, #9230 and #9235 each merged with `lint` **cancelled** — all +three touched `crates/`, none carried a fragment, and the work is consequently +absent from its release notes. + +If you are reviewing status before a merge, note that "no failing checks" is +not "all checks ran": `CANCELLED` is neither, and tooling that filters for +`FAILURE` will report a clean bill of health for a PR whose gate never +executed. + UI doc-tests launch real windows. On headless hosts, wrap in `xvfb-run -a` (Linux) or rely on `PERRY_UI_TEST_MODE=1` which auto-exits after one frame. ## Asking questions From 17beb3d97f42fff6ed0c3f645b079b90b6061b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 10:37:36 +0200 Subject: [PATCH 2/2] docs(contributing): teach 'pr-gate present and passing', not 'nothing red' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A gate that never ran is absent from the status list, so it reads as clean under any failure filter — the same way CANCELLED does. 'pr-gate: pass' is a positive assertion that the fan-in ran and every dependency was success or skipped; '0 failing' is satisfied equally by a PR whose gate never executed. Extends the note to release automation, where the same hole exists one level up: a skipped or absent required context satisfies 'not failing', so the dispatch condition has to require conclusion == success. --- CONTRIBUTING.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d32ecb0f8d..cc4a0fef54 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -162,10 +162,16 @@ Separately, #9215, #9230 and #9235 each merged with `lint` **cancelled** — all three touched `crates/`, none carried a fragment, and the work is consequently absent from its release notes. -If you are reviewing status before a merge, note that "no failing checks" is -not "all checks ran": `CANCELLED` is neither, and tooling that filters for -`FAILURE` will report a clean bill of health for a PR whose gate never -executed. +**Before merging, check that `pr-gate` is present and passing — not that +nothing is red.** Those are different assertions, and only the first one is +worth anything. `CANCELLED` is neither a pass nor a failure, and a gate that +never ran is *absent from the status list altogether*, so both states read as +clean under any check that looks for failures. `pr-gate: pass` is a positive +statement that the fan-in ran and every dependency was `success` or `skipped`; +"0 failing" is satisfied just as well by a PR whose gate never executed. The +same hole exists one level up for release automation: require the gate's +conclusion to be `success`, since a skipped or absent required context also +satisfies "not failing". UI doc-tests launch real windows. On headless hosts, wrap in `xvfb-run -a` (Linux) or rely on `PERRY_UI_TEST_MODE=1` which auto-exits after one frame.