Skip to content

Commit 4dc5c8f

Browse files
committed
fix(pm): read YAML block-scalar run: bodies in dispatch-gates discovery (#8410)
`extractCheckInvocations` read only the text on the `run:` line itself, so a step written as a block scalar (`run: |`) contributed the string "|" and none of its commands. Six gate families are invoked exclusively that way and were absent from the derivation entirely -- not matched, and not in the "undetermined" bucket either, since a family that is never discovered has no entry to fall into it. That is the one output shape the script's contract forbids, and it cost PR #8399 a CI round: its declared-breaking changeset derived check-changeset-no-major (a one-line `run:`) but not check-adr-0087-registration (a block-scalar body), the gate that reddened. The fix is in discovery only. check-adr-0087-registration.mjs already names `.changeset` in its own source, so the ordinary watch-hint match fires as soon as the family is discovered; nothing downstream changed, and no list was added. Whole-line comments are stripped from bodies so prose about a gate cannot be mistaken for an invocation of it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN
1 parent f58b1a8 commit 4dc5c8f

1 file changed

Lines changed: 148 additions & 3 deletions

File tree

scripts/pm/dispatch-gates.mjs

Lines changed: 148 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,91 @@ const ROOT = new URL('../..', import.meta.url).pathname;
6565
// ---------------------------------------------------------------------------
6666

6767
/**
68-
* Pull every `check:*` invocation out of a workflow file's `run:` lines,
68+
* A `run:` value that is a YAML block-scalar HEADER (`|`, `>`, with any
69+
* chomping/indentation indicator) carries no command of its own: the commands
70+
* are the lines indented beneath the key.
71+
*/
72+
const BLOCK_SCALAR_HEADER = /^[|>][+-]?\d*$/;
73+
74+
/**
75+
* The command text of every `run:` step in a workflow, one string per step,
76+
* with whole-line `#` comments removed.
77+
*
78+
* ## Why the body of a block scalar has to be read
79+
*
80+
* The obvious spelling — one regex for `run:` and take the rest of the line —
81+
* reads only the steps whose command fits on the `run:` line itself. A step
82+
* written as a block scalar puts `|` there and its commands on the FOLLOWING
83+
* lines, so that spelling collected the string "|" and never saw the commands.
84+
* Measured on this tree at the time of writing: six gate families were invoked
85+
* exclusively from block-scalar bodies (`check-adr-0087-registration`,
86+
* `check-empty-changeset`, `check-shard-attestation`, `check-osv-exemptions`,
87+
* `check-test-completeness`, `check-cross-package-test-inputs`) and were
88+
* therefore absent from the derivation ENTIRELY — not matched, and not in the
89+
* "undetermined" bucket either, because a family that is never discovered has
90+
* no entry to fall into it. That is the one output shape this script's contract
91+
* forbids (a gate the derivation cannot mention at all), and it cost PR #8399 a
92+
* CI round: its declared-breaking changeset derived `check-changeset-no-major`
93+
* (a one-line `run:`, so visible) but not `check-adr-0087-registration` (a
94+
* block-scalar body, so invisible), which is the gate that actually reddened.
95+
*
96+
* Nothing downstream needed changing: `check-adr-0087-registration.mjs` names
97+
* `.changeset` in its own source, so the ordinary watch-hint match fires as
98+
* soon as the family is discovered at all. The bug was never in the matching.
99+
*
100+
* ## Why comments are stripped, and why only whole-line ones
101+
*
102+
* Reading a block body means reading the shell comments inside it, and this
103+
* tree's workflow bodies discuss gates by name at length (ci.yml's shard job
104+
* spells `check-shard-attestation.mjs` in a comment explaining that gate's own
105+
* classifier). "Mentions a gate" is not "runs a gate", and a family discovered
106+
* from prose would be a fabricated lead — the same failure the "22 leads is the
107+
* same as none" note below rejects for a wider heuristic. Only whole-line
108+
* comments are dropped: a trailing `# note` after a real command sits on a line
109+
* whose command still has to be read, and stripping from the first `#` anywhere
110+
* would corrupt commands that legitimately contain one inside a quoted string.
111+
*
112+
* Measured both ways on this tree: stripping changes no family's discovery
113+
* today (every gate named in a body comment is also really invoked somewhere).
114+
* It is here so that stops being luck.
115+
*/
116+
export function runCommandTexts(workflowText) {
117+
const lines = workflowText.split('\n');
118+
const out = [];
119+
for (let i = 0; i < lines.length; i++) {
120+
const m = /^([ \t]*)run:[ \t]*(.*)$/.exec(lines[i]);
121+
if (!m) continue;
122+
const [, indent, inline] = m;
123+
if (!BLOCK_SCALAR_HEADER.test(inline.trim())) {
124+
out.push(inline.trim());
125+
continue;
126+
}
127+
// Block scalar: the body is every following line indented deeper than the
128+
// key (blank lines belong to it too). Advancing `i` past the body is what
129+
// keeps a `run:` MENTIONED inside a body from being parsed as a new key.
130+
const body = [];
131+
let j = i + 1;
132+
for (; j < lines.length; j++) {
133+
if (lines[j].trim() === '') {
134+
body.push('');
135+
continue;
136+
}
137+
if (/^[ \t]*/.exec(lines[j])[0].length <= indent.length) break;
138+
body.push(lines[j]);
139+
}
140+
out.push(body.filter((l) => !/^[ \t]*#/.test(l)).join('\n'));
141+
i = j - 1;
142+
}
143+
return out;
144+
}
145+
146+
/**
147+
* Pull every `check:*` invocation out of a workflow file's `run:` steps,
69148
* with the pnpm --filter package (if any) and the workflow's file name.
70149
*/
71150
export function extractCheckInvocations(workflowText, workflowFile) {
72151
const out = [];
73-
const runRe = /^\s*run:\s*(.+)$/gm;
74-
for (const [, cmd] of workflowText.matchAll(runRe)) {
152+
for (const cmd of runCommandTexts(workflowText)) {
75153
for (const m of cmd.matchAll(/pnpm\s+(?:--filter\s+(\S+)\s+)?(?:run\s+)?(check:[\w:-]+)/g)) {
76154
out.push({ check: m[2], filter: m[1] ?? null, workflow: workflowFile });
77155
}
@@ -475,6 +553,52 @@ function selfTest() {
475553
t('extracts direct node scripts/check-*.mjs', invs.some((i) => i.check === 'scripts/check-nul-bytes.mjs' && i.direct));
476554
t('ignores non-check runs', !invs.some((i) => String(i.check).includes('build')));
477555

556+
// Block-scalar bodies (#8410). A step written `run: |` keeps its commands on
557+
// the following lines; reading only the `run:` line collected "|" and missed
558+
// every gate invoked this way. Both scalar styles and both invocation shapes
559+
// are pinned, plus the two directions in which the body must END.
560+
const blockWf = [
561+
'jobs:',
562+
' changeset-check:',
563+
' steps:',
564+
' - name: Literal block, two commands',
565+
' env:',
566+
' MERGE_BASE: abc',
567+
' run: |',
568+
' node scripts/check-adr-0087-registration.mjs --self-test',
569+
' node scripts/check-adr-0087-registration.mjs --base "$MERGE_BASE"',
570+
'',
571+
' # A YAML comment BETWEEN steps naming `pnpm check:invented-by-prose`.',
572+
' - name: Folded block with a pnpm check',
573+
' run: >-',
574+
' pnpm --filter @objectstack/spec check:folded-surface',
575+
' - name: Body carrying a shell comment',
576+
' run: |',
577+
' # first run node scripts/check-mentioned-only.mjs, they said',
578+
' pnpm check:really-invoked',
579+
' - name: Back to a one-liner',
580+
' run: node scripts/check-nul-bytes.mjs',
581+
].join('\n');
582+
const blockInvs = extractCheckInvocations(blockWf, 'pr-automation.yml');
583+
const blockNames = blockInvs.map((i) => i.check);
584+
t('extracts a direct script from a literal block body', blockNames.includes('scripts/check-adr-0087-registration.mjs'));
585+
t('extracts a pnpm check from a folded block body, with its filter', blockInvs.some((i) => i.check === 'check:folded-surface' && i.filter === '@objectstack/spec'));
586+
t('a dedented step ends the block body (the one-liner after it still parses)', blockNames.includes('scripts/check-nul-bytes.mjs'));
587+
t('a blank line does NOT end the block body', blockNames.includes('check:folded-surface'));
588+
// The over-match guard: discovery must report what a step RUNS, never what a
589+
// comment mentions. Measured on this tree — four families (check:adr-links,
590+
// check:empty-changeset, check:platform-checklist, check:skill-frame-freshness)
591+
// appear in workflow prose only, and a naive any-token scan invents all four.
592+
t('a gate named only in a YAML comment between steps is not discovered', !blockNames.includes('check:invented-by-prose'));
593+
t('a gate named only in a shell comment inside a body is not discovered', !blockNames.includes('scripts/check-mentioned-only.mjs'));
594+
t('a real command in the same body as a comment is still discovered', blockNames.includes('check:really-invoked'));
595+
596+
// `runCommandTexts` on its own: one entry per step, in file order.
597+
const texts = runCommandTexts(blockWf);
598+
t('one command text per run step', texts.length === 4);
599+
t('a block body keeps its lines joined', texts[0].split('\n').filter((l) => l.trim()).length === 2);
600+
t('a one-line run yields its command verbatim', texts[3] === 'node scripts/check-nul-bytes.mjs');
601+
478602
// #7440: the printed line must be runnable as-is. The three shapes come from
479603
// the same three fixtures above, so the sample workflow and the print site
480604
// cannot drift apart.
@@ -558,6 +682,27 @@ function selfTest() {
558682
t('the i18n section names check:i18n exactly, runnably', i18nHit.some((l) => l.includes('- pnpm check:i18n —')));
559683
t('a path outside every owning package emits no i18n section', !changeKindLines(['packages/objectql/src/engine.ts'], resolved).some((l) => l.includes('check:i18n')));
560684

685+
// The measured incident (#8410 / PR #8399), pinned against the REAL workflow
686+
// rather than a fixture: a fixture proves the parser, only the live file
687+
// proves that THIS repo's changeset gate is reachable. `Check Changeset`
688+
// invokes check-adr-0087-registration.mjs from a block-scalar body, and that
689+
// is the gate PR #8399's declared-breaking changeset went red on after a
690+
// fully green local loop. If the step is ever rewritten as a one-liner this
691+
// case still passes (it asserts discovery, not the YAML style); if the gate
692+
// moves out of pr-automation.yml, re-point the case at its new home rather
693+
// than deleting it.
694+
const liveWf = readFileSync(join(ROOT, '.github/workflows/pr-automation.yml'), 'utf8');
695+
const liveInvs = extractCheckInvocations(liveWf, 'pr-automation.yml').map((i) => i.check);
696+
t('the live Check Changeset job discovers its ADR-0087 gate', liveInvs.includes('scripts/check-adr-0087-registration.mjs'));
697+
t('the live Check Changeset job discovers its empty-changeset gate', liveInvs.includes('scripts/check-empty-changeset.mjs'));
698+
t('the live one-line gate in that file still discovers', liveInvs.includes('scripts/check-changeset-no-major.mjs'));
699+
// The end-to-end direction: a `.changeset/` path must now REACH the ADR-0087
700+
// gate through the ordinary watch-hint match. That gate names `.changeset` in
701+
// its own source, so this asserts the whole chain (discover -> resolve ->
702+
// hint -> cover) rather than the parser alone.
703+
const adrHints = extractWatchHints(readFileSync(join(ROOT, 'scripts/check-adr-0087-registration.mjs'), 'utf8'));
704+
t('a .changeset path is covered by the ADR-0087 gate own hints', adrHints.some((h) => hintCovers(h, '.changeset/some-breaking-change.md')));
705+
561706
// The table's own rot detector: a name no live run discovers must say so,
562707
// never disappear quietly.
563708
const stale = changeKindLines(['a.test.ts'], () => null);

0 commit comments

Comments
 (0)