[object Object]

← back to Cli Printing Press

fix(ci): address Greptile review on conversation-resolution check

f55f57a8d908c5555d4aa3d989fb9ec56abf6b0a · 2026-05-16 14:41:23 -0700 · Trevin Chow

Three findings from Greptile on this PR:

1. (P1) Failure summary cited the Mergify condition by name in
.mergify.yml, but #1536 has not merged so that condition does not
exist on main yet. Replaced with a repo-convention message accurate
both today and after #1536 lands.

2. (P2) The --arg PR_NUMBER binding was passed to jq but no longer
referenced in the listing function (I removed the use in the
cli-printing-press port but left the binding). Dead arg, dropped.

3. (P2) The draft-PR guard only checked pull_request_target events,
so reviews and comments on draft PRs still fired the workflow.
Rewrote the guard to skip drafts across every PR event source while
allowing workflow_dispatch through as an explicit manual ask.

Files touched

Diff

commit f55f57a8d908c5555d4aa3d989fb9ec56abf6b0a
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat May 16 14:41:23 2026 -0700

    fix(ci): address Greptile review on conversation-resolution check
    
    Three findings from Greptile on this PR:
    
    1. (P1) Failure summary cited the Mergify condition by name in
    .mergify.yml, but #1536 has not merged so that condition does not
    exist on main yet. Replaced with a repo-convention message accurate
    both today and after #1536 lands.
    
    2. (P2) The --arg PR_NUMBER binding was passed to jq but no longer
    referenced in the listing function (I removed the use in the
    cli-printing-press port but left the binding). Dead arg, dropped.
    
    3. (P2) The draft-PR guard only checked pull_request_target events,
    so reviews and comments on draft PRs still fired the workflow.
    Rewrote the guard to skip drafts across every PR event source while
    allowing workflow_dispatch through as an explicit manual ask.
---
 .github/workflows/conversation-resolution-check.yml | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/conversation-resolution-check.yml b/.github/workflows/conversation-resolution-check.yml
index fe9c78b6..26aabe3b 100644
--- a/.github/workflows/conversation-resolution-check.yml
+++ b/.github/workflows/conversation-resolution-check.yml
@@ -45,7 +45,14 @@ permissions:
 jobs:
   evaluate:
     name: All conversations resolved
-    if: github.event_name != 'pull_request_target' || github.event.pull_request.draft == false
+    # 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
+    # object with .draft, and a review/comment on an in-progress draft
+    # should not post a failure check_run. workflow_dispatch has no
+    # pull_request in the payload, so we fall through and let the
+    # job run; manual dispatch is an explicit ask.
+    if: github.event.pull_request == null || github.event.pull_request.draft == false
     runs-on: ubuntu-latest
     timeout-minutes: 5
     # Coalesce bursts (a push + Greptile review + several thread resolves can
@@ -143,10 +150,10 @@ jobs:
                   (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.\n\nMerge is gated by Mergify (`#review-threads-unresolved = 0` in `.mergify.yml`); this check is the visibility layer so the blocker shows up here regardless of whether the `ready-to-merge` label has been applied yet."
+                "\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
-            ' --arg PR_NUMBER "${PR_NUMBER}" "${threads_json}")
+            ' "${threads_json}")
           fi
 
           payload="$(mktemp)"

← 21ae8a54 feat(ci): surface unresolved review threads as a status chec  ·  back to Cli Printing Press  ·  fix(ci): conversation-resolution check race + duplicate chec 162d9a32 →