Skip to content

ci(#6825): retry CreateBranch on fork replication errors - #6827

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6825-fork-branch-retry
Open

ci(#6825): retry CreateBranch on fork replication errors#6827
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/6825-fork-branch-retry

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Add retry logic around CreateBranch in fork PR scenarios to handle transient GitHub fork replication errors (409/422)
  • awaitForkReady confirms the default-branch ref is readable, but GitHub's eventually-consistent replication can make it temporarily unavailable again when CreateBranch re-fetches it moments later
  • New createForkBranch wrapper retries up to 5 times with 2s backoff; isReplicationError classifies 409 and 422 replication errors separately from permanent failures

Testing

  • 11 new unit tests covering isReplicationError (6 tests) and createForkBranch (5 tests) — all at 100% coverage
  • All 35 existing fork step tests continue to pass
  • go vet clean

Closes #6825

Post-script verification

  • Branch is not main/master (agent/6825-fork-branch-retry)
  • Secret scan passed (gitleaks — 2a29f774dbf5707909c4265d10b2b4b324c8a344..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

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
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 31, 2026 20:44
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 31, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:46 PM UTC · Completed 9:02 PM UTC

Commit: 22b7967 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.15

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 31, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small, focused change (2 files, 182 lines) adding retry logic for fork replication errors with good test coverage (50% test file ratio). Bot author with prior contributions. Elevated fix/revert churn in git history indicates an active bug area, but the change is well-scoped to the linked issue and easy to roll back.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [edge-case] pkg/behaviourtest/steps/fork.go:183isReplicationError uses bare substring matching on "409", which could false-positive on unrelated text containing that substring. In the current CreateBranch call chain the only source of "409" is the APIError format string, so this is safe today but fragile against future error message changes. Consider using errors.As to extract *github.APIError and inspecting the StatusCode field directly.

  • [error-handling-idiom] pkg/behaviourtest/steps/fork.go:175isReplicationError classifies errors by substring-matching err.Error(), which departs from the established codebase pattern of using typed error interfaces (forge.IsTransient, errors.As with *github.APIError). The same 409 status code check is done via errors.As in e2e/admin/admin_test.go. However, fork.go operates through the forge-agnostic scm.Driver interface, so introducing a direct github.APIError dependency would require either importing internal/forge/github or extending the forge error taxonomy with a status-code-bearing interface — an architectural decision beyond this PR's scope.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Agent PR ready for human review risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(behaviour): fork readiness race persists after initial polling

1 participant