Skip to content
Open
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
174 changes: 174 additions & 0 deletions torchci/lib/bot/crcrOncallBot.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
import { Probot } from "probot";
import { fetchCrcrAllowlist } from "../crcrAllowlist";
import { isPyTorchbotSupportedOrg } from "./utils";

export const FAILURE_CONCLUSIONS: ReadonlySet<string> = new Set([
"failure",
"cancelled",
"timed_out",
]);

function markerForRepo(downstreamRepo: string): string {
return `<!-- crcr-oncall:${downstreamRepo} -->`;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This marker is global per PR — if backend A fails first and gets a comment, then backend B fails later, the bot finds the existing marker and skips entirely. This means backend B's oncalls are never notified.

Consider using a per-repo marker (e.g., <!-- crcr-oncall:intel/torch-xpu-ops -->) so each downstream repo gets its own dedup scope.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

/**
* Parse the downstream ``owner/repo`` out of a CRCR check run name.
*
* Check runs are named ``crcr/<owner>/<repo>/<workflow_name>/<job_name>``
* (see the Python gh_helper.check_run_name), so the downstream repo is the
* first two path segments after the ``crcr/`` prefix.
*/
export function downstreamRepoFromCheckRunName(name: string): string | null {
const prefix = "crcr/";
if (!name.startsWith(prefix)) {
return null;
}
const parts = name.slice(prefix.length).split("/");
if (parts.length < 4 || !parts[0] || !parts[1]) {
return null;
}
return `${parts[0]}/${parts[1]}`;
}

/**
* Search existing PR comments for one with the CRCR on-call marker.
* Returns its ID (or 0 if none found). Uses pagination so the marker
* is found even when a PR has more than 30 comments.
*/
async function findExistingComment(
octokit: any,
owner: string,
repo: string,
prNumber: number,
downstreamRepo: string
): Promise<number> {
const marker = markerForRepo(downstreamRepo);
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
owner,
repo,
issue_number: prNumber,
});
for (const comment of comments) {
if (comment.body?.includes(marker)) {
return comment.id;
}
}
return 0;
}

export default function crcrOncallBot(app: Probot): void {
app.on("check_run.completed", async (ctx) => {
const owner = ctx.payload.repository.owner.login;
if (!isPyTorchbotSupportedOrg(owner)) {
return;
}

const repo = ctx.payload.repository.name;
const checkRun = ctx.payload.check_run;

// Only act on CRCR-created check runs
const downstreamRepo = downstreamRepoFromCheckRunName(checkRun.name);
if (!downstreamRepo) {
return;
}

// Only comment on failures
const conclusion = checkRun.conclusion ?? "";
if (!FAILURE_CONCLUSIONS.has(conclusion)) {
return;
}

// Get the PR this check run belongs to
const prs = checkRun.pull_requests ?? [];
if (prs.length === 0) {
ctx.log(
`crcrOncall: no PR associated with check run ${checkRun.name}, skipping`
);
return;
}

const headSha = checkRun.head_sha;
const checkRunUrl = checkRun.html_url ?? "";

// Load oncalls from the allowlist
let oncalls: string[];
try {
const allowlist = await fetchCrcrAllowlist(ctx.octokit);
oncalls = allowlist.getOncallsForRepo(downstreamRepo);
} catch (err) {
ctx.log({ err }, "crcrOncall: failed to load allowlist, skipping");
return;
}

if (oncalls.length === 0) {
ctx.log(
`crcrOncall: no oncalls configured for ${downstreamRepo}, skipping`
);
return;
}

const mentions = oncalls.map((o) => `@${o}`).join(" ");

// Post a comment on each associated PR (typically just one)
for (const pr of prs) {
try {
const prNumber = pr.number;

// Dedup by marker: only comment once per PR.
// NOTE: There is a theoretical TOCTOU race here — two concurrent
// check_run.completed events for the same PR could both pass the
// marker check and post duplicate comments. In practice, checks
// arrive sequentially, so this is unlikely to cause issues.
const existingId = await findExistingComment(
ctx.octokit,
owner,
repo,
prNumber,
downstreamRepo
);
if (existingId !== 0) {
ctx.log(
`crcrOncall: comment already exists on PR #${prNumber}, skipping`
);
continue;
}

const commentBody = `${markerForRepo(downstreamRepo)}
## :x: CRCR downstream CI failure

The downstream CI workflow in **${downstreamRepo}** has failed on commit \`${headSha.slice(
0,
7
)}\`.

${mentions} please investigate.

### Details

| Field | Value |
|---|---|
| **Check Run** | [${checkRun.name}](${checkRunUrl}) |
| **Commit** | \`${headSha}\` |
| **Conclusion** | \`${conclusion}\` |
`;

await ctx.octokit.issues.createComment({
owner,
repo,
issue_number: prNumber,
body: commentBody,
});

ctx.log(
`crcrOncall: commented on PR #${prNumber} for ${downstreamRepo} (${conclusion})`
);
} catch (err) {
ctx.log(
{ err },
`crcrOncall: failed to comment on PR for ${downstreamRepo}`
);
}
}
});
}
2 changes: 2 additions & 0 deletions torchci/lib/bot/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import cancelWorkflowsOnCloseBot from "./cancelWorkflowsOnCloseBot";
import checkLabelsBot from "./checkLabelsBot";
import ciflowPushTrigger from "./ciflowPushTrigger";
import codevNoWritePerm from "./codevNoWritePermBot";
import crcrOncallBot from "./crcrOncallBot";
import drciBot from "./drciBot";
import nitpickBot from "./nitpickBot";
import pytorchBot from "./pytorchBot";
Expand All @@ -22,6 +23,7 @@ export default function bot(app: Probot) {
checkLabelsBot(app);
ciflowPushTrigger(app);
codevNoWritePerm(app);
crcrOncallBot(app);
drciBot(app);
nitpickBot(app);
pytorchBot(app);
Expand Down
104 changes: 99 additions & 5 deletions torchci/lib/bot/pytorchBotHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ import _ from "lodash";
import { updateDrciComments } from "pages/api/drci/drci";
import shlex from "shlex";
import { queryClickhouseSaved } from "../clickhouse";
import { fetchCrcrAllowlist } from "../crcrAllowlist";
import { getHelp, getParser } from "./cliParser";
import { cherryPickClassifications } from "./Constants";
import {
downstreamRepoFromCheckRunName,
FAILURE_CONCLUSIONS,
} from "./crcrOncallBot";
import PytorchBotLogger from "./pytorchbotLogger";
import {
hasWritePermissions as _hasWP,
Expand Down Expand Up @@ -319,6 +324,30 @@ The explanation needs to be clear on why this is needed. Here are some good exam
}

await this.logger.log("merge", extra_data);

// Check for L4 CRCR blocking failures (L3 failures are non-blocking).
// Force merge (-f) bypasses this check, consistent with the existing
// force-merge semantics for in-repo CI failures.
// Only applies to pytorch/pytorch — the CRCR allowlist and check runs
// are specific to that repo.
if (!forceRequested && isPyTorchPyTorch(this.owner, this.repo)) {
try {
const blockingRepos = await this.getCrcrBlockingFailures();
if (blockingRepos.length > 0) {
const repoList = blockingRepos.join(", ");
await this.addComment(
`The following L4 downstream CI workflows have failed and are blocking this merge:\n\n` +
blockingRepos.map((r) => `- \`${r}\``).join("\n") +
`\n\nPlease investigate or use \`@pytorchbot merge -f\` to bypass.`
);
return;
}
} catch (err) {
// If the blocking check itself fails, log and proceed (fail open).
this.ctx.log({ err }, "CRCR blocking check failed, allowing merge");
}
}

if (!forceRequested && isPyTorchPyTorch(this.owner, this.repo)) {
let labels: string[] = this.ctx.payload?.issue?.labels.map(
(e: any) => e["name"]
Expand Down Expand Up @@ -401,10 +430,8 @@ The explanation needs to be clear on why this is needed. Here are some good exam
);
}

async hasWorkflowRunningPermissions(username: string): Promise<boolean> {
if (await _hasWP(this.ctx, username)) {
return true;
}
/** Lazy-load and cache the PR head SHA, then return it. */
private async ensureHeadSha(): Promise<string> {
if (this.headSha === undefined) {
const pullRequest = await this.ctx.octokit.pulls.get({
owner: this.owner,
Expand All @@ -413,12 +440,19 @@ The explanation needs to be clear on why this is needed. Here are some good exam
});
this.headSha = pullRequest.data.head.sha;
}
return this.headSha!;
}

async hasWorkflowRunningPermissions(username: string): Promise<boolean> {
if (await _hasWP(this.ctx, username)) {
return true;
}

return await hasApprovedPullRuns(
this.ctx.octokit,
this.ctx.payload.repository.owner.login,
this.ctx.payload.repository.name,
this.headSha!
await this.ensureHeadSha()
);
}

Expand Down Expand Up @@ -635,6 +669,66 @@ The explanation needs to be clear on why this is needed. Here are some good exam
headSha: this.headSha,
});
}

/**
* Return the list of L4 downstream repos whose CRCR check runs have failed
* on this PR's head commit. L3 failures are intentionally omitted — they
* are non-blocking.
*/
async getCrcrBlockingFailures(): Promise<string[]> {
const headSha = await this.ensureHeadSha();

// Query GitHub Check Runs API for all check runs on this commit.
// Use paginate to handle PRs with more than 100 check runs.
let checkRuns: any[] = [];
try {
checkRuns = await this.ctx.octokit.paginate(
this.ctx.octokit.checks.listForRef,
{
owner: this.owner,
repo: this.repo,
ref: headSha,
filter: "latest",
per_page: 100,
}
);
} catch {
// If we can't fetch check runs, fail open (don't block merge on
// an infrastructure error)
this.ctx.log("getCrcrBlockingFailures: failed to list check runs");
return [];
}

// Filter for CRCR check runs with a blocking conclusion
const crcrFailures = checkRuns.filter(
(cr: any) =>
cr.name.startsWith("crcr/") &&
FAILURE_CONCLUSIONS.has(cr.conclusion ?? "")
);

if (crcrFailures.length === 0) {
return [];
}

// Load the allowlist to classify each failed repo as L3 or L4
let allowlist;
try {
allowlist = await fetchCrcrAllowlist(this.ctx.octokit);
} catch {
this.ctx.log("getCrcrBlockingFailures: failed to load allowlist");
return [];
}

const blocking: string[] = [];
for (const cr of crcrFailures) {
const downstreamRepo = downstreamRepoFromCheckRunName(cr.name);
if (downstreamRepo && allowlist.isBlocking(downstreamRepo)) {
blocking.push(downstreamRepo);
}
}

return blocking;
}
}

export default PytorchBotHandler;
Loading
Loading