Skip to content

Commit 6c5772f

Browse files
committed
fix(pm): check-expected-skips derives schedule/dispatch-only jobs as expected skips under their raw name
WIP: the derivation and its wiring into the judge and --roster; self-test cases follow. Claude-Session: https://claude.ai/code/session_01KTZmMfzVzjNvyaLyQ8mHvg Co-authored-by: Claude <noreply@anthropic.com>
1 parent 73155fe commit 6c5772f

1 file changed

Lines changed: 208 additions & 9 deletions

File tree

‎scripts/pm/check-expected-skips.mjs‎

Lines changed: 208 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
* declares — so a renamed, deleted or re-gated job reddens the roster instead
6060
* of letting it rot into memory. A NEW gated job is caught from the other
6161
* side: its first skip is a name outside the roster, exit 4, until someone
62-
* declares it with its mechanism.
62+
* declares it — or `deriveNonPrEventSkips` below admits its one exact shape.
6363
*
6464
* ## What the API says about a skip, and what it does not
6565
*
@@ -132,7 +132,7 @@
132132

133133
import process from 'node:process';
134134
import { spawnSync } from 'node:child_process';
135-
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
135+
import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
136136
import { tmpdir } from 'node:os';
137137
import { join } from 'node:path';
138138
import { fileURLToPath } from 'node:url';
@@ -229,6 +229,11 @@ const CORE_REASON =
229229
* ⛔ Adding a row is a declaration that the skip is BY DESIGN, and the row must
230230
* name the mechanism — a row added to silence an exit 4 without one is the
231231
* finding written somewhere quieter.
232+
*
233+
* These are the LISTED rows. The judge reads them beside the DERIVED ones —
234+
* `deriveNonPrEventSkips` below, one exact `if:` shape read off the tree — and
235+
* ⛔ a job that shape admits is never listed here: the derivation refuses a
236+
* name both halves carry.
232237
*/
233238
export const EXPECTED_SKIPS = Object.freeze([
234239
{
@@ -402,6 +407,189 @@ export function workflowReader(root = ROOT) {
402407
};
403408
}
404409

410+
/** Every workflow file under a root, sorted; empty when the directory is absent. */
411+
export function listWorkflowFiles(root = ROOT) {
412+
const dir = join(root, WORKFLOW_DIR);
413+
if (!existsSync(dir)) return [];
414+
return readdirSync(dir)
415+
.filter((f) => /\.ya?ml$/.test(f))
416+
.sort();
417+
}
418+
419+
// ---------------------------------------------------------------------------
420+
// The DERIVED half — one job-level `if:` shape, read off the tree.
421+
// ---------------------------------------------------------------------------
422+
423+
/**
424+
* The events a job may be gated to for its skip to be by design on every PR
425+
* head. Neither ever fires FOR a pull request: `schedule` runs on the default
426+
* branch's tip, and `workflow_dispatch` runs only when someone asks — and a
427+
* dispatched run SELECTS the job, so it runs rather than skips. ⛔ Closed on
428+
* purpose: `push` fires on a PR branch in any workflow that listens to it, and
429+
* `merge_group` is the queue's own verdict — widening this list is a decision
430+
* about those events, never a spelling fix.
431+
*/
432+
export const NON_PR_EVENTS = Object.freeze(['schedule', 'workflow_dispatch']);
433+
434+
/** The one comparison a term may be: `github.event_name == '<lowercase event>'`. */
435+
const EVENT_NAME_TERM = /^github\.event_name\s*==\s*'([a-z_]+)'$/;
436+
/** One `${{ … }}` expression inside a job `name:` (lazy, so two in a name are two). */
437+
const NAME_EXPRESSION = /\$\{\{([\s\S]*?)\}\}/g;
438+
/** A bare matrix reference — the one expression a pre-expansion skip reports raw. */
439+
const MATRIX_REFERENCE = /^matrix\.[A-Za-z_][A-Za-z0-9_-]*$/;
440+
441+
/**
442+
* Read a job-level `if:` as a gate to non-PR events ONLY, or answer null.
443+
*
444+
* ⛔ A recogniser, never an evaluator. The header's boundary stands: this file
445+
* does not evaluate GitHub's expression language, and so it admits exactly one
446+
* shape — a disjunction whose EVERY term is `github.event_name == '<e>'` with
447+
* `<e>` in `NON_PR_EVENTS`, optionally wrapped whole in one `${{ … }}`. That
448+
* shape is false on every `pull_request`, `push` and `merge_group` run by
449+
* construction, so the job skips on each of them before it starts — and it
450+
* reads nothing a run could make true: no `needs`, no output, no `inputs`, no
451+
* `matrix`, no path, no label. Anything else answers null, and null keeps the
452+
* skip where it was: outside the roster, exit 4. Measured refusals on this
453+
* tree: `release.yml` › `version-pr` (a `workflow_dispatch` term conjoined with
454+
* `inputs.refresh_version_pr`) and `merged-branch-reaper.yml` › `reap`
455+
* (`success()` and `inputs.dry_run` beside its event terms).
456+
*
457+
* @param {unknown} cond the job's `if:` as the YAML parser returned it
458+
* @returns {string[]|null} the sorted, de-duplicated events; null when not this shape
459+
*/
460+
export function nonPrEventGate(cond) {
461+
if (typeof cond !== 'string') return null;
462+
let expr = cond.trim();
463+
const wrapped = /^\$\{\{([\s\S]*)\}\}$/.exec(expr);
464+
if (wrapped) expr = wrapped[1].trim();
465+
if (!expr || expr.includes('${{') || expr.includes('}}')) return null;
466+
const events = [];
467+
for (const term of expr.split('||')) {
468+
const m = EVENT_NAME_TERM.exec(term.trim());
469+
if (!m || !NON_PR_EVENTS.includes(m[1])) return null;
470+
if (!events.includes(m[1])) events.push(m[1]);
471+
}
472+
return events.length > 0 ? events.sort() : null;
473+
}
474+
475+
/**
476+
* The name a job's check-run carries when it is skipped by a job-level gate —
477+
* `name:` VERBATIM, else the job key — or the reason it cannot be told.
478+
*
479+
* GitHub evaluates the job-level `if:` BEFORE it expands a matrix, so a gated
480+
* matrix job's skipped check-run keeps each `${{ matrix.* }}` reference
481+
* literally (measured on PR #20748's head `a84b73af13`: the skipped check-run
482+
* of `scaffold-e2e.yml` › `registry-canary` is named
483+
* `Registry canary: ${{ matrix.template }}`, run 36658032070, as are ci.yml's
484+
* two listed templates). Only an expanded job's check-runs carry expanded
485+
* names, and an expanded job RAN, so no expanded name is ever a skip this
486+
* derivation must admit. Any expression other than a bare matrix reference —
487+
* `inputs.*`, `needs.*.outputs.*`, `github.*` — has an unmeasured skipped
488+
* spelling, so it is refused rather than guessed.
489+
*
490+
* @returns {{ name: string } | { refused: string }}
491+
*/
492+
export function skippedCheckRunName(key, job) {
493+
if (job?.name !== undefined && typeof job.name !== 'string') {
494+
return { refused: `\`name:\` is a ${typeof job.name}, not a string — the check-run name is not derivable` };
495+
}
496+
const name = typeof job?.name === 'string' ? job.name : key;
497+
const expressions = [...name.matchAll(NAME_EXPRESSION)].map((m) => m[1].trim());
498+
if (expressions.length === 0) return { name };
499+
const foreign = expressions.filter((e) => !MATRIX_REFERENCE.test(e));
500+
if (foreign.length > 0) {
501+
return {
502+
refused: `\`name:\` holds ${foreign.map((e) => `\`\${{ ${e} }}\``).join(', ')} — only a bare \`\${{ matrix.* }}\` reference is measured to survive a pre-expansion skip raw`,
503+
};
504+
}
505+
if (job?.strategy?.matrix === undefined) {
506+
return { refused: '`name:` references `matrix.*` but the job declares no `strategy.matrix` — the skipped spelling is unmeasured' };
507+
}
508+
return { name };
509+
}
510+
511+
/**
512+
* Derive the expected skips no one lists: every job, in every workflow file,
513+
* whose job-level `if:` `nonPrEventGate` admits, under the name
514+
* `skippedCheckRunName` gives it.
515+
*
516+
* The roster judges NAMES, so a derived name must mean ONE job. A candidate is
517+
* REFUSED — never admitted — when another job anywhere in the tree carries the
518+
* same check-run name (a skip of that other job would read expected), when the
519+
* listed roster already carries it (derive, don't list), or when its name
520+
* cannot be told. A workflow that cannot be read derives nothing and is
521+
* refused by file. Refusal is the safe direction: the skip stays exit 4. The
522+
* self-test holds the live tree's refusal list at empty, so the job's author
523+
* hears about it rather than a landing seat.
524+
*
525+
* @param {readonly string[]} files workflow file names under `WORKFLOW_DIR`
526+
* @param {(file: string) => object|null} readWorkflow
527+
* @param {readonly object[]} [listed] the listed roster the derived rows join
528+
* @returns {{ rows: object[], refused: { workflow: string, job: string|null, reason: string }[], scanned: number }}
529+
*/
530+
export function deriveNonPrEventSkips(files, readWorkflow, listed = EXPECTED_SKIPS) {
531+
const refused = [];
532+
const jobs = [];
533+
for (const workflow of files) {
534+
const wf = readWorkflow(workflow);
535+
if (!wf || typeof wf !== 'object') {
536+
refused.push({ workflow, job: null, reason: 'the workflow could not be read — none of its jobs is derived' });
537+
continue;
538+
}
539+
const entries = wf.jobs && typeof wf.jobs === 'object' ? Object.entries(wf.jobs) : [];
540+
for (const [key, job] of entries) {
541+
if (!job || typeof job !== 'object') continue;
542+
jobs.push({ workflow, key, job, named: skippedCheckRunName(key, job) });
543+
}
544+
}
545+
const bearers = new Map();
546+
for (const j of jobs) {
547+
if (!('name' in j.named)) continue;
548+
bearers.set(j.named.name, (bearers.get(j.named.name) ?? 0) + 1);
549+
}
550+
const listedNames = new Set(listed.map((r) => r.name));
551+
const rows = [];
552+
for (const { workflow, key, job, named } of jobs) {
553+
const events = nonPrEventGate(job.if);
554+
if (events === null) continue;
555+
if (!('name' in named)) {
556+
refused.push({ workflow, job: key, reason: named.refused });
557+
continue;
558+
}
559+
if (bearers.get(named.name) > 1) {
560+
refused.push({ workflow, job: key, reason: `${bearers.get(named.name)} jobs in the tree carry the check-run name ${JSON.stringify(named.name)} — a skip of the other would read expected` });
561+
continue;
562+
}
563+
if (listedNames.has(named.name)) {
564+
refused.push({ workflow, job: key, reason: `the listed roster already carries ${JSON.stringify(named.name)} — derive, don't list: delete the listed row` });
565+
continue;
566+
}
567+
const matrix = job.strategy?.matrix !== undefined;
568+
rows.push(
569+
Object.freeze({
570+
name: named.name,
571+
workflow,
572+
job: key,
573+
gate: Object.freeze({ kind: 'non-pr-event', events: Object.freeze(events) }),
574+
derived: true,
575+
reason:
576+
`derived, not listed: the job-level \`if:\` selects only ${events.map((e) => `\`${e}\``).join(' / ')}, so every pull_request, push and merge_group run skips it` +
577+
(matrix ? ' before its matrix expands, and the skipped check-run carries the raw `name:` template' : ''),
578+
}),
579+
);
580+
}
581+
return { rows, refused, scanned: files.length };
582+
}
583+
584+
/**
585+
* The roster the judge reads: the listed rows, then the rows derived from the
586+
* workflows under `root`. The derivation travels beside it for the report.
587+
*/
588+
export function expectedSkipRoster(root = ROOT) {
589+
const derivation = deriveNonPrEventSkips(listWorkflowFiles(root), workflowReader(root));
590+
return { roster: Object.freeze([...EXPECTED_SKIPS, ...derivation.rows]), derivation };
591+
}
592+
405593
// ---------------------------------------------------------------------------
406594
// The judge — pure over the REST check-runs shape.
407595
// ---------------------------------------------------------------------------
@@ -473,7 +661,9 @@ export function judgeCheckRuns(payload, roster = EXPECTED_SKIPS) {
473661
if (run.conclusion === 'skipped') {
474662
const row = byName.get(name);
475663
if (row) {
476-
if (!expected.has(name)) expected.set(name, { name, count: 0, reason: row.reason, workflow: row.workflow, job: row.job });
664+
if (!expected.has(name)) {
665+
expected.set(name, { name, count: 0, reason: row.reason, workflow: row.workflow, job: row.job, derived: row.derived === true });
666+
}
477667
expected.get(name).count += 1;
478668
} else {
479669
unexpected.push({
@@ -680,7 +870,7 @@ export function renderReport(judgement, meta = {}) {
680870
if (judgement.expected.length > 0) {
681871
L.push(` expected skips (${judgement.expected.length} name(s), ${judgement.expected.reduce((n, e) => n + e.count, 0)} run(s)):`);
682872
for (const e of [...judgement.expected].sort((a, b) => a.name.localeCompare(b.name))) {
683-
L.push(` ×${e.count} ${e.name} [${e.workflow} › ${e.job}]`);
873+
L.push(` ×${e.count} ${e.name} [${e.workflow} › ${e.job}${e.derived ? '; derived' : ''}]`);
684874
L.push(` ${e.reason}`);
685875
}
686876
} else {
@@ -707,12 +897,20 @@ export function renderReport(judgement, meta = {}) {
707897
}
708898

709899
/** The roster, rendered for a seat that wants to read it without a network. */
710-
export function renderRoster(roster = EXPECTED_SKIPS) {
711-
const L = [`check-expected-skips: ${roster.length} expected-skip name(s), declared in scripts/pm/check-expected-skips.mjs`];
900+
export function renderRoster(roster = EXPECTED_SKIPS, derivation = null) {
901+
const derived = roster.filter((r) => r.derived === true).length;
902+
const L = [
903+
`check-expected-skips: ${roster.length} expected-skip name(s) — ${roster.length - derived} listed in scripts/pm/check-expected-skips.mjs, ` +
904+
`${derived} derived from ${derivation ? `${derivation.scanned} workflow file(s)` : 'the workflows'}`,
905+
];
712906
for (const row of roster) {
713-
L.push(` ${row.name} [${row.workflow} › ${row.job}; gate: ${row.gate.kind}${row.gate.outputs ? ` ${row.gate.outputs.join('|')}` : row.gate.label ? ` ${row.gate.label}` : ''}]`);
907+
const detail = row.gate.outputs ? ` ${row.gate.outputs.join('|')}` : row.gate.label ? ` ${row.gate.label}` : row.gate.events ? ` ${row.gate.events.join('|')}` : '';
908+
L.push(` ${row.name} [${row.workflow} › ${row.job}; gate: ${row.gate.kind}${detail}${row.derived === true ? '; derived' : ''}]`);
714909
L.push(` ${row.reason}`);
715910
}
911+
for (const r of derivation?.refused ?? []) {
912+
L.push(` ⚠️ refused by the derivation — ${r.workflow}${r.job ? ` › ${r.job}` : ''}: ${r.reason}`);
913+
}
716914
return L;
717915
}
718916

@@ -764,7 +962,8 @@ async function run(argv) {
764962
return EXIT_OK;
765963
}
766964
if (opts.roster) {
767-
console.log(renderRoster().join('\n'));
965+
const { roster, derivation } = expectedSkipRoster(ROOT);
966+
console.log(renderRoster(roster, derivation).join('\n'));
768967
return EXIT_OK;
769968
}
770969
if (opts.errors.length > 0) {
@@ -809,7 +1008,7 @@ async function run(argv) {
8091008
meta.head = sha;
8101009
console.error(readPathLine());
8111010
}
812-
const judgement = judgeCheckRuns(payload);
1011+
const judgement = judgeCheckRuns(payload, expectedSkipRoster(ROOT).roster);
8131012
const exit = verdictExit(judgement);
8141013
if (opts.json) {
8151014
console.log(JSON.stringify({ ...meta, exit, judgement }, null, 2));

0 commit comments

Comments
 (0)