diff --git a/aws/lambda/cross_repo_ci_relay/callback/callback_handler.py b/aws/lambda/cross_repo_ci_relay/callback/callback_handler.py index 7d76a7395c..f7ad107ed8 100644 --- a/aws/lambda/cross_repo_ci_relay/callback/callback_handler.py +++ b/aws/lambda/cross_repo_ci_relay/callback/callback_handler.py @@ -203,6 +203,7 @@ def _create_upstream_check_run( workflow_name: str, job_name: str | None, details_url: str, + pr_number: str = "", ) -> None: """Create a new upstream check run mirroring the downstream job's status. @@ -233,7 +234,7 @@ def _create_upstream_check_run( details_url=details_url, # Store the downstream run_id so a check-run rerequest can re-run # the failed jobs of that workflow run. - external_id=str(run_id), + external_id=f"{run_id}:{pr_number}" if pr_number else str(run_id), output=output, ) logger.info( @@ -369,6 +370,7 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict: if repo_level.value >= AllowlistLevel.L3.value: pr_field = (body.get("payload") or {}).get("pull_request") or {} head_sha = (pr_field.get("head") or {}).get("sha", "") + pr_number = str(pr_field.get("number") or "") if head_sha: conclusion = (body.get("workflow") or {}).get("conclusion") details_url = f"https://github.com/{verified_repo}/actions/runs/{run_id}" @@ -408,6 +410,7 @@ def handle(config: RelayConfig, body: dict, verified_repo: str) -> dict: workflow_name=workflow_name, job_name=job_name, details_url=details_url, + pr_number=pr_number, ) if status == "in_progress": diff --git a/aws/lambda/cross_repo_ci_relay/callback/cleanup_handler.py b/aws/lambda/cross_repo_ci_relay/callback/cleanup_handler.py index e5f2b55c2a..972f6ac9d0 100644 --- a/aws/lambda/cross_repo_ci_relay/callback/cleanup_handler.py +++ b/aws/lambda/cross_repo_ci_relay/callback/cleanup_handler.py @@ -83,6 +83,7 @@ def _finalize_timed_out_check_run( pr_field = (body.get("payload") or {}).get("pull_request") or {} head_sha = (pr_field.get("head") or {}).get("sha", "") + pr_number = str(pr_field.get("number") or "") if not head_sha: return @@ -117,7 +118,7 @@ def _finalize_timed_out_check_run( status="completed", conclusion="timed_out", details_url=details_url, - external_id=run_id, + external_id=f"{run_id}:{pr_number}" if pr_number else run_id, output=output, ) logger.info( diff --git a/aws/lambda/cross_repo_ci_relay/tests/test_callback_handler.py b/aws/lambda/cross_repo_ci_relay/tests/test_callback_handler.py index f6193a813f..c0f7033d8b 100644 --- a/aws/lambda/cross_repo_ci_relay/tests/test_callback_handler.py +++ b/aws/lambda/cross_repo_ci_relay/tests/test_callback_handler.py @@ -365,7 +365,7 @@ def test_check_run_external_id_is_run_id(self): ) kw = self.mock_gh.create_check_run.call_args[1] - self.assertEqual(kw["external_id"], "99999") # run_id from _body + self.assertEqual(kw["external_id"], "99999:42") # run_id:pr_number def test_in_progress_callback_creates_check_run(self): self.mock_gh.create_check_run.return_value = 999 diff --git a/aws/lambda/cross_repo_ci_relay/tests/test_event_handler.py b/aws/lambda/cross_repo_ci_relay/tests/test_event_handler.py index e663c364bb..44df54ce6a 100644 --- a/aws/lambda/cross_repo_ci_relay/tests/test_event_handler.py +++ b/aws/lambda/cross_repo_ci_relay/tests/test_event_handler.py @@ -191,7 +191,7 @@ def test_scenario2_in_progress_job_creates_in_progress_check_run(self): self.assertEqual(kw["head_sha"], "abc123") self.assertEqual(kw["status"], "in_progress") self.assertIsNone(kw["conclusion"]) - self.assertEqual(kw["external_id"], "99999") # run_id + self.assertEqual(kw["external_id"], "99999:42") # run_id:pr_number def test_scenario2_backfills_every_job_not_just_one(self): """Multi-job workflow: a mid-run label backfills a check run for EVERY @@ -215,7 +215,7 @@ def test_scenario2_backfills_every_job_not_just_one(self): c.kwargs["external_id"] for c in self.mock_gh.create_check_run.call_args_list } - self.assertEqual(external_ids, {"99999"}) # run_id + self.assertEqual(external_ids, {"99999:42"}) # run_id:pr_number def test_scenario3_completed_job_creates_completed_check_run(self): """Scenario 3: label arrives after workflow completed → create completed CR directly.""" diff --git a/aws/lambda/cross_repo_ci_relay/webhook/event_handler.py b/aws/lambda/cross_repo_ci_relay/webhook/event_handler.py index 2573aa2a27..59157c6c9c 100644 --- a/aws/lambda/cross_repo_ci_relay/webhook/event_handler.py +++ b/aws/lambda/cross_repo_ci_relay/webhook/event_handler.py @@ -152,7 +152,7 @@ def _handle_pr_labeled(config: RelayConfig, payload: dict) -> dict: return {"ok": True, "created_check_runs": []} pr = payload.get("pull_request") or {} - pr_number = str(pr.get("number", "")) + pr_number = str(pr.get("number") or "") head_sha = (pr.get("head") or {}).get("sha", "") if not pr_number or not head_sha: return {"ignored": True, "reason": "missing pr context"} @@ -205,9 +205,12 @@ def _handle_pr_labeled(config: RelayConfig, payload: dict) -> dict: status=job_status, conclusion=(job_conclusion if job_status == "completed" else None), details_url=details_url, - external_id=str(run_id), + external_id=f"{run_id}:{pr_number}" if pr_number else str(run_id), output=gh_helper.build_check_run_output( - job_status, job_conclusion, details_url, downstream_repo + job_status, + job_conclusion, + details_url, + downstream_repo, ), ) created.append(f"{downstream_repo}/{job_name}") @@ -263,7 +266,7 @@ def _handle_check_run_rerequested(config: RelayConfig, payload: dict) -> dict: """ check_run = payload.get("check_run") or {} name = check_run.get("name", "") - run_id = check_run.get("external_id") or "" + run_id = (check_run.get("external_id") or "").split(":")[0] downstream_repo = _downstream_repo_from_check_run(name) if not downstream_repo or not run_id: return {"ignored": True, "reason": "not a crcr check run"} @@ -346,7 +349,7 @@ def _handle_check_suite_rerequested(config: RelayConfig, payload: dict) -> dict: rerun: list[str] = [] for check_run in check_runs: downstream_repo = _downstream_repo_from_check_run(check_run.get("name", "")) - run_id = check_run.get("external_id") or "" + run_id = (check_run.get("external_id") or "").split(":")[0] if not downstream_repo or not run_id: continue if (downstream_repo, run_id) in seen: diff --git a/torchci/lib/bot/crcrOncallBot.ts b/torchci/lib/bot/crcrOncallBot.ts index c77782ec95..0375489d14 100644 --- a/torchci/lib/bot/crcrOncallBot.ts +++ b/torchci/lib/bot/crcrOncallBot.ts @@ -80,27 +80,13 @@ export default function crcrOncallBot(app: Probot): void { } // Get the PRs this check run belongs to. - // checkRun.pull_requests is empty for cross-fork PRs (most pytorch - // contributions), so fall back to the commits API to resolve PRs - // from the head SHA — the same strategy used by the merge-blocking path. let prNumbers: number[] = []; if (checkRun.pull_requests && checkRun.pull_requests.length > 0) { prNumbers = checkRun.pull_requests.map((pr) => pr.number); - } else if (checkRun.head_sha) { - try { - const result = - await ctx.octokit.rest.repos.listPullRequestsAssociatedWithCommit({ - owner, - repo, - commit_sha: checkRun.head_sha, - }); - prNumbers = result.data.map((pr: any) => pr.number); - } catch (err) { - ctx.log( - { err }, - `crcrOncall: failed to resolve PRs for commit ${checkRun.head_sha}, skipping` - ); - return; + } else if (checkRun.external_id) { + const parts = checkRun.external_id.split(":"); + if (parts.length === 2 && parts[1]) { + prNumbers = [parseInt(parts[1], 10)]; } } diff --git a/torchci/test/crcrOncallBot.test.ts b/torchci/test/crcrOncallBot.test.ts index 368241557c..fa54d5fb28 100644 --- a/torchci/test/crcrOncallBot.test.ts +++ b/torchci/test/crcrOncallBot.test.ts @@ -176,16 +176,46 @@ L3: }); }); - test("does not comment when no PR is associated", async () => { + test("does not comment when external_id has no PR number", async () => { + // external_id carries bare run_id (no ":" separator) when pr_number + // is unavailable — the bot returns early without any API call. await probot.receive({ name: "check_run" as any, payload: checkRunPayload({ pull_requests: [], + external_id: "12345", }) as any, id: "8", }); }); + test("posts comment when external_id contains PR number for cross-fork PR", async () => { + const scope = nock("https://api.github.com") + .get(`/repos/${OWNER}/${REPO}/issues/${PR_NUMBER}/comments`) + .reply(200, []) + .post( + `/repos/${OWNER}/${REPO}/issues/${PR_NUMBER}/comments`, + (body: any) => { + expect(body.body).toContain( + "" + ); + expect(body.body).toContain("@oncall_xpu"); + return true; + } + ) + .reply(200); + + await probot.receive({ + name: "check_run" as any, + payload: checkRunPayload({ + pull_requests: [], // empty — simulates cross-fork PR + external_id: `12345:${PR_NUMBER}`, + }) as any, + id: "10", + }); + handleScope(scope); + }); + test("does nothing for unsupported org", async () => { const payload = checkRunPayload(); payload.repository = {