From 22b7967b0acb956aef95a025c0eb57b4105c4ce6 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:41:58 +0000 Subject: [PATCH] ci(#6825): retry CreateBranch on fork replication errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub's fork replication is eventually consistent — the default-branch ref can pass awaitForkReady's GetBranchRef poll but become temporarily unavailable when CreateBranch re-fetches it moments later, producing a 409 "Git Repository is empty" error. Add createForkBranch, a retry wrapper around CreateBranch that retries on 409/422 replication errors (up to 5 attempts with 2s backoff). whenForkPullRequestOpened now uses this wrapper instead of a bare CreateBranch call. Also add isReplicationError to classify transient fork replication failures (409, and 422 with "does not exist" or "empty" messages) separately from permanent errors like permission denied. Note: golangci-lint was not available in the sandbox. gofmt and go vet passed. Pre-commit could not fetch remote hook repos (network restriction); local hooks (gofmt, go vet) ran directly and passed. Closes #6825 --- pkg/behaviourtest/steps/fork.go | 71 ++++++++++++++++- pkg/behaviourtest/steps/fork_test.go | 111 +++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/pkg/behaviourtest/steps/fork.go b/pkg/behaviourtest/steps/fork.go index 54b1ce73e4..01b91561f6 100644 --- a/pkg/behaviourtest/steps/fork.go +++ b/pkg/behaviourtest/steps/fork.go @@ -36,6 +36,16 @@ const forkReadyMaxAttempts = 30 // forkReadyPoll is the delay between GetBranchRef polls. const forkReadyPoll = 2 * time.Second +// createBranchMaxAttempts is how many times createForkBranch +// retries CreateBranch when it fails with a replication error +// (409/422). Even after awaitForkReady passes, GitHub's +// eventually-consistent fork replication can cause transient +// failures when creating a branch. +const createBranchMaxAttempts = 5 + +// createBranchPoll is the delay between CreateBranch retries. +const createBranchPoll = 2 * time.Second + // givenFork creates a fork of the enrolled test repository if absent, or // reuses it if it already exists. The fork is created within the same // organization as the source repository. @@ -162,6 +172,57 @@ func resolveForkName(w *world.World, logicalName string) string { return w.RepoName + suffix } +// isReplicationError reports whether err looks like a GitHub fork +// replication race — a 409 "Git Repository is empty" or a 422 +// "Object does not exist" / "Tree SHA does not exist". These are +// transient: the underlying Git objects have not been replicated +// to the fork yet, but they will be shortly. +func isReplicationError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "409") || + (strings.Contains(msg, "422") && + (strings.Contains(msg, "does not exist") || + strings.Contains(msg, "empty"))) +} + +// createForkBranch wraps CreateBranch with retry logic for transient +// replication errors (409/422). Even after awaitForkReady confirms the +// default-branch ref is readable, GitHub's eventually-consistent fork +// replication can cause the ref to become temporarily unavailable again +// when CreateBranch re-fetches it. This retry closes that gap. +// +// maxAttempts and poll are explicit parameters so that unit tests can +// pass small values to avoid real sleeps. +func createForkBranch(ctx context.Context, w *world.World, owner, repo, branch string, maxAttempts int, poll time.Duration) error { + var lastErr error + for attempt := 1; attempt <= maxAttempts; attempt++ { + lastErr = w.SCM.CreateBranch(ctx, owner, repo, branch) + if lastErr == nil { + return nil + } + if !isReplicationError(lastErr) { + return lastErr + } + if attempt < maxAttempts { + select { + case <-ctx.Done(): + return fmt.Errorf( + "context cancelled retrying CreateBranch on %s/%s: %w", + owner, repo, ctx.Err(), + ) + case <-time.After(poll): + } + } + } + return fmt.Errorf( + "fork %s/%s branch creation failed after %d attempts: %w", + owner, repo, maxAttempts, lastErr, + ) +} + // whenForkPullRequestOpened commits a file to a new branch on the fork // and opens a cross-fork pull request against the base repository. func whenForkPullRequestOpened(w *world.World) error { @@ -174,10 +235,12 @@ func whenForkPullRequestOpened(w *world.World) error { ctx := context.Background() - // Create the branch on the fork first — GitHub's Contents API - // (used by CommitFileToFork → CreateOrUpdateFileOnBranch) requires - // the target branch to already exist. - if err := w.SCM.CreateBranch(ctx, w.ForkOwner, w.ForkRepo, branch); err != nil { + // Create the branch on the fork with retry for replication + // errors. awaitForkReady confirmed the default-branch ref was + // readable, but GitHub's eventually-consistent replication can + // cause transient 409/422 failures when CreateBranch re-fetches + // the ref moments later. + if err := createForkBranch(ctx, w, w.ForkOwner, w.ForkRepo, branch, createBranchMaxAttempts, createBranchPoll); err != nil { return fmt.Errorf("creating fork branch: %w", err) } // Record the branch immediately so CleanupScenario can delete it diff --git a/pkg/behaviourtest/steps/fork_test.go b/pkg/behaviourtest/steps/fork_test.go index 386bf4ed81..c3edcd239a 100644 --- a/pkg/behaviourtest/steps/fork_test.go +++ b/pkg/behaviourtest/steps/fork_test.go @@ -390,6 +390,98 @@ func TestAwaitForkReady_ContextCancelledDuringBranchNamePoll(t *testing.T) { assert.Contains(t, err.Error(), "context cancelled") } +// --- isReplicationError unit tests --- + +func TestIsReplicationError_409(t *testing.T) { + err := fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty") + assert.True(t, isReplicationError(err)) +} + +func TestIsReplicationError_422ObjectDoesNotExist(t *testing.T) { + err := fmt.Errorf("create branch: github api: 422 Object does not exist") + assert.True(t, isReplicationError(err)) +} + +func TestIsReplicationError_422TreeSHADoesNotExist(t *testing.T) { + err := fmt.Errorf("create tree: github api: 422 Tree SHA does not exist") + assert.True(t, isReplicationError(err)) +} + +func TestIsReplicationError_NilError(t *testing.T) { + assert.False(t, isReplicationError(nil)) +} + +func TestIsReplicationError_NonReplicationError(t *testing.T) { + err := fmt.Errorf("permission denied") + assert.False(t, isReplicationError(err)) +} + +func TestIsReplicationError_422WithoutObjectMessage(t *testing.T) { + // 422 without "does not exist" or "empty" is not replication. + err := fmt.Errorf("github api: 422 Validation Failed (field=sha, code=invalid)") + assert.False(t, isReplicationError(err)) +} + +// --- createForkBranch unit tests --- + +func TestCreateForkBranch_ImmediateSuccess(t *testing.T) { + scmDriver := &fakeForkSCM{} + w := &world.World{SCM: scmDriver} + err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0) + require.NoError(t, err) + assert.Equal(t, 1, scmDriver.createBranchCalls) +} + +func TestCreateForkBranch_RetriesThenSucceeds(t *testing.T) { + scmDriver := &fakeForkSCM{ + createBranchFailures: 2, + createBranchReplicaErr: fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty"), + } + w := &world.World{SCM: scmDriver} + err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0) + require.NoError(t, err) + assert.Equal(t, 3, scmDriver.createBranchCalls, + "CreateBranch should be called 2 failures + 1 success = 3 times") +} + +func TestCreateForkBranch_ExhaustsRetries(t *testing.T) { + scmDriver := &fakeForkSCM{ + createBranchFailures: -1, + createBranchReplicaErr: fmt.Errorf("get ref for default branch: github api: 409 Git Repository is empty"), + } + w := &world.World{SCM: scmDriver} + err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 3, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "branch creation failed after 3 attempts") + assert.Contains(t, err.Error(), "409") + assert.Equal(t, 3, scmDriver.createBranchCalls) +} + +func TestCreateForkBranch_NonReplicationErrorNotRetried(t *testing.T) { + scmDriver := &fakeForkSCM{ + createBranchErr: fmt.Errorf("permission denied"), + } + w := &world.World{SCM: scmDriver} + err := createForkBranch(context.Background(), w, "org", "repo-fork", "test-branch", 5, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "permission denied") + assert.Equal(t, 1, scmDriver.createBranchCalls, + "non-replication errors should not be retried") +} + +func TestCreateForkBranch_ContextCancelled(t *testing.T) { + scmDriver := &fakeForkSCM{ + createBranchFailures: -1, + createBranchReplicaErr: fmt.Errorf("github api: 409 Git Repository is empty"), + } + w := &world.World{SCM: scmDriver} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + err := createForkBranch(ctx, w, "org", "repo-fork", "test-branch", 30, 2*time.Second) + require.Error(t, err) + assert.Contains(t, err.Error(), "context cancelled") +} + // fakeForkSCM implements scm.Driver for fork step unit tests. type fakeForkSCM struct { forkRepo string @@ -420,6 +512,14 @@ type fakeForkSCM struct { // means GetBranchRef always fails. getBranchRefFailures int getBranchRefCalls int + + // createBranchFailures controls how many times CreateBranch + // returns a replication error before succeeding. Each call + // decrements the counter; when it reaches 0, CreateBranch + // returns success. A value of -1 means CreateBranch always fails. + createBranchFailures int + createBranchCalls int + createBranchReplicaErr error // error to return on replication failures } type addedLabelRecord struct { @@ -491,9 +591,20 @@ func (f *fakeForkSCM) CommitFile(context.Context, string, string, string, string func (f *fakeForkSCM) CreateBranch(_ context.Context, _, _, _ string) error { f.createBranchCalled = true + f.createBranchCalls++ if f.createBranchErr != nil { return f.createBranchErr } + // Support counted replication failures for retry tests. + if f.createBranchReplicaErr != nil { + if f.createBranchFailures == -1 { + return f.createBranchReplicaErr + } + if f.createBranchFailures > 0 { + f.createBranchFailures-- + return f.createBranchReplicaErr + } + } return nil }