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

[don-petry]

Problem

FORCE_REVIEW already overrides the advisory-bot, CHANGES_REQUESTED, idempotency, and cap gates for manual mention/dispatch runs — but not the ci-failing gate. So a fix to the CI gate itself can't be reviewed/approved through ring-0 self-host when the buggy gate marks its own fix PR as failing: a hard self-host deadlock with no break-glass (#619). #608 dissolved the specific CANCELLED case; the structural gap remained for the next CI-gate bug (and it's the same reason PRs carrying a failing non-required check — e.g. dev-lead/dispatch — sit un-approvable).

Fix

When FORCE_REVIEW=true, log a loud warning and proceed past the ci-failing gate instead of skipping.

Safe by construction:

• FORCE_REVIEW is manual-only — true only for repository_dispatch (human @mention) or an explicit force_review input. The scheduled sweep dispatches with force_review=false (sweep-stuck-reviews.sh: 'never force_review'), 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). It can't merge a genuinely-broken PR.

Tests

New test_review_one_pr_force_ci_failing.bats: normal run still skips reason=ci-failing (exit 100); FORCE_REVIEW=true bypasses it. Existing test_review_one_pr_ci_pending + test_ci_status (47) stay green. bash -n + shellcheck clean.

Resolves the structural half of #619 (#608 handled the CANCELLED half).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s

Summary by CodeRabbit

• New Features

  • Added an emergency override that allows reviews to proceed when CI is failing by setting FORCE_REVIEW=true.
  • Displays a warning when the CI-failing safeguard is bypassed.

• Bug Fixes

  • Preserved the existing behavior of skipping reviews when CI is failing unless the override is enabled.

• Tests

  • Added coverage for both standard and forced review flows.

[coderabbitai]

Warning

Review limit reached

@don-petry, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 10c272c9-2888-47ee-a51a-1d6fd0d48af0

📥 Commits

Reviewing files that changed from the base of the PR and between d6c940e and 1257363.

📒 Files selected for processing (2)

• .github/workflows/lint.yml
• scripts/review-one-pr.sh

📝 Walkthrough

Walkthrough

The review script now bypasses the CI-failing skip when FORCE_REVIEW=true. New BATS tests verify both normal skipping and forced execution, and the lint workflow runs the added test file.

Changes

CI-failing review behavior

Layer / File(s)	Summary
FORCE_REVIEW gate scripts/review-one-pr.sh	Failing CI continues to skip by default, while FORCE_REVIEW=true logs a bypass and continues without the skip sentinel.
Break-glass behavior validation tests/test_review_one_pr_force_ci_failing.bats, .github/workflows/lint.yml	BATS tests cover default and forced runs with stubbed CI and review engines; lint executes the new test suite.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• petry-projects/.github-private#899: Also changes CI-based skip handling and ci-failing reason propagation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: FORCE_REVIEW bypassing the ci-failing gate, with an apt break-glass context.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pr-review-force-review-ci-gate-619

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[gemini-code-assist]

Code Review

This pull request introduces a "break-glass" override (FORCE_REVIEW=true) in scripts/review-one-pr.sh to bypass the ci-failing gate, allowing PRs that fix the CI gate itself to be reviewed and approved. It also adds a new BATS test suite to verify this behavior. The feedback suggests using $BATS_TEST_TMPDIR instead of $BATS_TMPDIR in the new test file to ensure automatic cleanup and prevent parallel execution conflicts.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed.

[Copilot]

Pull request overview

This PR adds a manual “break-glass” path to the pr-review self-hosted reviewer so that FORCE_REVIEW=true can proceed even when the CI gate classifies the PR as ci-failing, preventing self-host deadlocks when the CI gate itself needs fixing.

Changes:

• Bypass the ci-failing skip gate when FORCE_REVIEW=true, emitting a loud workflow warning instead of exiting with the skip sentinel.
• Add a new Bats regression test to lock in the FORCE_REVIEW bypass behavior for ci-failing classification.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
scripts/review-one-pr.sh	Adds FORCE_REVIEW break-glass behavior for ci-failing skips (warn + proceed).
tests/test_review_one_pr_force_ci_failing.bats	Adds a regression guard ensuring FORCE_REVIEW bypasses the ci-failing gate.

Comments suppressed due to low confidence (1)

scripts/review-one-pr.sh:126

• The new FORCE_REVIEW bypass only covers the initial CI_STATUS==failing case. If CI starts as pending and (during the FORCE_REVIEW polling loop) transitions to failing, the script still exits 100 in the post-poll block ("CI checks are failing (detected after polling)"), which reintroduces the deadlock for mention-triggered runs where checks settle into a failing state after polling.

#      passing — all items completed AND every conclusion is SUCCESS, SKIPPED, or
#                NEUTRAL (or rollup empty). SKIPPED covers path-filtered checks;
#                NEUTRAL covers informational checks that don't gate merging.
#      failing — anything else (FAILURE, ACTION_REQUIRED, TIMED_OUT, CANCELLED,
#                STALE, STARTUP_FAILURE, or unknown conclusions)

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

Issues addressed: 0
Files changed: None
Skipped (informational): 0
No actionable issues found. SonarCloud quality gate passed with 0 new issues and 0 security hotspots. No Tier 1 blockers present in CI or reviews.
```

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 92a55437bfa1bff2dc917567466c1b5b0787df49
Review mode: triage-approved (single reviewer)

Summary

Adds a scoped break-glass to review-one-pr.sh: FORCE_REVIEW=true (manual-only) bypasses the ci-failing skip gate with a loud ::warning instead of exit 100, resolving the structural half of the #619 self-host deadlock. Includes a new bats regression suite (registered in lint.yml) covering both the normal skip and the bypass.

Linked issue analysis

#619 (self-host deadlock: force_review bypasses the advisory gate but not the CI gate) — the structural half is substantively addressed by extending FORCE_REVIEW to the ci-failing gate, exactly as the issue's proposed fix #1 describes. The CANCELLED-classification half was already handled by #608 (closed). Safety claims were verified in-repo: FORCE_REVIEW is true only for repository_dispatch (human @mention listener) or an explicit force_review input (pr-review.yml:195); the scheduled sweep dispatches via workflow_dispatch with no force flag (sweep-stuck-reviews.sh: 'never force_review'), so the bypass can never fire event-driven. GitHub rulesets still block merges on failing required checks, so the override only unblocks the review step.

Findings

• Verified locally at head 92a5543: bash -n clean; shellcheck shows only pre-existing info-level notes on untouched lines; the new bats suite passes (2/2).
• Prior review feedback applied: Gemini's BATS_TEST_TMPDIR suggestion and Copilot's 'assert status != 100' suggestion are both in the final test (the Copilot thread is unmarked but substantively resolved). CodeRabbit approved at head.
• Non-blocking gap (carried from Copilot's low-confidence note): if CI starts pending and settles to failing during the FORCE_REVIEW polling loop, the post-poll block (review-one-pr.sh, 'detected after polling') still exits 100 — the bypass does not cover that path. Suggest a follow-up to apply the same break-glass there for full coverage of #619.
• Nit (non-blocking): the test's teardown rm -rf of BATS_TEST_TMPDIR is redundant (bats auto-cleans) but harmless.
• Secret scan: run_secret_scanning MCP tool unavailable in this run; gitleaks CI check passed and the diff contains no credential-like content (test uses GH_TOKEN="fake" stub only).

CI status

All validating checks green at head 92a5543 (Lint, ShellCheck, bats, unit-tests, CodeQL, SonarCloud quality gate, gitleaks, AgentShield, holdout-guard, template-drift, etc.). Two CANCELLED non-required dev-lead orchestration checks (dispatch / ci-relay) from a superseded concurrency run — the exact benign pattern documented in #608/#619; they do not gate the merge.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/review-one-pr.sh (1)

311-328: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove duplicate code block.

This entire block evaluating CHANGES_REQUESTED is a verbatim duplicate of the block immediately preceding it (lines 293-310). It should be removed to reduce clutter and maintainability overhead.

♻️ Proposed refactor to delete the duplicate block

-# 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

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/review-one-pr.sh` around lines 311 - 328, Remove the duplicate
CHANGES_REQUESTED handling block shown in the diff, retaining the identical
block immediately preceding it. Preserve the existing FORCE_REVIEW bypass,
current-head SHA validation, skip output, and exit behavior in the retained
block.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/review-one-pr.sh`:
- Around line 142-159: Update the post-polling ci-failing check within the
polling loop to honor FORCE_REVIEW=true: replace its unconditional skip message
and exit 100 with a warning that records the failing status and allows execution
to continue. Keep the normal non-forced CI-failing behavior unchanged outside
this FORCE_REVIEW-only path.

---

Outside diff comments:
In `@scripts/review-one-pr.sh`:
- Around line 311-328: Remove the duplicate CHANGES_REQUESTED handling block
shown in the diff, retaining the identical block immediately preceding it.
Preserve the existing FORCE_REVIEW bypass, current-head SHA validation, skip
output, and exit behavior in the retained block.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 94d7d9a0-8924-45d8-9379-8982fd704306

📥 Commits

Reviewing files that changed from the base of the PR and between 116432f and d6c940e.

📒 Files selected for processing (3)

• .github/workflows/lint.yml
• scripts/review-one-pr.sh
• tests/test_review_one_pr_force_ci_failing.bats

[don-petry]

@coderabbitai resolve

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1230
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-07-14T18:02:34Z

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

**Analysis of all review findings:**
| Finding | Status | Details |
|---------|--------|---------|
| SonarCloud Quality Gate | ✅ Passed | 0 new issues, 0 security hotspots |
| CodeRabbit: Duplicate CHANGES_REQUESTED block | ✅ Fixed | Removed in commit `2f88530`, thread already resolved |
| CodeRabbit: Post-polling ci-failing check | ✅ Fixed | Now honors `FORCE_REVIEW=true` bypass, thread already resolved |
| Copilot: Test missing status assertion | ✅ Fixed | Line 55 asserts `[ "$status" -ne 100 ]`, thread now resolved |
| Gemini: Use $BATS_TEST_TMPDIR | ✅ Implemented | Already using in test setup (line 13) |
| CI Checks | ✅ All passing | Lint, ShellCheck, bats, CodeQL, SonarCloud all SUCCESS |
**No Tier 1 blockers:** No failed CI checks or CHANGES_REQUESTED reviews — PR is ready.