ci: resolve the PR by head repo + branch, not by commit sha - #490
Merged
Loup-Garou911XD merged 1 commit intoSep 8, 2026
Merged
Loup-Garou911XD merged 1 commit into
Loup-Garou911XD merged 1 commit into
Conversation
PR Apply has never once applied fixups to a fork PR. It resolved the
target PR through repos/{repo}/commits/{sha}/pulls, and that endpoint
cannot answer for a fork PR's head commit: the commit is not reachable
from any ref of this repo, so the association index has nothing to
return and the call yields [] forever. This is not eventual consistency
that a longer poll would ride out.
The failure used to be invisible. Before the loud-failure change the
lookup fell through to `skip`, which exits 0, so the job painted itself
green having applied nothing - which is how bombsquad-community#479 reached main unstamped.
Since then it fails loudly instead, correctly, and every fork PR has gone
red at that step (runs 34018273545, 34023650032, 34040502180, 34042614720,
34048956687, 34192652242).
Look the PR up by head repo owner + head branch instead. That filter is
exact, is available the moment the PR exists, and narrows on precisely
the fields HARD RULE 3 already requires the resolved PR to match. Nothing
is relaxed: state, base, head repo, head branch and head sha are all
still pinned against the workflow_run payload before anything is applied,
so a run can still only ever write to the branch it came from.
state=all, not state=open, so a PR merged between PR Check and this run
still resolves and takes the existing "PR not open; skipping" path rather
than looking unidentifiable. Several PRs from one head branch is ordinary
history for a reused branch, not the sha-adoption ambiguity HARD RULE 3
guards against - every candidate shares the same head repo and branch by
construction - so the newest by number wins instead of failing closed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSZrwNCXvibEJ9PXCW1uqi
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.
The bug
PR Applyhas never once applied fixups to a fork PR.It resolved the target PR through
repos/{repo}/commits/{sha}/pulls, and that endpoint cannot answer for a fork PR's head commit: the commit is not reachable from any ref of this repo, so the association index has nothing to return and the call yields[]. This is not eventual consistency that a longer poll rides out, it simply never resolves.Verified against the live API: that call returns
489for9def131now, but only because merging put the commit into main's history. During run34192652242it was empty for the full 50s poll while PR #489 was open with exactly that head.Why nobody noticed
The failure used to be invisible. Before the loud-failure change in #484, the lookup fell through to
skip, which exits 0, so the job painted itself green having applied nothing. Run34016889713is a "success" that did exactly that:That is how #479 reached main unstamped and broke it. #484 correctly turned the silent skip into a hard failure, which is why every fork PR has gone red at that step since:
34018273545,34023650032,34040502180,34042614720,34048956687,34192652242.So #484 did not cause this. It exposed it.
The fix
Look the PR up by head repo owner + head branch instead. That filter is exact, is available the moment the PR exists, and narrows on precisely the fields HARD RULE 3 already requires the resolved PR to match.
Nothing is relaxed. State, base, head repo, head branch and head sha are all still pinned against the
workflow_runpayload before anything is applied, so a run can still only ever write to the branch it came from.Two smaller decisions:
state=allrather thanstate=open, so a PR merged between PR Check and this run still resolves and takes the existingPR not open; skippingpath instead of looking unidentifiable.Owner and branch are shape-checked before they enter the URL.
Verification
YAML parses, the extracted step passes
bash -n, and the new lookup plus itsjqselection were run against the live API for #489.Note this cannot go fully green until the
secrets.PATexpiry is resolved (see the companion PR), since PR Apply's own merge to main runs the brokenci.ymlpath.🤖 Generated with Claude Code
https://claude.ai/code/session_01CSZrwNCXvibEJ9PXCW1uqi