From 13911f47344480ea0957730338a4c6476a168213 Mon Sep 17 00:00:00 2001 From: xfodev Date: Tue, 21 Jul 2026 15:13:14 +0200 Subject: [PATCH] test(scripts): fail loudly when REQUIRED_CONTEXTS drifts from ci.yml's job names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-stuck-required-checks.mjs's REQUIRED_CONTEXTS is hardcoded (branch protection's required-checks list needs Administration read, which the ephemeral workflow token can't get), so a workflow-side rename of the required aggregate check without a matching edit here silently blinds the stuck-check watchdog to it. - Add a test that parses .github/workflows/ci.yml's job names and asserts every workflow-sourced entry in REQUIRED_CONTEXTS still names a real job, plus that the single documented required aggregate ("validate") stays listed. Deriving which checks are required from YAML alone isn't possible (same permissions limitation), so this guarantees forward consistency and fails loudly on the drift it can detect (verified: injecting a bogus required context fails it). - Move the "which required checks are external / not workflow-declared" knowledge (currently "Superagent Security Scan", a third-party App check) into the script as an exported, documented EXTERNAL_REQUIRED_CHECKS constant beside REQUIRED_CONTEXTS, rather than hiding it in the test — the test imports it and also asserts it's a subset of REQUIRED_CONTEXTS. Closes #7774 --- scripts/check-stuck-required-checks.d.mts | 1 + scripts/check-stuck-required-checks.mjs | 7 +++ ...check-stuck-required-checks-script.test.ts | 58 +++++++++++++++++++ 3 files changed, 66 insertions(+) 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); + }); +});