Skip to content

Commit 88f97d5

Browse files
fix(pm): make check-family discovery fail loud on undiscoverable workflow verification steps (#9202)
* fix(pm): make check-family discovery fail loud on undiscoverable workflow verification steps dispatch-gates.mjs only recognized `pnpm check:NAME` and `node scripts/**check-NAME.mjs` as check-family invocations, so a workflow verification step using neither shape contributed zero discovered families and was invisible to BOTH halves of the tool's output (#9187). Measured: of the 25 workflow files, only 6 declare a `paths:` filter, and of those, `docs-drift-check.yml` (self-test via a non-`check-`-named script) and `scaffold-e2e.yml` (a genuine e2e pipeline, no check family at all) were the only two with zero discovered families — matching the issue's own count, so the fix is scoped to exactly those two rather than a broader regex widening. - dispatch-gates.mjs: add `checkFamilyCoverageGaps` + `declaredNoCheckFamiliesReason`, asserted in the tool's own self-test (both fixture-pinned and against the live workflow tree) — every paths-filtered workflow must now discover at least one check family, or declare why not via a `# dispatch-gates: no-check-families -- <reason>` comment carried IN the workflow file itself (never a hardcoded filename list here). - docs-drift-check.yml: name the mapper self-test through a new `check:docs-drift-mapper` package.json script instead of a raw `node` invocation, so it becomes discoverable. - scaffold-e2e.yml: declare the opt-out — its steps are an e2e build, not a named verification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza * fix(pm): make docs-drift-check's self-test discoverable without needing pnpm docs-drift-check.yml's job never runs `pnpm install` (the mapper it self-tests has zero dependencies, kept that way deliberately so the job stays fast on every PR touching packages/**), so a `pnpm check:NAME` invocation broke it outright (`pnpm: command not found`, PR #9202 CI). Route it through a thin wrapper script instead — scripts/docs-audit/check-affected-docs.mjs, spawned via a plain `node` call, mirroring scripts/pm/check-dispatch-gates.mjs's own established shape — which satisfies dispatch-gates.mjs's OTHER discovery form (`node scripts/**check-NAME.mjs`) without adding a pnpm dependency to a job that deliberately has none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 07ad424 commit 88f97d5

4 files changed

Lines changed: 217 additions & 1 deletion

File tree

.github/workflows/docs-drift-check.yml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,18 @@ jobs:
4646
# Self-test first, so a regression that widened an exclusion into dropping real
4747
# implementation changes — or collapsed nested packages into their container
4848
# again (#4162) — fails loudly here instead of quietly skewing this comment.
49+
#
50+
# Run through check-affected-docs.mjs, not the raw `node affected-docs.mjs`
51+
# invocation: scripts/pm/dispatch-gates.mjs only discovers a check family from
52+
# a `check:NAME` / `check-NAME.mjs` shape, and this step's own `paths:` filter
53+
# above used to give that derivation nothing to attach to — this was the one
54+
# paths-filtered workflow contributing zero of the discovered families (#9187).
55+
# A `pnpm check:NAME` script is deliberately NOT the fix here: this job never
56+
# runs `pnpm install` (the mapper has zero dependencies, kept that way so this
57+
# job stays fast on every packages/** PR), so `pnpm` isn't even on PATH — the
58+
# wrapper stays a plain `node` call for exactly that reason.
4959
- name: Self-test the change → docs mapper
50-
run: node scripts/docs-audit/affected-docs.mjs --self-test
60+
run: node scripts/docs-audit/check-affected-docs.mjs
5161

5262
- name: Compute affected docs
5363
id: affected

.github/workflows/scaffold-e2e.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
# ships (the pinned Dockerfile — see that step for why the split exists).
1717
# Job 2 (nightly) does the same via the *published* `create-objectstack@latest`
1818
# across every template in the registry — the new-user canary.
19+
#
20+
# Every step below is part of an install/build/boot/docker pipeline, not a named local
21+
# verification script a dev pre-runs with `pnpm check:x` — there is no check family here
22+
# to discover, deliberately (#9187):
23+
# dispatch-gates: no-check-families -- steps are an e2e pipeline, not named local checks
1924

2025
name: Scaffold E2E
2126

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
#!/usr/bin/env node
2+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
3+
4+
/**
5+
* check-affected-docs (#9187) — the discoverable name for affected-docs.mjs's
6+
* own `--self-test`.
7+
*
8+
* node scripts/docs-audit/check-affected-docs.mjs
9+
*
10+
* ## Why this file exists instead of calling affected-docs.mjs directly
11+
*
12+
* `docs-drift-check.yml`'s job checks out the repo and sets up Node — nothing
13+
* else. No `pnpm install`, no `corepack enable`, deliberately: the mapper this
14+
* job self-tests is dependency-free (`node:child_process`/`node:fs`/`node:path`
15+
* only), so the job skips the cost of a full workspace install to stay fast on
16+
* every PR touching `packages/**`. A `pnpm check:NAME` invocation would break
17+
* that job outright (`pnpm: command not found`) — measured, not hypothetical:
18+
* that is exactly what happened the first time this gate was made discoverable
19+
* through a `pnpm` script instead of this file.
20+
*
21+
* `scripts/pm/dispatch-gates.mjs` discovers a check family from exactly two
22+
* shapes: `pnpm check:NAME` or `node scripts/**check-NAME.mjs` — and the
23+
* mapper's own filename, `affected-docs.mjs`, carries no `check-` segment, so
24+
* its self-test contributed zero discovered families despite being a real,
25+
* one-second-local verification with a verdict (#9187). This thin wrapper is
26+
* the fix: it satisfies the naming convention AND stays runnable in a job that
27+
* never installs pnpm, by using the SAME direct-`node`-invocation shape every
28+
* other zero-pnpm-setup workflow gate already uses
29+
* (`check-adr-merge-approval.mjs`, `check-adr-links.mjs`, …) — never a `pnpm`
30+
* wrapper, which would reintroduce the exact breakage this file exists to
31+
* avoid.
32+
*
33+
* Spawned, not imported, so a card that touches the mapper's own logic is
34+
* matched via the CI trigger `docs-drift-check.yml` already declares
35+
* (`scripts/docs-audit/**`) rather than depending on this wrapper's own watch
36+
* hints to carry that weight.
37+
*/
38+
39+
import { spawnSync } from 'node:child_process';
40+
import { join } from 'node:path';
41+
import process from 'node:process';
42+
43+
const ROOT = new URL('../..', import.meta.url).pathname;
44+
45+
/** The script under test — also this gate's one watch hint. */
46+
const MAPPER = 'scripts/docs-audit/affected-docs.mjs';
47+
48+
const result = spawnSync(process.execPath, [join(ROOT, MAPPER), '--self-test'], { stdio: 'inherit' });
49+
50+
if (result.error) {
51+
console.error(`✗ check-affected-docs: could not run ${MAPPER}${result.error.message}`);
52+
process.exit(2);
53+
}
54+
if (result.signal) {
55+
console.error(`✗ check-affected-docs: ${MAPPER} --self-test was killed by ${result.signal}.`);
56+
process.exit(2);
57+
}
58+
process.exit(result.status ?? 2);

scripts/pm/dispatch-gates.mjs

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,72 @@ export function extractCheckInvocations(workflowText, workflowFile) {
325325
return out;
326326
}
327327

328+
/**
329+
* A workflow's OWN declaration that it deliberately has no check family to
330+
* discover — a whole-line comment anywhere in the workflow text:
331+
*
332+
* # dispatch-gates: no-check-families -- <reason>
333+
*
334+
* ## Why a marker IN the workflow, never a list in this script (#9187)
335+
*
336+
* `checkFamilyCoverageGaps` below turns "a paths-filtered workflow discovers
337+
* zero check families" from a silent omission into a CI failure — but one
338+
* real case (`scaffold-e2e.yml`: an install/build/boot/docker pipeline, not a
339+
* named local verification) is not a bug, it is a workflow that genuinely has
340+
* none. The tempting fix is a hardcoded exemption list ([`scaffold-e2e.yml`])
341+
* in THIS file — which reinstalls the exact failure this whole family exists
342+
* to retire: a second copy of a fact that belongs on the thing it describes,
343+
* silently drifting from it (the workflow gets renamed or a real gap gets
344+
* added beside the exempted one, and the list says nothing). A marker the
345+
* workflow carries is instead read fresh every run, same as `paths:` and
346+
* every `run:` step above — nothing to remember to update here.
347+
*
348+
* The reason is REQUIRED (not just the marker) — an opt-out with no reason
349+
* reads identically to a placeholder nobody will ever revisit, and is exactly
350+
* the shape a reviewer cannot tell apart from "forgot to name a family".
351+
*/
352+
const NO_CHECK_FAMILIES_MARKER = /^[ \t]*#[ \t]*dispatch-gates:[ \t]*no-check-families[ \t]*--[ \t]*(\S.*)$/m;
353+
354+
export function declaredNoCheckFamiliesReason(workflowText) {
355+
const m = NO_CHECK_FAMILIES_MARKER.exec(workflowText);
356+
return m ? m[1].trim() : null;
357+
}
358+
359+
/**
360+
* The workflows (by filename) that violate the #9187 coverage invariant:
361+
*
362+
* Every workflow that declares a `paths:` filter either discovers at least
363+
* one check family, or carries a `declaredNoCheckFamiliesReason`.
364+
*
365+
* ## Why scoped to paths-filtered workflows, not all of them
366+
*
367+
* The harm this closes is specific, not general: a `paths:` filter is CI
368+
* SCHEDULING a job for a SUBSET of PRs, and #9171 taught this tool to read
369+
* that schedule as a match key. A workflow with no `paths:` filter runs on
370+
* every PR regardless — it discriminates nothing, so a card touching it
371+
* derives no MORE from a family than it already would from every other
372+
* unfiltered job, and `residueLines`' "unfiltered" bucket already surfaces
373+
* that count honestly rather than as a silent absence. Widening this guard to
374+
* every workflow would fold that already-accounted-for bucket into a false
375+
* positive, and would also flag every zero-check workflow that is not a
376+
* verification job at all (release/publish/nightly-smoke pipelines) — the
377+
* "22 leads is the same as none" trap one level down. Measured on this tree
378+
* (#9187): 6 of the 25 workflow files declare a `paths:` filter; 4 of those 6
379+
* already discover a family, and the remaining 2 (`docs-drift-check.yml`,
380+
* `scaffold-e2e.yml`) are the whole known blast radius — one fixed by naming
381+
* its self-test through a `check:` script, one exempted by the marker above.
382+
*/
383+
export function checkFamilyCoverageGaps(workflowEntries) {
384+
const out = [];
385+
for (const { file, text } of workflowEntries) {
386+
if (extractTriggerPaths(text).length === 0) continue;
387+
if (extractCheckInvocations(text, file).length > 0) continue;
388+
if (declaredNoCheckFamiliesReason(text)) continue;
389+
out.push(file);
390+
}
391+
return out;
392+
}
393+
328394
/** Resolve a `check:x` script name to the script files it runs, via a package.json `scripts` map. */
329395
export function resolveCheckToFiles(checkName, scriptsMap) {
330396
const cmd = scriptsMap[checkName];
@@ -2484,6 +2550,83 @@ function selfTest() {
24842550
t('check:engine-double-contract is a live family, so naming it in the table is not a guess', liveFamilies.has('check:engine-double-contract'));
24852551
t('check:where-matcher is a live family too — the gate the prose never named', liveFamilies.has('check:where-matcher'));
24862552

2553+
// ── The check-family coverage guard (#9187) ───────────────────────────────
2554+
//
2555+
// `docs-drift-check.yml` declared a `paths:` filter and ran a real self-test
2556+
// (`node scripts/docs-audit/affected-docs.mjs --self-test`) that discovery
2557+
// could never see, because the naming convention every OTHER family follows
2558+
// — `check:NAME` or `check-NAME.mjs` — is enforced nowhere: the tree just
2559+
// happened to comply 103 times running up to this card. This section rules
2560+
// it normative: a paths-filtered workflow with no discovered family is now
2561+
// a CI failure, not a lead nobody could see. Fixture cases pin the shape;
2562+
// the live case at the end pins it against the real tree, the same pairing
2563+
// the census guard above uses.
2564+
const noFamilyWf = [
2565+
'name: X',
2566+
'on:',
2567+
' pull_request:',
2568+
' paths:',
2569+
" - 'packages/**'",
2570+
'jobs:',
2571+
' j:',
2572+
' steps:',
2573+
' - name: Self-test the mapper',
2574+
' run: node scripts/some-mapper.mjs --self-test',
2575+
].join('\n');
2576+
t(
2577+
'a paths-filtered workflow discovering no check family is a coverage gap',
2578+
checkFamilyCoverageGaps([{ file: 'x.yml', text: noFamilyWf }]).includes('x.yml'),
2579+
);
2580+
const familyWf = noFamilyWf.replace(
2581+
'node scripts/some-mapper.mjs --self-test',
2582+
'pnpm check:some-mapper',
2583+
);
2584+
t(
2585+
'a paths-filtered workflow that DOES discover a family is not a gap',
2586+
checkFamilyCoverageGaps([{ file: 'x.yml', text: familyWf }]).length === 0,
2587+
);
2588+
const unfilteredNoFamilyWf = [
2589+
'name: X',
2590+
'on:',
2591+
' pull_request: {}',
2592+
'jobs:',
2593+
' j:',
2594+
' steps:',
2595+
' - name: Self-test the mapper',
2596+
' run: node scripts/some-mapper.mjs --self-test',
2597+
].join('\n');
2598+
t(
2599+
'an UNFILTERED workflow with no family is not a gap — it runs on every PR regardless, the residue bucket already accounts for it',
2600+
checkFamilyCoverageGaps([{ file: 'x.yml', text: unfilteredNoFamilyWf }]).length === 0,
2601+
);
2602+
t(
2603+
'the declared opt-out reads its reason back',
2604+
declaredNoCheckFamiliesReason('# dispatch-gates: no-check-families -- e2e build, no named verification (#9187)\n')
2605+
=== 'e2e build, no named verification (#9187)',
2606+
);
2607+
t('no marker present reads as no declared reason', declaredNoCheckFamiliesReason('# just a comment\n') === null);
2608+
t('the marker with no reason text does not count as declared', declaredNoCheckFamiliesReason('# dispatch-gates: no-check-families\n') === null);
2609+
const exemptedWf = noFamilyWf.replace(
2610+
'jobs:',
2611+
'# dispatch-gates: no-check-families -- fixture, not a real verification step\njobs:',
2612+
);
2613+
t(
2614+
"a paths-filtered, zero-family workflow carrying the marker is NOT a gap — the declared opt-out this card's route requires",
2615+
checkFamilyCoverageGaps([{ file: 'x.yml', text: exemptedWf }]).length === 0,
2616+
);
2617+
2618+
// The live guard: every REAL paths-filtered workflow either discovers a
2619+
// family or declares why not. This is what actually fails CI the day a new
2620+
// paths-filtered workflow adds an undiscoverable verification step and
2621+
// forgets both halves of the fix.
2622+
const liveWfDir = join(ROOT, '.github/workflows');
2623+
const liveWorkflowEntries = readdirSync(liveWfDir)
2624+
.filter((f) => /\.ya?ml$/.test(f))
2625+
.map((file) => ({ file, text: readFileSync(join(liveWfDir, file), 'utf8') }));
2626+
t('the live tree has at least one paths-filtered workflow (the guard is not vacuous)', liveWorkflowEntries.some((e) => extractTriggerPaths(e.text).length > 0));
2627+
const liveGaps = checkFamilyCoverageGaps(liveWorkflowEntries);
2628+
t(`every real paths-filtered workflow discovers a check family or declares why not (gaps: ${liveGaps.join(', ') || 'none'})`, liveGaps.length === 0);
2629+
24872630
// ── The residue accounting (#8632) ────────────────────────────────────────
24882631
//
24892632
// Two properties, both of which the deleted prose lacked: it accounts for

0 commit comments

Comments
 (0)