fix(pr-merge): refuse to merge a PR whose forge checks are red or unreadable - #2423
Open
Nikita-Guzenko wants to merge 1 commit into
Open
fix(pr-merge): refuse to merge a PR whose forge checks are red or unreadable#2423Nikita-Guzenko wants to merge 1 commit into
Nikita-Guzenko wants to merge 1 commit into
Conversation
…eadable bin/fm-pr-merge.sh called gh-axi pr merge without ever reading the forge's check state, so "never merge a red PR" lived only in AGENTS.md and depended on session attention. On 2026-08-10 that gap shipped a broken commit: the merge-queue run for nguzen/aln pull request 182 failed at 17:54 UTC, the pull request was merged at 17:59 UTC, and the defect in it blocked every production deploy until 2026-08-12. The merge path now reads the verdict itself through the new bin/fm-pr-checks-lib.sh and refuses before recording any state: * failing at least one check the forge reports as failed * unreadable the state could not be established - never treated as a pass Two sources are read, because either can be red alone: the pull request head's status check rollup, and the newest merge-queue attempt, whose checks run again on a combined commit published as gh-readonly-queue/<base>/pr-<n>-<sha>. Only the newest queue attempt is judged, so a superseded red attempt cannot block a re-queued green one. `gh pr checks` is deliberately unused: it exits non-zero both when a check failed and when one is still running, so its exit status cannot classify a result. --allow-failing-checks is the single deliberate override, and it announces itself; AGENTS.md records that using it is a red merge and needs the same explicit captain instruction any red merge needs. Verified against the live incident: the library classifies nguzen/aln#182 as failing (branch and merge-queue) and kunchenguid#183 as green. Tests: tests/fm-pr-merge.test.sh gains the red-branch, red-merge-queue-with- green-branch, superseded-attempt, unreadable, wrong-payload, override and unknown-flag cases; both guards were mutation-checked (blinding the queue read fails only the queue case, accepting an unreadable verdict fails only the unreadable case).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Make
bin/fm-pr-merge.shrefuse to merge a pull request whose forge reports a failing or unreadable check verdict, so "never merge a red PR" is enforced by the one merge path instead of by session attention.Why
On 2026-08-10 the merge-queue run for a production repo's pull request failed at 17:54 UTC and the pull request was merged at 17:59 UTC by the same account; the defect in that commit then blocked every production deploy until 2026-08-12. The merge path never read the forge's check state, so the "never merge a red PR" rule lived only in
AGENTS.mdand depended on attention rather than enforcement.What changed
The merge path now reads the verdict itself through the new
bin/fm-pr-checks-lib.shand refuses before recording any state:Two sources are read, because either can be red on its own:
gh-readonly-queue/<base>/pr-<n>-<sha>.Only the newest queue attempt is judged, so a superseded red attempt cannot block a re-queued green one.
gh pr checksis deliberately unused: it exits non-zero both when a check failed and when one is still running, so its exit status cannot classify a result.--allow-failing-checksis the single deliberate override, and it announces itself;AGENTS.mdrecords that using it is a red merge and needs the same explicit captain instruction any red merge needs.Testing
tests/fm-pr-merge.test.sh— all cases pass, including the new red-branch, red-merge-queue-with-green-branch, superseded-attempt, unreadable, wrong-payload, override, and unknown-flag cases; both guards were mutation-checked.tests/fm-pr-check-security.test.sh— passes.bin/fm-lint.sh(pinned ShellCheck 0.11.0) — clean.bin/fm-test-run.sh --check-coverage— partition intact.Supersedes
This PR replaces #2299, which was built from a base ~125 commits behind current
main. This branch is the same change (commit49d3ab1) cherry-picked cleanly onto current upstreammainwith no conflicts. #2299 will be closed with a cross-link to this PR.