Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,44 @@ 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/<PR-number>-<slug>.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.

**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.

## Asking questions
Expand Down
Loading