Skip to content

Commit d1f8ce8

Browse files
ci(pr-automation): run the ADR-0087 and major-bump steps past a designed Check Changeset red (#20976)
Fixes #20784 Clause-②: no ## What changes `.github/workflows/pr-automation.yml`, job `changeset-check` (`Check Changeset`). The three steps after `Reject an empty-frontmatter changeset added by this PR` now take: ```yaml if: >- !cancelled() && steps.labels.outputs.skip != 'true' && steps.labels_settled.outputs.skip != 'true' && steps.diffbase.outputs.merge_base != '' # the major guard alone adds: && steps.allow_major.outputs.allow != 'true' ``` - `Require an ADR-0087 disposition on a declared-breaking changeset` (`check-adr-0087-registration.mjs`) - `Re-read this PR's allow-major label live` (id `allow_major`; it only feeds the guard below) - `Guard against accidental major bumps (launch window)` (`check-changeset-no-major.mjs`, the level axis) There is no `always()`, so a cancelled run still stops. The job stays red on the DELIBERATE CORRECTION class. It now also shows the ADR-0087 and level-axis verdicts on that same run. The route-0 text that `Require a changeset` prints said "the steps after it are skipped". This change makes that false, so the text now says which step is still skipped and that the two verdict steps after it run. The pin is the file's existing shape pin: `scripts/check-empty-changeset.mjs --self-test`, battery "The consumer: this gate's own CI step". It gains 11 cases, and the battery floor moves from 23 to 34. It asserts four things: - each of the three steps begins its `if:` with `!cancelled() &&` and has no `||`; - each of them requires `steps.diffbase.outputs.merge_base != ''`; - no other step of the job names a status function; - nothing in the workflow uses `always()`. No gate-script logic changed. Only the self-test block and its floor did. ## The three hypotheses, measured before building **H1: which steps follow the empty-changeset step.** There are exactly three, the ones listed above. All three should run past its red. None of them is a consequence of that red: two are independent verdicts on the same diff, and the allow-major read is an input to one of them. A second designed red hides the same three steps. On a correction-only PR (it adds no changeset of its own), route 0 makes `Require a changeset` fail by design. Both later gates diff with `--diff-filter=AMR`, so such a PR still gives them something to judge. The triage direction's literal form, `!cancelled() && LABEL_CONDITIONS`, runs the steps past both reds, and this PR keeps that reach. Fixture F2 below shows the verdicts that were being hidden. **H2: the outputs the later steps read.** No later step reads an output of the empty-changeset step, which has no `id`. When that step is red, every step before it succeeded, so `merge_base` is set, `labels.skip` is `false`, and `labels_settled.skip` is `false` or unwritten (never `'true'`). The later steps see correct inputs. The literal form has a wider reach than that, though. It would also run the three steps past the unusable-base red (`Require a usable diff base`), where `steps.diffbase.outputs.merge_base` is the empty string. I measured all three changeset scripts on this tree with an empty base. Each one treats it as "no base" and answers against `origin/main` with exit 0: ```text node scripts/check-adr-0087-registration.mjs --base "" -> exit 0 "this PR adds no declared-breaking changeset" node scripts/check-changeset-no-major.mjs --base "" -> exit 0 "Diffing HEAD from 3693a1b (merge base with origin/main)" node scripts/check-empty-changeset.mjs --base "" -> exit 0 (same fallback) ``` That verdict would be about a base this job had already refused to trust. So each step also requires `merge_base != ''`. This is the same step-level `!cancelled()` plus "the output that proves the input exists" pattern that `release.yml` and `cut-rc.yml` already use. This is not the dispatch's stop condition: `merge_base` is not an output of the empty-changeset step. It is the minimal guard the literal form needs. No other input needs a guard. The two scripts import only node builtins and repo-local modules, so a failed `pnpm install` cannot change their verdict. An unwritten label output is not `'true'`, which is the enforcing direction. **H3: where the shape pin lives.** It lives in `scripts/check-empty-changeset.mjs --self-test`, in the battery "The consumer: this gate's own CI step". That battery already pins this job's step conditions: the settling read, both label guards on every step that can fail, and the allow-major read. `check-workflow-status-functions` does not fit: its own scope note limits it to job-level `if:`. `check-changeset-no-major.mjs`'s wiring battery pins the guard step's `run:` and `env:`, and it passes unchanged. ## Evidence **Ablation.** Each leg ran once, on the committed tree, through `scripts/ablation-replace.mjs`. After every leg the tool proved the file was restored: its blob equals HEAD (`e0b36ccdfa02`) and `git diff HEAD` is empty. | Leg | Mutation | Self-test result | |---|---|---| | a | ADR-0087 step without `!cancelled()` | exit 1, 2 failures: the leading conjunct, and the closed set (found steps 13 and 14 instead of 12 to 14) | | b | major guard with `base_error == ''` in place of the `merge_base` read | exit 1, 1 failure | | c | allow-major read with `always()` | exit 1, 2 failures: the leading conjunct, and no `always()` | | d | `Require a changeset` also given `!cancelled()` | exit 1, 1 failure: the closed set (found steps 10 and 12 to 14) | | e | whole workflow as it was at the base commit, i.e. without this change (trap-guarded swap; blob proven equal to HEAD after restore) | exit 1, 7 failures: 3 leading conjunct, 3 `merge_base`, and the closed set (found none) | The first attempt at leg b did nothing. Its replacement text was a substring of its anchor, so the tool refused before running anything. The leg was re-run with a replacement that can be told apart. **Fixture run.** This used a shared clone in scratch whose commits were never pushed. Each script ran with `--base` set to this branch's head. | Scenario | empty-changeset | ADR-0087 | no-major | |---|---|---|---| | F1: the shape of PR #20780. It edits a pending note and adds its own `minor` changeset that carries a **BREAKING** banner and no disposition. | exit 1 (foreign-changeset refusal) | exit 1 (no disposition) | exit 0 | | F2: correction only. It edits a pending note from `minor` to `major` and adds nothing. The count step would report 0, so route 0 turns `Require a changeset` red. | exit 1 | exit 1 (turned breaking, no disposition) | exit 1 (majored in place) | Under the old conditions, none of the ADR-0087 or no-major verdicts in F1 or F2 would have run in CI. ## Gates at HEAD 4888e25 `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, with no paths, derived 52 commands from this diff. All 52 were run and every one exited 0. `dispatch-gates --ran` reports: "52 derived famil(ies) accounted for, 52 run, 0 NOT-MEASURED". Among them: ```text exit 0 node scripts/check-empty-changeset.mjs --self-test (170 assertions) exit 0 pnpm check:changeset-gate-self-tests (empty-changeset, adr-0087 and no-major self-tests; no-major 339 assertions) exit 0 pnpm check:workflow-status-functions exit 0 pnpm check:workflow-step-name-quoting exit 0 node scripts/check-self-test-workflow-commands.mjs (+ --self-test) exit 0 node scripts/check-step-collectors.mjs (+ --self-test) exit 0 pnpm check:nul-bytes exit 0 pnpm check:pm-dispatch-gates (1976 cases; 1243 s on this box) ``` **Narrowed lint.** eslint's population is `**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}`, and `.github/workflows/pr-automation.yml` falls outside it. eslint itself reports "File ignored because no matching configuration was supplied". The `--format json` output has 2 entries: `scripts/check-empty-changeset.mjs` with 0 errors and 0 warnings, and the ignored YAML. Nothing in the config enables type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot change the verdict on any file it does not touch. No changeset: this diff changes workflow wiring and a root `scripts/` self-test, and neither publishes anything. ## Acceptance notes - **Live proof on GitHub is NOT MEASURED in this PR.** A `pull_request` run takes its workflow from the PR's own merge ref. A correction-class PR will show both verdicts on one run only after this lands. PR #20780 would then need a new event such as a push or a `main` merge. A re-run is not enough: `rerun_failed_jobs` replays the original merge ref. - **This PR's own `Check Changeset` run carries `skip-changeset`.** That only exercises the exempt direction: the three steps must stay skipped when either label read exempts the PR. - **Boundary, not widened:** `Reject an empty-frontmatter changeset added by this PR` keeps the implicit `success()`. On a correction-only PR it is still skipped behind route 0, and route 0's own text says so. - **Dormant, not filed:** the three changeset scripts treat an explicit empty `--base ""` as absent and fall back to `origin/main` with exit 0. With the `merge_base` conjunct, CI never reaches that path. Carrier: none. - **Base behind `origin/main`:** this branch sits on `3693a1b50`. No upstream commit since then touches either changed file or the three changeset scripts, and `merge-tree` is clean. CI judges the merge ref. --- _Generated by [Claude Code](https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d67b942 commit d1f8ce8

2 files changed

Lines changed: 143 additions & 17 deletions

File tree

‎.github/workflows/pr-automation.yml‎

Lines changed: 71 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -739,8 +739,9 @@ jobs:
739739
This is said HERE rather than in that refusal on purpose. The step below
740740
prints the two-class remedy (COLLISION vs DELIBERATE CORRECTION), but on
741741
this class it never speaks: a PR that only corrects somebody else's note
742-
adds none of its own, so THIS step fails first and the steps after it
743-
are skipped.
742+
adds none of its own, so THIS step fails first and that step is skipped.
743+
The ADR-0087 disposition and major-bump steps after it still run and
744+
judge the note you changed -- read their verdicts on this run too.
744745
745746
A PR that corrects a pending note AND releases something of its own does
746747
both: route 1 for its own release, under a name that cannot collide
@@ -929,10 +930,49 @@ jobs:
929930
# `check-*.mjs` gates: its fixtures are the only place the red directions are
930931
# pinned, and each of them was verified to flip green when the corresponding
931932
# check is ablated. Real temp git repositories, well under a second.
933+
#
934+
# ── Past a designed red (#20784) ─────────────────────────────────────────
935+
#
936+
# This step and the two after it (the allow-major read and the major guard)
937+
# name `!cancelled()`, so they run whatever the steps above concluded. Two
938+
# steps above are RED BY DESIGN on the DELIBERATE CORRECTION class, and the
939+
# red is left standing on purpose: `Require a changeset` on a PR that only
940+
# corrects somebody else's pending note (route 0), and the empty-changeset
941+
# step on one that corrects a note AND adds its own (the foreign-changeset
942+
# refusal). Under GitHub's implicit `success()` either red skipped all three
943+
# verdicts below, on exactly the PRs most likely to carry a breaking
944+
# changeset, so neither the ADR-0087 disposition nor the level axis was ever
945+
# read on them in CI. Both gates judge `M` rows as well as `A`, so a
946+
# correction-only PR has something for them to judge. The job stays red on
947+
# that class; it now reports every verdict on the same run.
948+
#
949+
# ⛔ Not `always()`: a cancelled run must still stop.
950+
#
951+
# `steps.diffbase.outputs.merge_base != ''` is what `!cancelled()` costs,
952+
# and it is not optional. Under the implicit `success()` an unusable base
953+
# skipped these steps too, so a non-empty `$MERGE_BASE` came for free. Run
954+
# past a red, they would also run past `Require a usable diff base`, where
955+
# the base is the empty string, and all three changeset scripts read
956+
# `--base ""` as NO base and answer against `origin/main` at exit 0
957+
# (measured). That is the base-free reading #6129 and #7005 removed: a
958+
# verdict about a base this job refused to trust. So each of the three names
959+
# the input it consumes. Nothing else needs a guard: the two scripts import
960+
# only node builtins and repo-local modules, so a failed install cannot
961+
# change their verdict, and an unwritten label output is not 'true', which
962+
# is the enforcing direction every read in this job already takes (#4690).
963+
#
964+
# The set is CLOSED. Each step above reads an output that an earlier step
965+
# may not have written (the count, the settled label), so each keeps the
966+
# implicit `success()`: `Require a changeset` run past a failed count would
967+
# test an empty `$ADDED` and announce a green (see the note on `Require a
968+
# usable diff base`). check-empty-changeset.mjs's consumer battery pins
969+
# this shape: the three steps, their two conjuncts, and no `always()`.
932970
- name: Require an ADR-0087 disposition on a declared-breaking changeset
933971
if: >-
934-
steps.labels.outputs.skip != 'true'
972+
!cancelled()
973+
&& steps.labels.outputs.skip != 'true'
935974
&& steps.labels_settled.outputs.skip != 'true'
975+
&& steps.diffbase.outputs.merge_base != ''
936976
env:
937977
MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }}
938978
run: |
@@ -1005,10 +1045,16 @@ jobs:
10051045
id: allow_major
10061046
# Both skip-changeset reads, for the same reason the guard below names
10071047
# them: a PR the changeset gate exempts must not pay an API call for a
1008-
# guard that is not going to run.
1048+
# guard that is not going to run. The same argument gives it the guard's
1049+
# other two conjuncts: `!cancelled()`, because this read is the guard's
1050+
# input and the guard runs past a designed red (#20784, the note on the
1051+
# ADR-0087 step), and the `merge_base` read, because without a base the
1052+
# guard does not run and this call would buy nothing.
10091053
if: >-
1010-
steps.labels.outputs.skip != 'true'
1054+
!cancelled()
1055+
&& steps.labels.outputs.skip != 'true'
10111056
&& steps.labels_settled.outputs.skip != 'true'
1057+
&& steps.diffbase.outputs.merge_base != ''
10121058
env:
10131059
GH_TOKEN: ${{ github.token }}
10141060
PR_NUMBER: ${{ github.event.pull_request.number }}
@@ -1037,17 +1083,24 @@ jobs:
10371083
# version. During the launch window we ship breaking changes as `minor`.
10381084
# Add the `allow-major` PR label when a whole-stack major is intended.
10391085
#
1040-
# The first two clauses keep this step exempt exactly when the changeset
1041-
# check above is: before #5580 the `skip-changeset` label skipped the whole
1042-
# job, this step included, and a live-read label must not quietly re-arm
1043-
# it. Both reads are named for the same reason -- after #6378 the exemption
1044-
# can be established by either one, and a step that honoured only the fast
1045-
# path would re-arm itself on precisely the PRs the settling read rescued.
1086+
# `!cancelled()` and the `merge_base` clause are the pair the note on the
1087+
# ADR-0087 step argues (#20784): this verdict is read even when a step
1088+
# above is red by design, and never against a base this job could not
1089+
# resolve. ⛔ Not `always()`.
10461090
#
1047-
# The third clause is the LIVE `allow-major` read directly above (#5620),
1048-
# never the event payload. Its comment block carries the argument: what the
1049-
# payload read cost, where this diverges from #5580's `skip-changeset` read
1050-
# and why, the residual it leaves, and what re-arms the whole thing.
1091+
# The two skip-changeset clauses keep this step exempt exactly when the
1092+
# changeset check above is: before #5580 the `skip-changeset` label skipped
1093+
# the whole job, this step included, and a live-read label must not
1094+
# quietly re-arm it. Both reads are named for the same reason -- after
1095+
# #6378 the exemption can be established by either one, and a step that
1096+
# honoured only the fast path would re-arm itself on precisely the PRs the
1097+
# settling read rescued.
1098+
#
1099+
# The `allow_major` clause is the LIVE `allow-major` read directly above
1100+
# (#5620), never the event payload. Its comment block carries the
1101+
# argument: what the payload read cost, where this diverges from #5580's
1102+
# `skip-changeset` read and why, the residual it leaves, and what re-arms
1103+
# the whole thing.
10511104
#
10521105
# `--base` takes the same merge base the two gates above take, for the
10531106
# same #6129 reason one defect along (#7005). This script used to read
@@ -1067,8 +1120,10 @@ jobs:
10671120
# without the fork -- #6129's argument, which never depended on how big
10681121
# the stock was.
10691122
if: >-
1070-
steps.labels.outputs.skip != 'true'
1123+
!cancelled()
1124+
&& steps.labels.outputs.skip != 'true'
10711125
&& steps.labels_settled.outputs.skip != 'true'
1126+
&& steps.diffbase.outputs.merge_base != ''
10721127
&& steps.allow_major.outputs.allow != 'true'
10731128
env:
10741129
MERGE_BASE: ${{ steps.diffbase.outputs.merge_base }}

‎scripts/check-empty-changeset.mjs‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
690690
"A' (#17712): a changeset the PR did not add is neither modified nor deleted": 29,
691691
'D (#18160): the refusal names BOTH classes, body and annotation pinned equal': 12,
692692
'#4690, one step later: no merge base at all is a failure': 1,
693-
'The consumer: this gate\'s own CI step (#6129)': 23,
693+
'The consumer: this gate\'s own CI step (#6129)': 34,
694694
'The second consumer: where THIS SELF-TEST runs (#6509)': 12,
695695
'Parser unit rows': 8,
696696
'THE FIX (#7004): comments and quoted bump values': 12,
@@ -1745,6 +1745,77 @@ function selfTest() {
17451745
`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`,
17461746
);
17471747

1748+
// ── Past a designed red (#20784) ─────────────────────────────────────────
1749+
//
1750+
// Two steps of this job are red BY DESIGN on the DELIBERATE CORRECTION
1751+
// class, and the red is left standing: route 0 of `Require a changeset`,
1752+
// and this gate's own foreign-changeset refusal. The three steps after
1753+
// this gate -- the ADR-0087 disposition, the live allow-major read that
1754+
// feeds the major guard, and the major guard itself -- are independent
1755+
// verdicts on the same diff. Under GitHub's implicit `success()` a designed
1756+
// red above them skipped all three, so the PRs that correct a pending note
1757+
// (the ones most likely to carry a breaking changeset) went through CI with
1758+
// neither verdict read, and every assertion in this battery stayed green.
1759+
//
1760+
// What is pinned, read as one sentence: each of the three LEADS its `if:`
1761+
// with `!cancelled() &&` and carries no `||` (so no disjunct can route
1762+
// around it); each also requires `steps.diffbase.outputs.merge_base != ''`,
1763+
// because a step run past the unusable-base red would hand its script
1764+
// `--base ""`, which this script and its two siblings read as NO base and
1765+
// answer against `origin/main` at exit 0; the steps of this job that name a
1766+
// status function are exactly these three, because every step above them
1767+
// reads an output an earlier step may not have written; and nothing in the
1768+
// workflow uses `always()`, because a cancelled run must still stop.
1769+
const ifOf = (step) => {
1770+
const lines = step.split('\n');
1771+
const at = lines.findIndex((l) => /^ {8}if:/.test(l));
1772+
if (at === -1) return null;
1773+
const head = lines[at].replace(/^ {8}if:\s*/, '').trim();
1774+
if (!/^[>|][+-]?$/.test(head)) return head;
1775+
const body = [];
1776+
for (const l of lines.slice(at + 1)) {
1777+
if (l.trim() !== '' && !/^ {9,}/.test(l)) break;
1778+
body.push(l.trim());
1779+
}
1780+
return body.filter(Boolean).join(' ');
1781+
};
1782+
const pastRed = [
1783+
['the ADR-0087 disposition', (c) => /node scripts\/check-adr-0087-registration\.mjs --base/.test(c)],
1784+
['the live allow-major read', (c) => /grep -qxF 'allow-major'/.test(c)],
1785+
['the major guard', (c) => /node scripts\/check-changeset-no-major\.mjs --base/.test(c)],
1786+
].map(([label, isIt]) => {
1787+
const at = namedSteps.findIndex(isIt);
1788+
return { label, at, cond: at === -1 ? null : ifOf(namedSteps[at]) };
1789+
});
1790+
for (const { label, cond } of pastRed) {
1791+
assert(
1792+
cond !== null,
1793+
`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`,
1794+
);
1795+
assert(
1796+
/^!cancelled\(\)\s*&&/.test(cond ?? '') && !/\|\|/.test(cond ?? ''),
1797+
`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)}`,
1798+
);
1799+
assert(
1800+
/steps\.diffbase\.outputs\.merge_base != ''/.test(cond ?? ''),
1801+
`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)}`,
1802+
);
1803+
}
1804+
const NAMES_STATUS = /\b(?:always|cancelled|success|failure)\s*\(\s*\)/;
1805+
const namingStatus = namedSteps
1806+
.map((c, i) => ({ i, cond: ifOf(c) }))
1807+
.filter(({ cond }) => NAMES_STATUS.test(cond ?? ''))
1808+
.map(({ i }) => i);
1809+
const expectedStatus = pastRed.map(({ at }) => at).sort((a, b) => a - b);
1810+
assert(
1811+
expectedStatus.every((i) => i >= 0) && JSON.stringify(namingStatus) === JSON.stringify(expectedStatus),
1812+
`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`,
1813+
);
1814+
assert(
1815+
!/\balways\s*\(\s*\)/.test(yaml.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n')),
1816+
'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',
1817+
);
1818+
17481819
// The hard constraint of #6378, stated as structure: none of this may have
17491820
// made the gate softer. A PR with no changeset and no label still has to
17501821
// hit a real non-zero exit, and no step of this job may be excused from

0 commit comments

Comments
 (0)