Skip to content

fix(ci): 无法解析的 diff 基准改为落成 output，由已认两次标签读取的步骤裁决 (#6434) #6508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hotlong merged 1 commit into from
Aug 8, 2026
+170 −19
Merged

fix(ci): 无法解析的 diff 基准改为落成 output，由已认两次标签读取的步骤裁决 (#6434) #6508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
106 changes: 96 additions & 10 deletions .github/workflows/pr-automation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,23 @@ jobs:
# someone gives the checkout a `ref:`, where parent^1 becomes the PR's
# previous commit. `merge-base` is right under BOTH checkouts, which is
# why it is the one written here.
#
# #6434: this step REPORTS an unusable base, it does not adjudicate one.
# Both of its former `exit 1`s are now `base_error` outputs, and the verdict
# moved to `Require a usable diff base` further down -- the one place that
# has seen BOTH label reads. The reason is the same #5580/#6378 race one
# defect along: this step runs before the settling read, so a
# `skip-changeset` label that lands in its window (the COMMON case, ~+10..45s
# from PR creation) was still invisible here. Failing on the spot therefore
# reddened a PR the job was about to exempt -- two conditions at once, the
# late label plus a genuine git-base failure, which is why no instance was
# ever observed and why the gap was recorded rather than guessed at.
#
# What is deliberately NOT changed: an unusable base is still a FAILURE for
# every PR the changeset gate applies to. #4690 governs the verdict, not its
# address. The warning below is emitted where the fact is discovered so the
# infrastructure flake stays visible in the log even on the exempt path,
# where nothing else would ever mention it.
- name: Resolve the diff base (merge base with the base branch)
id: diffbase
if: steps.labels.outputs.skip != 'true'
Expand All @@ -282,8 +299,9 @@ jobs:
PINNED_BASE_SHA: ${{ github.event.pull_request.base.sha }}
run: |
if [ -z "$BASE_REF" ]; then
echo "::error::This event carries no base branch, so the changeset diff base cannot be computed. A gate that cannot read its input has verified nothing, so this is a failure rather than a pass (#4690)."
exit 1
echo "::warning::This event carries no base branch, so the changeset diff base cannot be computed. Adjudicated below, once the skip-changeset window has settled."
echo 'base_error=This event carries no base branch, so the changeset diff base cannot be computed.' >> "$GITHUB_OUTPUT"
exit 0
fi
if ! git rev-parse --verify --quiet "refs/remotes/origin/$BASE_REF^{commit}" >/dev/null; then
git fetch --no-tags --quiet origin "+refs/heads/$BASE_REF:refs/remotes/origin/$BASE_REF" \
Expand All @@ -292,11 +310,17 @@ jobs:
# `if !` rather than a bare assignment on purpose: these steps run under
# `bash -e` (no `shell:` key anywhere in this file), where a failing
# command substitution kills the step with no message at all. The gate
# is allowed to fail here -- it is NOT allowed to fail unexplained.
# is allowed to fail over this -- it is NOT allowed to fail unexplained.
if ! MERGE_BASE=$(git merge-base "refs/remotes/origin/$BASE_REF" HEAD); then
echo "::error::Could not compute merge-base(origin/$BASE_REF, HEAD), so the changeset diff has no trustworthy starting point. Failing rather than falling back to the frozen base.sha, which is the #6129 defect itself."
exit 1
echo "::warning::Could not compute merge-base(origin/$BASE_REF, HEAD), so the changeset diff has no trustworthy starting point. Adjudicated below, once the skip-changeset window has settled."
echo "base_error=Could not compute merge-base(origin/$BASE_REF, HEAD), so the changeset diff has no trustworthy starting point." >> "$GITHUB_OUTPUT"
exit 0
fi
# Written explicitly rather than left unset. An unwritten output already
# reads as the empty string, but every downstream `if:` here spells the
# success case as `base_error == ''`, and a contract that load-bearing
# should be visible in the shell that establishes it.
echo 'base_error=' >> "$GITHUB_OUTPUT"
echo "merge_base=$MERGE_BASE" >> "$GITHUB_OUTPUT"
# The drift is printed, not just corrected. #6129 was invisible for as
# long as it was because nothing in the log ever said which commit the
Expand All @@ -306,18 +330,37 @@ jobs:
echo "Diff base: $MERGE_BASE (merge-base of origin/$BASE_REF and HEAD)"
echo "Frozen payload base.sha: $PINNED_BASE_SHA -- $BASE_REF has moved $DRIFT commit(s) since it was frozen, and that drift is exactly what this gate used to count as this PR's own."

# The three toolchain steps and the counting step below all carry the
# `base_error == ''` conjunct as well (#6434). For the counting step it is
# CORRECTNESS: handed an empty `$MERGE_BASE`, `git diff --diff-filter=A ""
# HEAD` fails, but it fails inside a pipeline whose last command is `tr`, so
# the step would report `added=0` -- a fabricated count that reads exactly
# like "this PR forgot its changeset" and would send the verdict step to the
# wrong error message entirely. For the three toolchain steps it is COST and
# legibility: an unusable base can no longer be adjudicated until the label
# window settles, and there is no reason to buy a Node toolchain and a
# `--frozen-lockfile` install first for a run that will either be exempted or
# failed without ever compiling anything. It also keeps the failure signature
# single: on this path the only red is the adjudication step, never an
# install that happened to flake on the same shaky network.
- name: Setup Node.js
if: steps.labels.outputs.skip != 'true'
if: >-
steps.labels.outputs.skip != 'true'
&& steps.diffbase.outputs.base_error == ''
uses: actions/setup-node@v7
with:
node-version: '22'

- name: Enable Corepack
if: steps.labels.outputs.skip != 'true'
if: >-
steps.labels.outputs.skip != 'true'
&& steps.diffbase.outputs.base_error == ''
run: corepack enable

- name: Install dependencies
if: steps.labels.outputs.skip != 'true'
if: >-
steps.labels.outputs.skip != 'true'
&& steps.diffbase.outputs.base_error == ''
run: pnpm install --frozen-lockfile

# COUNTING ONLY -- the verdict is two steps down (#6378). The split is not
Expand All @@ -327,7 +370,9 @@ jobs:
# that fact before any waiting is considered.
- name: Count the changesets this PR adds
id: changeset_count
if: steps.labels.outputs.skip != 'true'
if: >-
steps.labels.outputs.skip != 'true'
&& steps.diffbase.outputs.base_error == ''
env:
MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }}
run: |
Expand Down Expand Up @@ -405,11 +450,21 @@ jobs:
# already behind us, and the loop does exactly one read and no sleeping.
# Only a genuinely fresh PR can wait at all. 120s is ~3x the worst label
# latency measured on this repo (~40s, #6358).
#
# #6434 adds the second disjunct, and it is the SAME invariant rather than a
# widening of it: "charge the wait only to a PR that is headed for red". A PR
# whose diff base could not be resolved is headed for red just as surely as
# one that added no changeset -- it simply gets there by the other route, and
# the counting step is skipped on that path, so `added` is `''` and the first
# disjunct can never speak for it. No PR waits that would not have waited
# under #6378: a resolvable base plus a changeset still skips this step
# entirely, at no wall time and no API call.
- name: Settle the skip-changeset window (only when this PR would otherwise fail)
id: labels_settled
if: >-
steps.labels.outputs.skip != 'true'
&& steps.changeset_count.outputs.added == '0'
&& (steps.changeset_count.outputs.added == '0'
|| steps.diffbase.outputs.base_error != '')
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
Expand Down Expand Up @@ -464,6 +519,37 @@ jobs:
echo "Label window closed after $ATTEMPT read(s). Labels on PR #$PR_NUMBER right now: ${LABELS:-(none)}"
echo 'skip=false' >> "$GITHUB_OUTPUT"

# #6434: the verdict on an unusable diff base. It is a SEPARATE step rather
# than a branch inside the step below, for the reason this job already gives
# for splitting `Reject an empty-frontmatter changeset` off: the two failures
# are different facts and deserve different messages. "The git base is
# unavailable" is not "you forgot a changeset", and an author who reads the
# second when the first is true will go looking in the wrong place.
#
# It sits HERE, immediately after the settling read, because both of its
# neighbours constrain it. It cannot move up: above the settling read there
# is no settled label state, which is the whole defect being fixed. It cannot
# move down: the step below evaluates `[ "$ADDED" -eq 0 ]` on an `ADDED` that
# is the empty string whenever the counting step was skipped, and `test`
# answering "not a number" is a NON-zero status, so the `if` would take its
# else branch and announce "This PR adds changeset(s)" -- a green verdict on
# a gate that never ran. The ordering is load-bearing in both directions.
#
# This is NOT the route-2 move #6378's cost argument rules out. The settling
# read stays exactly where it is, after the count; nothing about which PRs
# pay for the window changed. What moved is one verdict, downward, past a
# read that had already been paid for.
- name: Require a usable diff base (adjudicated after the label window)
if: >-
steps.labels.outputs.skip != 'true'
&& steps.labels_settled.outputs.skip != 'true'
&& steps.diffbase.outputs.base_error != ''
env:
BASE_ERROR: ${{ steps.diffbase.outputs.base_error }}
run: |
echo "::error::$BASE_ERROR Failing rather than falling back to the frozen base.sha, which is the #6129 defect itself: a gate that cannot read its input has verified nothing, so this is a failure rather than a pass (#4690). Neither live label read found 'skip-changeset' on this PR, so the changeset gate does apply to it and cannot be evaluated. If this PR releases nothing, apply the label and re-run; if the base branch is genuinely fetchable, this is infrastructure and a re-run will clear it."
exit 1

# The VERDICT. Everything it needs was decided above; this step only
# announces it, which is what makes the failure message a single block of
# prose rather than something interleaved with counting and polling.
Expand Down
83 changes: 74 additions & 9 deletions scripts/check-empty-changeset.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,20 @@ function selfTest() {
settle,
'consumer: the settling label read must be conditioned on `steps.changeset_count.outputs.added == \'0\'` -- the wait #6378 introduces is charged ONLY to a PR headed for red, and un-conditioning it taxes every run instead',
);
// The condition above is a substring test, so it would go on passing if a
// later edit bolted an unrelated `|| <anything>` onto it and quietly taxed
// every run again. #6434 legitimately adds ONE disjunct -- the unusable diff
// base, which is the other way a PR arrives at red and which the count
// cannot speak for, because the counting step is skipped on that path. So
// the whole condition is pinned, not just its first term: exactly these two
// ways in, and no third without an argument.
const settleIf = yaml.match(
/steps\.labels\.outputs\.skip != 'true'\s*\n\s*&& \(steps\.changeset_count\.outputs\.added == '0'\s*\n\s*\|\| steps\.diffbase\.outputs\.base_error != ''\)/,
);
assert(
settleIf !== null,
"consumer: the settling read's condition must be exactly `no fast-path label AND (added == '0' OR base_error != '')` -- both disjuncts are ways of being headed for RED, which is the only thing that may buy the #6378 wait",
);

// Both reads, one matcher. A divergence (say a substring `grep -q` on one
// path) would be a gate that exempts on one read and enforces on the
Expand All @@ -744,20 +758,56 @@ function selfTest() {
`consumer: exactly two live \`grep -qxF 'skip-changeset'\` reads are expected (the fast path and the settling read); found ${matchers.length}`,
);

// Every step that can FAIL a PR over the changeset rule must honour both
// reads. Scoped to those steps by what they run, not by name: a step that
// shells out to a `check-*.mjs` gate, or that emits the "no changeset"
// error. `Resolve the diff base` is deliberately outside this set -- it
// exits 1 over an unusable git base, which is not a changeset verdict and
// was never label-exempt (recorded, not implied).
// Every step of this job that can FAIL a PR must honour both reads. Scoped
// by what a step RUNS, never by its name: it shells out to a `check-*.mjs`
// gate, or it contains a literal `exit 1` outside a comment.
//
// #6434 widened this boundary, and the widening is the point rather than an
// accident of it. The predicate used to be "runs a `check-*.mjs` OR emits
// the no-changeset error", which described the four steps that existed and
// nothing else. `Resolve the diff base` sat outside it carrying two `exit
// 1`s of its own -- deliberately, on the argument that "the git base is
// unavailable" is not a changeset verdict and so was never label-exempt.
// That argument is sound about the VERDICT and wrong about its ADDRESS: the
// step ran before the settling read, so on a PR whose `skip-changeset` label
// landed in the ordinary +10..45s window it could red a run the job was
// about to exempt. #6434 moved that verdict to `Require a usable diff base`,
// which honours both reads, and the boundary here moved with it.
//
// Naming `exit 1` instead of one specific error string is what makes the
// rule outlive the steps it was written for: the gap #6434 closed existed
// precisely because the old predicate could not see a failing step it had
// not been told about, and the next one added here would have been invisible
// the same way. Residual, stated rather than implied: a step that fails by
// running a command that returns non-zero, with no literal `exit 1` and no
// `check-*.mjs`, is still outside this set. That is a smaller hole than the
// one it replaces, not no hole.
const jobText = yaml.slice(yaml.indexOf('\n changeset-check:'));
const chunks = jobText
.split(/\n(?= - name: )/)
.map((c) => c.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'))
.filter((c) => /node scripts\/check-\S+\.mjs/.test(c) || /::error::This PR adds no changeset/.test(c));
.filter((c) => /node scripts\/check-\S+\.mjs/.test(c) || /\bexit 1\b/.test(c));
assert(
chunks.length === 4,
`consumer: expected 4 changeset-verdict steps in the Check Changeset job, found ${chunks.length} -- a new one that this rule cannot see is a new way to red an exempt PR`,
chunks.length === 5,
`consumer: expected 5 failable steps in the Check Changeset job, found ${chunks.length} -- a new one that this rule cannot see is a new way to red an exempt PR`,
);
// The step whose relocation #6434 IS. Pinned in the negative as well as the
// positive: base resolution reports into an output and the verdict is taken
// downstream, so re-introducing an `exit 1` here would restore the fast-path
// -only failure this card exists to remove. The count above would catch that
// as a 6th failable step; this says which one and why, so the next reader
// gets the reason and not just an arithmetic mismatch.
const diffbaseStep = jobText
.split(/\n(?= - name: )/)
.map((c) => c.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'))
.find((c) => /- name: Resolve the diff base/.test(c));
assert(
diffbaseStep !== undefined && !/\bexit 1\b/.test(diffbaseStep),
'consumer: `Resolve the diff base` must not fail on the spot -- it runs BEFORE the settling read, so its verdict would be taken on the fast path alone and would red a PR whose skip-changeset label was still in flight (#6434). It reports `base_error` and the adjudication step below decides.',
);
assert(
diffbaseStep !== undefined && /base_error=/.test(diffbaseStep),
'consumer: `Resolve the diff base` must report an unusable base as a `base_error` output -- dropping it silently would leave the downstream adjudication permanently un-triggerable, i.e. a gate that passes because it never runs (#4690)',
);
const unguarded = chunks.filter((c) => !/steps\.labels_settled\.outputs\.skip != 'true'/.test(c));
assert(
Expand All @@ -778,6 +828,21 @@ function selfTest() {
/::error::This PR adds no changeset[\s\S]{0,900}?\n\s+exit 1\n/.test(yaml),
'consumer: the "no changeset" verdict must still exit 1 -- #6378 removes a structural FALSE red, it does not relax the gate',
);
// The same constraint for #6434, and the reason it is written as a POSITIVE
// is the asymmetry that makes the negative one above worthless on its own:
// "the diff base no longer reds an exempt PR" is satisfied just as well by a
// step that stopped running, or by an adjudication whose `if:` can never be
// true. So the exempt direction is not asserted at all -- what is asserted
// is that the enforcing direction survived, in the one place it now lives.
const adjudication = jobText
.split(/\n(?= - name: )/)
.map((c) => c.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n'))
.find((c) => /- name: Require a usable diff base/.test(c));
assert(adjudication !== undefined, 'consumer: the unusable-diff-base verdict step must exist (#6434) -- without it `base_error` is written and never read, which is a gate deleted rather than relocated');
assert(
/steps\.diffbase\.outputs\.base_error != ''/.test(adjudication ?? '') && /\n\s+exit 1\n/.test(adjudication ?? ''),
"consumer: the unusable-diff-base verdict must fire on `base_error != ''` and exit 1 -- #6434 relocates a failure past the settling read, it does not forgive one. A PR the label reads did not exempt is still failed over a base that could not be resolved (#4690).",
);
assert(
!/continue-on-error/.test(yaml),
'consumer: no step in pr-automation.yml may carry `continue-on-error` -- that would turn this gate into a warning, which is the one outcome #6378 rules out',
Expand Down
Loading