fix: recover fork PR re-runs when GitHub omits PR metadata by chmouel · Pull Request #2655 · tektoncd/pipelines-as-code

chmouel · 2026-04-08T09:02:29Z

When GitHub receives a Re-run request for a Pipelines as Code check tied to a pull request from a fork, the webhook payload can omit the usual pull request metadata. PAC could already recover by looking up pull requests for the head SHA, but fork PRs still failed when GitHub's commits/{sha}/pulls API returned no matches.

This change makes rerequest handling more resilient and keeps the selection rules safe:

check_run rerequests now honor pull request data attached directly to the event when GitHub provides it
if the commit lookup API returns no open PR, PAC falls back to listing open pull requests and matching by head SHA so fork PR rerequests can still resolve
the SHA fallback now preserves the existing ambiguity guard and fails if more than one open PR shares the same head SHA instead of picking one arbitrarily
tests now cover the direct-event path, fork fallback success, missing-PR failures, and the ambiguous fallback regression

📝 Description of the Change

Improve GitHub rerequest PR resolution so fork pull requests can be recovered safely even when GitHub omits PR metadata or the commit lookup API returns no match.

🔗 Linked GitHub Issue

Not linked.

🧪 Testing Strategy

Ran:

go test ./pkg/provider/github

🤖 AI Assistance

I have not used any AI assistance for this PR.
I have used AI assistance for this PR.

AI assistance was used to help draft and refine the code change, tests, and PR description. The resulting changes were reviewed and validated with the package test run above.

✅ Submitter Checklist

codecov-commenter · 2026-04-08T09:05:17Z

Codecov Report

❌ Patch coverage is 96.42857% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.87%. Comparing base (fff1dac) to head (6eb152f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/provider/github/parse_payload.go	96.42%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2655      +/-   ##
==========================================
+ Coverage   59.77%   59.87%   +0.10%     
==========================================
  Files         210      210              
  Lines       21135    21180      +45     
==========================================
+ Hits        12633    12682      +49     
+ Misses       7707     7703       -4     
  Partials      795      795

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gemini-code-assist

Code Review

This pull request introduces a fallback mechanism to resolve pull requests by SHA when the standard commit API fails, which is particularly useful for fork PRs. It also adds logic to extract pull request information directly from the check_run event. Feedback focuses on ensuring consistency and handling ambiguity when multiple pull requests might match a single SHA or check run, suggesting the use of existing helper functions to validate that only a single open pull request is processed.

chmouel · 2026-04-08T11:07:12Z

tested on dogfooding as working

zakisk · 2026-04-10T14:17:28Z

+
+	// ListPullRequestsWithCommit may return no matches for fork PR commits.
+	v.Logger.Infof("No PR found via commits API for SHA %s, falling back to open PR listing", runevent.SHA)
+	return v.findOpenPullRequestBySHA(ctx, runevent.Organization, runevent.Repository, runevent.SHA)


can we call the findOpenPullRequestBySHA only if its fork because if its not fork and there is no matching PRs as well then it would be just redundant API calls! (hint: event.pull_request.head.fork)

the whole reason we end up here is that github stripped the PR metadata from the webhook — there's no checkSuite.PullRequests, no checkRun.PullRequests, nothing. so there's no event.pull_request.head.fork to look at either, that data is exactly what's missing. the fallback exists precisely because we have zero PR context to work with.

zakisk · 2026-04-15T06:32:53Z

+	if len(checkSuite.PullRequests) > 0 {
+		runevent.PullRequestNumber = checkSuite.PullRequests[0].GetNumber()
+		runevent.TriggerTarget = triggertype.PullRequest
+		v.Logger.Infof("Recheck of PR %s/%s#%d has been requested", runevent.Organization, runevent.Repository, runevent.PullRequestNumber)
+		return v.getPullRequest(ctx, runevent)
+	}


Suggested change

if len(checkSuite.PullRequests) > 0 {

runevent.PullRequestNumber = checkSuite.PullRequests[0].GetNumber()

runevent.TriggerTarget = triggertype.PullRequest

v.Logger.Infof("Recheck of PR %s/%s#%d has been requested", runevent.Organization, runevent.Repository, runevent.PullRequestNumber)

return v.getPullRequest(ctx, runevent)

}

you can delete this as you're already doing this on line 557 below

these are two different things they come from different levels of the webhook payload and github can populate one without the other. can't remove the first block.

chmouel

Code review: found 2 issue(s).

chmouel · 2026-06-25T09:35:28Z

 		}
-		if runevent.HeadBranch == "" {
-			return nil, fmt.Errorf("cannot determine branch for check_run rerequest: head_branch is null and no associated PR found for SHA %s", runevent.SHA)
+		if pr != nil {


This still leaves the fork rerun authorized as the PR author rather than the user who clicked Re-run. runevent.Sender is empty on this resolved-PR path, so populateRunEventFromPullRequest fills it with pr.GetUser().GetLogin(). The later non-push ACL check then evaluates the external fork contributor, and /ok-to-test replay does not handle CheckRunEvent, so a maintainer-rerequested fork PR can still be denied after this fallback successfully finds it. Set runevent.Sender and v.userType from event.GetSender() before returning the PR path, like the push path below does.

Addressed in 6eb152f: rerun events now keep the GitHub actor who clicked Re-run as runevent.Sender before PR population, so the ACL check no longer falls back to the fork PR author.

chmouel · 2026-06-25T09:35:28Z

+						Title: github.Ptr("fork PR"),
+					},
+				},
+				"/repos/owner/reponame/pulls/987": github.PullRequest{


This /pulls/987 fixture is never used by the fork fallback path: after /pulls returns the matching PR, the code calls populateRunEventFromPullRequest(runevent, pr) directly and never fetches the single-PR endpoint. That makes the test look like it covers a full PR fetch when it only relies on the list response. Please remove the dead handler or add assertions for fields populated from the list-response PR so the intended coverage is explicit.

Addressed in 6eb152f: the unused /pulls/987 fixture was removed, and the test now asserts fields populated from the list-response PR so the fork fallback coverage is explicit.

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>

chmouel · 2026-06-25T10:03:35Z

Jira for the GitHub fork PR rerun fix in this PR: SRVKP-12590.

gemini-code-assist Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread pkg/provider/github/parse_payload.go

Comment thread pkg/provider/github/parse_payload.go Outdated

chmouel marked this pull request as draft April 8, 2026 09:11

chmouel force-pushed the fix-check-run-rerequest-fork-pr branch from 3282631 to 212adf3 Compare April 8, 2026 09:17

chmouel marked this pull request as ready for review April 8, 2026 11:07

chmouel force-pushed the fix-check-run-rerequest-fork-pr branch from 5f94900 to 8a5873e Compare April 8, 2026 14:06

zakisk reviewed Apr 10, 2026

View reviewed changes

Comment thread pkg/provider/github/parse_payload.go Outdated

chmouel marked this pull request as draft April 14, 2026 13:25

chmouel force-pushed the fix-check-run-rerequest-fork-pr branch 2 times, most recently from c6a4c49 to 6ecab48 Compare April 14, 2026 15:30

zakisk reviewed Apr 15, 2026

View reviewed changes

tekton-robot added the needs-rebase label Jun 4, 2026

chmouel force-pushed the fix-check-run-rerequest-fork-pr branch from 6ecab48 to 9319477 Compare June 25, 2026 09:18

tekton-robot removed the needs-rebase label Jun 25, 2026

chmouel marked this pull request as ready for review June 25, 2026 09:19

chmouel commented Jun 25, 2026

View reviewed changes

chmouel force-pushed the fix-check-run-rerequest-fork-pr branch from 9319477 to 6eb152f Compare June 25, 2026 09:53

Uh oh!

Conversation

chmouel commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description of the Change

🔗 Linked GitHub Issue

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

codecov-commenter commented Apr 8, 2026 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

chmouel commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chmouel Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chmouel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chmouel commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chmouel commented Apr 8, 2026 •

edited

Loading

codecov-commenter commented Apr 8, 2026 •

edited by codecov Bot

Loading

chmouel Jun 25, 2026 •

edited

Loading

chmouel commented Jun 25, 2026 •

edited

Loading