From 95d6b572fe87193a47f019371b370c1894ee8fc5 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Fri, 16 Oct 2026 12:22:02 -0700 Subject: [PATCH] fix(pull): don't auto-detect empty pull requests as manually merged (#39807) (#39812) Backport #39807 by @gomitrah With "Autodetect manual merge" enabled, an empty pull request (one with no commits of its own, for example opened to discuss a branch) was closed as manually merged as soon as it was checked. Its head commit is already an ancestor of the base branch, so the detection took the "fast-forwarded" path and recorded the head itself as the merge commit, even though nothing was merged. Detection now skips a PR whose head commit is its merge base, the same condition Gitea already uses to show a PR as "already included in the target branch". Real manual merges are still detected, including when the base branch and the PR branch are pushed together. The merge base is refreshed on recheck, so a fast-forward manual merge whose merge base was already refreshed to the head is not detected either. This only misses a detection and never closes a PR wrongly; telling the two apart would need a value stored at PR creation. Fixes https://github.com/go-gitea/gitea/issues/31947 Co-authored-by: Mitrahsoft Co-authored-by: wxiaoguang --- services/pull/check.go | 16 ++++++++-------- tests/integration/manual_merge_test.go | 11 +++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/services/pull/check.go b/services/pull/check.go index 424d464d023..48ab332ed36 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -29,6 +29,7 @@ import ( "gitea.dev/modules/queue" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" asymkey_service "gitea.dev/services/asymkey" "gitea.dev/services/automergequeue" ) @@ -313,8 +314,7 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com cmd := gitcmd.NewCommand("merge-base", "--is-ancestor").AddDynamicArguments(prHeadRef, pr.BaseBranch) if err := cmd.WithRepo(pr.BaseRepo).RunWithStderr(ctx); err != nil { if gitcmd.IsErrorExitCode(err, 1) { - // prHeadRef is not an ancestor of the base branch - return nil, nil //nolint:nilnil // return nil to indicate that the PR head is not merged + return nil, util.ErrNotExist // prHeadRef is not an ancestor of the base branch, no merge commit is found } // Errors are signaled by a non-zero status that is not 1 return nil, fmt.Errorf("%-v git merge-base --is-ancestor: %w", pr, err) @@ -327,6 +327,9 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com if err != nil { return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err) } + if prHeadCommitID == pr.MergeBase { + return nil, util.ErrNotExist // empty PR, no merge commit is found + } gitRepo, err := git.OpenRepository(ctx, pr.BaseRepo) if err != nil { @@ -403,13 +406,10 @@ func manuallyMergedByPushedCommit(ctx context.Context, pr *issues_model.PullRequ } commit, err := getMergeCommit(ctx, pr) - if err != nil { - log.Error("%-v getMergeCommit: %v", pr, err) + if errors.Is(err, util.ErrNotExist) { return false - } - - if commit == nil { - // no merge commit found + } else if err != nil { + log.Error("%-v getMergeCommit: %v", pr, err) return false } diff --git a/tests/integration/manual_merge_test.go b/tests/integration/manual_merge_test.go index ae94101844a..cf6781a1e9a 100644 --- a/tests/integration/manual_merge_test.go +++ b/tests/integration/manual_merge_test.go @@ -5,6 +5,7 @@ package integration import ( "fmt" + "net/http" "net/url" "strings" "testing" @@ -16,6 +17,7 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/queue" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" @@ -59,6 +61,10 @@ func TestManualMergeAutodetect(t *testing.T) { require.NoError(t, err) } + testAPICreateBranch(t, user2Ctx.Session, user2.Name, repoName, defaultBranch, "empty-pr", http.StatusCreated) + emptyAPIPull, err := doAPICreatePullRequest(user1Ctx, user2.Name, repoName, defaultBranch, "empty-pr")(t) + require.NoError(t, err) + // user1 clones, then merges every branch sequentially, then pushes once. // The first merge fast-forwards; the rest produce real merge commits, // which generates multiple commits for "git rev-list --ancestry-path --merges ...". @@ -100,5 +106,10 @@ func TestManualMergeAutodetect(t *testing.T) { assert.Equalf(t, user1.ID, pr.MergerID, "PR %d (%s) merger should be the pusher", i+1, branchNames[i]) assert.Equalf(t, expectedMergeCommits[i], pr.MergedCommitID, "PR %d (%s) should be attributed to its own merge commit, not another PR's", i+1, branchNames[i]) } + + require.NoError(t, queue.GetManager().FlushAll(t.Context(), 5*time.Second)) + emptyPR := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: emptyAPIPull.ID}) + assert.False(t, emptyPR.HasMerged, "empty PR must not be detected as manually merged") + assert.Equal(t, issues_model.PullRequestStatusAncestor, emptyPR.Status) }) }