Skip to content

Commit be64163

Browse files
os-zhuangclaude
andcommitted
fix(ci): allow-major 改为求值时刻实时读标签,红 run 不再永久红 (#5620)
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn
1 parent 6595262 commit be64163

2 files changed

Lines changed: 145 additions & 10 deletions

File tree

.github/workflows/pr-automation.yml

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,90 @@ jobs:
721721
node scripts/check-adr-0087-registration.mjs --self-test
722722
node scripts/check-adr-0087-registration.mjs --base "$MERGE_BASE"
723723
724+
# ── The allow-major read (#5620) ─────────────────────────────────────────
725+
#
726+
# The twin of #5580, one label along. Until this step existed, the guard
727+
# below read `contains(github.event.pull_request.labels.*.name,
728+
# 'allow-major')` -- the same frozen snapshot, with the same two
729+
# consequences: an `allow-major` applied after the event fired is invisible
730+
# to the run, and `rerun_failed_jobs` replays that SAME payload (pm-dispatch
731+
# Operational notes 5), so the red it produces cannot be re-run green. Only
732+
# a fresh real event could ever displace it.
733+
#
734+
# Same shape as #5580's read, deliberately rather than by coincidence: same
735+
# endpoint, same `grep -qxF` whole-line matcher (so an `allow-major-audit`
736+
# label buys no exemption), same here-string so `grep -q` never sits in a
737+
# pipeline where a writer can take SIGPIPE under `set -o pipefail`, and the
738+
# same #4690 tolerance direction -- an unreadable label list resolves to
739+
# `allow=false`, i.e. RUN THE GUARD. A gate that could not read its input has
740+
# verified nothing, and handing out a whole-stack-major exemption on that
741+
# basis is the anti-pattern itself.
742+
#
743+
# Two deliberate divergences from #5580, both because this label is consumed
744+
# at the END of the job rather than at the top:
745+
#
746+
# 1. NO payload fast path. #5580 kept `contains(...)` as a short-circuit
747+
# because a `skip-changeset` already visible in the payload skips an
748+
# entire job -- checkout, install, everything -- for one API call. Here
749+
# the only thing downstream is one `node
750+
# scripts/check-changeset-no-major.mjs`, so a fast path would save
751+
# nothing while preserving #5580's one acknowledged stale cell (a label
752+
# REMOVED after the event still exempting this run). Dropping it means
753+
# the exemption is established only by a label really observed.
754+
# 2. NO #6378-style settling window, and that is a residual recorded rather
755+
# than an oversight. #6378's wait may be charged ONLY to a PR headed for
756+
# red, and "headed for red" here means "declares a major AND is
757+
# unlabelled" -- a fact only check-changeset-no-major.mjs knows, and it
758+
# reports it by exiting 1. Conditioning a wait on that means running the
759+
# script twice, or swallowing its output and replaying it, i.e.
760+
# restructuring how this guard reports, for a race this step's POSITION
761+
# already narrows: it runs after checkout + setup-node + `pnpm install
762+
# --frozen-lockfile` + the count + two `check-*.mjs` gates, where the
763+
# fast-path read at the top of the job sits at ~+10s from PR creation
764+
# (measured, #6378 on run 31204438874) against a label latency of
765+
# ~+10..45s (#6310, #6358). And unlike #5580's direction, a residual
766+
# first red here HAS a rescue: because the read is live,
767+
# `rerun_failed_jobs` after labelling converges to green. Removing the
768+
# permanent red is the whole of what #5620 records.
769+
#
770+
# DORMANT TODAY, and the dormancy is deliberately not load-bearing.
771+
# `check-changeset-no-major.mjs` stands aside for the whole pre-release
772+
# window (its RC EXEMPTION note) and `.changeset/pre.json` currently says
773+
# `"mode": "pre"`, so the guard below cannot fail and the label is never
774+
# needed. It re-arms by itself at `changeset pre exit` -- precisely the day
775+
# whole-stack majors are under discussion and `allow-major` is most likely to
776+
# be applied by hand, seconds after `gh pr create`. Nothing here leans on the
777+
# dormancy, and it must not become a reason to weaken the read.
778+
- name: Re-read this PR's allow-major label live (the event payload can predate it)
779+
id: allow_major
780+
# Both skip-changeset reads, for the same reason the guard below names
781+
# them: a PR the changeset gate exempts must not pay an API call for a
782+
# guard that is not going to run.
783+
if: >-
784+
steps.labels.outputs.skip != 'true'
785+
&& steps.labels_settled.outputs.skip != 'true'
786+
env:
787+
GH_TOKEN: ${{ github.token }}
788+
PR_NUMBER: ${{ github.event.pull_request.number }}
789+
run: |
790+
if [ -z "$PR_NUMBER" ]; then
791+
echo "::warning::No PR number on this event, so the 'allow-major' label could not be read. Enforcing the launch-window major guard."
792+
echo 'allow=false' >> "$GITHUB_OUTPUT"
793+
exit 0
794+
fi
795+
if ! LABELS=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" --jq '.labels[].name'); then
796+
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."
797+
echo 'allow=false' >> "$GITHUB_OUTPUT"
798+
exit 0
799+
fi
800+
echo "Labels on PR #$PR_NUMBER right now: ${LABELS:-(none)}"
801+
if grep -qxF 'allow-major' <<<"$LABELS"; then
802+
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."
803+
echo 'allow=true' >> "$GITHUB_OUTPUT"
804+
else
805+
echo 'allow=false' >> "$GITHUB_OUTPUT"
806+
fi
807+
724808
- name: Guard against accidental major bumps (launch window)
725809
# Every publishable package is in one Changesets "fixed" (lockstep) group,
726810
# so a single `major` bump promotes the ENTIRE monorepo to a new major
@@ -734,17 +818,12 @@ jobs:
734818
# can be established by either one, and a step that honoured only the fast
735819
# path would re-arm itself on precisely the PRs the settling read rescued.
736820
#
737-
# The second clause still reads the frozen payload, and so still carries
738-
# the #5580 race in its own right: an `allow-major` applied after the event
739-
# fired is invisible to this run and a rerun replays the same payload.
740-
# It is DORMANT while Changesets is in pre-release mode, because
741-
# check-changeset-no-major.mjs stands aside for the whole RC window (see
742-
# its RC EXEMPTION note), so the label is currently never needed. Tracked
743-
# as #5620 rather than fixed here: #5580 scoped this change to the
744-
# `skip-changeset` read, and widening a green gate's exemption path under
745-
# cover of another issue is how exemptions grow unnoticed.
821+
# The third clause is the LIVE `allow-major` read directly above (#5620),
822+
# never the event payload. Its comment block carries the argument: what the
823+
# payload read cost, where this diverges from #5580's `skip-changeset` read
824+
# and why, the residual it leaves, and what re-arms the whole thing.
746825
if: >-
747826
steps.labels.outputs.skip != 'true'
748827
&& steps.labels_settled.outputs.skip != 'true'
749-
&& !contains(github.event.pull_request.labels.*.name, 'allow-major')
828+
&& steps.allow_major.outputs.allow != 'true'
750829
run: node scripts/check-changeset-no-major.mjs

scripts/check-empty-changeset.mjs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,62 @@ function selfTest() {
820820
`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)`,
821821
);
822822

823+
// ── The same race, one label along (#5620) ───────────────────────────────
824+
//
825+
// `allow-major` is the launch-window guard's escape hatch, and it was read
826+
// from `github.event.pull_request.labels` for exactly as long as the
827+
// `skip-changeset` half was: same frozen snapshot, same permanent red under
828+
// `rerun_failed_jobs`, which replays the payload. It is pinned HERE, beside
829+
// the skip-changeset reads, because it is the same defect in the same job
830+
// and a second home for it would be one more thing to remember.
831+
//
832+
// Two properties of this one make the pin worth more than usual, not less.
833+
// It is DORMANT -- check-changeset-no-major.mjs stands aside for the whole
834+
// pre-release window -- so no CI run can currently exercise the live read
835+
// and a revert to the payload would be invisible until `changeset pre exit`
836+
// re-arms the guard, which is the day whole-stack majors are being argued
837+
// about. And it has no settling read to fall back on, by the argument in
838+
// the workflow; the position of the read is doing that work instead.
839+
//
840+
// What is pinned is the SHAPE OF THE EXEMPTION, never its permissiveness:
841+
// no payload read anywhere, exactly one live read, the guard consuming that
842+
// read, and an input that could not be read still RUNNING the guard.
843+
assert(
844+
!/contains\(github\.event\.pull_request\.labels\.\*\.name, 'allow-major'\)/.test(yaml),
845+
"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)",
846+
);
847+
const allowMajorReads = [...yaml.matchAll(/grep -qxF 'allow-major'/g)];
848+
assert(
849+
allowMajorReads.length === 1,
850+
`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.`,
851+
);
852+
const namedSteps = jobText
853+
.split(/\n(?= - name: )/)
854+
.map((c) => c.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'));
855+
const majorGuard = namedSteps.find((c) => /- name: Guard against accidental major bumps/.test(c));
856+
assert(
857+
majorGuard !== undefined && /steps\.allow_major\.outputs\.allow != 'true'/.test(majorGuard),
858+
'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',
859+
);
860+
const allowMajorStep = namedSteps.find((c) => /grep -qxF 'allow-major'/.test(c));
861+
assert(
862+
allowMajorStep !== undefined
863+
&& /steps\.labels\.outputs\.skip != 'true'/.test(allowMajorStep)
864+
&& /steps\.labels_settled\.outputs\.skip != 'true'/.test(allowMajorStep),
865+
'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',
866+
);
867+
// The tolerance direction, pinned by POSITION rather than by counting the
868+
// enforcing branches: `allow=true` may be written once, and only downstream
869+
// of the grep that actually observed the label. Every other exit -- no PR
870+
// number, an unreadable label list -- reaches the guard (#4690).
871+
const allowLines = (allowMajorStep ?? '').split('\n');
872+
const grepAt = allowLines.findIndex((l) => /grep -qxF 'allow-major'/.test(l));
873+
const allowTrueAt = allowLines.map((l, i) => (/allow=true/.test(l) ? i : -1)).filter((i) => i >= 0);
874+
assert(
875+
grepAt >= 0 && allowTrueAt.length === 1 && allowTrueAt[0] > grepAt,
876+
`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`,
877+
);
878+
823879
// The hard constraint of #6378, stated as structure: none of this may have
824880
// made the gate softer. A PR with no changeset and no label still has to
825881
// hit a real non-zero exit, and no step of this job may be excused from

0 commit comments

Comments
 (0)