diff --git a/scripts/check-stuck-required-checks.d.mts b/scripts/check-stuck-required-checks.d.mts index 744acee61d..af99b5b3e8 100644 --- a/scripts/check-stuck-required-checks.d.mts +++ b/scripts/check-stuck-required-checks.d.mts @@ -27,6 +27,7 @@ export type FindStuckOptions = CommentScope & { thresholdMinutes: number }; export declare const MARKER: string; export declare const REQUIRED_CONTEXTS: Set; +export declare const EXTERNAL_REQUIRED_CHECKS: Set; export declare function minutesSince(isoString: string): number; diff --git a/scripts/check-stuck-required-checks.mjs b/scripts/check-stuck-required-checks.mjs index 4bef2cb84e..5f1c585dd7 100644 --- a/scripts/check-stuck-required-checks.mjs +++ b/scripts/check-stuck-required-checks.mjs @@ -25,6 +25,13 @@ export const MARKER = ""; // with a real user token, to check the current list). export const REQUIRED_CONTEXTS = new Set(["validate", "Superagent Security Scan"]); +// The subset of REQUIRED_CONTEXTS that no workflow in this repo declares -- currently only the third-party +// "Superagent Security Scan" GitHub App check (see the header comment). Kept beside REQUIRED_CONTEXTS, and +// exported, so a workflow-drift test can tell workflow-sourced required checks (which must still match a +// ci.yml job name) apart from genuinely external ones, and so this "which of these are external" knowledge +// is documented next to the hardcoded list rather than hidden in a test (#7774). +export const EXTERNAL_REQUIRED_CHECKS = new Set(["Superagent Security Scan"]); + /** Build the authenticated GitHub API caller used by the live entrypoint. Kept as a factory so the * pure/testable logic below can take `githubApi` as an injected dependency instead of closing over a * module-level token. */ diff --git a/test/unit/check-stuck-required-checks-script.test.ts b/test/unit/check-stuck-required-checks-script.test.ts index ffe38f53e8..b9381f3714 100644 --- a/test/unit/check-stuck-required-checks-script.test.ts +++ b/test/unit/check-stuck-required-checks-script.test.ts @@ -1,6 +1,10 @@ +import { readFileSync } from "node:fs"; + import { describe, expect, it } from "vitest"; +import { parse } from "yaml"; import { + EXTERNAL_REQUIRED_CHECKS, MARKER, REQUIRED_CONTEXTS, findStuckChecksForPr, @@ -125,3 +129,57 @@ describe("runStuckCheckWatchdog (#7455)", () => { expect(calls.some((c) => c.method === "POST")).toBe(false); }); }); + +// #7774: REQUIRED_CONTEXTS is hardcoded because branch protection's required-checks list can't be read live +// from the ephemeral workflow token (Administration read is not grantable). That makes it silent-drift-prone: +// if the workflow's required aggregate check is renamed without a matching edit here, the watchdog goes blind +// to it with no signal anywhere. These tests fail loudly on that divergence. Deriving "which checks are +// required" from YAML alone isn't possible (the same permissions limitation), so the achievable, genuine +// guarantee is forward consistency: every workflow-sourced required context must still name a real ci.yml job. +describe("REQUIRED_CONTEXTS stays in sync with .github/workflows/ci.yml (#7774)", () => { + // EXTERNAL_REQUIRED_CHECKS (from the script) is the documented set of required checks no workflow declares — + // currently the third-party "Superagent Security Scan" GitHub App check — so they intentionally have no + // ci.yml job counterpart and are excluded from the workflow-consistency assertion below. + const EXTERNAL_CHECKS = EXTERNAL_REQUIRED_CHECKS; + + function ciJobCheckNames(): Set { + const doc = parse(readFileSync(".github/workflows/ci.yml", "utf8")) as { + jobs: Record; + }; + const names = new Set(); + // A job's status-check context is its `name:` when set, else its job id. + for (const [jobId, job] of Object.entries(doc.jobs)) names.add(job.name ?? jobId); + return names; + } + + it("every workflow-sourced required context is a real ci.yml job name", () => { + const jobNames = ciJobCheckNames(); + const workflowSourced = [...REQUIRED_CONTEXTS].filter((context) => !EXTERNAL_CHECKS.has(context)); + // Premise guard: at least one required context must come from a workflow (else EXTERNAL_CHECKS is stale). + expect(workflowSourced.length).toBeGreaterThan(0); + const missing = workflowSourced.filter((context) => !jobNames.has(context)); + expect( + missing, + `REQUIRED_CONTEXTS has ${missing.join(", ")} which is not a job name in .github/workflows/ci.yml — ` + + `update scripts/check-stuck-required-checks.mjs's REQUIRED_CONTEXTS (or EXTERNAL_CHECKS in this test) to match.`, + ).toEqual([]); + }); + + it("still lists the single required aggregate the workflow documents ('validate')", () => { + // ci.yml declares one required status check that branch protection points at: the `validate` aggregate. + // Renaming it without updating REQUIRED_CONTEXTS is exactly the drift the watchdog would silently miss. + const jobNames = ciJobCheckNames(); + expect(jobNames.has("validate")).toBe(true); + expect(REQUIRED_CONTEXTS.has("validate")).toBe(true); + }); + + it("EXTERNAL_CHECKS lists only genuinely external checks (none is a ci.yml job)", () => { + const jobNames = ciJobCheckNames(); + for (const external of EXTERNAL_CHECKS) expect(jobNames.has(external)).toBe(false); + }); + + it("every EXTERNAL_REQUIRED_CHECKS entry is actually a required context", () => { + // Excluding a check from the workflow-consistency assertion only makes sense if it's genuinely required. + for (const external of EXTERNAL_REQUIRED_CHECKS) expect(REQUIRED_CONTEXTS.has(external)).toBe(true); + }); +});