diff --git a/master/github_app_check_push.py b/master/github_app_check_push.py index 84b7111..dc710a5 100644 --- a/master/github_app_check_push.py +++ b/master/github_app_check_push.py @@ -86,15 +86,21 @@ def createStatus( # The check run's id isn't threaded through between calls, so look it up by name instead # of tracking build-run state; one extra GET, but no persisted state. A build's queued, - # started, and completed reports all update the same check run this way. + # started, and completed reports all update the same check run this way -- but only while + # that run is still open. A rebuild (via the Buildbot UI's "Rebuild", or a future handler + # for GitHub's "Re-run" webhook) reports a fresh "pending" after the previous run already + # went to "completed"; GitHub's own guidance for handling reruns is to start a new check + # run rather than resurrect a completed one (see "Building CI checks with a GitHub App" / + # handling the check_run "rerequested" event), so only PATCH runs that are still open. resp = yield self._http.get( f"/repos/{repo_user}/{repo_name}/commits/{sha}/check-runs", params={"check_name": context}, headers=headers, ) runs = (yield resp.json())["check_runs"] - if runs: - run_id = max(runs, key=lambda r: r["id"])["id"] + open_runs = [r for r in runs if r["status"] != "completed"] + if open_runs: + run_id = max(open_runs, key=lambda r: r["id"])["id"] # HTTPSession has no patch() wrapper (only get/put/post/delete); the Checks API # update endpoint is PATCH-only, so fall through to the generic dispatcher it's # built on. @@ -104,7 +110,8 @@ def createStatus( ) ) - # No existing check run (this is the first report for this build, or GitHub is still - # processing the previous write) -- create one from scratch. + # No open check run (this is the first report for this build, GitHub is still + # processing the previous write, or the previous run already completed and this is a + # rebuild) -- create one from scratch. payload = {**payload, "name": context, "head_sha": sha, "external_id": issue} return (yield self._http.post(base, json=payload, headers=headers)) diff --git a/master/master.cfg b/master/master.cfg index 19069b1..24d7b12 100644 --- a/master/master.cfg +++ b/master/master.cfg @@ -1296,6 +1296,89 @@ class SafeGitHubEventHandler(GitHubEventHandler): self._log(f'missing key "{e}" in malformed payload: {payload}') return self.skip() + # GitHub's "Re-run" / "Re-run all jobs" buttons (and the corresponding buttons in its own + # Checks UI) don't touch our push/pull_request webhook at all -- they're delivered as + # check_run/check_suite events (from the App's own Checks-permission subscription) with + # action "rerequested". Handle those by re-running the exact Buildbot build(s) the check + # run(s) point at, via the same data-API path as the web UI's own "Rebuild" button + # (BuildEndpoint.actionRebuild), so behavior is identical either way. + _CHECK_RUN_DETAILS_URL_RE = re.compile(r"#/builders/(\d+)/builds/(\d+)$") + + @inlineCallbacks + def _rebuild_check_run(self, check_run): + name = check_run.get("name") + details_url = check_run.get("details_url") or "" + m = self._CHECK_RUN_DETAILS_URL_RE.search(details_url) + if not m: + self._log(f"Can't rebuild check run '{name}': unrecognized details_url '{details_url}'") + return + + builderid, build_number = int(m.group(1)), int(m.group(2)) + build = yield self.master.data.get(("builders", builderid, "builds", build_number)) # ty: ignore[unresolved-attribute] + if build is None: + self._log(f"Can't rebuild check run '{name}': no such build {builderid}/{build_number}") + return + + buildrequest = yield self.master.data.get(("buildrequests", build["buildrequestid"])) # ty: ignore[unresolved-attribute] + yield self.master.data.updates.rebuildBuildrequest(buildrequest) # ty: ignore[unresolved-attribute] + self._log(f"Rebuilding check run '{name}' (builder {builderid}, build {build_number})") + + def handle_check_run(self, payload, event): + if payload.get("action") == "rerequested": + return self._handle_check_run_rerequested(payload) + return self.skip() + + @inlineCallbacks + def _handle_check_run_rerequested(self, payload): + yield self._rebuild_check_run(payload["check_run"]) + return self.skip() + + def handle_check_suite(self, payload, event): + if payload.get("action") == "rerequested": + return self._handle_check_suite_rerequested(payload) + return self.skip() + + @inlineCallbacks + def _handle_check_suite_rerequested(self, payload): + check_suite = payload["check_suite"] + check_runs_url = check_suite.get("check_runs_url") + if not check_runs_url: + self._log(f"check_suite {check_suite.get('id')} has no check_runs_url; can't rebuild") + return self.skip() + + headers = {"User-Agent": "Buildbot"} + if self._token: + p = Properties() + p.master = self.master + p.setProperty("full_name", payload["repository"]["full_name"], "change_hook") + token = yield p.render(self._token) + headers["Authorization"] = "token " + token + + # check_runs_url is a full API URL; HTTPSession is bound to github_api_endpoint, so + # request the path relative to that base. + path = check_runs_url.removeprefix(self.github_api_endpoint) + + http = yield httpclientservice.HTTPSession( + self.master.httpservice, # ty: ignore[unresolved-attribute] + self.github_api_endpoint, + headers=headers, + debug=self.debug, + verify=self.verify, + ) + res = yield http.get(path) + if not (200 <= res.code < 300): + self._log(f"Failed listing check runs for suite {check_suite.get('id')}: response code {res.code}") + return self.skip() + + data = yield res.json() + for check_run in data.get("check_runs", []): + # Only rebuild our own check runs (see GitHubAppCheckPush's context format) -- + # a suite re-run may cover other apps' checks on the same commit. + if (check_run.get("name") or "").startswith("buildbot/"): + yield self._rebuild_check_run(check_run) + + return self.skip() + @staticmethod def skip(): return [], "git"