fix(#2432): retry merge on 409 after updating PR branch

internal/forge/github/github.go

@@ -2057,13 +2057,41 @@ func (c *LiveClient) DismissPullRequestReview(ctx context.Context, owner, repo s
 }
 
 // MergeChangeProposal squash-merges a pull request by number.
+// If the merge fails with a 409 (head branch out of date), it updates the PR
+// branch and retries up to 3 times with a short delay between attempts.
 func (c *LiveClient) MergeChangeProposal(ctx context.Context, owner, repo string, number int) error {
-	resp, err := c.put(ctx, fmt.Sprintf("/repos/%s/%s/pulls/%d/merge", owner, repo, number), map[string]string{"merge_method": "squash"})
-	if err != nil {
-		return fmt.Errorf("merge pull request #%d: %w", number, err)
+	const maxAttempts = 3
+	mergePath := fmt.Sprintf("/repos/%s/%s/pulls/%d/merge", owner, repo, number)
+	updatePath := fmt.Sprintf("/repos/%s/%s/pulls/%d/update-branch", owner, repo, number)
+
+	for attempt := range maxAttempts {
+		resp, err := c.put(ctx, mergePath, map[string]string{"merge_method": "squash"})
+		if err == nil {
+			resp.Body.Close()
+			return nil
+		}
+
+		var apiErr *APIError
+		if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusConflict {
+			return fmt.Errorf("merge pull request #%d: %w", number, err)
+		}
+
+		// Update the PR branch to incorporate base branch changes.
+		updateResp, updateErr := c.do(ctx, http.MethodPut, updatePath, map[string]string{})
+		if updateErr == nil {
+			updateResp.Body.Close()
+		}
+
+		if attempt < maxAttempts-1 {
+			select {
+			case <-time.After(3 * time.Second):
+			case <-ctx.Done():
+				return ctx.Err()
+			}
+		}
 	}
-	resp.Body.Close()
-	return nil
+
+	return fmt.Errorf("merge pull request #%d: branch remained out of date after %d update-and-retry attempts", number, maxAttempts)
 }
 
 // UpdatePullRequestBranch updates a PR's head branch by merging the base

internal/forge/github/github_merge_test.go

@@ -0,0 +1,121 @@
+package github
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMergeChangeProposal_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodPut, r.Method)
+		assert.Equal(t, "/repos/org/repo/pulls/42/merge", r.URL.Path)
+
+		var body map[string]string
+		json.NewDecoder(r.Body).Decode(&body)
+		assert.Equal(t, "squash", body["merge_method"])
+
+		w.WriteHeader(http.StatusOK)
+		json.NewEncoder(w).Encode(map[string]string{"sha": "abc123"})
+	}))
+	defer srv.Close()
+
+	client := newTestClient(t, srv)
+	err := client.MergeChangeProposal(context.Background(), "org", "repo", 42)
+	require.NoError(t, err)
+}
+
+func TestMergeChangeProposal_409UpdatesBranchAndRetries(t *testing.T) {
+	var mergeAttempts atomic.Int32
+	var updateCalls atomic.Int32
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodPut && r.URL.Path == "/repos/org/repo/pulls/7/merge":
+			attempt := mergeAttempts.Add(1)
+			if attempt == 1 {
+				// First merge attempt: 409 conflict.
+				w.WriteHeader(http.StatusConflict)
+				json.NewEncoder(w).Encode(map[string]string{
+					"message": "Head branch is out of date",
+				})
+				return
+			}
+			// Second merge attempt: success.
+			w.WriteHeader(http.StatusOK)
+			json.NewEncoder(w).Encode(map[string]string{"sha": "def456"})
+
+		case r.Method == http.MethodPut && r.URL.Path == "/repos/org/repo/pulls/7/update-branch":
+			updateCalls.Add(1)
+			w.WriteHeader(http.StatusAccepted)
+			json.NewEncoder(w).Encode(map[string]string{"message": "Updating pull request branch."})
+
+		default:
+			t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer srv.Close()
+
+	client := newTestClient(t, srv)
+	err := client.MergeChangeProposal(context.Background(), "org", "repo", 7)
+	require.NoError(t, err)
+	assert.Equal(t, int32(2), mergeAttempts.Load(), "should have attempted merge twice")
+	assert.Equal(t, int32(1), updateCalls.Load(), "should have called update-branch once")
+}
+
+func TestMergeChangeProposal_NonConflictErrorNotRetried(t *testing.T) {
+	var mergeAttempts atomic.Int32
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mergeAttempts.Add(1)
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		json.NewEncoder(w).Encode(map[string]string{
+			"message": "Pull Request is not mergeable",
+		})
+	}))
+	defer srv.Close()
+
+	client := newTestClient(t, srv)
+	err := client.MergeChangeProposal(context.Background(), "org", "repo", 7)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "not mergeable")
+	assert.Equal(t, int32(1), mergeAttempts.Load(), "should not retry non-409 errors")
+}
+
+func TestMergeChangeProposal_409PersistsAfterRetries(t *testing.T) {
+	var mergeAttempts atomic.Int32
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodPut && r.URL.Path == "/repos/org/repo/pulls/7/merge":
+			mergeAttempts.Add(1)
+			w.WriteHeader(http.StatusConflict)
+			json.NewEncoder(w).Encode(map[string]string{
+				"message": "Head branch is out of date",
+			})
+
+		case r.Method == http.MethodPut && r.URL.Path == "/repos/org/repo/pulls/7/update-branch":
+			w.WriteHeader(http.StatusAccepted)
+			json.NewEncoder(w).Encode(map[string]string{"message": "Updating pull request branch."})
+
+		default:
+			t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer srv.Close()
+
+	client := newTestClient(t, srv)
+	err := client.MergeChangeProposal(context.Background(), "org", "repo", 7)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "merge pull request #7")
+	// Should have tried multiple times before giving up.
+	assert.Greater(t, mergeAttempts.Load(), int32(1), "should have retried merge")
+}