Skip to content

Commit 611a03a

Browse files
qq9340100claude
andauthored
fix(ci): check-required-contexts guards types: against dropped defaults (#8394)
Extends assertion 7 (the pull_request trigger guard) to also require that, when a required-context workflow's pull_request trigger names `types:`, the list is a superset of GitHub's default [opened, synchronize, reopened]. Naming any `types:` replaces that default set rather than extending it, so a hand-restated list that drops one produces the identical permanent-pending wedge the existing `paths:` guard exists to catch, through a different key on the same trigger. adr-merge-approval.yml (since #8302) is the only required-context workflow naming types: today, and its list already restates all three defaults, so the pin stays green on main as-is. Self-test gains fixtures proving all three directions named in the triage scope: dropping a default (or two) from adr-merge-approval.yml's list names the omission and goes red; a workflow with no types: key at all stays green; a strict superset (an extra activity beyond the defaults) stays green. Fixes #8304 Co-authored-by: Claude <noreply@anthropic.com>
1 parent f3fb618 commit 611a03a

1 file changed

Lines changed: 79 additions & 3 deletions

File tree

scripts/check-required-contexts.mjs

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,15 @@
5151
* it (#5617's audit lists `Console Pin Freshness` as exactly this shape:
5252
* a file whose own comment invites required-ization it cannot survive);
5353
* 7. its workflow's `pull_request:` trigger exists and carries no `paths:` /
54-
* `paths-ignore:`. A path-filtered trigger produces NO check run on a PR
55-
* that misses the glob — not a skip, an absence — which is permanent
56-
* pending (the audit's `Spec property liveness` exclusion).
54+
* `paths-ignore:`, and, if it names `types:` at all, that list is a
55+
* superset of GitHub's default `[opened, synchronize, reopened]`. A
56+
* path-filtered trigger produces NO check run on a PR that misses the
57+
* glob, and naming `types:` REPLACES (never extends) the default set,
58+
* so dropping one of the three from a hand-written list produces NO
59+
* check run on that activity — neither is a skip, both are an absence,
60+
* which is permanent pending (the audit's `Spec property liveness`
61+
* exclusion for the `paths:` half; #8304 for the `types:` half, live
62+
* since `adr-merge-approval.yml` started naming `types:` in #8302).
5763
*
5864
* Plus two whole-registry properties:
5965
*
@@ -205,6 +211,16 @@ function scriptRepoRoot() {
205211
return resolve(dirname(fileURLToPath(import.meta.url)), '..');
206212
}
207213

214+
/**
215+
* GitHub's default `pull_request:` activity types. Naming any `types:` at
216+
* all REPLACES this set rather than extending it, so a workflow that names
217+
* `types:` must restate every one of these (or more) or it silently stops
218+
* publishing a check run for the dropped activity — the same permanent-
219+
* pending wedge assertion 7's `paths:` guard exists to catch, through a
220+
* different key on the same trigger (#8304).
221+
*/
222+
const DEFAULT_PULL_REQUEST_TYPES = ['opened', 'synchronize', 'reopened'];
223+
208224
/**
209225
* A workflow's trigger block.
210226
*
@@ -310,6 +326,23 @@ export function judge({ registry, workflows }) {
310326
);
311327
}
312328
}
329+
// (7b) a `types:` list that drops one of GitHub's defaults. Naming any
330+
// `types:` REPLACES the default `[opened, synchronize, reopened]`
331+
// rather than adding to it, so a hand-restated list that misses one is
332+
// the identical permanent-pending wedge as a `paths:` filter, through a
333+
// different key on the same trigger (#8304).
334+
if (pr && typeof pr === 'object' && Object.prototype.hasOwnProperty.call(pr, 'types')) {
335+
const types = Array.isArray(pr.types) ? pr.types : [];
336+
const missing = DEFAULT_PULL_REQUEST_TYPES.filter((t) => !types.includes(t));
337+
if (missing.length > 0) {
338+
problems.push(
339+
`.github/workflows/${file}'s \`pull_request:\` trigger names \`types:\` but omits GitHub's default activity type(s) ` +
340+
`${missing.map((t) => `'${t}'`).join(', ')}. Naming any \`types:\` REPLACES that default set instead of extending it, so a PR reaching ` +
341+
`that activity produces NO check run — not a skip, an absence — and every required context in this file sits permanently pending ` +
342+
`on those PRs (#8304; the \`types:\` counterpart to assertion 7's \`paths:\` guard above).`,
343+
);
344+
}
345+
}
313346
}
314347
}
315348

@@ -558,6 +591,49 @@ async function selfTest() {
558591
const noPr = fixture('drop pull_request from ci.yml', 'ci.yml', (s) => s.replace(' pull_request:\n branches:\n - main\n', ''));
559592
assert(noPr.problems.some((p) => p.includes('no `pull_request:` trigger')), 'a required-context workflow with no pull_request trigger ⇒ red');
560593

594+
// ── (7b) a `types:` list that drops a GitHub default ──────────────────────
595+
// adr-merge-approval.yml is (as of #8302) the only required-context
596+
// workflow that names `types:` at all — its list hand-restates the three
597+
// defaults alongside the two auto-merge activities #8012 needs, which is
598+
// exactly the load-bearing-but-unverified shape #8304 is about.
599+
const droppedReopened = fixture('drop reopened from adr-merge-approval.yml types', 'adr-merge-approval.yml', (s) =>
600+
s.replace(
601+
'types: [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled]',
602+
'types: [opened, synchronize, auto_merge_enabled, auto_merge_disabled]',
603+
),
604+
);
605+
assert(
606+
droppedReopened.problems.some((p) => p.includes('adr-merge-approval.yml') && p.includes("omits GitHub's default activity type(s) 'reopened'")),
607+
"pruning 'reopened' from a hand-restated types: list ⇒ red, naming the dropped default (#8304)",
608+
);
609+
const droppedTwo = fixture('drop opened and synchronize from adr-merge-approval.yml types', 'adr-merge-approval.yml', (s) =>
610+
s.replace(
611+
'types: [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled]',
612+
'types: [reopened, auto_merge_enabled, auto_merge_disabled]',
613+
),
614+
);
615+
assert(
616+
droppedTwo.problems.some((p) => p.includes("'opened', 'synchronize'")),
617+
'dropping two defaults at once ⇒ red naming both, in default order',
618+
);
619+
const noTypesAtAll = fixture('remove the types: key entirely from adr-merge-approval.yml', 'adr-merge-approval.yml', (s) =>
620+
s.replace(' types: [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled]\n', ''),
621+
);
622+
assert(
623+
noTypesAtAll.problems.length === 0,
624+
`a pull_request trigger with no \`types:\` key at all ⇒ green — GitHub's own defaults apply, nothing was replaced (got ${JSON.stringify(noTypesAtAll.problems)})`,
625+
);
626+
const supersetTypes = fixture('extend adr-merge-approval.yml types with an extra activity', 'adr-merge-approval.yml', (s) =>
627+
s.replace(
628+
'types: [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled]',
629+
'types: [opened, synchronize, reopened, auto_merge_enabled, auto_merge_disabled, ready_for_review]',
630+
),
631+
);
632+
assert(
633+
supersetTypes.problems.length === 0,
634+
`a types: list that is a strict superset of the three defaults ⇒ green (got ${JSON.stringify(supersetTypes.problems)})`,
635+
);
636+
561637
// ── (9) the shadowing collision, on the live specimen ────────────────────
562638
// ci.yml's sharded `test` job is named `Test Core (${{ matrix.shard }}/3)`
563639
// and its aggregate gate is named `Test Core`. Dropping the suffix makes two

0 commit comments

Comments
 (0)