diff --git a/.github/workflows/pr-automation.yml b/.github/workflows/pr-automation.yml index e5dcc511ee..4bf52ad4db 100644 --- a/.github/workflows/pr-automation.yml +++ b/.github/workflows/pr-automation.yml @@ -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 @@ -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 diff --git a/scripts/check-empty-changeset.mjs b/scripts/check-empty-changeset.mjs index 9be9d2c954..9e5e9d742a 100644 --- a/scripts/check-empty-changeset.mjs +++ b/scripts/check-empty-changeset.mjs @@ -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(, ...)\` 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