Skip to content

Commit 58e3cbc

Browse files
authored
[CRCR] Implement on-call bot and allowlist functionality for downstream CI failures (#8183)
## Summary Implement CRCR on-call bot and merge-blocking logic for downstream CI failures, driven by a structured allowlist (L1-L4) that maps downstream repos to severity levels and on-call contacts. ## Changes ### CRCR Allowlist (`lib/crcrAllowlist.ts`) Parses `.github/allowlist.yml` from `pytorch/pytorch`, mapping downstream repos to four levels: - **L1/L2**: Tracked only, no on-call behavior. - **L3**: Non-blocking failure — on-call is notified but merge is not blocked. - **L4**: Blocking failure — on-call is notified and merge is blocked. Includes a 15-minute in-memory cache and strict YAML validation. Errors fail open. ### On-call Bot (`lib/bot/crcrOncallBot.ts`) A Probot bot on `check_run.completed`: when a CRCR check run fails, looks up on-calls from the allowlist and posts a tagged comment on the associated PR. Deduplicates via an HTML comment marker (theoretical TOCTOU race exists but is negligible in practice since checks arrive sequentially). ### Merge Blocking (`lib/bot/pytorchBotHandler.ts`) `@pytorchbot merge` checks the GitHub Checks API for CRCR failures on the PR head commit. L4 failures block the merge with a comment listing failing repos. `-f` bypasses this, consistent with existing force-merge semantics. Errors fail open. - Introduced `ensureHeadSha()` helper to deduplicate the lazy-load pattern for `headSha` across methods. ### Dr.CI Integration - `fetchRecentWorkflows.ts`: New `fetchOotWorkflows()` queries `oot_workflow_job` from ClickHouse. Uses a new `downstreamLevel` field on `RecentWorkflowsData` instead of overloading `failure_context`. - `drci.ts`: L4 failures appear as blocking; L3 failures appear in a new "OUT OF TREE (non-blocking)" section. ### Tests Unit tests for allowlist parsing and on-call bot behavior, plus updated Dr.CI test helpers.
1 parent 64a6965 commit 58e3cbc

12 files changed

Lines changed: 1257 additions & 42 deletions

torchci/lib/bot/crcrOncallBot.ts

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
import { Probot } from "probot";
2+
import { fetchCrcrAllowlist } from "../crcrAllowlist";
3+
import { isPyTorchbotSupportedOrg } from "./utils";
4+
5+
export const FAILURE_CONCLUSIONS: ReadonlySet<string> = new Set([
6+
"failure",
7+
"cancelled",
8+
"timed_out",
9+
]);
10+
11+
function markerForRepo(downstreamRepo: string): string {
12+
return `<!-- crcr-oncall:${downstreamRepo} -->`;
13+
}
14+
15+
/**
16+
* Parse the downstream ``owner/repo`` out of a CRCR check run name.
17+
*
18+
* Check runs are named ``crcr/<owner>/<repo>/<workflow_name>/<job_name>``
19+
* (see the Python gh_helper.check_run_name), so the downstream repo is the
20+
* first two path segments after the ``crcr/`` prefix.
21+
*/
22+
export function downstreamRepoFromCheckRunName(name: string): string | null {
23+
const prefix = "crcr/";
24+
if (!name.startsWith(prefix)) {
25+
return null;
26+
}
27+
const parts = name.slice(prefix.length).split("/");
28+
if (parts.length < 4 || !parts[0] || !parts[1]) {
29+
return null;
30+
}
31+
return `${parts[0]}/${parts[1]}`;
32+
}
33+
34+
/**
35+
* Search existing PR comments for one with the CRCR on-call marker.
36+
* Returns its ID (or 0 if none found). Uses pagination so the marker
37+
* is found even when a PR has more than 30 comments.
38+
*/
39+
async function findExistingComment(
40+
octokit: any,
41+
owner: string,
42+
repo: string,
43+
prNumber: number,
44+
downstreamRepo: string
45+
): Promise<number> {
46+
const marker = markerForRepo(downstreamRepo);
47+
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
48+
owner,
49+
repo,
50+
issue_number: prNumber,
51+
});
52+
for (const comment of comments) {
53+
if (comment.body?.includes(marker)) {
54+
return comment.id;
55+
}
56+
}
57+
return 0;
58+
}
59+
60+
export default function crcrOncallBot(app: Probot): void {
61+
app.on("check_run.completed", async (ctx) => {
62+
const owner = ctx.payload.repository.owner.login;
63+
if (!isPyTorchbotSupportedOrg(owner)) {
64+
return;
65+
}
66+
67+
const repo = ctx.payload.repository.name;
68+
const checkRun = ctx.payload.check_run;
69+
70+
// Only act on CRCR-created check runs
71+
const downstreamRepo = downstreamRepoFromCheckRunName(checkRun.name);
72+
if (!downstreamRepo) {
73+
return;
74+
}
75+
76+
// Only comment on failures
77+
const conclusion = checkRun.conclusion ?? "";
78+
if (!FAILURE_CONCLUSIONS.has(conclusion)) {
79+
return;
80+
}
81+
82+
// Get the PRs this check run belongs to.
83+
// checkRun.pull_requests is empty for cross-fork PRs (most pytorch
84+
// contributions), so fall back to the commits API to resolve PRs
85+
// from the head SHA — the same strategy used by the merge-blocking path.
86+
let prNumbers: number[] = [];
87+
if (checkRun.pull_requests && checkRun.pull_requests.length > 0) {
88+
prNumbers = checkRun.pull_requests.map((pr) => pr.number);
89+
} else if (checkRun.head_sha) {
90+
try {
91+
const result =
92+
await ctx.octokit.rest.repos.listPullRequestsAssociatedWithCommit({
93+
owner,
94+
repo,
95+
commit_sha: checkRun.head_sha,
96+
});
97+
prNumbers = result.data.map((pr: any) => pr.number);
98+
} catch (err) {
99+
ctx.log(
100+
{ err },
101+
`crcrOncall: failed to resolve PRs for commit ${checkRun.head_sha}, skipping`
102+
);
103+
return;
104+
}
105+
}
106+
107+
if (prNumbers.length === 0) {
108+
ctx.log(
109+
`crcrOncall: no PR associated with check run ${checkRun.name}, skipping`
110+
);
111+
return;
112+
}
113+
114+
const headSha = checkRun.head_sha;
115+
const checkRunUrl = checkRun.html_url ?? "";
116+
117+
// Load oncalls from the allowlist
118+
let oncalls: string[];
119+
try {
120+
const allowlist = await fetchCrcrAllowlist(ctx.octokit);
121+
oncalls = allowlist.getOncallsForRepo(downstreamRepo);
122+
} catch (err) {
123+
ctx.log({ err }, "crcrOncall: failed to load allowlist, skipping");
124+
return;
125+
}
126+
127+
if (oncalls.length === 0) {
128+
ctx.log(
129+
`crcrOncall: no oncalls configured for ${downstreamRepo}, skipping`
130+
);
131+
return;
132+
}
133+
134+
const mentions = oncalls.map((o) => `@${o}`).join(" ");
135+
136+
// Post a comment on each associated PR (typically just one)
137+
for (const prNumber of prNumbers) {
138+
try {
139+
// Dedup by marker: only comment once per PR.
140+
// NOTE: There is a theoretical TOCTOU race here — two concurrent
141+
// check_run.completed events for the same PR could both pass the
142+
// marker check and post duplicate comments. In practice, checks
143+
// arrive sequentially, so this is unlikely to cause issues.
144+
const existingId = await findExistingComment(
145+
ctx.octokit,
146+
owner,
147+
repo,
148+
prNumber,
149+
downstreamRepo
150+
);
151+
if (existingId !== 0) {
152+
ctx.log(
153+
`crcrOncall: comment already exists on PR #${prNumber}, skipping`
154+
);
155+
continue;
156+
}
157+
158+
const commentBody = `${markerForRepo(downstreamRepo)}
159+
## :x: CRCR downstream CI failure
160+
161+
The downstream CI workflow in **${downstreamRepo}** has failed on commit \`${headSha.slice(
162+
0,
163+
7
164+
)}\`.
165+
166+
${mentions} please investigate.
167+
168+
### Details
169+
170+
| Field | Value |
171+
|---|---|
172+
| **Check Run** | [${checkRun.name}](${checkRunUrl}) |
173+
| **Commit** | \`${headSha}\` |
174+
| **Conclusion** | \`${conclusion}\` |
175+
`;
176+
177+
await ctx.octokit.issues.createComment({
178+
owner,
179+
repo,
180+
issue_number: prNumber,
181+
body: commentBody,
182+
});
183+
184+
ctx.log(
185+
`crcrOncall: commented on PR #${prNumber} for ${downstreamRepo} (${conclusion})`
186+
);
187+
} catch (err) {
188+
ctx.log(
189+
{ err },
190+
`crcrOncall: failed to comment on PR for ${downstreamRepo}`
191+
);
192+
}
193+
}
194+
});
195+
}

torchci/lib/bot/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import cancelWorkflowsOnCloseBot from "./cancelWorkflowsOnCloseBot";
66
import checkLabelsBot from "./checkLabelsBot";
77
import ciflowPushTrigger from "./ciflowPushTrigger";
88
import codevNoWritePerm from "./codevNoWritePermBot";
9+
import crcrOncallBot from "./crcrOncallBot";
910
import drciBot from "./drciBot";
1011
import nitpickBot from "./nitpickBot";
1112
import pytorchBot from "./pytorchBot";
@@ -22,6 +23,7 @@ export default function bot(app: Probot) {
2223
checkLabelsBot(app);
2324
ciflowPushTrigger(app);
2425
codevNoWritePerm(app);
26+
crcrOncallBot(app);
2527
drciBot(app);
2628
nitpickBot(app);
2729
pytorchBot(app);

torchci/lib/bot/pytorchBotHandler.ts

Lines changed: 103 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import _ from "lodash";
33
import { updateDrciComments } from "pages/api/drci/drci";
44
import shlex from "shlex";
55
import { queryClickhouseSaved } from "../clickhouse";
6+
import { fetchCrcrAllowlist } from "../crcrAllowlist";
67
import { getHelp, getParser } from "./cliParser";
78
import { cherryPickClassifications } from "./Constants";
9+
import { downstreamRepoFromCheckRunName } from "./crcrOncallBot";
810
import PytorchBotLogger from "./pytorchbotLogger";
911
import {
1012
hasWritePermissions as _hasWP,
@@ -319,7 +321,23 @@ The explanation needs to be clear on why this is needed. Here are some good exam
319321
}
320322

321323
await this.logger.log("merge", extra_data);
324+
325+
// Check for L4 CRCR blocking failures (L3 failures are non-blocking).
326+
// Force merge (-f) bypasses this check, consistent with the existing
327+
// force-merge semantics for in-repo CI failures.
328+
// Only applies to pytorch/pytorch — the CRCR allowlist and check runs
329+
// are specific to that repo.
322330
if (!forceRequested && isPyTorchPyTorch(this.owner, this.repo)) {
331+
const blockingRepos = await this.getCrcrBlockingFailures();
332+
if (blockingRepos.length > 0) {
333+
await this.addComment(
334+
`The following L4 downstream CI workflows are blocking this merge (failed or still running):\n\n` +
335+
blockingRepos.map((r) => `- \`${r}\``).join("\n") +
336+
`\n\nPlease investigate or use \`@pytorchbot merge -f\` to bypass.`
337+
);
338+
return;
339+
}
340+
323341
let labels: string[] = this.ctx.payload?.issue?.labels.map(
324342
(e: any) => e["name"]
325343
);
@@ -401,10 +419,8 @@ The explanation needs to be clear on why this is needed. Here are some good exam
401419
);
402420
}
403421

404-
async hasWorkflowRunningPermissions(username: string): Promise<boolean> {
405-
if (await _hasWP(this.ctx, username)) {
406-
return true;
407-
}
422+
/** Lazy-load and cache the PR head SHA, then return it. */
423+
private async ensureHeadSha(): Promise<string> {
408424
if (this.headSha === undefined) {
409425
const pullRequest = await this.ctx.octokit.pulls.get({
410426
owner: this.owner,
@@ -413,12 +429,19 @@ The explanation needs to be clear on why this is needed. Here are some good exam
413429
});
414430
this.headSha = pullRequest.data.head.sha;
415431
}
432+
return this.headSha!;
433+
}
434+
435+
async hasWorkflowRunningPermissions(username: string): Promise<boolean> {
436+
if (await _hasWP(this.ctx, username)) {
437+
return true;
438+
}
416439

417440
return await hasApprovedPullRuns(
418441
this.ctx.octokit,
419442
this.ctx.payload.repository.owner.login,
420443
this.ctx.payload.repository.name,
421-
this.headSha!
444+
await this.ensureHeadSha()
422445
);
423446
}
424447

@@ -635,6 +658,81 @@ The explanation needs to be clear on why this is needed. Here are some good exam
635658
headSha: this.headSha,
636659
});
637660
}
661+
662+
/**
663+
* Return the list of L4 downstream repos whose CRCR check runs are failing
664+
* or still pending on this PR's head commit. L3 workflows are intentionally
665+
* omitted — they are non-blocking.
666+
*
667+
* A pending (not-yet-completed) L4 check run also blocks merge: a still-
668+
* running check could fail, so merging before it completes would bypass
669+
* the downstream gating. Use ``@pytorchbot merge -f`` to override.
670+
*/
671+
async getCrcrBlockingFailures(): Promise<string[]> {
672+
// Only "failure" blocks merge — "cancelled" and "timed_out" are often
673+
// superseded / infra-related and should not gate merge.
674+
const BLOCKING_CONCLUSIONS = new Set(["failure"]);
675+
676+
// Query GitHub Check Runs API for all check runs on this commit.
677+
// Use paginate to handle PRs with more than 100 check runs.
678+
let checkRuns: any[] = [];
679+
try {
680+
const headSha = await this.ensureHeadSha();
681+
checkRuns = await this.ctx.octokit.paginate(
682+
this.ctx.octokit.checks.listForRef,
683+
{
684+
owner: this.owner,
685+
repo: this.repo,
686+
ref: headSha,
687+
filter: "latest",
688+
per_page: 100,
689+
}
690+
);
691+
} catch {
692+
// If we can't fetch check runs, fail open (don't block merge on
693+
// an infrastructure error, including transient ensureHeadSha failure)
694+
this.ctx.log("getCrcrBlockingFailures: failed to list check runs");
695+
return [];
696+
}
697+
698+
// Find all CRCR check runs (not just failures — we also need to
699+
// gate on pending L4 checks so a still-running check isn't bypassed).
700+
const crcrCheckRuns = checkRuns.filter((cr: any) =>
701+
cr.name.startsWith("crcr/")
702+
);
703+
704+
if (crcrCheckRuns.length === 0) {
705+
return [];
706+
}
707+
708+
// Load the allowlist to classify each repo as L3 or L4
709+
let allowlist;
710+
try {
711+
allowlist = await fetchCrcrAllowlist(this.ctx.octokit);
712+
} catch {
713+
this.ctx.log("getCrcrBlockingFailures: failed to load allowlist");
714+
return [];
715+
}
716+
717+
const blocking = new Set<string>();
718+
for (const cr of crcrCheckRuns) {
719+
const downstreamRepo = downstreamRepoFromCheckRunName(cr.name);
720+
if (!downstreamRepo || !allowlist.isBlocking(downstreamRepo)) {
721+
continue; // Not L4
722+
}
723+
724+
// Block when the L4 check has failed OR is still pending.
725+
// Successful/completed L4 checks don't block.
726+
const isFailure = BLOCKING_CONCLUSIONS.has(cr.conclusion ?? "");
727+
const isPending = cr.status !== "completed";
728+
729+
if (isFailure || isPending) {
730+
blocking.add(downstreamRepo);
731+
}
732+
}
733+
734+
return [...blocking];
735+
}
638736
}
639737

640738
export default PytorchBotHandler;

0 commit comments

Comments
 (0)