fix: Clarify error when GitHub PR merge refspec fetch fails - #4254
Review submitted — approved
Review summary
Change reviewed
PR #4254 clarifies checkout failures for GitHub's speculative refs/pull/N/merge ref. When GitHub cannot create that ref—commonly because the pull request has a merge conflict—the agent now appends a user-facing hint instead of returning only the generic Git fetch error.
The implementation covers both checkout paths that can fetch the merge ref:
internal/job/checkout_fetch.goappends the hint to failures from the canonical PR-ref fetch, for both known-commit andHEADflows.internal/job/checkout_mirror.goapplies the same hint when an existing--git-mirrors-pathmirror is updated. It also stops applying the approximately two-minute PR-head retry policy to merge-ref fetches, matching the existing fail-fast behavior infetchSource.internal/job/checkout_test.gonow asserts the hint in the existing canonical missing-merge-ref test.internal/job/checkout_mirror_remote_test.goadds coverage for the corresponding existing-mirror failure path.
This matches the behavior requested by Linear issues A-1711 and A-1722.
Review coverage
I read the complete four-file diff and the surrounding implementations. I traced checkout from defaultCheckoutPhase through getOrUpdateMirrorDir/updateGitMirror and then fetchSource, and inspected gitFetch to verify its retry and error-classification behavior. I also checked remote-mirror eligibility handling to confirm merge-ref builds remain on the canonical path. The new fmt.Errorf wrapping preserves the underlying Git error while adding the diagnostic suffix.
The previous blocking review identified that updateGitMirror returned the raw error before fetchSource could add the hint. The current revision addresses that path directly and includes a focused regression test. The author also resolved the review thread; I acknowledged the fix and dismissed the superseded changes-requested review.
Verification
Executed locally with Go 1.26.5:
- The two new/changed missing-merge-ref tests passed.
- The existing PR-head mirror fallback test passed.
TestDefaultCheckoutPhasepassed.- The complete
internal/jobpackage test suite passed after installing the repository's documented SSH alias test fixture in/etc/ssh/ssh_config.d/. - The changed files produced no
gofmtdiff.
I also attempted go test ./.... The relevant internal/job package passed, but the overall command could not complete in this environment because the compiler was killed while building the large AWS EC2 SDK package, consistent with a local resource limit rather than a test assertion.
Buildkite build #14026 briefly reported a failure and was pending again at the final status check. Its job logs were not available through the Buildkite API credentials in this environment, so I could not verify the reason for that CI transition.
Verdict
Approved revision 5ef69bde5eae7f0db27fc0008136f56242ecc8a0 with no new inline findings. The prior mirror-update concern is resolved, and the changed checkout behavior is covered by focused tests.
Trigger source: automatic.