ci(pr-automation): run the ADR-0087 and major-bump steps past a designed Check Changeset red

.github/workflows/pr-automation.yml

@@ -739,8 +739,9 @@ jobs:
                This is said HERE rather than in that refusal on purpose. The step below
                prints the two-class remedy (COLLISION vs DELIBERATE CORRECTION), but on
                this class it never speaks: a PR that only corrects somebody else's note
-               adds none of its own, so THIS step fails first and the steps after it
-               are skipped.
+               adds none of its own, so THIS step fails first and that step is skipped.
+               The ADR-0087 disposition and major-bump steps after it still run and
+               judge the note you changed -- read their verdicts on this run too.
 
                A PR that corrects a pending note AND releases something of its own does
                both: route 1 for its own release, under a name that cannot collide
@@ -929,10 +930,49 @@ jobs:
       # `check-*.mjs` gates: its fixtures are the only place the red directions are
       # pinned, and each of them was verified to flip green when the corresponding
       # check is ablated. Real temp git repositories, well under a second.
+      #
+      # ── Past a designed red (#20784) ─────────────────────────────────────────
+      #
+      # This step and the two after it (the allow-major read and the major guard)
+      # name `!cancelled()`, so they run whatever the steps above concluded. Two
+      # steps above are RED BY DESIGN on the DELIBERATE CORRECTION class, and the
+      # red is left standing on purpose: `Require a changeset` on a PR that only
+      # corrects somebody else's pending note (route 0), and the empty-changeset
+      # step on one that corrects a note AND adds its own (the foreign-changeset
+      # refusal). Under GitHub's implicit `success()` either red skipped all three
+      # verdicts below, on exactly the PRs most likely to carry a breaking
+      # changeset, so neither the ADR-0087 disposition nor the level axis was ever
+      # read on them in CI. Both gates judge `M` rows as well as `A`, so a
+      # correction-only PR has something for them to judge. The job stays red on
+      # that class; it now reports every verdict on the same run.
+      #
+      # ⛔ Not `always()`: a cancelled run must still stop.
+      #
+      # `steps.diffbase.outputs.merge_base != ''` is what `!cancelled()` costs,
+      # and it is not optional. Under the implicit `success()` an unusable base
+      # skipped these steps too, so a non-empty `$MERGE_BASE` came for free. Run
+      # past a red, they would also run past `Require a usable diff base`, where
+      # the base is the empty string, and all three changeset scripts read
+      # `--base ""` as NO base and answer against `origin/main` at exit 0
+      # (measured). That is the base-free reading #6129 and #7005 removed: a
+      # verdict about a base this job refused to trust. So each of the three names
+      # the input it consumes. Nothing else needs a guard: the two scripts import
+      # only node builtins and repo-local modules, so a failed install cannot
+      # change their verdict, and an unwritten label output is not 'true', which
+      # is the enforcing direction every read in this job already takes (#4690).
+      #
+      # The set is CLOSED. Each step above reads an output that an earlier step
+      # may not have written (the count, the settled label), so each keeps the
+      # implicit `success()`: `Require a changeset` run past a failed count would
+      # test an empty `$ADDED` and announce a green (see the note on `Require a
+      # usable diff base`). check-empty-changeset.mjs's consumer battery pins
+      # this shape: the three steps, their two conjuncts, and no `always()`.
       - name: Require an ADR-0087 disposition on a declared-breaking changeset
         if: >-
-          steps.labels.outputs.skip != 'true'
+          !cancelled()
+          && steps.labels.outputs.skip != 'true'
           && steps.labels_settled.outputs.skip != 'true'
+          && steps.diffbase.outputs.merge_base != ''
         env:
           MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }}
         run: |
@@ -1005,10 +1045,16 @@ jobs:
         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.
+        # guard that is not going to run. The same argument gives it the guard's
+        # other two conjuncts: `!cancelled()`, because this read is the guard's
+        # input and the guard runs past a designed red (#20784, the note on the
+        # ADR-0087 step), and the `merge_base` read, because without a base the
+        # guard does not run and this call would buy nothing.
         if: >-
-          steps.labels.outputs.skip != 'true'
+          !cancelled()
+          && steps.labels.outputs.skip != 'true'
           && steps.labels_settled.outputs.skip != 'true'
+          && steps.diffbase.outputs.merge_base != ''
         env:
           GH_TOKEN: ${{ github.token }}
           PR_NUMBER: ${{ github.event.pull_request.number }}
@@ -1037,17 +1083,24 @@ jobs:
         # version. During the launch window we ship breaking changes as `minor`.
         # Add the `allow-major` PR label when a whole-stack major is intended.
         #
-        # The first two clauses keep this step exempt exactly when the changeset
-        # check above is: before #5580 the `skip-changeset` label skipped the whole
-        # job, this step included, and a live-read label must not quietly re-arm
-        # it. Both reads are named for the same reason -- after #6378 the exemption
-        # 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.
+        # `!cancelled()` and the `merge_base` clause are the pair the note on the
+        # ADR-0087 step argues (#20784): this verdict is read even when a step
+        # above is red by design, and never against a base this job could not
+        # resolve. ⛔ Not `always()`.
         #
-        # 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.
+        # The two skip-changeset clauses keep this step exempt exactly when the
+        # changeset check above is: before #5580 the `skip-changeset` label skipped
+        # the whole job, this step included, and a live-read label must not
+        # quietly re-arm it. Both reads are named for the same reason -- after
+        # #6378 the exemption 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 `allow_major` 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.
         #
         # `--base` takes the same merge base the two gates above take, for the
         # same #6129 reason one defect along (#7005). This script used to read
@@ -1067,8 +1120,10 @@ jobs:
         # without the fork -- #6129's argument, which never depended on how big
         # the stock was.
         if: >-
-          steps.labels.outputs.skip != 'true'
+          !cancelled()
+          && steps.labels.outputs.skip != 'true'
           && steps.labels_settled.outputs.skip != 'true'
+          && steps.diffbase.outputs.merge_base != ''
           && steps.allow_major.outputs.allow != 'true'
         env:
           MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }}

scripts/check-empty-changeset.mjs

@@ -690,7 +690,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
   "A' (#17712): a changeset the PR did not add is neither modified nor deleted": 29,
   'D (#18160): the refusal names BOTH classes, body and annotation pinned equal': 12,
   '#4690, one step later: no merge base at all is a failure': 1,
-  'The consumer: this gate\'s own CI step (#6129)': 23,
+  'The consumer: this gate\'s own CI step (#6129)': 34,
   'The second consumer: where THIS SELF-TEST runs (#6509)': 12,
   'Parser unit rows': 8,
   'THE FIX (#7004): comments and quoted bump values': 12,
@@ -1745,6 +1745,77 @@ function selfTest() {
         `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`,
       );
 
+      // ── Past a designed red (#20784) ─────────────────────────────────────────
+      //
+      // Two steps of this job are red BY DESIGN on the DELIBERATE CORRECTION
+      // class, and the red is left standing: route 0 of `Require a changeset`,
+      // and this gate's own foreign-changeset refusal. The three steps after
+      // this gate -- the ADR-0087 disposition, the live allow-major read that
+      // feeds the major guard, and the major guard itself -- are independent
+      // verdicts on the same diff. Under GitHub's implicit `success()` a designed
+      // red above them skipped all three, so the PRs that correct a pending note
+      // (the ones most likely to carry a breaking changeset) went through CI with
+      // neither verdict read, and every assertion in this battery stayed green.
+      //
+      // What is pinned, read as one sentence: each of the three LEADS its `if:`
+      // with `!cancelled() &&` and carries no `||` (so no disjunct can route
+      // around it); each also requires `steps.diffbase.outputs.merge_base != ''`,
+      // because a step run past the unusable-base red would hand its script
+      // `--base ""`, which this script and its two siblings read as NO base and
+      // answer against `origin/main` at exit 0; the steps of this job that name a
+      // status function are exactly these three, because every step above them
+      // reads an output an earlier step may not have written; and nothing in the
+      // workflow uses `always()`, because a cancelled run must still stop.
+      const ifOf = (step) => {
+        const lines = step.split('\n');
+        const at = lines.findIndex((l) => /^ {8}if:/.test(l));
+        if (at === -1) return null;
+        const head = lines[at].replace(/^ {8}if:\s*/, '').trim();
+        if (!/^[>|][+-]?$/.test(head)) return head;
+        const body = [];
+        for (const l of lines.slice(at + 1)) {
+          if (l.trim() !== '' && !/^ {9,}/.test(l)) break;
+          body.push(l.trim());
+        }
+        return body.filter(Boolean).join(' ');
+      };
+      const pastRed = [
+        ['the ADR-0087 disposition', (c) => /node scripts\/check-adr-0087-registration\.mjs --base/.test(c)],
+        ['the live allow-major read', (c) => /grep -qxF 'allow-major'/.test(c)],
+        ['the major guard', (c) => /node scripts\/check-changeset-no-major\.mjs --base/.test(c)],
+      ].map(([label, isIt]) => {
+        const at = namedSteps.findIndex(isIt);
+        return { label, at, cond: at === -1 ? null : ifOf(namedSteps[at]) };
+      });
+      for (const { label, cond } of pastRed) {
+        assert(
+          cond !== null,
+          `consumer: ${label} step could not be sliced out of the Check Changeset job with an \`if:\` -- the two assertions after this one would judge nothing`,
+        );
+        assert(
+          /^!cancelled\(\)\s*&&/.test(cond ?? '') && !/\|\|/.test(cond ?? ''),
+          `consumer: ${label} step must lead its \`if:\` with \`!cancelled() &&\` and carry no \`||\` -- under the implicit \`success()\` a designed red above it (route 0, or the foreign-changeset refusal) skips it, and the PRs that correct a pending note get no verdict from it in CI. Got ${JSON.stringify(cond)}`,
+        );
+        assert(
+          /steps\.diffbase\.outputs\.merge_base != ''/.test(cond ?? ''),
+          `consumer: ${label} step must require \`steps.diffbase.outputs.merge_base != ''\` -- run past a red it also runs past the unusable-base red, and a changeset script handed \`--base ""\` reads NO base and answers against origin/main at exit 0. Got ${JSON.stringify(cond)}`,
+        );
+      }
+      const NAMES_STATUS = /\b(?:always|cancelled|success|failure)\s*\(\s*\)/;
+      const namingStatus = namedSteps
+        .map((c, i) => ({ i, cond: ifOf(c) }))
+        .filter(({ cond }) => NAMES_STATUS.test(cond ?? ''))
+        .map(({ i }) => i);
+      const expectedStatus = pastRed.map(({ at }) => at).sort((a, b) => a - b);
+      assert(
+        expectedStatus.every((i) => i >= 0) && JSON.stringify(namingStatus) === JSON.stringify(expectedStatus),
+        `consumer: in the Check Changeset job exactly the three steps after the empty-changeset gate may name a status function (steps ${JSON.stringify(expectedStatus)}), found ${JSON.stringify(namingStatus)} -- every step above them reads an output an earlier step may not have written, and run past a red one of them announces a verdict on an empty input`,
+      );
+      assert(
+        !/\balways\s*\(\s*\)/.test(yaml.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n')),
+        'consumer: nothing in pr-automation.yml may use `always()` -- `!cancelled()` runs a step past a red, `always()` also runs it on a cancelled run, which must still stop',
+      );
+
       // 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