fix(pr-review): FORCE_REVIEW bypasses the ci-failing gate (self-host break-glass #619)

.github/workflows/lint.yml

@@ -110,6 +110,7 @@ jobs:
             tests/test_engine_unavailable_notice.bats \
             tests/test_batch_skip_reporting.bats \
             tests/test_review_one_pr_ci_pending.bats \
+            tests/test_review_one_pr_force_ci_failing.bats \
             tests/test_release_registry.bats \
             tests/test_readme_refresh_helpers.bats \
             tests/test_validate_personas.bats \

scripts/review-one-pr.sh

@@ -139,9 +139,24 @@ echo "    review decision: $REVIEW_DECISION"
 # no-op and does not count it against the MAX_PRS review budget.
 # Reasons that produce a skip: already-reviewed-at-head, ci-failing, ci-pending, changes-requested.
 if [ "$CI_STATUS" = "failing" ]; then
-  echo "    skip: CI checks are failing — will re-evaluate after fixes are pushed"
-  echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"ci-failing\"}"
-  exit 100
+  if [ "${FORCE_REVIEW:-false}" = "true" ]; then
+    # Break-glass (#619): a manual mention / dispatch (FORCE_REVIEW=true) overrides
+    # the ci-failing gate so a fix to the CI gate *itself* can be reviewed and
+    # approved through ring-0 self-host — the one case the pinned-stable design
+    # otherwise can't ship without an out-of-band admin merge. This is safe:
+    #   • FORCE_REVIEW is manual-only — it is true only for repository_dispatch
+    #     (human @mention) or an explicit force_review input; the scheduled sweep
+    #     dispatches with force_review=false, so this never fires event-driven.
+    #   • GitHub's ruleset still blocks the merge on any failing REQUIRED check, so
+    #     an override only unblocks the codeowner-approval gate for PRs whose
+    #     failing checks are non-required (e.g. superseded dev-lead orchestration
+    #     jobs) — it cannot merge a PR with a genuinely failing required check.
+    echo "::warning::force-review: CI is failing but FORCE_REVIEW=true — bypassing the ci-failing gate (break-glass, #619). Required-check failures still block the merge at the ruleset."
+  else
+    echo "    skip: CI checks are failing — will re-evaluate after fixes are pushed"
+    echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"ci-failing\"}"
+    exit 100
+  fi
 fi
 
 # Pending CI gate: for normal runs, skip immediately.
@@ -188,9 +203,13 @@ if [ "$CI_STATUS" = "pending" ]; then
     unset _poll _FORCE_POLL_MAX _FORCE_POLL_SEC _gh_poll_err _gh_poll_err_content
 
     if [ "$CI_STATUS" = "failing" ]; then
-      echo "    skip: CI checks are failing (detected after polling) — will re-evaluate after fixes are pushed"
-      echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"ci-failing\"}"
-      exit 100
+      if [ "${FORCE_REVIEW:-false}" = "true" ]; then
+        echo "::warning::force-review: CI is failing but FORCE_REVIEW=true — bypassing the ci-failing gate (break-glass, #619). Required-check failures still block the merge at the ruleset."
+      else
+        echo "    skip: CI checks are failing (detected after polling) — will re-evaluate after fixes are pushed"
+        echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"ci-failing\"}"
+        exit 100
+      fi
     fi
 
     if [ "$CI_STATUS" = "pending" ]; then
@@ -275,24 +294,6 @@ fi
   fi
 }
 
-# Skip when a human has requested changes, with two guards:
-#   1. FORCE_REVIEW bypasses the skip — mention-triggered runs always proceed so
-#      authors can request a re-review after addressing feedback.
-#   2. Only skip when a CHANGES_REQUESTED review targets the current head SHA.
-#      On repos that don't dismiss stale reviews, reviewDecision can stay
-#      CHANGES_REQUESTED after the author pushes new commits; in that case the
-#      review is stale and the cascade should re-engage with the updated code.
-if [ "$REVIEW_DECISION" = "CHANGES_REQUESTED" ] && [ "${FORCE_REVIEW:-false}" != "true" ]; then
-  CHANGES_REQUESTED_AT_HEAD=$(echo "$PR_SNAPSHOT" | jq -r --arg sha "$PR_HEAD_SHA" '
-    [.reviews[] | select(.state == "CHANGES_REQUESTED" and .commit.oid == $sha)]
-    | if length > 0 then "true" else "false" end
-  ')
-  if [ "$CHANGES_REQUESTED_AT_HEAD" = "true" ]; then
-    echo "    skip: changes requested at current head — awaiting author response before reviewing"
-    echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"changes-requested\"}"
-    exit 100
-  fi
-fi
 # Skip when a human has requested changes, with two guards:
 #   1. FORCE_REVIEW bypasses the skip — mention-triggered runs always proceed so
 #      authors can request a re-review after addressing feedback.

tests/test_review_one_pr_force_ci_failing.bats

@@ -0,0 +1,56 @@
+#!/usr/bin/env bats
+# Guards the #619 break-glass in scripts/review-one-pr.sh:
+# a manual FORCE_REVIEW=true run overrides the ci-failing gate (so a fix to the
+# CI gate itself can be reviewed through ring-0 self-host), while a normal run
+# still skips on failing CI. FORCE_REVIEW is manual-only (repository_dispatch /
+# explicit input); the scheduled sweep dispatches with force_review=false.
+
+setup() {
+  REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  export REVIEW_SCRIPT="$REPO_ROOT/scripts/review-one-pr.sh"
+  export SHA="deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+  export PR_URL="https://github.com/petry-projects/.github-private/pull/999"
+  export TEST_DIR="$BATS_TEST_TMPDIR"
+  mkdir -p "$TEST_DIR/bin"; cd "$TEST_DIR"
+  # A single external check FAILED -> compute_ci_status classifies "failing".
+  cat > "$TEST_DIR/snapshot.json" <<EOF
+{
+  "headRefOid": "$SHA",
+  "statusCheckRollup": [
+    { "name": "dev-lead / dispatch", "status": "COMPLETED", "conclusion": "FAILURE" }
+  ],
+  "reviewDecision": "",
+  "reviews": [],
+  "labels": [],
+  "comments": []
+}
+EOF
+  cat > "$TEST_DIR/bin/gh" <<EOF
+#!/bin/bash
+if [ "\$1" = "pr" ] && [ "\$2" = "view" ]; then cat "$TEST_DIR/snapshot.json"; exit 0; fi
+exit 0
+EOF
+  chmod +x "$TEST_DIR/bin/gh"
+  # Stub engines so a bypassed run can't block on a real CLI.
+  for e in claude copilot gemini; do printf '#!/bin/bash\nexit 0\n' > "$TEST_DIR/bin/$e"; chmod +x "$TEST_DIR/bin/$e"; done
+  export PATH="$TEST_DIR/bin:$PATH"
+  export REVIEW_ENGINE="claude" GH_TOKEN="fake" DRY_RUN="true"
+}
+teardown() { rm -rf "$TEST_DIR"; }
+
+@test "normal run (no FORCE_REVIEW): failing CI → skip reason=ci-failing, exit 100" {
+  unset FORCE_REVIEW
+  run timeout 25 bash "$REVIEW_SCRIPT" "$PR_URL"
+  [ "$status" -eq 100 ]
+  [[ "$output" == *'"reason":"ci-failing"'* ]]
+  [[ "$output" != *"bypassing the ci-failing gate"* ]]
+}
+
+@test "FORCE_REVIEW=true: failing CI → ci-failing gate bypassed (break-glass #619)" {
+  export FORCE_REVIEW="true"
+  run timeout 25 bash "$REVIEW_SCRIPT" "$PR_URL"
+  # Must NOT emit the ci-failing skip; must announce the break-glass bypass.
+  [[ "$output" != *'"reason":"ci-failing"'* ]]
+  [[ "$output" == *"bypassing the ci-failing gate"* ]]
+  [ "$status" -ne 100 ]
+}