Skip to content

Commit 3282631

Browse files
committed
fix(github): recover fork PR re-runs when GitHub omits PR metadata
When someone clicks Re-run on a Pipelines as Code check for a pull request coming from a fork, GitHub can leave out the usual pull request details in the webhook payload. That left PAC with no obvious way to tell which pull request the check belonged to, so the re-run stopped with an error instead of starting again. This change teaches PAC to keep looking in a more practical way. If the first GitHub API says it cannot find the pull request for the commit, PAC now lists open pull requests in the repository and matches the one whose head commit SHA is the same. In simple terms: when GitHub does not hand us the answer directly, we now look through the open pull requests and find the one built from that exact commit. The change also checks pull request data attached directly to the check_run event before falling back to SHA-based lookup, and the tests now cover both the new fork pull request path and the empty-result error cases. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent 92a3273 commit 3282631

2 files changed

Lines changed: 143 additions & 1 deletion

File tree

pkg/provider/github/parse_payload.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,13 +314,53 @@ func selectSingleOpenPullRequest(prs []*github.PullRequest) (*github.PullRequest
314314
}
315315
}
316316

317+
func (v *Provider) findOpenPullRequestBySHA(ctx context.Context, org, repo, sha string) (*github.PullRequest, error) {
318+
opts := &github.PullRequestListOptions{
319+
State: "open",
320+
Sort: "updated",
321+
ListOptions: github.ListOptions{PerPage: 100},
322+
}
323+
324+
for {
325+
prs, resp, err := wrapAPI(v, "list_pull_requests", func() ([]*github.PullRequest, *github.Response, error) {
326+
return v.Client().PullRequests.List(ctx, org, repo, opts)
327+
})
328+
if err != nil {
329+
return nil, fmt.Errorf("failed to list open pull requests in %s/%s: %w", org, repo, err)
330+
}
331+
332+
for _, pr := range prs {
333+
if pr.GetHead().GetSHA() == sha {
334+
return pr, nil
335+
}
336+
}
337+
338+
if resp.NextPage == 0 {
339+
break
340+
}
341+
opts.Page = resp.NextPage
342+
}
343+
344+
return nil, nil
345+
}
346+
317347
func (v *Provider) resolveReRequestPullRequest(ctx context.Context, runevent *info.Event) (*github.PullRequest, error) {
318348
prs, err := v.getPullRequestsWithCommit(ctx, runevent.SHA, runevent.Organization, runevent.Repository, false)
319349
if err != nil {
320350
return nil, err
321351
}
322352

323-
return selectSingleOpenPullRequest(prs)
353+
pr, err := selectSingleOpenPullRequest(prs)
354+
if err != nil {
355+
return nil, err
356+
}
357+
if pr != nil {
358+
return pr, nil
359+
}
360+
361+
// ListPullRequestsWithCommit may return no matches for fork PR commits.
362+
v.Logger.Infof("No PR found via commits API for SHA %s, falling back to open PR listing", runevent.SHA)
363+
return v.findOpenPullRequestBySHA(ctx, runevent.Organization, runevent.Repository, runevent.SHA)
324364
}
325365

326366
func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt any) (*info.Event, error) {
@@ -503,6 +543,12 @@ func (v *Provider) handleReRequestEvent(ctx context.Context, event *github.Check
503543
runevent.HeadURL = event.GetCheckRun().GetCheckSuite().GetRepository().GetHTMLURL()
504544
// If we don't have a pull_request in this it probably mean a push
505545
if len(event.GetCheckRun().GetCheckSuite().PullRequests) == 0 {
546+
if len(event.GetCheckRun().PullRequests) > 0 {
547+
runevent.PullRequestNumber = event.GetCheckRun().PullRequests[0].GetNumber()
548+
runevent.TriggerTarget = triggertype.PullRequest
549+
v.Logger.Infof("Recheck of PR %s/%s#%d has been requested (from check_run)", runevent.Organization, runevent.Repository, runevent.PullRequestNumber)
550+
return v.getPullRequest(ctx, runevent)
551+
}
506552
// If head_branch is null, try to find a PR by SHA before assuming push
507553
if runevent.HeadBranch == "" && runevent.SHA != "" {
508554
pr, err := v.resolveReRequestPullRequest(ctx, runevent)

pkg/provider/github/parse_payload_test.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,99 @@ func TestParsePayLoad(t *testing.T) {
644644
shaRet: "samplePRsha",
645645
wantedPullRequestNumber: 54321,
646646
},
647+
{
648+
name: "good/rerequest check_run resolves PR from check_run pull requests",
649+
eventType: "check_run",
650+
githubClient: true,
651+
triggerTarget: string(triggertype.PullRequest),
652+
payloadEventStruct: github.CheckRunEvent{
653+
Action: github.Ptr("rerequested"),
654+
Repo: sampleRepo,
655+
CheckRun: &github.CheckRun{
656+
PullRequests: []*github.PullRequest{&samplePR},
657+
CheckSuite: &github.CheckSuite{
658+
HeadSHA: github.Ptr("samplePRsha"),
659+
},
660+
},
661+
},
662+
muxReplies: map[string]any{
663+
"/repos/owner/reponame/pulls/54321": samplePR,
664+
},
665+
shaRet: "samplePRsha",
666+
wantedPullRequestNumber: 54321,
667+
},
668+
{
669+
name: "good/rerequest check_run null head_branch resolves fork PR from open PR list",
670+
eventType: "check_run",
671+
githubClient: true,
672+
triggerTarget: string(triggertype.PullRequest),
673+
payloadEventStruct: github.CheckRunEvent{
674+
Action: github.Ptr("rerequested"),
675+
Repo: sampleRepo,
676+
CheckRun: &github.CheckRun{
677+
CheckSuite: &github.CheckSuite{
678+
HeadSHA: github.Ptr("forkPRsha"),
679+
},
680+
},
681+
},
682+
muxReplies: map[string]any{
683+
"/repos/owner/reponame/commits/forkPRsha/pulls": []*github.PullRequest{},
684+
"/repos/owner/reponame/pulls": []*github.PullRequest{
685+
{
686+
Number: github.Ptr(987),
687+
State: github.Ptr("open"),
688+
HTMLURL: github.Ptr("https://github.com/owner/reponame/pull/987"),
689+
Head: &github.PullRequestBranch{
690+
SHA: github.Ptr("forkPRsha"),
691+
Ref: github.Ptr("fork-feature"),
692+
Repo: &github.Repository{
693+
Owner: &github.User{
694+
Login: github.Ptr("fork-owner"),
695+
},
696+
Name: github.Ptr("reponame"),
697+
HTMLURL: github.Ptr("https://github.com/fork-owner/reponame"),
698+
},
699+
},
700+
Base: &github.PullRequestBranch{
701+
Ref: github.Ptr("main"),
702+
SHA: github.Ptr("basesha"),
703+
Repo: sampleRepo,
704+
},
705+
User: &github.User{
706+
Login: github.Ptr("fork-contributor"),
707+
},
708+
Title: github.Ptr("fork PR"),
709+
},
710+
},
711+
"/repos/owner/reponame/pulls/987": github.PullRequest{
712+
Number: github.Ptr(987),
713+
State: github.Ptr("open"),
714+
HTMLURL: github.Ptr("https://github.com/owner/reponame/pull/987"),
715+
Head: &github.PullRequestBranch{
716+
SHA: github.Ptr("forkPRsha"),
717+
Ref: github.Ptr("fork-feature"),
718+
Repo: &github.Repository{
719+
Owner: &github.User{
720+
Login: github.Ptr("fork-owner"),
721+
},
722+
Name: github.Ptr("reponame"),
723+
HTMLURL: github.Ptr("https://github.com/fork-owner/reponame"),
724+
},
725+
},
726+
Base: &github.PullRequestBranch{
727+
Ref: github.Ptr("main"),
728+
SHA: github.Ptr("basesha"),
729+
Repo: sampleRepo,
730+
},
731+
User: &github.User{
732+
Login: github.Ptr("fork-contributor"),
733+
},
734+
Title: github.Ptr("fork PR"),
735+
},
736+
},
737+
shaRet: "forkPRsha",
738+
wantedPullRequestNumber: 987,
739+
},
647740
{
648741
name: "good/rerequest check_suite null head_branch resolves PR from SHA",
649742
eventType: "check_suite",
@@ -678,6 +771,7 @@ func TestParsePayLoad(t *testing.T) {
678771
},
679772
muxReplies: map[string]any{
680773
"/repos/owner/reponame/commits/orphanSHA/pulls": []*github.PullRequest{},
774+
"/repos/owner/reponame/pulls": []*github.PullRequest{},
681775
},
682776
},
683777
{
@@ -694,6 +788,7 @@ func TestParsePayLoad(t *testing.T) {
694788
},
695789
muxReplies: map[string]any{
696790
"/repos/owner/reponame/commits/orphanSHA/pulls": []*github.PullRequest{},
791+
"/repos/owner/reponame/pulls": []*github.PullRequest{},
697792
},
698793
},
699794
{
@@ -717,6 +812,7 @@ func TestParsePayLoad(t *testing.T) {
717812
State: github.Ptr("closed"),
718813
},
719814
},
815+
"/repos/owner/reponame/pulls": []*github.PullRequest{},
720816
},
721817
},
722818
{

0 commit comments

Comments
 (0)