fix(pr-validation): scope, cancellation and the commit-list cap

.github/workflows/breaking-change-approval.yml

@@ -384,6 +384,7 @@ jobs:
         # that matters most here.
         if: >-
           needs.detect.result == 'success'
+          && needs.detect.outputs.in-scope == 'true'
           && needs.detect.outputs.has-breaking-changes == 'true'
           && needs.detect.outputs.fingerprint != ''
         env:
@@ -510,6 +511,7 @@ jobs:
         id: codeowners
         if: >-
           needs.detect.result == 'success'
+          && needs.detect.outputs.in-scope == 'true'
           && needs.detect.outputs.has-breaking-changes == 'true'
           && inputs.approval_reviewers == ''
           && inputs.create_approval_environment
@@ -519,10 +521,14 @@ jobs:
           github-token: ${{ secrets.MANAGE_TOKEN || github.token }}
           ref: ${{ needs.detect.outputs.base-ref }}
 
+      # Creating or verifying the Environment is only meaningful when an
+      # approval is actually coming. Out of scope this fails the job over an
+      # Environment nobody is going to use.
       - name: Verify the breaking-approval Environment is protected
         id: environment
         if: >-
           needs.detect.result == 'success'
+          && needs.detect.outputs.in-scope == 'true'
           && needs.detect.outputs.has-breaking-changes == 'true'
           && inputs.require_protected_environment
           && !inputs.dry_run
@@ -593,8 +599,15 @@ jobs:
     name: Approve Breaking Change
     runs-on: ${{ vars.GENERAL_RUNNERS || inputs.runner_type }}
     needs: [detect, request]
+    # `in-scope` is not optional here. Out of scope means no approval is
+    # required — #918 started preserving the caller's real detection so the
+    # acknowledgement could still be reported, and in doing so removed the only
+    # thing keeping this job out of scope. A pull request excluded by a branch
+    # rule, a skip label or an ignored author would wait on an Environment it
+    # has no business touching, and the verdict would discard the answer.
     if: >-
       needs.detect.result == 'success'
+      && needs.detect.outputs.in-scope == 'true'
       && needs.request.result == 'success'
       && needs.detect.outputs.has-breaking-changes == 'true'
       && needs.request.outputs.already-approved != 'true'
@@ -659,6 +672,8 @@ jobs:
           GH_TOKEN: ${{ secrets.MANAGE_TOKEN || github.token }}
           REPO: ${{ github.repository }}
           HEAD_SHA: ${{ needs.detect.outputs.head-sha }}
+          HEAD_REF: ${{ needs.detect.outputs.head-ref }}
+          RUN_ID: ${{ github.run_id }}
           DETECT_RESULT: ${{ needs.detect.result }}
           REQUEST_RESULT: ${{ needs.request.result }}
           APPROVAL_RESULT: ${{ needs.approval.result }}
@@ -700,18 +715,58 @@ jobs:
           # approval did not complete", which is true of the dead run and
           # misleading about the live one.
           #
-          # A cancelled approval is the first and broader signal. Nothing cancels
-          # it except the concurrency group replacing this run, or a human
-          # cancelling by hand, and neither is a decision to report. It matters
-          # on its own because the head does not have to change for a run to be
-          # replaced: editing the description fires `edited`, which queues a new
-          # run on the very same commit — and the comparison below would see
-          # nothing wrong.
-          if [ "$APPROVAL_RESULT" = "cancelled" ]; then
-            state=superseded
-            emit
-            echo "::notice::This run's approval was cancelled — another run is carrying the decision; leaving PR #${PR_NUMBER} alone"
-            exit 0
+          # A cancelled approval is a reason to look, not a verdict on its own.
+          # #912 treated every cancellation as supersession, because a run can
+          # be replaced without the head moving — editing the description fires
+          # `edited`, which queues a new run on the same commit. True, and too
+          # wide: a cancellation with nothing replacing it then reported success
+          # for a revision nobody approved, while `result=failure` sat in the
+          # outputs saying the opposite.
+          #
+          # Supersession has to be established. Either the head moved on (the
+          # comparison below), or a newer run exists for this branch to carry
+          # the decision. Neither means the cancellation stands unresolved,
+          # which does not pass.
+          if [ "$APPROVAL_RESULT" = "cancelled" ] && [ -n "${PR_NUMBER:-}" ]; then
+            # Scoped to this workflow, and to this pull request.
+            # `actions/runs?branch=` returns runs of every workflow on the
+            # branch, and the other ones from the same push start later with
+            # higher ids — so any cancellation would have found a "newer run"
+            # and passed, which is the false success this whole change exists
+            # to remove. Measured on a real branch: a run of Self — Repository
+            # Routines sat above the gate's own run.
+            #
+            # One head branch can carry more than one pull request — same
+            # branch, different bases — so the workflow scope alone still lets
+            # another pull request's run stand in for this one's. `.pull_requests`
+            # carries the run's own pull request and settles it. It comes back
+            # empty for runs from forks, and there the branch scope is the best
+            # evidence left: a run of this workflow, on this head branch, newer
+            # than this one.
+            workflow_id=$(
+              gh api "repos/${REPO}/actions/runs/${RUN_ID}" --jq '.workflow_id' 2>/dev/null || true
+            )
+
+            newer=0
+            if [ -n "$workflow_id" ]; then
+              newer=$(
+                gh api "repos/${REPO}/actions/workflows/${workflow_id}/runs?branch=${HEAD_REF}&per_page=50" \
+                  --jq "[.workflow_runs[]
+                          | select(.id > ${RUN_ID})
+                          | select((.pull_requests | length) == 0
+                                   or any(.pull_requests[]; .number == ${PR_NUMBER}))]
+                         | length" 2>/dev/null || echo 0
+              )
+            fi
+
+            if [ "${newer:-0}" -gt 0 ]; then
+              state=superseded
+              emit
+              echo "::notice::This run's approval was cancelled and ${newer} newer run(s) exist for ${HEAD_REF} — leaving PR #${PR_NUMBER} alone"
+              exit 0
+            fi
+
+            echo "::warning::This run's approval was cancelled and nothing was found replacing it. Not treating that as supersession."
           fi
 
           if [ -n "${PR_NUMBER:-}" ] && [ -n "${HEAD_SHA:-}" ]; then

src/validate/breaking-change-commits/README.md

@@ -48,6 +48,19 @@ and no others:
 mentions the phrase mid-sentence — because the release parser treats neither as
 breaking.
 
+## Pull requests above 250 commits
+
+`/pulls/{n}/commits` caps at 250. The action compares what the endpoint
+returned against the `commits` count the pull request declares, and when the
+two disagree it **exits non-zero** instead of reporting on a partial list — a
+fingerprint built from the first 250 of 300 commits would miss a break living
+in the tail and say "already approved" about a change nobody saw.
+
+So a pull request over 250 commits fails the step rather than passing it. The
+gate reads that as `unknown`, which blocks, and the way through is to split the
+pull request — not to widen the cap, since the endpoint has no page beyond it.
+The same refusal fires if the declared count cannot be read at all.
+
 ## Commit subjects are untrusted text
 
 The subjects go into `GITHUB_OUTPUT` as a multiline value, and whoever opens the

src/validate/breaking-change-commits/resolve.sh

@@ -45,6 +45,30 @@ if ! commits=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}/commits" --paginate \
   exit 1
 fi
 
+# The commits endpoint returns at most 250, pagination or not. GitHub documents
+# this, and answering from a truncated list is worse than not answering: a break
+# living past the cap reads as "no breaking change", and — with reuse — an
+# approved break inside the window plus an omitted one outside it leaves the
+# fingerprint unchanged, so an approval is honoured for a change it never
+# covered.
+#
+# Refuse instead. A gate that cannot see the whole change says so.
+returned=$(grep -c . <<< "${commits}" || true)
+[[ -z "${commits}" ]] && returned=0
+
+if ! declared=$(gh api "repos/${REPO}/pulls/${PR_NUMBER}" --jq '.commits' 2>/dev/null); then
+  printf 'Breaking Change Commits: could not read the commit count of PR #%s in %s.\n' \
+    "${PR_NUMBER}" "${REPO}" >&2
+  exit 1
+fi
+
+if [[ "${returned}" -ne "${declared}" ]]; then
+  printf 'Breaking Change Commits: PR #%s declares %s commits but the API returned %s.\n' \
+    "${PR_NUMBER}" "${declared}" "${returned}" >&2
+  printf 'The commits endpoint caps at 250, so this enumeration is incomplete and a verdict from it would be a guess.\n' >&2
+  exit 1
+fi
+
 shas=''
 subjects=''
 count=0

src/validate/breaking-change-commits/test.sh

@@ -19,6 +19,14 @@ cat > "${TEST_ROOT}/bin/gh" <<'STUB'
 #!/usr/bin/env bash
 case "$*" in
   *"/commits"*) printf '%s\n' ${COMMITS_B64} ;;
+  # The declared total. DECLARED_COMMITS overrides it to simulate the 250 cap,
+  # where the endpoint returns fewer commits than the pull request has.
+  *"/pulls/"*)
+    if [ -n "${DECLARED_COMMITS:-}" ]; then
+      printf '%s\n' "${DECLARED_COMMITS}"
+    else
+      printf '%s\n' ${COMMITS_B64} | grep -c . || true
+    fi ;;
   *) exit 1 ;;
 esac
 STUB
@@ -178,5 +186,40 @@ assert 'adding an ordinary commit does not change it' "${same_a}" "${ordinary_ad
 refute 'adding a breaking commit changes it' "${same_a}" "${added}"
 assert 'no breaking commits means no fingerprint' "${none}" ''
 
+# ----------------- The 250-commit cap -----------------
+# The endpoint returns at most 250 commits however much you paginate. Answering
+# from a truncated list reports "no breaking change" for a break living past the
+# cap — and, with approval reuse, leaves the fingerprint unchanged while the
+# change gained a break nobody approved.
+# `|| capped_status=$?` on the assignment itself: under `set -e` a command
+# substitution that exits non-zero takes the script down before the next line
+# can read `$?`. This is the same trap the `run` helper above already sidesteps.
+capped_status=0
+capped=$(
+  env GH_TOKEN=stub REPO=LerianStudio/example PR_NUMBER=42 \
+    COMMITS_B64="${PLAIN}" DECLARED_COMMITS=300 \
+    bash "${RESOLVER}" 2>/dev/null
+) || capped_status=$?
+if [[ ${capped_status} -ne 0 && -z "${capped}" ]]; then
+  PASSED=$((PASSED + 1))
+  printf 'ok - an incomplete enumeration is refused, not answered\n'
+else
+  FAILED=$((FAILED + 1))
+  printf 'not ok - a truncated commit list produced a verdict (%s)\n' "${capped}"
+fi
+
+complete=$(
+  env GH_TOKEN=stub REPO=LerianStudio/example PR_NUMBER=42 \
+    COMMITS_B64="${BANG} ${PLAIN}" DECLARED_COMMITS=2 \
+    bash "${RESOLVER}" 2>/dev/null | sed -n 's/^count=//p'
+)
+if [[ "${complete}" == '1' ]]; then
+  PASSED=$((PASSED + 1))
+  printf 'ok - a complete enumeration is answered normally\n'
+else
+  FAILED=$((FAILED + 1))
+  printf 'not ok - a complete enumeration was refused (count=%s)\n' "${complete}"
+fi
+
 printf '\n%s passed, %s failed\n' "${PASSED}" "${FAILED}"
 [[ ${FAILED} -eq 0 ]]