Skip to content

Commit 6ec55ef

Browse files
fix(ci): run the changeset family's --self-test in lint.yml, out of reach of skip-changeset (#6509) (#6917)
* fix(ci): run the changeset family's --self-test in lint.yml, out of reach of skip-changeset (#6509) `check-empty-changeset.mjs` and `check-adr-0087-registration.mjs` were called from exactly one place in the repository: pr-automation.yml's `changeset-check` job, which a PR carrying `skip-changeset` is exempted from wholesale. A PR that edits a CI-internal script is the textbook case for that label, so a PR editing these two checkers was routinely the PR that skipped their own fixtures. The self-test halves are pure static checks with no merge-base dependency, so they move to lint.yml's ESLint job as one unconditional step. The real scans stay in `changeset-check`, which is where $MERGE_BASE exists — running them in a job with no branch point would be #6129 in the false-RED direction. The `chunks.length === 5` job invariant is untouched. `check-empty-changeset.mjs`'s consumer block gains 11 assertions pinning the new wiring: the step exists exactly once, carries no `if:`, lint.yml reads no label and carries no paths filter, the gates are invoked only through the self-test-only pnpm script, and that script covers both checkers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn * TEMP ABLATION (reverted in the next commit) — reverse-verification for #6509 Weakens the live allow-major matcher in pr-automation.yml from `grep -qxF` to `grep -qF`, i.e. reintroduces exactly the #5620-class defect that PR #6876's consumer assertions were added to catch — and which never executed once on #6876's own CI, because that PR carried skip-changeset. This PR carries skip-changeset too. Predicted direction: `Check Changeset` skips its self-test steps again, and the NEW lint.yml step goes RED. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn * Revert the temporary ablation — restore the whole-line allow-major matcher (#6509) Restores `grep -qxF 'allow-major'` in pr-automation.yml, undoing 56f7018. That commit existed only to prove on CI that the new lint.yml step goes RED on a PR carrying skip-changeset; run 31294323300 is the evidence and this returns the branch to the intended state. pr-automation.yml is now byte-identical to origin/main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ab54608 commit 6ec55ef

3 files changed

Lines changed: 167 additions & 0 deletions

File tree

.github/workflows/lint.yml

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,48 @@ jobs:
345345
- name: objectui pin-changeset digest guard
346346
run: pnpm check:objectui-changeset
347347

348+
# Changeset-family gate self-tests (#6509). The SELF-TEST halves only —
349+
# the real scans stay in pr-automation.yml's `changeset-check`, and the
350+
# split is the whole point of this step.
351+
#
352+
# Why they moved here. `check-empty-changeset.mjs` and
353+
# `check-adr-0087-registration.mjs` were called from exactly one place in
354+
# the repository: the `changeset-check` job, which a PR carrying
355+
# `skip-changeset` is exempted from WHOLESALE (job-level `if:` plus the two
356+
# per-step label reads). And "this PR edits a CI-internal script" is the
357+
# textbook `skip-changeset` case — such a PR releases nothing, so by the
358+
# workflow's own prescription it takes the label. The consequence is the
359+
# exact inversion of #4690: a PR that edits these two checkers is the PR
360+
# most likely to skip their own fixtures.
361+
#
362+
# Not hypothetical, and not this card's own reasoning either. PR #6876
363+
# (#5620, merged) added FIVE consumer assertions to
364+
# `check-empty-changeset.mjs` that never executed once on its own CI: it
365+
# carried `skip-changeset`, so on run 31289894461 the step
366+
# `Reject an empty-frontmatter changeset added by this PR` — the only thing
367+
# that runs `--self-test` — reports `conclusion: skipped`. Every assertion
368+
# in that PR was verified locally and by nothing else.
369+
#
370+
# This step is deliberately UNCONDITIONAL. No `if:`, no label read, no
371+
# paths filter: an exemption is precisely what the two self-tests must not
372+
# have, or the gap simply moves. That property is not left to prose —
373+
# `check-empty-changeset.mjs`'s own consumer block asserts this step's
374+
# shape (present, unguarded, self-test halves only), so removing or
375+
# conditioning it reds the very check it was removing.
376+
#
377+
# Why the SELF-TEST halves only, and not `pnpm check:empty-changeset`:
378+
# that script chains the real scan, whose verdict is a function of the
379+
# PR's DIFF and therefore needs `$MERGE_BASE`. This job has no branch
380+
# point, so a real scan here would fall back to reading stock — which is
381+
# #6129, "judge the author for what main gained while their PR was open",
382+
# in the direction of a false RED. The self-test halves have no such
383+
# dependency: measured, both pass with REPO_ROOT pointing at a directory
384+
# that is not a git repository at all. They build their own throwaway
385+
# repos in $TMPDIR and read two files (this workflow and
386+
# pr-automation.yml). ~0.8s + ~5.0s.
387+
- name: Changeset-family gate self-tests
388+
run: pnpm check:changeset-gate-self-tests
389+
348390
# Release-notes drift guard: the platform is one version-locked train, so
349391
# every released @objectstack/spec major must have a curated, navigable
350392
# release page at content/docs/releases/v<major>.mdx. Catches the gap that

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
"check:prerelease-pins": "node scripts/check-prerelease-pin-watch.mjs --self-test && node scripts/check-prerelease-pin-watch.mjs",
6868
"check:empty-changeset": "node scripts/check-empty-changeset.mjs --self-test && node scripts/check-empty-changeset.mjs",
6969
"check:adr-0087-registration": "node scripts/check-adr-0087-registration.mjs --self-test && node scripts/check-adr-0087-registration.mjs",
70+
"check:changeset-gate-self-tests": "node scripts/check-empty-changeset.mjs --self-test && node scripts/check-adr-0087-registration.mjs --self-test",
7071
"check:override-consistency": "node scripts/check-override-consistency.mjs --self-test && node scripts/check-override-consistency.mjs",
7172
"check:release-notes": "node scripts/check-release-notes.mjs",
7273
"check:release-body": "node scripts/release-github-releases.mjs --self-test",

scripts/check-empty-changeset.mjs

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,130 @@ function selfTest() {
905905
);
906906
}
907907

908+
// ── The second consumer: where THIS SELF-TEST runs (#6509) ───────────────
909+
//
910+
// Everything above pins the job that runs the REAL scan. This block pins the
911+
// job that runs the self-test, and it exists because for a long time they
912+
// were the same job -- which made the assertions above skippable by a label.
913+
//
914+
// The defect, stated once. `changeset-check` is exempt WHOLESALE when a PR
915+
// carries `skip-changeset`, and a PR that edits a CI-internal script is the
916+
// textbook case for that label (it releases nothing; the workflow itself
917+
// prescribes the label for exactly that). So a PR editing this file was
918+
// routinely the PR that skipped this file's own fixtures. Measured specimen,
919+
// not a worry: PR #6876 (#5620, merged) added the five `allow-major`
920+
// assertions above and NONE of them executed on its own CI -- it carried the
921+
// label, and on run 31289894461 the step that runs `--self-test` reports
922+
// `conclusion: skipped`. Both directions of that gap are real, and they are
923+
// not symmetric: a BROKEN assertion is merely deferred onto the next
924+
// unlabelled PR (an unrelated author eats the red), while a DELETED one is
925+
// silent forever, because nothing afterwards remembers it existed.
926+
//
927+
// The fix is wiring, so the fixture for it has to read the wiring. Without
928+
// this block `lint.yml`'s step could be deleted tomorrow with every
929+
// assertion above still green -- the same "phantom check" shape (#4690) this
930+
// whole family is written against.
931+
//
932+
// What is pinned is the property that closes the gap, not the step's prose:
933+
// the self-test halves run in a job NO PR-level exemption reaches, and the
934+
// merge-base-dependent halves stay out of it.
935+
//
936+
// RESIDUAL, recorded rather than implied. This assertion is run BY the step
937+
// it pins, so a PR that deletes that step AND carries `skip-changeset` is
938+
// still not caught -- both places that would have run it are gone in the
939+
// same diff. That is a strictly smaller hole than the one it replaces (which
940+
// swallowed EVERY labelled PR, including one that merely edits an
941+
// assertion), it is a deletion visible in a `.github/**` diff rather than a
942+
// silent no-op, and closing it entirely would need a gate outside this
943+
// family asserting this family's wiring, which is a coupling with its own
944+
// cost. Stated so the next reader inherits the fact and not a false sense of
945+
// closure.
946+
{
947+
const lintPath = join(REPO_ROOT, '.github/workflows/lint.yml');
948+
const lintPresent = existsSync(lintPath);
949+
assert(lintPresent, 'consumer: .github/workflows/lint.yml must exist -- it is where this self-test runs unconditionally (#6509)');
950+
const lintYaml = lintPresent ? readFileSync(lintPath, 'utf8') : '';
951+
const uncommented = (text) => text.split('\n').filter((l) => !/^\s*#/.test(l)).join('\n');
952+
953+
// Scoped to the `lint` job: `typecheck` is a separate required job with a
954+
// separate purpose, and a step landing there instead would be a different
955+
// fact than the one asserted here.
956+
const lintJobStart = lintYaml.indexOf('\n lint:');
957+
const lintJobEnd = lintYaml.indexOf('\n typecheck:');
958+
const lintJob = lintJobStart === -1 ? '' : lintYaml.slice(lintJobStart, lintJobEnd === -1 ? undefined : lintJobEnd);
959+
const lintSteps = lintJob.split(/\n(?= - name: )/).map(uncommented);
960+
const wiredSteps = lintSteps.filter((s) => /run: pnpm check:changeset-gate-self-tests\b/.test(s));
961+
assert(
962+
wiredSteps.length === 1,
963+
`consumer: the ESLint job of lint.yml must run \`pnpm check:changeset-gate-self-tests\` exactly once (found ${wiredSteps.length}) -- that step is the only place these two checkers' fixtures are executed on a PR carrying \`skip-changeset\` (#6509)`,
964+
);
965+
// The load-bearing half. A conditioned step is the defect again with one
966+
// more hop: whatever the condition reads, it is a way for a PR to arrange
967+
// that this self-test does not run on it.
968+
assert(
969+
wiredSteps.every((s) => !/^\s*if:/m.test(s)),
970+
'consumer: the changeset-family self-test step in lint.yml must carry NO `if:` -- an exemptable self-test is #6509 itself, and the exemption it must not have is the one that hid PR #6876\'s five assertions',
971+
);
972+
// Same property, one level up: a label read anywhere in this workflow
973+
// would mean some step of it can be waived by the author of the PR under
974+
// test.
975+
assert(
976+
!/skip-changeset/.test(uncommented(lintYaml)),
977+
'consumer: lint.yml must not read the `skip-changeset` label anywhere -- the whole point of running the self-tests here is that this workflow has no PR-level exemption',
978+
);
979+
// And one level up again: "runs on every PR" is what "unconditional"
980+
// means in practice. A `paths:` filter is a condition written in the
981+
// trigger instead of in an `if:`, and it would silently restore the gap
982+
// for every PR whose file list misses the glob.
983+
const onBlock = uncommented((lintYaml.match(/\non:\n([\s\S]*?)\n(?=[A-Za-z_])/) ?? ['', ''])[1]);
984+
assert(
985+
/^\s+pull_request:/m.test(onBlock),
986+
'consumer: lint.yml must keep its `pull_request:` trigger -- a self-test that does not run on pull requests is not wired at all (#6509)',
987+
);
988+
assert(
989+
!/\bpaths(-ignore)?\s*:/.test(onBlock),
990+
'consumer: lint.yml must carry no `paths:`/`paths-ignore:` filter -- a path-filtered trigger is an exemption written one level up, and this self-test may not have one (#6509)',
991+
);
992+
993+
// The other half of the split: the merge-base-dependent scans stay out of
994+
// this job. `check:empty-changeset` / `check:adr-0087-registration` chain
995+
// the REAL scan, whose verdict is a function of the PR's diff; this job
996+
// has no branch point, so running one here reads stock and reports main's
997+
// drift against the author -- #6129 in the false-RED direction.
998+
const strayScans = uncommented(lintYaml)
999+
.split('\n')
1000+
.filter((l) => /check-empty-changeset\.mjs|check-adr-0087-registration\.mjs|pnpm check:empty-changeset\b|pnpm check:adr-0087-registration\b/.test(l));
1001+
assert(
1002+
strayScans.length === 0,
1003+
`consumer: lint.yml must invoke these gates ONLY through \`pnpm check:changeset-gate-self-tests\` (found ${strayScans.length} direct invocation(s)) -- the real scans need $MERGE_BASE and this job has no branch point, which is #6129 in the false-RED direction`,
1004+
);
1005+
1006+
// What that pnpm script actually is. The step above is a name; this is the
1007+
// thing the name resolves to, and it is where "self-test halves only" and
1008+
// "BOTH of them" are actually true or false.
1009+
const pkgPath = join(REPO_ROOT, 'package.json');
1010+
const pkgPresent = existsSync(pkgPath);
1011+
assert(pkgPresent, 'consumer: the repository root package.json must exist -- it carries the script lint.yml runs');
1012+
let wiring = '';
1013+
try {
1014+
wiring = JSON.parse(pkgPresent ? readFileSync(pkgPath, 'utf8') : '{}').scripts?.['check:changeset-gate-self-tests'] ?? '';
1015+
} catch {
1016+
wiring = '';
1017+
}
1018+
assert(
1019+
/check-empty-changeset\.mjs --self-test/.test(wiring),
1020+
'consumer: `check:changeset-gate-self-tests` must run `check-empty-changeset.mjs --self-test` -- the step in lint.yml is only as real as the script it resolves to',
1021+
);
1022+
assert(
1023+
/check-adr-0087-registration\.mjs --self-test/.test(wiring),
1024+
'consumer: `check:changeset-gate-self-tests` must run `check-adr-0087-registration.mjs --self-test` too -- both checkers live in the same exempted job and both were unwired by it (#6509). `check-changeset-no-major.mjs` is deliberately absent: it has no `--self-test` to run.',
1025+
);
1026+
assert(
1027+
!/check-(?:empty-changeset|adr-0087-registration)\.mjs(?! --self-test)/.test(wiring),
1028+
'consumer: every invocation in `check:changeset-gate-self-tests` must carry `--self-test` -- chaining a real scan into the lint job is the #6129 direction this split exists to avoid',
1029+
);
1030+
}
1031+
9081032
// ── Parser unit rows ─────────────────────────────────────────────────────
9091033
assert(isEmptyDeclaration('---\n---\n\nbody\n'), 'parser: the canonical empty shape is empty');
9101034
assert(isEmptyDeclaration('\n---\n\n---\n\nbody\n'), 'parser: blank lines around/inside the fence stay empty');

0 commit comments

Comments
 (0)