Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/check-stuck-required-checks.d.mts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type FindStuckOptions = CommentScope & { thresholdMinutes: number };

export declare const MARKER: string;
export declare const REQUIRED_CONTEXTS: Set<string>;
export declare const EXTERNAL_REQUIRED_CHECKS: Set<string>;

export declare function minutesSince(isoString: string): number;

Expand Down
7 changes: 7 additions & 0 deletions scripts/check-stuck-required-checks.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ export const MARKER = "<!-- stuck-required-check-watchdog -->";
// 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. */
Expand Down
58 changes: 58 additions & 0 deletions test/unit/check-stuck-required-checks-script.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<string> {
const doc = parse(readFileSync(".github/workflows/ci.yml", "utf8")) as {
jobs: Record<string, { name?: string }>;
};
const names = new Set<string>();
// 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);
});
});