fix(scripts): guard pr-wait on mergeable, not a mergeStateStatus value that cannot exist - #603
Merged
Merged
Conversation
…lue that cannot exist Both twins tested `mergeStateStatus == "CONFLICTING"` and exited 2. That branch was unreachable. CONFLICTING is a value of the separate `mergeable` field (MERGEABLE / CONFLICTING / UNKNOWN); the MergeStateStatus enum is BEHIND, BLOCKED, CLEAN, DIRTY, DRAFT, HAS_HOOKS, UNKNOWN, UNSTABLE, and it spells a conflict DIRTY. So the guard never fired, on any PR, ever -- and what it exists to prevent is precisely what happened. Checked live against PR #395 on 2026-08-04, which is genuinely conflicting: pr-wait.ps1 printed "merge state: DIRTY", sailed past, and went on to `gh pr checks --watch` a PR that GitHub dispatches no workflows for at all, where it would have sat until the timeout. Both fields are now tested. `mergeable` is the one that means this; DIRTY is the same fact from the other side, and either arriving first is enough to stop. GitHub computes mergeability asynchronously and answers UNKNOWN until it has -- asking is what schedules the computation -- so UNKNOWN re-asks a few times rather than being read as either verdict. The exit code and the message are unchanged; the status line now prints both fields, since printing only mergeStateStatus is what kept this invisible. The decision is exposed as `pr-wait.sh --merge-verdict MERGEABLE CLEAN`, the way --summarize-log already exposes the log parsing, and scripts/pr-wait_test.py covers it -- including the pair PR #395 actually returns and every other MergeStateStatus member, so nobody folds the guard back onto the wrong field. Nothing short of a live conflicting PR could catch this before. AGENTS.md said "Check `mergeStateStatus`", which is where the wrong field came from; it now names `mergeable`, shows the JSON a conflicting PR returns, and notes the UNKNOWN window.
Owner
Author
|
/no-changelog Local tooling only: fixes the conflicting-PR guard in scripts/pr-wait.sh and scripts\pr-wait.ps1, plus its unit test and the AGENTS.md line that named the wrong field. Nothing in a shipped artifact changes -- no runtime, service, config, packaging or published docs is touched, and the only files are scripts/pr-wait.*, scripts/pr-wait_test.py and AGENTS.md. |
Contributor
No changelog fragment, deliberately
The check is green and this PR ships with no line in the release notes. The waiver covers the PR, not the commit it was written on — if later work here |
Neaox
enabled auto-merge (squash)
August 4, 2026 02:52
Compatibility Tests✅ No regressions against the baseline
Pass rate (excluding skips and N/A): 100.0% |
Neaox
added a commit
that referenced
this pull request
Aug 4, 2026
…ing it (#607) #603 fixed the conflict guard, but that guard only sees the PR as it was before the wait. Nothing was watching the other end. A PR goes conflicting mid-watch whenever main moves under it, and every tripwire in the script misses that. It does not touch this PR's head, so the stale-head check cannot fire. It dispatches no new checks, so --watch returns its usual green. The closing `gh pr view` asked for state, mergeStateStatus and headRefOid -- not `mergeable` -- so it printed `mergeState=DIRTY`, the exact spelling that hid the original bug, and only printed it: nothing tested it, and the exit code stayed gh's check status. So the run ended by reporting success on a PR that will now never merge. With auto-merge armed that is silent: GitHub just does not merge, and nothing says why. The closing view now asks for `mergeable` too, prints it, and tests both fields through the same merge_verdict the opening guard uses. A run that ends conflicting says so and exits 2. Two things it deliberately does not do: - It does not fire on a PR that is no longer OPEN. A PR that merges mid-watch -- the happy ending -- answers state=MERGED with mergeState=UNKNOWN, which must not read as trouble. Observed on #603 itself, whose pr-wait run closed with exactly that pair. - It does not override a check failure. If something actually failed, exit 1 and the evidence above it are the more actionable signal, so the conflict is reported as a warning and 1 stands. Verified in both twins against a stubbed gh, since the path needs a conflict to arrive mid-run and cannot be reached against a live PR on demand: OPEN + CONFLICTING/DIRTY exits 2 with the new message, OPEN + MERGEABLE/CLEAN exits 0, MERGED + UNKNOWN/UNKNOWN exits 0. Identical output and exit codes from pr-wait.sh and pr-wait.ps1. Live runs still correct on #395 (conflicting, exit 2) and #603 (merged, exit 0). Note this narrows the window rather than closing it: a conflict arriving mid-run is reported when the run ends, not the moment it happens. Detecting it sooner would mean polling mergeability throughout, which is the poll loop the script exists to avoid.
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.
Both
pr-waittwins guarded against a conflicting PR withmergeStateStatus == "CONFLICTING"and exited 2. That branch was unreachable, so the guard never fired on any PR, ever.CONFLICTINGis a value of the separatemergeablefield (MERGEABLE / CONFLICTING / UNKNOWN). It is not a member of theMergeStateStatusenum at all — BEHIND, BLOCKED, CLEAN, DIRTY, DRAFT, HAS_HOOKS, UNKNOWN, UNSTABLE — which spells a conflictDIRTY.What the guard exists to prevent is exactly what happened. PR #395 is genuinely conflicting, and on 2026-08-04
scripts\pr-wait.ps1 395printedmerge state: DIRTY, sailed past the guard, and went on togh pr checks --watcha PR that GitHub dispatches no workflows for — where it would have sat until the timeout. AGENTS.md § Stacked pull requests documents that exact failure mode.The fix
merge_verdict/Get-MergeVerdictcollapses both fields into one decision:mergeablemergeStateStatusCONFLICTINGDIRTYMERGEABLECLEAN/BEHIND/BLOCKED/DRAFT/HAS_HOOKS/UNSTABLEUNKNOWNUNKNOWNmergeableis the field that means this;DIRTYis the same fact from the other side, and either arriving first is enough to stop.GitHub computes mergeability asynchronously and answers
UNKNOWNuntil it has — asking is what schedules the computation — soUNKNOWNre-asks up to 3 times at 2s (PR_WAIT_MERGEABILITY_TRIES/PR_WAIT_MERGEABILITY_INTERVAL) rather than being read as either verdict. If it is still unknown the script proceeds and the existing appear-timeout catches it.Exit code 2 and both message lines are unchanged. The status line now prints both fields, since printing only
mergeStateStatusis what kept this invisible.Worth knowing:
gh pr listreportsUNKNOWNfor most PRs because the list query does not compute mergeability per-PR, whilegh pr viewreturns the real value immediately.pr-waitusespr view. A guard readingpr listwould beunknownon nearly every PR.Coverage
The decision is exposed as
pr-wait.sh --merge-verdict <mergeable> <state>, the way--summarize-logalready exposes the log parsing, andscripts/pr-wait_test.pygains 6 tests — including the pair PR #395 actually returns and every otherMergeStateStatusmember, so nobody folds the guard back onto the wrong field. Nothing short of a live conflicting PR could catch this before.Verified
pr-waittests and 2powershell_asciitests pass; allscripts/*_test.pygreen.scripts/pr-wait.sh 395andscripts\pr-wait.ps1 395both exit 2 with the CONFLICTING message.(mergeable: MERGEABLE, merge state: BLOCKED)before reaching the appear-timeout. The twins stay behaviorally identical.Also
AGENTS.md said "Check
mergeStateStatus", which is where the wrong field came from. It now namesmergeable, shows the JSON a conflicting PR returns, and notes theUNKNOWNwindow.Builds on #600 — no overlap with it beyond sitting on the same files.