From b89fcf7496d812c99694fe4e9743f10f464b02ed Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 31 Oct 2026 13:48:26 +0100 Subject: [PATCH] fix(actions): run review-triggered workflows on the pull request head (#40099) Runs triggered by pull request reviews took their ref and commit from the review's `commit_id` instead of the pull request. They now use the pull request head like other pull request events, and their commit status is set on the commit that ran. --- services/actions/commit_status.go | 14 ++------------ services/actions/notifier.go | 3 ++- services/actions/notifier_helper.go | 6 ------ services/actions/notifier_helper_test.go | 6 +++--- tests/integration/actions_trigger_test.go | 3 ++- 5 files changed, 9 insertions(+), 23 deletions(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index f174adc3142..08df31f575c 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -116,21 +116,11 @@ func getCommitStatusEventNameAndCommitID(run *actions_model.ActionRun) (event, c return "", "", errors.New("head of pull request is missing in event payload") } commitID = payload.PullRequest.Head.Sha - case // pull_request_review events share the same PullRequestPayload as pull_request - webhook_module.HookEventPullRequestReviewApproved, + case webhook_module.HookEventPullRequestReviewApproved, webhook_module.HookEventPullRequestReviewRejected, webhook_module.HookEventPullRequestReviewComment: event = run.TriggerEvent - payload, err := run.GetPullRequestEventPayload() - if err != nil { - return "", "", fmt.Errorf("GetPullRequestEventPayload: %w", err) - } - if payload.PullRequest == nil { - return "", "", errors.New("pull request is missing in event payload") - } else if payload.PullRequest.Head == nil { - return "", "", errors.New("head of pull request is missing in event payload") - } - commitID = payload.PullRequest.Head.Sha + commitID = run.CommitSHA case webhook_module.HookEventRelease: event = string(run.Event) commitID = run.CommitSHA diff --git a/services/actions/notifier.go b/services/actions/notifier.go index cca6283d2fc..fa6122437be 100644 --- a/services/actions/notifier.go +++ b/services/actions/notifier.go @@ -450,7 +450,8 @@ func (n *actionsNotifier) PullRequestReview(ctx context.Context, pr *issues_mode return } - newPullRequestReviewNotifyInput(review.Issue.Repo, review.Reviewer, reviewHookType, review.CommitID, pr). + newNotifyInput(review.Issue.Repo, review.Reviewer, reviewHookType). + WithPullRequest(pr). WithPayload(&api.PullRequestPayload{ Action: api.HookIssueReviewed, Index: review.Issue.Index, diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index ea1ac5dbf79..61462acc1c3 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -86,12 +86,6 @@ func newNotifyInputForSchedules(repo *repo_model.Repository) *notifyInput { return newNotifyInput(repo, user_model.NewActionsUser(), webhook_module.HookEventSchedule) } -func newPullRequestReviewNotifyInput(repo *repo_model.Repository, reviewer *user_model.User, event webhook_module.HookEventType, commitID string, pr *issues_model.PullRequest) *notifyInput { - return newNotifyInput(repo, reviewer, event). - WithRef(commitID). - WithPullRequest(pr) -} - func (input *notifyInput) WithDoer(doer *user_model.User) *notifyInput { input.Doer = doer return input diff --git a/services/actions/notifier_helper_test.go b/services/actions/notifier_helper_test.go index ae65cbf924a..260eb36da5d 100644 --- a/services/actions/notifier_helper_test.go +++ b/services/actions/notifier_helper_test.go @@ -108,12 +108,12 @@ func TestGetApprovalUsersAddsForkPullRequestAuthorUnlessDefaultBranchWorkflow(t } func TestFilteredWorkflowCommitStatusForForkPullRequest(t *testing.T) { - forkPR := &issues_model.PullRequest{ + input := new(notifyInput).WithPullRequest(&issues_model.PullRequest{ Flow: issues_model.PullRequestFlowGithub, BaseRepoID: 1, HeadRepoID: 2, - } - input := newPullRequestReviewNotifyInput(&repo_model.Repository{ID: 1}, &user_model.User{ID: 2}, actions_module.GithubEventPullRequest, "refs/pull/1/head", forkPR) + Index: 1, + }) assert.True(t, isForkPullRequestInput(input)) assert.Equal(t, "refs/pull/1/head", input.Ref.String()) diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index e39bead7c57..1c2d0250923 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -1373,11 +1373,12 @@ jobs: assert.NoError(t, err) defer gitRepo.Close() - _, _, err = pull_service.SubmitReview(t.Context(), user4, gitRepo, pullIssue, issues_model.ReviewTypeApprove, "lgtm", sha, nil) + _, _, err = pull_service.SubmitReview(t.Context(), user4, gitRepo, pullIssue, issues_model.ReviewTypeApprove, "lgtm", addWorkflow.Commit.SHA, nil) assert.NoError(t, err) // verify that a commit status was created for the review event waitCommitStatusAndInsertFakeStatus(t, repo, sha) + assert.Equal(t, sha, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: repo.ID}).CommitSHA) }) }