Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
21 changes: 18 additions & 3 deletions scripts/review-one-pr.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
fi

# Pending CI gate: for normal runs, skip immediately.
Expand Down
56 changes: 56 additions & 0 deletions tests/test_review_one_pr_force_ci_failing.bats
Original file line number Diff line number Diff line change
@@ -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"* ]]
Comment thread
don-petry marked this conversation as resolved.
Resolved
[ "$status" -ne 100 ]
}
Loading