Fix the wrong push commits in the pull request when force push

Author: lunny

services/git/compare.go

@@ -42,6 +42,38 @@ func (ci *CompareInfo) DirectComparison() bool {
 	return ci.CompareSeparator == ".."
 }
 
+// GetCompareCommitIDsWithMergeBase get commit IDs from repo in between oldCommitID and newCommitID
+// Commit on baseBranch will skip
+func GetCompareCommitIDsWithMergeBase(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID string) (commitIDs []string, err error) {
+	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
+	if err != nil {
+		return nil, err
+	}
+	defer closer.Close()
+
+	newCommit, err := gitRepo.GetCommit(newCommitID)
+	if err != nil {
+		return nil, err
+	}
+
+	mergeBase, err := gitrepo.MergeBase(ctx, repo, oldCommitID, newCommitID)
+	if err != nil {
+		return nil, fmt.Errorf("MergeBase: %w", err)
+	}
+
+	commits, err := gitRepo.ShowPrettyFormatLogToList(ctx, mergeBase+".."+newCommit.ID.String())
+	if err != nil {
+		return nil, fmt.Errorf("ShowPrettyFormatLogToList: %w", err)
+	}
+
+	commitIDs = make([]string, 0, len(commits))
+	for i := len(commits) - 1; i >= 0; i-- {
+		commitIDs = append(commitIDs, commits[i].ID.String())
+	}
+
+	return commitIDs, err
+}
+
 // GetCompareInfo generates and returns compare information between base and head branches of repositories.
 func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseRef, headRef git.RefName, directComparison, fileOnly bool) (_ *CompareInfo, err error) {
 	compareInfo := &CompareInfo{

services/git/compare_test.go

@@ -0,0 +1,45 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"testing"
+
+	issues_model "code.gitea.io/gitea/models/issues"
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/gitrepo"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetCompareCommitIDsWithMergeBase(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
+	assert.NoError(t, pr.LoadBaseRepo(t.Context()))
+
+	gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo)
+	assert.NoError(t, err)
+	defer gitRepo.Close()
+
+	headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch)
+	assert.NoError(t, err)
+
+	baseCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch)
+	assert.NoError(t, err)
+
+	commitIDs, err := GetCompareCommitIDsWithMergeBase(t.Context(), pr.BaseRepo, pr.BaseBranch, headCommit.ID.String())
+	assert.NoError(t, err)
+	assert.NotEmpty(t, commitIDs)
+
+	commits, err := gitRepo.CommitsBetweenNotBase(headCommit, baseCommit, pr.BaseBranch)
+	assert.NoError(t, err)
+
+	expected := make([]string, 0, len(commits))
+	for i := len(commits) - 1; i >= 0; i-- {
+		expected = append(expected, commits[i].ID.String())
+	}
+
+	assert.Equal(t, expected, commitIDs)
+}

services/pull/comment.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
+	git_service "code.gitea.io/gitea/services/git"
 )
 
 // getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
@@ -62,19 +63,24 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
 	}
 
 	var data issues_model.PushActionContent
-	data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
-	if err != nil {
-		// For force-push events, a missing/unreachable old commit should not prevent
-		// deleting stale push comments or creating the force-push timeline entry.
-		if !isForcePush {
+	if isForcePush {
+		// if it's a force push, we need to get the whole pull request commits
+		data.CommitIDs, err = git_service.GetCompareCommitIDsWithMergeBase(ctx, pr.BaseRepo, pr.BaseBranch, newCommitID)
+		if err != nil {
+			// For force-push events, a missing/unreachable old commit should not prevent
+			// deleting stale push comments or creating the force-push timeline entry.
+			log.Error("GetCompareCommitIDsWithMergeBase: %v", err)
+		}
+	} else {
+		data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
+		if err != nil {
 			return nil, err
 		}
-		log.Error("getCommitIDsFromRepo: %v", err)
-	}
-	// It maybe an empty pull request. Only non-empty pull request need to create push comment
-	// for force push, we always need to delete the old push comment so don't return here.
-	if len(data.CommitIDs) == 0 && !isForcePush {
-		return nil, nil //nolint:nilnil // return nil because no comment needs to be created
+		// It maybe an empty pull request. Only non-empty pull request need to create push comment
+		// for force push, we always need to delete the old push comment so don't return here.
+		if len(data.CommitIDs) == 0 {
+			return nil, nil //nolint:nilnil // return nil because no comment needs to be created
+		}
 	}
 
 	return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {

services/pull/comment_test.go

@@ -93,6 +93,31 @@ func TestCreatePushPullCommentForcePushDeletesOldComments(t *testing.T) {
 		assert.Equal(t, 1, forcePushCount)
 	})
 
+	t.Run("force-push-ignores-missing-old-commit", func(t *testing.T) {
+		assert.NoError(t, unittest.PrepareTestDatabase())
+
+		pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
+		assert.NoError(t, pr.LoadIssue(t.Context()))
+		assert.NoError(t, pr.LoadBaseRepo(t.Context()))
+
+		pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+
+		gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo)
+		assert.NoError(t, err)
+		defer gitRepo.Close()
+
+		headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch)
+		assert.NoError(t, err)
+
+		comment, err := CreatePushPullComment(t.Context(), pusher, pr, "0000000000000000000000000000000000000000", headCommit.ID.String(), true)
+		assert.NoError(t, err)
+		assert.NotNil(t, comment)
+		var createdData issues_model.PushActionContent
+		assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData))
+		assert.True(t, createdData.IsForcePush)
+		assert.NotEmpty(t, createdData.CommitIDs)
+	})
+
 	t.Run("head-vs-base-branch", func(t *testing.T) {
 		assert.NoError(t, unittest.PrepareTestDatabase())