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
52 changes: 52 additions & 0 deletions scripts/check-stuck-required-checks.d.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
export type GithubApi = (
path: string,
options?: { method?: string; body?: string; headers?: Record<string, string> },
) => Promise<unknown>;

export type PullRequestRef = {
number: number;
draft?: boolean;
head: { sha: string };
};

export type StuckCheck = {
name: string;
status: string;
startedAt: string;
elapsedMinutes: number;
htmlUrl?: string;
};

export type CommentScope = {
githubApi: GithubApi;
owner: string;
repoName: string;
};

export type FindStuckOptions = CommentScope & { thresholdMinutes: number };

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

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

export declare function findStuckChecksForPr(
pr: PullRequestRef,
requiredContexts: Set<string>,
options: FindStuckOptions,
): Promise<StuckCheck[]>;

export declare function hasExistingWatchdogComment(
prNumber: number,
options: CommentScope,
): Promise<boolean>;

export declare function runStuckCheckWatchdog(options: {
githubApi: GithubApi;
owner: string;
repoName: string;
thresholdMinutes: number;
dryRun?: boolean;
requiredContexts?: Set<string>;
log?: (message: string) => void;
}): Promise<number>;
131 changes: 77 additions & 54 deletions scripts/check-stuck-required-checks.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,9 @@
// situation visible fast instead of requiring another manual multi-PR investigation like the one that
// found the original incident.

const STUCK_THRESHOLD_MINUTES = Number(process.argv.find((a) => a.startsWith("--threshold-minutes="))?.split("=")[1] ?? 20);
const DRY_RUN = process.argv.includes("--dry-run");
const MARKER = "<!-- stuck-required-check-watchdog -->";

const repo = process.env.GITHUB_REPOSITORY;
if (!repo) throw new Error("GITHUB_REPOSITORY is required");
const [owner, repoName] = repo.split("/");
const token = process.env.GITHUB_TOKEN;
if (!token) throw new Error("GITHUB_TOKEN is required");

async function githubApi(path, options = {}) {
const response = await fetch(`https://api.github.com${path}`, {
...options,
headers: {
Authorization: `Bearer ${token}`,
Accept: "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
...options.headers,
},
});
if (!response.ok) {
throw new Error(`GitHub API error ${response.status} on ${path}: ${await response.text()}`);
}
return response.status === 204 ? null : response.json();
}
import { pathToFileURL } from "node:url";

export const MARKER = "<!-- stuck-required-check-watchdog -->";

// Hardcoded, not read live from branch protection: GET /branches/main/protection/required_status_checks
// needs "Administration" repository read permission, which the default GITHUB_TOKEN does not get even
Expand All @@ -45,37 +23,53 @@ async function githubApi(path, options = {}) {
// protection is considered too privileged). Update this list by hand if the required checks on `main`
// ever change (`gh api repos/{owner}/{repo}/branches/main/protection/required_status_checks` locally,
// with a real user token, to check the current list).
const REQUIRED_CONTEXTS = new Set(["validate", "Superagent Security Scan"]);

async function getOpenPRs() {
const prs = await githubApi(`/repos/${owner}/${repoName}/pulls?state=open&per_page=100`);
return prs.filter((pr) => !pr.draft);
export const REQUIRED_CONTEXTS = new Set(["validate", "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. */
function makeGithubApi(token) {
return async function githubApi(path, options = {}) {
const response = await fetch(`https://api.github.com${path}`, {
...options,
headers: {
Authorization: `Bearer ${token}`,
Accept: "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
...options.headers,
},
});
if (!response.ok) {
throw new Error(`GitHub API error ${response.status} on ${path}: ${await response.text()}`);
}
return response.status === 204 ? null : response.json();
};
}

function minutesSince(isoString) {
export function minutesSince(isoString) {
return (Date.now() - new Date(isoString).getTime()) / 60000;
}

async function findStuckChecksForPr(pr, requiredContexts) {
export async function findStuckChecksForPr(pr, requiredContexts, { githubApi, owner, repoName, thresholdMinutes }) {
const checkRuns = await githubApi(`/repos/${owner}/${repoName}/commits/${pr.head.sha}/check-runs?per_page=100`);
const stuck = [];
for (const run of checkRuns.check_runs ?? []) {
if (!requiredContexts.has(run.name)) continue;
if (run.status === "completed") continue;
const elapsedMinutes = run.started_at ? minutesSince(run.started_at) : null;
if (elapsedMinutes !== null && elapsedMinutes >= STUCK_THRESHOLD_MINUTES) {
if (elapsedMinutes !== null && elapsedMinutes >= thresholdMinutes) {
stuck.push({ name: run.name, status: run.status, startedAt: run.started_at, elapsedMinutes: Math.round(elapsedMinutes), htmlUrl: run.html_url });
}
}
return stuck;
}

async function hasExistingWatchdogComment(prNumber) {
export async function hasExistingWatchdogComment(prNumber, { githubApi, owner, repoName }) {
const comments = await githubApi(`/repos/${owner}/${repoName}/issues/${prNumber}/comments?per_page=100`);
return comments.some((comment) => comment.body?.includes(MARKER));
}

async function postComment(prNumber, stuckChecks) {
async function postComment(prNumber, stuckChecks, { githubApi, owner, repoName }) {
const lines = [
MARKER,
"## ⚠️ A required check looks stuck",
Expand All @@ -94,28 +88,57 @@ async function postComment(prNumber, stuckChecks) {
});
}

const prs = await getOpenPRs();
let flaggedCount = 0;

for (const pr of prs) {
const stuckChecks = await findStuckChecksForPr(pr, REQUIRED_CONTEXTS);
if (stuckChecks.length === 0) continue;

console.log(`PR #${pr.number}: ${stuckChecks.length} required check(s) stuck past ${STUCK_THRESHOLD_MINUTES}min: ${stuckChecks.map((c) => `${c.name} (~${c.elapsedMinutes}min)`).join(", ")}`);
/** Scan every open, non-draft PR for stuck required checks, printing a line per stuck PR and posting a
* one-time (marker-guarded) comment unless `dryRun`. `githubApi` is injected so this whole flow is
* testable without live GitHub calls. Returns the number of new comments posted. */
export async function runStuckCheckWatchdog({
githubApi,
owner,
repoName,
thresholdMinutes,
dryRun = false,
requiredContexts = REQUIRED_CONTEXTS,
log = console.log,
}) {
const prs = (await githubApi(`/repos/${owner}/${repoName}/pulls?state=open&per_page=100`)).filter((pr) => !pr.draft);
let flaggedCount = 0;

for (const pr of prs) {
const stuckChecks = await findStuckChecksForPr(pr, requiredContexts, { githubApi, owner, repoName, thresholdMinutes });
if (stuckChecks.length === 0) continue;

log(`PR #${pr.number}: ${stuckChecks.length} required check(s) stuck past ${thresholdMinutes}min: ${stuckChecks.map((c) => `${c.name} (~${c.elapsedMinutes}min)`).join(", ")}`);

if (await hasExistingWatchdogComment(pr.number, { githubApi, owner, repoName })) {
log(` Already flagged this PR -- skipping (idempotent).`);
continue;
}

if (await hasExistingWatchdogComment(pr.number)) {
console.log(` Already flagged this PR -- skipping (idempotent).`);
continue;
}
if (dryRun) {
log(` --dry-run: would post a comment on PR #${pr.number}, skipping the actual POST.`);
continue;
}

if (DRY_RUN) {
console.log(` --dry-run: would post a comment on PR #${pr.number}, skipping the actual POST.`);
continue;
await postComment(pr.number, stuckChecks, { githubApi, owner, repoName });
flaggedCount += 1;
log(` Posted a new comment on PR #${pr.number}.`);
}

await postComment(pr.number, stuckChecks);
flaggedCount += 1;
console.log(` Posted a new comment on PR #${pr.number}.`);
log(`Checked ${prs.length} open PR(s); flagged ${flaggedCount} new stuck-check comment(s).`);
return flaggedCount;
}

console.log(`Checked ${prs.length} open PR(s); flagged ${flaggedCount} new stuck-check comment(s).`);
// Entrypoint guard (#7455): the pure detection logic above is importable for tests with an injected
// githubApi; only direct invocation reads env, builds the live API caller, and makes real calls.
if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) {
const thresholdMinutes = Number(process.argv.find((a) => a.startsWith("--threshold-minutes="))?.split("=")[1] ?? 20);
const dryRun = process.argv.includes("--dry-run");

const repo = process.env.GITHUB_REPOSITORY;
if (!repo) throw new Error("GITHUB_REPOSITORY is required");
const [owner, repoName] = repo.split("/");
const token = process.env.GITHUB_TOKEN;
if (!token) throw new Error("GITHUB_TOKEN is required");

await runStuckCheckWatchdog({ githubApi: makeGithubApi(token), owner, repoName, thresholdMinutes, dryRun });
}
127 changes: 127 additions & 0 deletions test/unit/check-stuck-required-checks-script.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { describe, expect, it } from "vitest";

import {
MARKER,
REQUIRED_CONTEXTS,
findStuckChecksForPr,
minutesSince,
runStuckCheckWatchdog,
} from "../../scripts/check-stuck-required-checks.mjs";

// #7455: findStuckChecksForPr's stuck/threshold decision and the watchdog's dry-run + marker-idempotency
// only ran inside the un-guarded live-GitHub driver. With githubApi injected and the driver behind an
// entrypoint guard, both are now testable with mock responses and zero network.

type CheckRun = { name: string; status: string; started_at?: string; html_url?: string };
type ApiOptions = { method?: string; body?: string; headers?: Record<string, string> };

function minutesAgoIso(minutes: number): string {
return new Date(Date.now() - minutes * 60_000).toISOString();
}

const scope = { owner: "acme", repoName: "widget", thresholdMinutes: 20 };

function checkRunApi(checkRuns: CheckRun[]) {
return async (path: string): Promise<unknown> => {
if (path.includes("/check-runs")) return { check_runs: checkRuns };
throw new Error(`unexpected path: ${path}`);
};
}

describe("minutesSince (#7455)", () => {
it("returns elapsed minutes since an ISO timestamp", () => {
expect(minutesSince(minutesAgoIso(30))).toBeGreaterThanOrEqual(29.9);
expect(minutesSince(minutesAgoIso(30))).toBeLessThanOrEqual(30.1);
});
});

describe("findStuckChecksForPr (#7455)", () => {
const pr = { number: 1, head: { sha: "deadbeef" } };

it("flags a required check pending past the threshold", async () => {
const stuck = await findStuckChecksForPr(pr, REQUIRED_CONTEXTS, {
githubApi: checkRunApi([{ name: "validate", status: "in_progress", started_at: minutesAgoIso(30) }]),
...scope,
});
expect(stuck).toHaveLength(1);
expect(stuck[0]!.name).toBe("validate");
expect(stuck[0]!.elapsedMinutes).toBeGreaterThanOrEqual(20);
});

it("does not flag a required check still under the threshold", async () => {
const stuck = await findStuckChecksForPr(pr, REQUIRED_CONTEXTS, {
githubApi: checkRunApi([{ name: "validate", status: "in_progress", started_at: minutesAgoIso(5) }]),
...scope,
});
expect(stuck).toHaveLength(0);
});

it("excludes a not-completed check that has no started_at (elapsedMinutes === null), e.g. still queued", async () => {
const stuck = await findStuckChecksForPr(pr, REQUIRED_CONTEXTS, {
githubApi: checkRunApi([{ name: "validate", status: "queued" }]),
...scope,
});
expect(stuck).toHaveLength(0);
});

it("ignores non-required and already-completed checks even when old", async () => {
const stuck = await findStuckChecksForPr(pr, REQUIRED_CONTEXTS, {
githubApi: checkRunApi([
{ name: "some-other-check", status: "in_progress", started_at: minutesAgoIso(60) },
{ name: "validate", status: "completed", started_at: minutesAgoIso(60) },
]),
...scope,
});
expect(stuck).toHaveLength(0);
});
});

describe("runStuckCheckWatchdog (#7455)", () => {
function watchdogApi(opts: { prs: unknown[]; checkRuns: CheckRun[]; comments: Array<{ body?: string }> }) {
const calls: Array<{ path: string; method: string }> = [];
const githubApi = async (path: string, options: ApiOptions = {}): Promise<unknown> => {
const method = options.method ?? "GET";
calls.push({ path, method });
if (path.includes("/pulls?")) return opts.prs;
if (path.includes("/check-runs")) return { check_runs: opts.checkRuns };
if (path.includes("/comments")) return method === "POST" ? {} : opts.comments;
throw new Error(`unexpected path: ${path}`);
};
return { githubApi, calls };
}

const stuckRun: CheckRun = { name: "validate", status: "in_progress", started_at: minutesAgoIso(30) };

it("posts a comment for a stuck PR that has not been flagged yet", async () => {
const { githubApi, calls } = watchdogApi({
prs: [{ number: 7, draft: false, head: { sha: "s" } }],
checkRuns: [stuckRun],
comments: [],
});
const flagged = await runStuckCheckWatchdog({ githubApi, ...scope, log: () => {} });
expect(flagged).toBe(1);
expect(calls.filter((c) => c.method === "POST")).toHaveLength(1);
});

it("is idempotent: skips a PR that already has the watchdog marker comment (no POST)", async () => {
const { githubApi, calls } = watchdogApi({
prs: [{ number: 7, draft: false, head: { sha: "s" } }],
checkRuns: [stuckRun],
comments: [{ body: `${MARKER}\n## previously flagged` }],
});
const flagged = await runStuckCheckWatchdog({ githubApi, ...scope, log: () => {} });
expect(flagged).toBe(0);
expect(calls.some((c) => c.method === "POST")).toBe(false);
});

it("--dry-run never calls the comment-post endpoint", async () => {
const { githubApi, calls } = watchdogApi({
prs: [{ number: 7, draft: false, head: { sha: "s" } }],
checkRuns: [stuckRun],
comments: [],
});
const flagged = await runStuckCheckWatchdog({ githubApi, ...scope, dryRun: true, log: () => {} });
expect(flagged).toBe(0);
expect(calls.some((c) => c.method === "POST")).toBe(false);
});
});