fix(622): classify every non-success CI state, and word a negative verdict correctly

Fourth review round, two Low findings, both in the condition-(a) message change I added last round. Both are message-only — every path still denies — but both would have printed something flatly false at the moment someone is trying to understand why a merge is blocked. 1. The filter selected only `pending` and `failure`, but Gitea also has `error` (and `warning`). With `review-verdict/h10=pending` and a build in `error`, the errored build fell out of the set, leaving the verdict as the lone entry — and the hook then claimed "every CI check is green" while a build was erroring. Non-green is now anything that is not `success`. `skipped` is deliberately still counted as GREEN: the image-push job skips on every PR (#593 — a skipped context is not red), so treating it as non-green would have permanently suppressed the H10-specific message in the one situation it exists for. 2. If the sole non-green context was `review-verdict/h10=failure` — i.e. someone reviewed this head and REJECTED it — the message said no verdict existed yet and told the reader to post MERGEABLE. It now branches on the verdict's own state: pending means nobody has reviewed this head, failure/error means it was reviewed and rejected, so resolve the findings. Also folds the two combined-status fetches into one. Verified against eight payloads: verdict-only-pending, verdict-pending-plus-error, verdict-failure-alone, skipped-plus-verdict-pending, two-pending, a real build failure, a warning state, and an unreadable body — each producing the intended message and no other. Both review channels independently confirmed this round that the rename validation is correct at both call sites, that the tests meaningfully pin it, and that no ALLOW/GRANT path became more permissive.
2026-07-25 23:16:30 +02:00
parent 07e1e8cfbc
commit 9f5317bfa5
1 changed files with 25 additions and 9 deletions
@@ -154,21 +154,37 @@ done
 # --- (a) CI combined status must be green (unless deferring to Gitea's own check-gate). ---
 if [ "$mwcs" != "true" ]; then
  [ -n "$sha" ] || decide ask "H6 merge gate: could not resolve PR #$pr head sha to check CI. Verify CI is green before merging."
-  state=$(gq "repos/$owner/$repo/commits/$sha/status" | jq -r '.state // ""' 2>/dev/null || true)
+  cistatus=$(gq "repos/$owner/$repo/commits/$sha/status?limit=100")
+  state=$(printf '%s' "$cistatus" | jq -r '.state // ""' 2>/dev/null || true)
  case "$state" in
    success) : ;;
    "")      decide ask "H6 merge gate: could not read CI status for PR #$pr ($sha). Verify CI is green before merging." ;;
    *)
      # `review-verdict/h10` is itself one of the contexts folded into the COMBINED state, so a PR
      # awaiting its verdict reports combined 'pending' and would otherwise be reported as a CI
-      # problem — sending the reader to look at build logs when the actual missing thing is the
-      # review. Name the real blocker when the verdict is the only thing outstanding.
-      vpending=$(gq "repos/$owner/$repo/commits/$sha/status?limit=100" \
-                 | jq -r '[.statuses[]? | select(.status == "pending" or .status == "failure")] | map(.context) | join(", ")' 2>/dev/null || true)
-      if [ "$vpending" = "review-verdict/h10" ]; then
-        decide deny "H6/H10 merge gate: BLOCKED — every CI check on PR #$pr is green; the only outstanding context is 'review-verdict/h10' on head ${sha:0:7}, i.e. this head has no review verdict yet. Review it and run: scripts/post-review-verdict.sh $pr MERGEABLE"
-      fi
-      decide deny "H6 merge gate: BLOCKED — PR #$pr CI status is '$state', not 'success' (outstanding: ${vpending:-unknown}). Wait for a green build (or pass merge_when_checks_succeed to let Gitea gate it) before merging."
+      # problem — sending the reader to build logs when the missing thing is the review. Name the
+      # real blocker when the verdict is the only thing outstanding.
+      #
+      # "Not green" is anything that is not `success`, NOT just pending/failure: Gitea also has
+      # `error` (and `warning`), and omitting those would let an errored build hide behind the
+      # verdict and produce the flatly false claim "every CI check is green". `skipped` IS treated
+      # as green — the image-push job skips on every PR (ersatztv#593: a skipped context is not red).
+      nongreen=$(printf '%s' "$cistatus" \
+                 | jq -r '[.statuses[]? | select(.status != "success" and .status != "skipped")]
+                          | map("\(.context)=\(.status)") | join(", ")' 2>/dev/null || true)
+      # The verdict's OWN state decides the wording: absent/pending means nobody has reviewed this
+      # head, while failure/error means someone reviewed it and said no. Telling a reviewer to "post
+      # a verdict" when they already posted a BLOCKED one would be actively misleading.
+      vonly=$(printf '%s' "$cistatus" \
+              | jq -r '[.statuses[]? | select(.status != "success" and .status != "skipped")]
+                       | if (length == 1 and .[0].context == "review-verdict/h10") then .[0].status else "" end' 2>/dev/null || true)
+      case "$vonly" in
+        pending)
+          decide deny "H6/H10 merge gate: BLOCKED — every CI check on PR #$pr is green; the only outstanding context is 'review-verdict/h10' on head ${sha:0:7}, i.e. this head has no review verdict yet. Review it and run: scripts/post-review-verdict.sh $pr MERGEABLE" ;;
+        failure|error)
+          decide deny "H6/H10 merge gate: BLOCKED — every CI check on PR #$pr is green, but 'review-verdict/h10' is '$vonly' on head ${sha:0:7}: this head was reviewed and REJECTED. Resolve the findings, then run: scripts/post-review-verdict.sh $pr MERGEABLE" ;;
+      esac
+      decide deny "H6 merge gate: BLOCKED — PR #$pr CI status is '$state', not 'success' (not green: ${nongreen:-unknown}). Wait for a green build (or pass merge_when_checks_succeed to let Gitea gate it) before merging."
      ;;
  esac
 else