fix(pull): refresh commits behind when an AGit pull request is updated (#39613) (#39625)

Backport #39613 by @dziulatex

Fixes #39598

When the base branch has "Block merge if the pull request is outdated"
enabled, an AGit pull request that fell behind could never be unblocked:
merging the base branch into the topic and pushing it again to the same
`refs/for/<branch>/<topic>` left the PR "blocked because it's outdated"
forever.

The outdated block reads the stored `commits_behind`. Pushing a branch
refreshes it for GitHub-flow PRs (`AddTestPullRequestTask`), but the
AGit update path in `services/agit` moved `refs/pull/N/head` without
recomputing it, so the value only changed when the base branch was
pushed, which is the event that makes a PR behind again.

The AGit update path now calls `syncCommitDivergence` in
`UpdateRefForAgit`, the same as AGit PR creation (`NewPullRequest`)
already does.

Signed-off-by: dziulatex <paweldziurasoftware@yahoo.com>
Co-authored-by: dziulatex <38464243+dziulatex@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

services/agit/agit.go

@@ -252,7 +252,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 		oldHeadCommitID := pr.HeadCommitID
 
 		pr.HeadCommitID = opts.NewCommitIDs[i]
-		if err = pull_service.UpdateRef(ctx, pr); err != nil {
+		if err = pull_service.UpdateRefForAgit(ctx, pr); err != nil {
 			return nil, fmt.Errorf("failed to update pull ref. Error: %w", err)
 		}
 

services/pull/pull.go

@@ -98,8 +98,9 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
 	assigneeCommentMap := make(map[int64]*issues_model.Comment)
 	assignees := make(map[int64]*user_model.User)
 	var reviewNotifiers []*issue_service.ReviewRequestNotifier
-	if err := db.WithTx(ctx, func(ctx context.Context) error {
+	err := db.WithTx(ctx, func(ctx context.Context) error {
-		if err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr); err != nil {
+		err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr)
+		if err != nil {
 			return err
 		}
 
@@ -126,20 +127,17 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
 		pr.Issue = issue
 		issue.PullRequest = pr
 
-		var err error
 		if pr.Flow == issues_model.PullRequestFlowGithub {
-			err = PushToBaseRepo(ctx, pr)
+			if err = PushToBaseRepo(ctx, pr); err != nil {
-		} else {
+				return err
-			err = UpdateRef(ctx, pr)
+			}
-		}
+			if err = syncCommitDivergence(ctx, pr); err != nil {
-		if err != nil {
+				return err
-			return err
+			}
-		}
+		} else { // agit
-
+			if err = UpdateRefForAgit(ctx, pr); err != nil {
-		// Update Commit Divergence
+				return err
-		err = syncCommitDivergence(ctx, pr)
+			}
-		if err != nil {
-			return err
 		}
 
 		// add first push codes comment
@@ -154,7 +152,8 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
 			}
 		}
 		return nil
-	}); err != nil {
+	})
+	if err != nil {
 		// cleanup: this will only remove the reference, the real commit will be clean up when next GC
 		if err1 := git.RemoveRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName()); err1 != nil {
 			log.Error("RemoveRef: %v", err1)
@@ -602,19 +601,15 @@ func UpdatePullsRefs(ctx context.Context, repo *repo_model.Repository, update *r
 	}
 }
 
-// UpdateRef update refs/pull/id/head directly for agit flow pull request
+// UpdateRefForAgit update refs/pull/id/head directly for agit flow pull request
-func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
+func UpdateRefForAgit(ctx context.Context, pr *issues_model.PullRequest) (err error) {
-	log.Trace("UpdateRef[%d]: upgate pull request ref in base repo '%s'", pr.ID, pr.GetGitHeadRefName())
 	if err := pr.LoadBaseRepo(ctx); err != nil {
-		log.Error("Unable to load base repository for PR[%d] Error: %v", pr.ID, err)
 		return err
 	}
-
 	if err := git.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadCommitID); err != nil {
-		log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err)
+		return fmt.Errorf("unable to update ref %s for base repo %s, err: %w", pr.GetGitHeadRefName(), pr.BaseRepo.FullName(), err)
 	}
-
+	return syncCommitDivergence(ctx, pr)
-	return err
 }
 
 // retargetBranchPulls change target branch for all pull requests whose base branch is the branch

tests/integration/git_misc_test.go

@@ -109,6 +109,8 @@ func TestAgitPullPush(t *testing.T) {
 		assert.NoError(t, pr.LoadIssue(t.Context()))
 		assert.Equal(t, "test-title", pr.Issue.Title)
 		assert.Equal(t, "test-description", pr.Issue.Content)
+		assert.Equal(t, 0, pr.CommitsBehind)
+		assert.Equal(t, 1, pr.CommitsAhead)
 
 		// commit 2
 		_, err = generateCommitWithNewData(t.Context(), testFileSizeSmall, dstPath, "user2@example.com", "User Two", "branch-data-file-2-")
@@ -119,6 +121,9 @@ func TestAgitPullPush(t *testing.T) {
 			WithDir(dstPath).
 			Run(t.Context())
 		assert.NoError(t, err)
+		pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: 1, Flow: issues_model.PullRequestFlowAGit, HeadBranch: "user2/test-agit-push"})
+		assert.Equal(t, 0, pr.CommitsBehind)
+		assert.Equal(t, 2, pr.CommitsAhead)
 
 		// reset to first commit
 		err = gitcmd.NewCommand("reset", "--hard", "HEAD~1").WithDir(dstPath).Run(t.Context())