Skip to content

fix(scripts): catch a PR that goes conflicting while pr-wait is watching it - #607

Merged
Neaox merged 1 commit into
mainfrom
fix/pr-wait-conflict-after-watch
Aug 4, 2026
Merged

fix(scripts): catch a PR that goes conflicting while pr-wait is watching it#607
Neaox merged 1 commit into
mainfrom
fix/pr-wait-conflict-after-watch

Conversation

@Neaox

@Neaox Neaox commented Aug 4, 2026

Copy link
Copy Markdown
Owner

fix(scripts): catch a PR that goes conflicting while pr-wait is watching it

#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 fix(scripts): guard pr-wait on mergeable, not a mergeStateStatus value that cannot exist #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.

…ing it

#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.
@Neaox

Neaox commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

/no-changelog Local tooling only: extends the pr-wait conflict guard from #603 to the end of the run, plus its unit test and the AGENTS.md description. 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.

@Neaox
Neaox enabled auto-merge (squash) August 4, 2026 03:05
@Neaox
Neaox disabled auto-merge August 4, 2026 03:05
@overcast-release

Copy link
Copy Markdown
Contributor

No changelog fragment, deliberately

@Neaox said so:

Local tooling only: extends the pr-wait conflict guard from #603 to the end of the run, plus its unit test and the AGENTS.md description. 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.

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
adds something a user would want to read about, comment /needs-changelog and I
will ask again.

@Neaox
Neaox enabled auto-merge (squash) August 4, 2026 03:13
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Compatibility Tests

No regressions against the baseline

Suite Pass Fail Unimpl. Skip N/A Total
cdk 35 0 0 0 0 35
cli 505 0 0 0 1 506
dotnet-sdk 212 0 0 329 0 541
go-sdk 506 0 0 0 0 506
java-sdk 506 0 0 35 0 541
node-js-sdk 506 0 0 0 0 506
python-sdk 506 0 0 0 0 506
rust-sdk 172 0 0 369 0 541
Total 2948 0 0 733 1 3682

Pass rate (excluding skips and N/A): 100.0%

Full report

@Neaox
Neaox merged commit 254a153 into main Aug 4, 2026
44 checks passed
@Neaox
Neaox deleted the fix/pr-wait-conflict-after-watch branch August 4, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant