Fix Actions pull_request.paths being triggered incorrectly by rebase (#36045) (#36054)

Backport #36045

Partially fix #34710

The bug described in #34710 can be divided into two parts: `push.paths`
and `pull_request.paths`. This PR fixes the issue related to
`pull_request.paths`. The root cause is that the check for whether the
workflow can be triggered happens **before** updating the PR’s merge
base. This causes the file-change detection to use the old merge base.
Therefore, we need to update the merge base first and then check whether
the workflow can be triggered.

services/mailer/incoming/incoming.go

@@ -6,6 +6,7 @@ package incoming
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	net_mail "net/mail"
 	"regexp"
@@ -221,7 +222,7 @@ loop:
 			err := func() error {
 				r := msg.GetBody(section)
 				if r == nil {
-					return fmt.Errorf("could not get body from message: %w", err)
+					return errors.New("could not get body from message")
 				}
 
 				env, err := enmime.ReadEnvelope(r)

services/pull/pull.go

@@ -425,10 +425,16 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 				for _, pr := range prs {
 					objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
 					if opts.NewCommitID != "" && opts.NewCommitID != objectFormat.EmptyObjectID().String() {
-						changed, err := checkIfPRContentChanged(ctx, pr, opts.OldCommitID, opts.NewCommitID)
+						changed, newMergeBase, err := checkIfPRContentChanged(ctx, pr, opts.OldCommitID, opts.NewCommitID)
 						if err != nil {
 							log.Error("checkIfPRContentChanged: %v", err)
 						}
+						if newMergeBase != "" && pr.MergeBase != newMergeBase {
+							pr.MergeBase = newMergeBase
+							if err := pr.UpdateColsIfNotMerged(ctx, "merge_base"); err != nil {
+								log.Error("Update merge base for %-v: %v", pr, err)
+							}
+						}
 						if changed {
 							// Mark old reviews as stale if diff to mergebase has changed
 							if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil {
@@ -502,30 +508,30 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 
 // checkIfPRContentChanged checks if diff to target branch has changed by push
 // A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged
-func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) {
+func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, mergeBase string, err error) {
 	prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
 	if err != nil {
 		log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err)
-		return false, err
+		return false, "", err
 	}
 	defer cancel()
 
 	tmpRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath)
 	if err != nil {
-		return false, fmt.Errorf("OpenRepository: %w", err)
+		return false, "", fmt.Errorf("OpenRepository: %w", err)
 	}
 	defer tmpRepo.Close()
 
 	// Find the merge-base
-	_, base, err := tmpRepo.GetMergeBase("", "base", "tracking")
+	mergeBase, _, err = tmpRepo.GetMergeBase("", "base", "tracking")
 	if err != nil {
-		return false, fmt.Errorf("GetMergeBase: %w", err)
+		return false, "", fmt.Errorf("GetMergeBase: %w", err)
 	}
 
-	cmd := gitcmd.NewCommand("diff", "--name-only", "-z").AddDynamicArguments(newCommitID, oldCommitID, base)
+	cmd := gitcmd.NewCommand("diff", "--name-only", "-z").AddDynamicArguments(newCommitID, oldCommitID, mergeBase)
 	stdoutReader, stdoutWriter, err := os.Pipe()
 	if err != nil {
-		return false, fmt.Errorf("unable to open pipe for to run diff: %w", err)
+		return false, mergeBase, fmt.Errorf("unable to open pipe for to run diff: %w", err)
 	}
 
 	stderr := new(bytes.Buffer)
@@ -542,19 +548,19 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest,
 		},
 	}); err != nil {
 		if err == util.ErrNotEmpty {
-			return true, nil
+			return true, mergeBase, nil
 		}
 		err = gitcmd.ConcatenateError(err, stderr.String())
 
 		log.Error("Unable to run diff on %s %s %s in tempRepo for PR[%d]%s/%s...%s/%s: Error: %v",
-			newCommitID, oldCommitID, base,
+			newCommitID, oldCommitID, mergeBase,
 			pr.ID, pr.BaseRepo.FullName(), pr.BaseBranch, pr.HeadRepo.FullName(), pr.HeadBranch,
 			err)
 
-		return false, fmt.Errorf("Unable to run git diff --name-only -z %s %s %s: %w", newCommitID, oldCommitID, base, err)
+		return false, mergeBase, fmt.Errorf("Unable to run git diff --name-only -z %s %s %s: %w", newCommitID, oldCommitID, mergeBase, err)
 	}
 
-	return false, nil
+	return false, mergeBase, nil
 }
 
 // PushToBaseRepo pushes commits from branches of head repository to

services/pull/review.go

@@ -309,7 +309,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos
 		if headCommitID == commitID {
 			stale = false
 		} else {
-			stale, err = checkIfPRContentChanged(ctx, pr, commitID, headCommitID)
+			stale, _, err = checkIfPRContentChanged(ctx, pr, commitID, headCommitID)
 			if err != nil {
 				return nil, nil, err
 			}

services/wiki/wiki.go

@@ -137,7 +137,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 	if hasDefaultBranch {
 		if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil {
 			log.Error("Unable to read HEAD tree to index in: %s %v", basePath, err)
-			return fmt.Errorf("fnable to read HEAD tree to index in: %s %w", basePath, err)
+			return fmt.Errorf("unable to read HEAD tree to index in: %s %w", basePath, err)
 		}
 	}
 

tests/integration/actions_trigger_test.go

@@ -31,6 +31,7 @@ import (
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
+	webhook_module "code.gitea.io/gitea/modules/webhook"
 	issue_service "code.gitea.io/gitea/services/issue"
 	pull_service "code.gitea.io/gitea/services/pull"
 	release_service "code.gitea.io/gitea/services/release"
@@ -1604,3 +1605,56 @@ jobs:
 		assert.NotNil(t, run)
 	})
 }
+
+func TestPullRequestWithPathsRebase(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		session := loginUser(t, user2.Name)
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
+
+		repoName := "actions-pr-paths-rebase"
+		apiRepo := createActionsTestRepo(t, token, repoName, false)
+		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID})
+		apiCtx := NewAPITestContext(t, "user2", repoName, auth_model.AccessTokenScopeWriteRepository)
+		runner := newMockRunner()
+		runner.registerAsRepoRunner(t, "user2", repoName, "mock-runner", []string{"ubuntu-latest"}, false)
+
+		// init files and dirs
+		testCreateFile(t, session, "user2", repoName, repo.DefaultBranch, "", "dir1/dir1.txt", "1")
+		testCreateFile(t, session, "user2", repoName, repo.DefaultBranch, "", "dir2/dir2.txt", "2")
+		wfFileContent := `name: ci
+on: 
+  pull_request:
+    paths:
+      - 'dir1/**'
+jobs:
+  ci-job:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo 'ci'
+`
+		testCreateFile(t, session, "user2", repoName, repo.DefaultBranch, "", ".gitea/workflows/ci.yml", wfFileContent)
+
+		// create a PR to modify "dir1/dir1.txt", the workflow will be triggered
+		testEditFileToNewBranch(t, session, "user2", repoName, repo.DefaultBranch, "update-dir1", "dir1/dir1.txt", "11")
+		_, err := doAPICreatePullRequest(apiCtx, "user2", repoName, repo.DefaultBranch, "update-dir1")(t)
+		assert.NoError(t, err)
+		pr1Task := runner.fetchTask(t)
+		_, _, pr1Run := getTaskAndJobAndRunByTaskID(t, pr1Task.Id)
+		assert.Equal(t, webhook_module.HookEventPullRequest, pr1Run.Event)
+
+		// create a PR to modify "dir2/dir2.txt" then update main branch and rebase, the workflow will not be triggered
+		testEditFileToNewBranch(t, session, "user2", repoName, repo.DefaultBranch, "update-dir2", "dir2/dir2.txt", "22")
+		apiPull, err := doAPICreatePullRequest(apiCtx, "user2", repoName, repo.DefaultBranch, "update-dir2")(t)
+		runner.fetchNoTask(t)
+		assert.NoError(t, err)
+		testEditFile(t, session, "user2", repoName, repo.DefaultBranch, "dir1/dir1.txt", "11") // change the file in "dir1"
+		req := NewRequestWithValues(t, "POST",
+			fmt.Sprintf("/%s/%s/pulls/%d/update?style=rebase", "user2", repoName, apiPull.Index), // update by rebase
+			map[string]string{
+				"_csrf": GetUserCSRFToken(t, session),
+			})
+		session.MakeRequest(t, req, http.StatusSeeOther)
+		runner.fetchNoTask(t)
+	})
+}