← back to Cli Printing Press

fix(ci): conversation-resolution check race + duplicate check_run

162d9a32bf983fb88a1df1fe75a61a197090626a · 2026-05-16 15:47:45 -0700 · Trevin Chow

Three flaws in the workflow shipped with this PR that printing-press-library
#587 surfaced (then closed in mvanhorn/printing-press-library#623). Backing
the same fixes into the cli-printing-press copy before this merges:

1. Race condition. synchronize fires the moment a push lands, but GitHub
marks review threads outdated/resolved asynchronously based on the new
file content. Querying immediately races that resolution and can report
stale 'unresolved' for a thread that is about to flip resolved a second
later. Add 8s settle delay on pull_request_target events specifically.

2. Job name collision. The job was named the same as the user-facing
check ('All conversations resolved'), so GitHub Actions auto-created a
check_run with that name reflecting only the job's exit status (always
success). Our explicit POST created a SECOND check_run with the same
name reflecting actual thread state. Both showed up on the SHA. Rename
the job to 'Evaluate review threads' so the auto-created check_run has
a distinct internal name and stays out of the merge widget.

3. Duplicate check_runs across runs. Every workflow run POSTed a new
check_run rather than updating, so a stale failure could sit next to a
fresh success indefinitely. Switch POST to PATCH-if-exists keyed on
(name=All conversations resolved, app=github-actions).

After this lands the merge widget shows exactly one
'All conversations resolved' entry per PR, with state always matching
the latest workflow run.

Files touched

• M .github/workflows/conversation-resolution-check.yml

Diff

commit 162d9a32bf983fb88a1df1fe75a61a197090626a
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 15:47:45 2026 -0700

    fix(ci): conversation-resolution check race + duplicate check_run
    
    Three flaws in the workflow shipped with this PR that printing-press-library
    #587 surfaced (then closed in mvanhorn/printing-press-library#623). Backing
    the same fixes into the cli-printing-press copy before this merges:
    
    1. Race condition. synchronize fires the moment a push lands, but GitHub
    marks review threads outdated/resolved asynchronously based on the new
    file content. Querying immediately races that resolution and can report
    stale 'unresolved' for a thread that is about to flip resolved a second
    later. Add 8s settle delay on pull_request_target events specifically.
    
    2. Job name collision. The job was named the same as the user-facing
    check ('All conversations resolved'), so GitHub Actions auto-created a
    check_run with that name reflecting only the job's exit status (always
    success). Our explicit POST created a SECOND check_run with the same
    name reflecting actual thread state. Both showed up on the SHA. Rename
    the job to 'Evaluate review threads' so the auto-created check_run has
    a distinct internal name and stays out of the merge widget.
    
    3. Duplicate check_runs across runs. Every workflow run POSTed a new
    check_run rather than updating, so a stale failure could sit next to a
    fresh success indefinitely. Switch POST to PATCH-if-exists keyed on
    (name=All conversations resolved, app=github-actions).
    
    After this lands the merge widget shows exactly one
    'All conversations resolved' entry per PR, with state always matching
    the latest workflow run.
---
 .../workflows/conversation-resolution-check.yml    | 42 ++++++++++++++++++++--
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/conversation-resolution-check.yml b/.github/workflows/conversation-resolution-check.yml
index 26aabe3b..ec7a60ac 100644
--- a/.github/workflows/conversation-resolution-check.yml
+++ b/.github/workflows/conversation-resolution-check.yml
@@ -44,7 +44,15 @@ permissions:
 
 jobs:
   evaluate:
-    name: All conversations resolved
+    # Deliberately NOT named "All conversations resolved" — GitHub Actions
+    # auto-creates a check_run for every job using the job name, and we
+    # explicitly POST our own check_run named "All conversations resolved"
+    # at the end of the run. If the job were named the same, the auto
+    # check_run (always success since the script exits 0) would collide
+    # with our POSTed one (which reflects actual thread state). Using a
+    # distinct internal name keeps them separate so only our POSTed
+    # check_run shows up in the merge widget under the user-facing name.
+    name: Evaluate review threads
     # Skip drafts across every event source. The original guard only
     # covered pull_request_target, but pull_request_review and
     # pull_request_review_comment events also carry a pull_request
@@ -88,12 +96,25 @@ jobs:
         env:
           PR_NUMBER: ${{ steps.pr.outputs.number }}
           HEAD_SHA: ${{ steps.pr.outputs.head_sha }}
+          EVENT_NAME: ${{ github.event_name }}
         run: |
           set -euo pipefail
 
           owner="${GITHUB_REPOSITORY%/*}"
           repo="${GITHUB_REPOSITORY#*/}"
 
+          # Synchronize event fires the moment a push lands, but GitHub
+          # marks threads outdated/resolved asynchronously based on the
+          # new file content. Querying immediately races that resolution
+          # and can report a stale "1 unresolved" for a thread that is
+          # about to flip resolved a second later (printing-press-library
+          # #587 hit this; #623 backed the fix into the downstream copy
+          # of this workflow). Short settle delay on push only; other
+          # event types don't race.
+          if [ "${EVENT_NAME}" = "pull_request_target" ]; then
+            sleep 8
+          fi
+
           threads_json="$(mktemp)"
           gh api graphql --paginate \
             -F owner="${owner}" -F repo="${repo}" -F number="${PR_NUMBER}" \
@@ -166,5 +187,20 @@ jobs:
             '{name: $name, head_sha: $head_sha, status: "completed", conclusion: $conclusion, output: {title: $title, summary: $summary}}' \
             > "${payload}"
 
-          gh api -X POST "repos/${GITHUB_REPOSITORY}/check-runs" --input "${payload}" >/dev/null
-          echo "Posted check_run: ${title} (${conclusion}) on ${HEAD_SHA}"
+          # POST creates a new check_run every time, so successive runs
+          # on the same SHA stack up — a stale failure can sit alongside
+          # a fresh success and confuse the merge widget. PATCH-if-exists
+          # makes later runs update the same check_run so only the
+          # latest state is shown. Match by name + app (own runs only)
+          # so we don't trample anyone else's check.
+          existing_id="$(gh api \
+            "repos/${GITHUB_REPOSITORY}/commits/${HEAD_SHA}/check-runs?per_page=100&check_name=All+conversations+resolved" \
+            --jq '[.check_runs[] | select(.app.slug == "github-actions")] | sort_by(.started_at) | .[-1].id // empty')"
+
+          if [ -n "${existing_id}" ]; then
+            gh api -X PATCH "repos/${GITHUB_REPOSITORY}/check-runs/${existing_id}" --input "${payload}" >/dev/null
+            echo "Updated existing check_run ${existing_id}: ${title} (${conclusion}) on ${HEAD_SHA}"
+          else
+            gh api -X POST "repos/${GITHUB_REPOSITORY}/check-runs" --input "${payload}" >/dev/null
+            echo "Posted new check_run: ${title} (${conclusion}) on ${HEAD_SHA}"
+          fi

← f55f57a8 fix(ci): address Greptile review on conversation-resolution  ·  back to Cli Printing Press  ·  fix(ci): scope settle delay to synchronize, strip head_sha f 5b8a8141 →