From e737502119cf206c1fcb3f9a4b4bb67c9ee7d27b Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Fri, 4 Sep 2026 17:33:46 -0400 Subject: [PATCH 1/2] Drop BuildRequestGenerator from GitHubAppCheckPush's default generators GitHubStatusPush's default generators report both a build-request-queued event and the actual build start/end. Both produce state == "pending" in sendMessage() (build['complete'] is False for a queued request too), so createStatus() was creating a check run for the queued-but-not-yet-started event as well as the real one for build start -- but unlike statuses, check runs are persistent objects with their own id, so the queued-request one was never completed and sat forever as "in progress" pointing at a buildrequest page instead of a build. --- master/github_app_check_push.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/master/github_app_check_push.py b/master/github_app_check_push.py index bdfb907..0b4577e 100644 --- a/master/github_app_check_push.py +++ b/master/github_app_check_push.py @@ -3,7 +3,9 @@ import jwt import requests from buildbot.interfaces import IRenderable +from buildbot.reporters.generators.build import BuildStartEndStatusGenerator from buildbot.reporters.github import GitHubStatusPush +from buildbot.reporters.message import MessageFormatterRenderable from twisted.internet import defer, threads from zope.interface import implementer @@ -56,6 +58,19 @@ class GitHubAppCheckPush(GitHubStatusPush): doesn't work here; only the Checks API used by this class requires App auth). """ + def _create_default_generators(self): + # GitHubStatusPush's defaults also include a BuildRequestGenerator, reporting once a + # build is merely queued (before any worker picks it up). Unlike statuses, check runs + # are persistent objects with their own id: creating one for the queued build-request + # report and another for the actual build start would leave the first orphaned forever + # (nothing ever completes it), rather than just being overwritten as a status would be. + return [ + BuildStartEndStatusGenerator( + start_formatter=MessageFormatterRenderable("Build started."), + end_formatter=MessageFormatterRenderable("Build done."), + ) + ] + @defer.inlineCallbacks def _get_auth_header(self, props): token = yield props.render(self.token) From f021c3c81c84693d0fbbf14f27d90eaa18d7593d Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Fri, 4 Sep 2026 17:38:54 -0400 Subject: [PATCH 2/2] Keep reporting queued builds, but update one check run through its lifecycle The previous commit dropped BuildRequestGenerator entirely, losing the queued-but-not-started feedback (a static dot, distinct from an actively running build) that we actually want. The real bug wasn't reporting the queued state -- it was creating a brand new check run for it instead of reusing the one for the build's later start/completion. Both the queued (BuildRequestGenerator) and started (BuildStartEndStatus- Generator) reports produce state == "pending" in the inherited sendMessage(), but their target_url differs -- a buildrequest page vs. an actual build page -- which is enough to tell "queued" from "in progress" apart. createStatus() now always looks up the existing check run by name first and PATCHes it, falling back to POST only when none exists yet, so a build's queued, started, and completed reports all update the same check run. Verified against a fake HTTP session driving createStatus() through all three stages: exactly one check run exists throughout, transitioning queued -> in_progress -> completed. --- master/github_app_check_push.py | 80 ++++++++++++++------------------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/master/github_app_check_push.py b/master/github_app_check_push.py index 0b4577e..84b7111 100644 --- a/master/github_app_check_push.py +++ b/master/github_app_check_push.py @@ -3,9 +3,7 @@ import jwt import requests from buildbot.interfaces import IRenderable -from buildbot.reporters.generators.build import BuildStartEndStatusGenerator from buildbot.reporters.github import GitHubStatusPush -from buildbot.reporters.message import MessageFormatterRenderable from twisted.internet import defer, threads from zope.interface import implementer @@ -53,24 +51,12 @@ def _fetch(self): class GitHubAppCheckPush(GitHubStatusPush): """Like GitHubStatusPush, but reports through the Checks API instead of the legacy Statuses - API, so a build in progress shows GitHub's spinner instead of a static pending dot. Requires - a GitHub App: pass an AppInstallationToken as `token=` (the Statuses API's PAT-based token - doesn't work here; only the Checks API used by this class requires App auth). + API, so a build shows GitHub's queued/in-progress states (and eventual spinner) instead of a + single static pending dot. Requires a GitHub App: pass an AppInstallationToken as `token=` + (the Statuses API's PAT-based token doesn't work here; only the Checks API used by this class + requires App auth). """ - def _create_default_generators(self): - # GitHubStatusPush's defaults also include a BuildRequestGenerator, reporting once a - # build is merely queued (before any worker picks it up). Unlike statuses, check runs - # are persistent objects with their own id: creating one for the queued build-request - # report and another for the actual build start would leave the first orphaned forever - # (nothing ever completes it), rather than just being overwritten as a status would be. - return [ - BuildStartEndStatusGenerator( - start_formatter=MessageFormatterRenderable("Build started."), - end_formatter=MessageFormatterRenderable("Build done."), - ) - ] - @defer.inlineCallbacks def _get_auth_header(self, props): token = yield props.render(self.token) @@ -85,40 +71,40 @@ def createStatus( output = {"title": context, "summary": description or ""} if state == "pending": - payload = { - "name": context, - "head_sha": sha, - "status": "in_progress", - "details_url": target_url, - "output": output, - "external_id": issue, - } - return (yield self._http.post(base, json=payload, headers=headers)) + # GitHubStatusPush's default generators report both a build being merely queued + # (BuildRequestGenerator, before any worker picks it up) and a build actually + # starting (BuildStartEndStatusGenerator) with state == "pending" -- neither is + # "complete" yet. Only the queued report's target_url points at a buildrequest page + # rather than an actual build, so use that to tell the two apart. + status = "queued" if target_url and "/buildrequests/" in target_url else "in_progress" + payload = {"status": status, "details_url": target_url, "output": output} + else: + # GitHubStatusPush.sendMessage() already collapsed several build results into + # "error"; both "failure" and "error" map to the same GitHub conclusion. + conclusion = "success" if state == "success" else "failure" + payload = {"status": "completed", "conclusion": conclusion, "details_url": target_url, "output": output} - # The check run's id isn't threaded through from the "pending" call above, so look it up - # by name instead of tracking build-run state; one extra GET, but no persisted state. + # 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. 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 not runs: - return None - - # GitHubStatusPush.sendMessage() already collapsed several build results into "error"; - # both "failure" and "error" map to the same GitHub conclusion. - conclusion = "success" if state == "success" else "failure" - payload = { - "status": "completed", - "conclusion": conclusion, - "details_url": target_url, - "output": output, - } - # 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. - return ( - yield self._http.http._do_request( - self._http, "patch", f"{base}/{runs[0]['id']}", json=payload, headers=headers + if runs: + run_id = max(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. + return ( + yield self._http.http._do_request( + self._http, "patch", f"{base}/{run_id}", json=payload, headers=headers + ) ) - ) + + # No existing check run (this is the first report for this build, or GitHub is still + # processing the previous write) -- create one from scratch. + payload = {**payload, "name": context, "head_sha": sha, "external_id": issue} + return (yield self._http.post(base, json=payload, headers=headers))