feat: implement issue #680 — pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch

.github/scripts/pr-auto-review/README.md

@@ -0,0 +1,51 @@
+# pr-auto-review scripts
+
+Supporting logic for the org-level **PR Auto-Review — Ready Check** reusable
+workflow (`.github/workflows/pr-auto-review-reusable.yml`). All bash/jq decision
+logic lives here so it can be unit-tested with bats
+(`test/workflows/pr-auto-review/`) instead of being trapped inline in YAML.
+
+The reusable workflow checks this repo out at `github.job_workflow_sha` (using
+the `GH_PAT_WORKFLOWS` secret for authentication, so no extra caller permission
+is required) and sources `lib/ready-check.sh` to decide whether a PR's CI checks
+satisfy the passing-gate before the review agent is dispatched. Sourcing from the
+reusable's own commit keeps the predicate matched to the pinned workflow version.
+
+## `lib/ready-check.sh`
+
+Pure, side-effect-free helpers. Source the file, then call:
+
+| Function | Input | Returns |
+|----------|-------|---------|
+| `pr_auto_review_required_contexts` | branch-rules JSON on stdin (`GET /repos/{owner}/{repo}/rules/branches/{branch}`) | prints a compact JSON array of required status-check context names (`[]` if none / non-array) |
+| `pr_auto_review_checks_ready REQUIRED_JSON SELF_NAME` | checks JSON on stdin (`gh pr checks --json bucket,name`) | prints a one-line reason; `0` ready, `1` not ready |
+
+### Passing-gate semantics (issue #680)
+
+The old gate counted **every** check context, so a non-required or **cancelled**
+advisory context (e.g. a superseded `dev-lead / ci-relay` run, cancelled by
+per-PR concurrency) was scored as "not passing" and silently blocked
+auto-dispatch on PRs that were actually mergeable.
+
+`pr_auto_review_checks_ready` instead evaluates only the checks that actually
+gate merge:
+
+- **`REQUIRED_JSON` non-empty** — gate on the required contexts only. The PR is
+  **not ready** if any required context has no matching check reported yet, or a
+  matching check is not in the `pass`/`skipping` bucket (i.e. `fail`, `pending`
+  or `cancel`). Every non-required context — including cancelled advisory
+  runs — is ignored.
+- **`REQUIRED_JSON` empty** — fallback (issue option **B**) for branches with no
+  configured required status checks: block only when a non-self check is
+  `fail` or `pending`; `cancel`, `skipping` and `pass` are non-blocking.
+
+`SELF_NAME` is this workflow's own check-run name; it is excluded from the gate
+so an in-progress run never blocks itself.
+
+### Context name matching
+
+Rulesets store the bare context (e.g. a job name `Lint`, or a third-party status
+context `SonarCloud Code Analysis`), while `gh pr checks` renders Actions checks
+as `"<workflow> / <job>"` (e.g. `CI / Lint`). A check matches a required context
+when the names are equal, or when either ends with `" / <the other>"`, so both
+forms resolve to the same required check.

.github/scripts/pr-auto-review/lib/ready-check.sh

@@ -0,0 +1,95 @@
+#!/usr/bin/env bash
+# Ready-check decision logic for the pr-auto-review reusable workflow.
+#
+# These are pure, side-effect-free functions so they can be unit-tested with
+# bats (see test/workflows/pr-auto-review/ready-check.bats). The reusable
+# workflow sources this file and uses the predicate to decide whether a PR's
+# CI checks satisfy the passing-gate before dispatching the review agent.
+#
+# Contract: see .github/scripts/pr-auto-review/README.md
+# Pins issue #680.
+
+# pr_auto_review_required_contexts
+#   Reads a branch-rules JSON array on stdin (the response of
+#   `GET /repos/{owner}/{repo}/rules/branches/{branch}`) and emits a compact
+#   JSON array of the required status-check context names. Emits `[]` when
+#   there is no required_status_checks rule or the input is not an array
+#   (e.g. a `{"message": "Not Found"}` error body).
+pr_auto_review_required_contexts() {
+  jq -c '
+    if type == "array" then
+      [ .[]
+        | select(.type == "required_status_checks")
+        | .parameters?.required_status_checks?[]?
+        | .context
+        | select(type == "string")
+      ]
+    else
+      []
+    end
+  '
+}
+
+# pr_auto_review_checks_ready REQUIRED_JSON SELF_NAME
+#   Reads a checks JSON array on stdin (the response of
+#   `gh pr checks --json bucket,name`; each element has .name and .bucket) and
+#   decides whether the passing-gate is satisfied. Prints a one-line reason to
+#   stdout. Returns 0 (ready) or 1 (not ready).
+#
+#   REQUIRED_JSON  JSON array of required status-check context names (may be []).
+#   SELF_NAME      name of this workflow's own check run, excluded from the gate
+#                  so an in-progress run does not block itself (may be "").
+#
+#   A check "matches" a required context when their names are equal, or when the
+#   check name ends with " / <context>" (GitHub renders Actions checks as
+#   "<workflow> / <job>" while rulesets store the bare job name), or the reverse.
+#
+#   Decision:
+#     * REQUIRED_JSON non-empty → gate on the required contexts only. Not ready
+#       if any required context has no matching check reported yet, or a matching
+#       check is not in the "pass"/"skipping" bucket. Non-required contexts
+#       (including cancelled advisory runs) are ignored entirely — issue #680.
+#     * REQUIRED_JSON empty → fallback (option B): ignore the required set and
+#       block only when a non-self check is failing or pending; "cancel",
+#       "skipping" and "pass" are treated as non-blocking.
+pr_auto_review_checks_ready() {
+  local required_json="${1:-[]}" self_name="$2" decision reason
+  local result
+  result=$(jq -r \
+    --argjson required "$required_json" \
+    --arg self "$self_name" '
+      def matches($ctx):
+        .name as $n
+        | ($n | type == "string") and (
+            ($n == $ctx)
+            or ($n | endswith(" / " + $ctx))
+            or ($ctx | endswith(" / " + $n))
+          );
+
+      (map(select(.name != $self))) as $checks
+      | if ($required | length) == 0 then
+          ([ $checks[] | select(.bucket == "fail" or .bucket == "pending") ]) as $blocking
+          | if ($blocking | length) > 0 then
+              "not-ready\t\($blocking | length) non-passing check(s) and no required set — skipping"
+            else
+              "ready\tno required set; no failing or pending checks"
+            end
+        else
+          ([ $required[] as $ctx
+             | { ctx: $ctx, runs: [ $checks[] | select(matches($ctx)) ] } ]) as $req
+          | ([ $req[] | select(.runs | length == 0) | .ctx ]) as $missing
+          | ([ $req[] | .runs[] | select(.bucket != "pass" and .bucket != "skipping") ]) as $notpassing
+          | if ($missing | length) > 0 then
+              "not-ready\trequired check(s) not reported yet: \($missing | join(", "))"
+            elif ($notpassing | length) > 0 then
+              "not-ready\t\($notpassing | length) required check(s) not yet passing — skipping"
+            else
+              "ready\tall \($req | length) required check(s) passing"
+            end
+        end
+    ')
+  decision="${result%%$'\t'*}"
+  reason="${result#*$'\t'}"
+  echo "$reason"
+  [[ "$decision" == "ready" ]]
+}

.github/workflows/pr-auto-review-reusable.yml

@@ -5,7 +5,9 @@
 #
 # Fires a review-agent dispatch when a PR meets ALL of the following criteria:
 #   1. PR is open and not a draft
-#   2. All CI checks are completed and passing (no pending / failing)
+#   2. All REQUIRED status checks are completed and passing (no pending /
+#      failing / cancelled). Non-required advisory contexts are ignored — see
+#      .github/scripts/pr-auto-review/lib/ready-check.sh (issue #680).
 #   3. Effective review decision is not CHANGES_REQUESTED
 #   4. No unresolved review threads
 #
@@ -99,6 +101,21 @@ jobs:
           echo "skip=false" >> "$GITHUB_OUTPUT"
           echo "PR URL: $PR_URL"
 
+      # Source the ready-check decision logic from this reusable's own commit so
+      # the passing-gate always matches the pinned workflow version. Authenticated
+      # with GH_PAT_WORKFLOWS (already required by this workflow) so no extra
+      # `contents:` permission is needed from callers.
+      - name: Checkout ready-check tooling
+        if: steps.pr.outputs.skip != 'true'
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          repository: petry-projects/.github
+          ref: ${{ github.job_workflow_sha }}
+          path: .pr-auto-review-tooling
+          token: ${{ secrets.GH_PAT_WORKFLOWS }}
+          fetch-depth: 1
+          persist-credentials: false
+
       - name: Check PR readiness criteria
         id: criteria
         if: steps.pr.outputs.skip != 'true'
@@ -108,18 +125,23 @@ jobs:
         run: |
           set -euo pipefail
 
+          # shellcheck source=/dev/null
+          . .pr-auto-review-tooling/.github/scripts/pr-auto-review/lib/ready-check.sh
+
           # Derive the base repository (owner/repo) from the PR URL.
           # gh pr view --json does not expose baseRepository; parsing the URL
           # is simpler and works for both same-repo and fork PRs.
           REPO=$(echo "$PR_URL" | sed 's|https://github.com/||; s|/pull/.*||')
 
-          # Fetch PR metadata in one call, including the effective review decision.
+          # Fetch PR metadata in one call, including the effective review decision
+          # and the base branch (needed to resolve required status checks).
           PR_META=$(gh pr view "$PR_URL" \
-            --json state,isDraft,number,reviewDecision)
+            --json state,isDraft,number,reviewDecision,baseRefName)
           STATE=$(echo "$PR_META" | jq -r '.state')
           IS_DRAFT=$(echo "$PR_META" | jq -r '.isDraft')
           PR_NUMBER=$(echo "$PR_META" | jq -r '.number')
           REVIEW_DECISION=$(echo "$PR_META" | jq -r '.reviewDecision // ""')
+          BASE_BRANCH=$(echo "$PR_META" | jq -r '.baseRefName')
 
           # 1. PR must be open and not a draft.
           if [ "$STATE" != "OPEN" ] || [ "$IS_DRAFT" = "true" ]; then
@@ -129,7 +151,7 @@ jobs:
           fi
           echo "PR is open and not a draft ✓"
 
-          # 2. All CI checks must be completed and passing.
+          # 2. All REQUIRED CI checks must be completed and passing.
           #    gh pr checks --json may exit non-zero when checks are
           #    failing/pending but still writes the JSON payload to stdout;
           #    use || true so set -e doesn't discard that output.
@@ -144,26 +166,35 @@ jobs:
             exit 0
           fi
 
+          # Resolve the base branch's required status-check contexts. The gate
+          # counts ONLY these — non-required and cancelled advisory contexts
+          # (e.g. a superseded "dev-lead / ci-relay" run) must not block dispatch
+          # (issue #680). An empty set falls back to a fail/pending-only gate.
+          # gh api writes the error body to stdout on a 4xx (e.g. no ruleset), so
+          # capture it inside the `if` condition — a failing pipeline concatenated
+          # with a fallback would otherwise yield two JSON values.
+          if RULES_JSON=$(gh api "/repos/${REPO}/rules/branches/${BASE_BRANCH}" 2>/dev/null); then
+            REQUIRED_JSON=$(printf '%s' "$RULES_JSON" | pr_auto_review_required_contexts 2>/dev/null || echo "[]")
+          else
+            REQUIRED_JSON="[]"
+          fi
+          if [ -z "${REQUIRED_JSON}" ]; then
+            REQUIRED_JSON="[]"
+          fi
+
           # Get the name of this workflow's own check run so it can be excluded
           # from the gate — an in-progress run shows as "pending" and would
           # otherwise block itself on every trigger.
           SELF_CHECK=$(gh api \
             "/repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/jobs" \
             --jq '.jobs[0].name // empty' 2>/dev/null || echo "")
 
-          # Use double-quoted jq expression with \$self so the shell produces
-          # a literal "$self" for jq without triggering SC2016 (which flags
-          # shell variables in single-quoted strings). $self is a jq variable.
-          NOT_PASSING=$(echo "$CHECKS" | jq \
-            --arg self "$SELF_CHECK" \
-            "map(select(.name != \$self)) | map(select(.bucket != \"pass\" and .bucket != \"skipping\")) | length")
-
-          if [ "$NOT_PASSING" -gt 0 ]; then
-            echo "$NOT_PASSING of $TOTAL check(s) are not yet passing — skipping"
+          if ! REASON=$(echo "$CHECKS" | pr_auto_review_checks_ready "$REQUIRED_JSON" "$SELF_CHECK"); then
+            echo "$REASON"
             echo "ready=false" >> "$GITHUB_OUTPUT"
             exit 0
           fi
-          echo "All $TOTAL CI check(s) passing ✓"
+          echo "$REASON ✓"
 
           # 3. Effective review decision must not be CHANGES_REQUESTED.
           #    reviewDecision reflects the aggregate current state (accounts for

.github/workflows/pr-auto-review-tests.yml

@@ -0,0 +1,66 @@
+# Quality gates for the reusable pr-auto-review workflow and its supporting
+# ready-check scripts.
+#
+# Triggered on any PR that touches:
+#   - .github/workflows/pr-auto-review*.yml
+#   - .github/scripts/pr-auto-review/**
+#   - test/workflows/pr-auto-review/**
+#   - standards/workflows/pr-auto-review.yml
+#
+# Gates (all must pass before merge):
+#   1. shellcheck — static analysis for the ready-check lib
+#   2. bats       — unit tests for the passing-gate decision logic
+#
+# Standard: this workflow enforces the contract documented in
+# .github/scripts/pr-auto-review/README.md.
+
+name: PR Auto-Review Tests
+
+on:
+  pull_request:
+    paths:
+      - '.github/workflows/pr-auto-review*.yml'
+      - '.github/scripts/pr-auto-review/**'
+      - 'test/workflows/pr-auto-review/**'
+      - 'standards/workflows/pr-auto-review.yml'
+  push:
+    branches:
+      - main
+    paths:
+      - '.github/workflows/pr-auto-review*.yml'
+      - '.github/scripts/pr-auto-review/**'
+      - 'test/workflows/pr-auto-review/**'
+      - 'standards/workflows/pr-auto-review.yml'
+
+permissions:
+  contents: read
+
+concurrency:
+  group: pr-auto-review-tests-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  test:
+    name: Lint and bats
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          fetch-depth: 1
+
+      - name: Install bats, shellcheck, and jq
+        run: |
+          set -euo pipefail
+          sudo apt-get update -qq
+          sudo apt-get install -y --no-install-recommends bats shellcheck jq
+
+      - name: shellcheck
+        run: |
+          set -euo pipefail
+          shellcheck -x \
+            .github/scripts/pr-auto-review/lib/ready-check.sh
+
+      - name: Run bats suite
+        run: bats --print-output-on-failure test/workflows/pr-auto-review/

test/workflows/pr-auto-review/helpers/setup.bash

@@ -0,0 +1,9 @@
+#!/usr/bin/env bash
+# Common test helpers for the pr-auto-review bats suite.
+
+# Repo root, regardless of where bats is invoked from.
+TT_REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/../../../.." && pwd)"
+export TT_REPO_ROOT
+
+TT_SCRIPTS_DIR="${TT_REPO_ROOT}/.github/scripts/pr-auto-review"
+export TT_SCRIPTS_DIR