Skip to content
Merged
Show file tree
Hide file tree
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
99 changes: 89 additions & 10 deletions .github/workflows/pr-automation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,90 @@ jobs:
node scripts/check-adr-0087-registration.mjs --self-test
node scripts/check-adr-0087-registration.mjs --base "$MERGE_BASE"

# ── The allow-major read (#5620) ─────────────────────────────────────────
#
# The twin of #5580, one label along. Until this step existed, the guard
# below read `contains(github.event.pull_request.labels.*.name,
# 'allow-major')` -- the same frozen snapshot, with the same two
# consequences: an `allow-major` applied after the event fired is invisible
# to the run, and `rerun_failed_jobs` replays that SAME payload (pm-dispatch
# Operational notes 5), so the red it produces cannot be re-run green. Only
# a fresh real event could ever displace it.
#
# Same shape as #5580's read, deliberately rather than by coincidence: same
# endpoint, same `grep -qxF` whole-line matcher (so an `allow-major-audit`
# label buys no exemption), same here-string so `grep -q` never sits in a
# pipeline where a writer can take SIGPIPE under `set -o pipefail`, and the
# same #4690 tolerance direction -- an unreadable label list resolves to
# `allow=false`, i.e. RUN THE GUARD. A gate that could not read its input has
# verified nothing, and handing out a whole-stack-major exemption on that
# basis is the anti-pattern itself.
#
# Two deliberate divergences from #5580, both because this label is consumed
# at the END of the job rather than at the top:
#
# 1. NO payload fast path. #5580 kept `contains(...)` as a short-circuit
# because a `skip-changeset` already visible in the payload skips an
# entire job -- checkout, install, everything -- for one API call. Here
# the only thing downstream is one `node
# scripts/check-changeset-no-major.mjs`, so a fast path would save
# nothing while preserving #5580's one acknowledged stale cell (a label
# REMOVED after the event still exempting this run). Dropping it means
# the exemption is established only by a label really observed.
# 2. NO #6378-style settling window, and that is a residual recorded rather
# than an oversight. #6378's wait may be charged ONLY to a PR headed for
# red, and "headed for red" here means "declares a major AND is
# unlabelled" -- a fact only check-changeset-no-major.mjs knows, and it
# reports it by exiting 1. Conditioning a wait on that means running the
# script twice, or swallowing its output and replaying it, i.e.
# restructuring how this guard reports, for a race this step's POSITION
# already narrows: it runs after checkout + setup-node + `pnpm install
# --frozen-lockfile` + the count + two `check-*.mjs` gates, where the
# fast-path read at the top of the job sits at ~+10s from PR creation
# (measured, #6378 on run 31204438874) against a label latency of
# ~+10..45s (#6310, #6358). And unlike #5580's direction, a residual
# first red here HAS a rescue: because the read is live,
# `rerun_failed_jobs` after labelling converges to green. Removing the
# permanent red is the whole of what #5620 records.
#
# DORMANT TODAY, and the dormancy is deliberately not load-bearing.
# `check-changeset-no-major.mjs` stands aside for the whole pre-release
# window (its RC EXEMPTION note) and `.changeset/pre.json` currently says
# `"mode": "pre"`, so the guard below cannot fail and the label is never
# needed. It re-arms by itself at `changeset pre exit` -- precisely the day
# whole-stack majors are under discussion and `allow-major` is most likely to
# be applied by hand, seconds after `gh pr create`. Nothing here leans on the
# dormancy, and it must not become a reason to weaken the read.
- name: Re-read this PR's allow-major label live (the event payload can predate it)
id: allow_major
# Both skip-changeset reads, for the same reason the guard below names
# them: a PR the changeset gate exempts must not pay an API call for a
# guard that is not going to run.
if: >-
steps.labels.outputs.skip != 'true'
&& steps.labels_settled.outputs.skip != 'true'
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
if [ -z "$PR_NUMBER" ]; then
echo "::warning::No PR number on this event, so the 'allow-major' label could not be read. Enforcing the launch-window major guard."
echo 'allow=false' >> "$GITHUB_OUTPUT"
exit 0
fi
if ! LABELS=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.labels[].name'); then
echo "::warning::Could not read the labels of PR #$PR_NUMBER, so this run cannot see an 'allow-major' applied after the event fired. Enforcing the launch-window major guard."
echo 'allow=false' >> "$GITHUB_OUTPUT"
exit 0
fi
echo "Labels on PR #$PR_NUMBER right now: ${LABELS:-(none)}"
if grep -qxF 'allow-major' <<<"$LABELS"; then
echo "::notice::'allow-major' is on PR #$PR_NUMBER (read live, not from the event payload), so a whole-stack major is intended here and the launch-window guard stands aside."
echo 'allow=true' >> "$GITHUB_OUTPUT"
else
echo 'allow=false' >> "$GITHUB_OUTPUT"
fi

- name: Guard against accidental major bumps (launch window)
# Every publishable package is in one Changesets "fixed" (lockstep) group,
# so a single `major` bump promotes the ENTIRE monorepo to a new major
Expand All @@ -734,17 +818,12 @@ jobs:
# can be established by either one, and a step that honoured only the fast
# path would re-arm itself on precisely the PRs the settling read rescued.
#
# The second clause still reads the frozen payload, and so still carries
# the #5580 race in its own right: an `allow-major` applied after the event
# fired is invisible to this run and a rerun replays the same payload.
# It is DORMANT while Changesets is in pre-release mode, because
# check-changeset-no-major.mjs stands aside for the whole RC window (see
# its RC EXEMPTION note), so the label is currently never needed. Tracked
# as #5620 rather than fixed here: #5580 scoped this change to the
# `skip-changeset` read, and widening a green gate's exemption path under
# cover of another issue is how exemptions grow unnoticed.
# The third clause is the LIVE `allow-major` read directly above (#5620),
# never the event payload. Its comment block carries the argument: what the
# payload read cost, where this diverges from #5580's `skip-changeset` read
# and why, the residual it leaves, and what re-arms the whole thing.
if: >-
steps.labels.outputs.skip != 'true'
&& steps.labels_settled.outputs.skip != 'true'
&& !contains(github.event.pull_request.labels.*.name, 'allow-major')
&& steps.allow_major.outputs.allow != 'true'
run: node scripts/check-changeset-no-major.mjs
56 changes: 56 additions & 0 deletions scripts/check-empty-changeset.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,62 @@ function selfTest() {
`consumer: every changeset-verdict step must honour the fast-path read too (${halfGuarded.length} do not) -- dropping it makes an already-labelled PR pay a whole job (#5580)`,
);

// ── The same race, one label along (#5620) ───────────────────────────────
//
// `allow-major` is the launch-window guard's escape hatch, and it was read
// from `github.event.pull_request.labels` for exactly as long as the
// `skip-changeset` half was: same frozen snapshot, same permanent red under
// `rerun_failed_jobs`, which replays the payload. It is pinned HERE, beside
// the skip-changeset reads, because it is the same defect in the same job
// and a second home for it would be one more thing to remember.
//
// Two properties of this one make the pin worth more than usual, not less.
// It is DORMANT -- check-changeset-no-major.mjs stands aside for the whole
// pre-release window -- so no CI run can currently exercise the live read
// and a revert to the payload would be invisible until `changeset pre exit`
// re-arms the guard, which is the day whole-stack majors are being argued
// about. And it has no settling read to fall back on, by the argument in
// the workflow; the position of the read is doing that work instead.
//
// What is pinned is the SHAPE OF THE EXEMPTION, never its permissiveness:
// no payload read anywhere, exactly one live read, the guard consuming that
// read, and an input that could not be read still RUNNING the guard.
assert(
!/contains\(github\.event\.pull_request\.labels\.\*\.name, 'allow-major'\)/.test(yaml),
"consumer: the allow-major exemption must never be read from `github.event.pull_request.labels` -- that snapshot is frozen when the event fires and `rerun_failed_jobs` replays it, so the red it produces cannot be re-run green (#5620)",
);
const allowMajorReads = [...yaml.matchAll(/grep -qxF 'allow-major'/g)];
assert(
allowMajorReads.length === 1,
`consumer: exactly one live \`grep -qxF 'allow-major'\` read is expected in the workflow; found ${allowMajorReads.length}. The matcher is whole-line and fixed for the same two reasons as the skip-changeset reads: \`contains(<array>, ...)\` matched an array ELEMENT, so a substring match would newly exempt an \`allow-major-audit\` label, and a piped \`grep -q\` can take SIGPIPE under pipefail.`,
);
const namedSteps = jobText
.split(/\n(?= - name: )/)
.map((c) => c.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'));
const majorGuard = namedSteps.find((c) => /- name: Guard against accidental major bumps/.test(c));
assert(
majorGuard !== undefined && /steps\.allow_major\.outputs\.allow != 'true'/.test(majorGuard),
'consumer: the launch-window major guard must take its exemption from the LIVE allow-major read (`steps.allow_major.outputs.allow`) -- reading the event payload there is #5620 itself',
);
const allowMajorStep = namedSteps.find((c) => /grep -qxF 'allow-major'/.test(c));
assert(
allowMajorStep !== undefined
&& /steps\.labels\.outputs\.skip != 'true'/.test(allowMajorStep)
&& /steps\.labels_settled\.outputs\.skip != 'true'/.test(allowMajorStep),
'consumer: the live allow-major read must honour both skip-changeset reads -- a PR the changeset gate exempts must not buy an API call for a guard that will not run',
);
// The tolerance direction, pinned by POSITION rather than by counting the
// enforcing branches: `allow=true` may be written once, and only downstream
// of the grep that actually observed the label. Every other exit -- no PR
// number, an unreadable label list -- reaches the guard (#4690).
const allowLines = (allowMajorStep ?? '').split('\n');
const grepAt = allowLines.findIndex((l) => /grep -qxF 'allow-major'/.test(l));
const allowTrueAt = allowLines.map((l, i) => (/allow=true/.test(l) ? i : -1)).filter((i) => i >= 0);
assert(
grepAt >= 0 && allowTrueAt.length === 1 && allowTrueAt[0] > grepAt,
`consumer: the live allow-major read must write \`allow=true\` exactly once and only after the label was really observed (found ${allowTrueAt.length} at ${JSON.stringify(allowTrueAt)}, grep at ${grepAt}) -- an exemption handed out because the label list could not be read is the #4690 anti-pattern, and here it would wave a whole-stack major through`,
);

// The hard constraint of #6378, stated as structure: none of this may have
// made the gate softer. A PR with no changeset and no label still has to
// hit a real non-zero exit, and no step of this job may be excused from
Expand Down
Loading