[object Object]

← back to Cli Printing Press

fix(ci): use job exit-code instead of POST/PATCH for check_run state

9433241780080b9aa21e0fd23244a1034a5533e0 · 2026-05-16 16:07:02 -0700 · Trevin Chow

Mirrors printing-press-library#623 redesign. POST/PATCH approach
hits HTTP 403 in CI as of Feb 2025: GitHub Actions runtime rejects
workflow-side modification of check_run status/conclusion. Switch
to relying on the auto-created check_run for the job (its conclusion
mirrors the job's exit status) and exit 1 when threads are unresolved.

Net: simpler workflow, no API calls, no duplicates, behavior matches
the post-Feb-2025 GitHub Actions runtime.

Files touched

Diff

commit 9433241780080b9aa21e0fd23244a1034a5533e0
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 16:07:02 2026 -0700

    fix(ci): use job exit-code instead of POST/PATCH for check_run state
    
    Mirrors printing-press-library#623 redesign. POST/PATCH approach
    hits HTTP 403 in CI as of Feb 2025: GitHub Actions runtime rejects
    workflow-side modification of check_run status/conclusion. Switch
    to relying on the auto-created check_run for the job (its conclusion
    mirrors the job's exit status) and exit 1 when threads are unresolved.
    
    Net: simpler workflow, no API calls, no duplicates, behavior matches
    the post-Feb-2025 GitHub Actions runtime.
---
 .../workflows/conversation-resolution-check.yml    | 110 ++++++++-------------
 1 file changed, 43 insertions(+), 67 deletions(-)

diff --git a/.github/workflows/conversation-resolution-check.yml b/.github/workflows/conversation-resolution-check.yml
index df47379a..7d234f4a 100644
--- a/.github/workflows/conversation-resolution-check.yml
+++ b/.github/workflows/conversation-resolution-check.yml
@@ -38,21 +38,20 @@ on:
         required: true
 
 permissions:
-  checks: write
   contents: read
   pull-requests: read
 
 jobs:
   evaluate:
-    # 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
+    # GitHub Actions auto-creates a check_run with this exact name when
+    # the job runs, and its conclusion mirrors the job's exit status.
+    # We rely on that auto-created check_run rather than POSTing our
+    # own because as of Feb 2025 the GitHub Actions runtime rejects
+    # external workflow attempts to update check_run status/conclusion
+    # via GITHUB_TOKEN (HTTP 403). The job exits non-zero when threads
+    # are unresolved, the auto check_run goes red, the merge widget
+    # shows the failure — no API calls, no duplicates, no PATCH races.
+    name: All conversations resolved
     # 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
@@ -149,65 +148,42 @@ jobs:
           echo "total=${total} unresolved=${unresolved} (active=${unresolved_active} outdated=${unresolved_outdated})"
 
           if [ "${unresolved}" -eq 0 ]; then
-            conclusion="success"
-            title="All ${total} conversation(s) resolved"
-            summary=$'No unresolved review threads on this PR.'
-          else
-            conclusion="failure"
-            if [ "${unresolved_active}" -gt 0 ] && [ "${unresolved_outdated}" -gt 0 ]; then
-              title="${unresolved} unresolved (${unresolved_active} on current code, ${unresolved_outdated} outdated)"
-            elif [ "${unresolved_active}" -gt 0 ]; then
-              title="${unresolved} unresolved conversation(s)"
-            else
-              title="${unresolved} unresolved on outdated lines (likely addressed)"
-            fi
-
-            summary=$(jq -r '
-              def listing:
-                [.[] | select(.isResolved == false)] as $u |
-                ($u | length) as $count |
-                "**" + ($count|tostring) + " unresolved review thread(s):**\n\n" +
-                ([$u[0:10] | .[] |
-                  "- `" + .path + (if .line then ":" + (.line | tostring) else "" end) + "` by `" +
-                  (.comments.nodes[0].author.login // "unknown") + "`" +
-                  (if .isOutdated then " *(outdated — line no longer exists; likely addressed by a later commit)*" else "" end)
-                ] | join("\n")) +
-                (if $count > 10 then "\n\n_… and " + ($count - 10 | tostring) + " more._" else "" end) +
-                "\n\n---\n\n**To unblock the merge**, click \"Resolve conversation\" on each unresolved thread in the GitHub UI. Per repo convention every Greptile finding should be resolved (in code, or with a concrete reply explaining the deferral) before merge."
-              ;
-              listing
-            ' "${threads_json}")
+            printf '## All %s conversation(s) resolved\n\nNo unresolved review threads on this PR.\n' \
+              "${total}" >> "${GITHUB_STEP_SUMMARY}"
+            echo "PASS: 0 unresolved threads on ${HEAD_SHA}"
+            exit 0
           fi
 
-          payload="$(mktemp)"
-          jq -n \
-            --arg name "All conversations resolved" \
-            --arg head_sha "${HEAD_SHA}" \
-            --arg conclusion "${conclusion}" \
-            --arg title "${title}" \
-            --arg summary "${summary}" \
-            '{name: $name, head_sha: $head_sha, status: "completed", conclusion: $conclusion, output: {title: $title, summary: $summary}}' \
-            > "${payload}"
+          # Build the detailed thread listing once, reused for the step
+          # summary (rendered on the workflow run page) and the
+          # ::error:: annotation (rendered in the check_run UI).
+          listing="$(jq -r '
+            [.[] | select(.isResolved == false)] as $u |
+            ($u | length) as $count |
+            ([$u[0:10] | .[] |
+              "- `" + .path + (if .line then ":" + (.line | tostring) else "" end) + "` by `" +
+              (.comments.nodes[0].author.login // "unknown") + "`" +
+              (if .isOutdated then " *(outdated — line no longer exists; likely addressed by a later commit)*" else "" end)
+            ] | join("\n")) +
+            (if $count > 10 then "\n\n_… and " + ($count - 10 | tostring) + " more._" else "")
+          ' "${threads_json}")"
 
-          # 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
-            # head_sha is a POST-only field for /check-runs; PATCH rejects
-            # unknown body params with 422, which would `set -e` us out
-            # (greptile P1 on #623).
-            patch_payload="$(mktemp)"
-            jq 'del(.head_sha)' "${payload}" > "${patch_payload}"
-            gh api -X PATCH "repos/${GITHUB_REPOSITORY}/check-runs/${existing_id}" --input "${patch_payload}" >/dev/null
-            echo "Updated existing check_run ${existing_id}: ${title} (${conclusion}) on ${HEAD_SHA}"
+          if [ "${unresolved_active}" -gt 0 ] && [ "${unresolved_outdated}" -gt 0 ]; then
+            heading="${unresolved} unresolved (${unresolved_active} on current code, ${unresolved_outdated} outdated)"
+          elif [ "${unresolved_active}" -gt 0 ]; then
+            heading="${unresolved} unresolved conversation(s)"
           else
-            gh api -X POST "repos/${GITHUB_REPOSITORY}/check-runs" --input "${payload}" >/dev/null
-            echo "Posted new check_run: ${title} (${conclusion}) on ${HEAD_SHA}"
+            heading="${unresolved} unresolved on outdated lines (likely addressed)"
           fi
+
+          {
+            printf '## %s\n\n%s\n\n---\n\n' "${heading}" "${listing}"
+            printf '**To unblock the merge**, click "Resolve conversation" on each unresolved thread in the GitHub UI. Per repo convention every Greptile finding should be resolved (in code, or with a concrete reply explaining the deferral) before merge.\n'
+          } >> "${GITHUB_STEP_SUMMARY}"
+
+          # ::error:: annotation surfaces in the check_run UI without
+          # needing a POST. The job's non-zero exit drives the auto
+          # check_run's conclusion to failure, which is what the merge
+          # widget displays.
+          echo "::error::${heading} on PR #${PR_NUMBER}. See the workflow run summary for the list. Click \"Resolve conversation\" on each thread to unblock."
+          exit 1

← c96ac5da fix(cli): writeThroughCache caches single-object responses k  ·  back to Cli Printing Press  ·  fix(ci): close jq if-then-else with end so >10-thread path d f7365c26 →