Fix checks not updating on rebuild; handle GitHub's own re-run webhooks

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))

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"