From be64163736ed339a0538be693cee052d793a5b55 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sun, 9 Aug 2026 02:10:44 +0000 Subject: [PATCH] =?UTF-8?q?fix(ci):=20allow-major=20=E6=94=B9=E4=B8=BA?= =?UTF-8?q?=E6=B1=82=E5=80=BC=E6=97=B6=E5=88=BB=E5=AE=9E=E6=97=B6=E8=AF=BB?= =?UTF-8?q?=E6=A0=87=E7=AD=BE,=E7=BA=A2=20run=20=E4=B8=8D=E5=86=8D?= =?UTF-8?q?=E6=B0=B8=E4=B9=85=E7=BA=A2=20(#5620)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pr-automation.yml` 的 `Guard against accidental major bumps (launch window)` 第三个 clause 一直读 `contains(github.event.pull_request.labels.*.name, 'allow-major')` —— 与 #5580 修掉的是同一个读法、同一个后果:载荷是事件触发那一刻的 快照,开 PR 后再补 `allow-major` 该 run 看不见;而 `rerun_failed_jobs` 复用同一份 载荷(pm-dispatch Operational notes 5),于是这个红 run 按构造重跑不成绿,只能靠一次 新的真实事件顶掉。 复用 #5580 的落地形状(`a9c7e6ba0` 在同一 job 新增 `id: labels` 实时读标签步骤, 以 `steps.labels.outputs.skip` 供下游 `if:` 消费):新增 `id: allow_major` 步骤, 同一个 `gh api repos/$REPO/pulls/$PR` 端点、同一个 `grep -qxF` 整行精确匹配、同一个 here-string(避免 `grep -q` 进管道后写入端吃 SIGPIPE、在 pipefail 下翻转判定)、 同一个 #4690 容忍方向 —— 读不到标签判 `allow=false`,即照常执行守卫。守卫步骤的第三个 clause 改读 `steps.allow_major.outputs.allow != 'true'`。 与 #5580 的两处刻意分歧,均因本标签在 job **末尾**被消费而非开头: 1. **不保留载荷 fast-path。** #5580 保留它是因为载荷里已有 `skip-changeset` 可跳过 整个 job(checkout + install 全免)换一次 API 调用;这里下游只剩一条 `node scripts/check-changeset-no-major.mjs`,fast-path 省不下任何东西,却会一并 保留 #5580 承认的那个反向 stale 格(事件后被**移除**的标签仍豁免本 run)。去掉后, 豁免只能由「真的观测到的标签」建立。 2. **不加 #6378 式结算窗口**,并按残留记录而非当作已解决。#6378 的等待只能记在 「注定要红」的 PR 头上,而这里的「注定要红」= 「声明了 major 且没有标签」—— 只有 `check-changeset-no-major.mjs` 知道,且它以 exit 1 表达。要据此设条件就得把 脚本跑两遍、或吞掉输出再回放,即改变该守卫的报错形状;而本步骤的**位置**已经把 竞态窗口收窄:它排在 checkout + setup-node + `pnpm install --frozen-lockfile` + 计数 + 两个 `check-*.mjs` 之后,而 job 开头的 fast-path 读位于开 PR 后约 +10s (#6378 实测 run 31204438874),标签落地延迟实测 +10..45s(#6310、#6358)。 更关键:与 #5580 的方向不同,这里的首红**有救** —— 读是实时的,补标签后 `rerun_failed_jobs` 即收敛为绿。消掉「永久红」正是 #5620 记的那笔。 **休眠不作为依据。** `check-changeset-no-major.mjs` 在 pre-release 窗口整体让位 (其 RC EXEMPTION 头注),`.changeset/pre.json` 现为 `"mode": "pre"`,故该守卫今天 不可能红、标签也用不上。它在 `changeset pre exit` 当刻自动重新武装 —— 恰是全栈 major 讨论最密集、`allow-major` 最可能被现场手工补标签的窗口。文件内注释写明了这一点。 回归钉:`scripts/check-empty-changeset.mjs` 的 consumer 自测块(该仓唯一断言本 job 标签读取形状的地方)增 5 条断言 —— 工作流内不得再出现 `allow-major` 的载荷读法、 恰好一处实时 `grep -qxF 'allow-major'`、守卫必须消费 `steps.allow_major.outputs.allow`、 实时读须同时尊重两处 skip-changeset 读、`allow=true` 只能写一次且必须在真正观测到 标签的 grep 之后(容忍方向按位置钉死,而非数分支)。 ⛔ 未动:job 名与既有 step 命名约定(无新增 job,必需检查上下文不受影响)、计数逻辑、 `BASE_SHA`/merge-base、release 与 pre-mode 策略、`docs/adr/**`。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn --- .github/workflows/pr-automation.yml | 99 ++++++++++++++++++++++++++--- scripts/check-empty-changeset.mjs | 56 ++++++++++++++++ 2 files changed, 145 insertions(+), 10 deletions(-) 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