fix: Clarify error when GitHub PR merge refspec fetch fails - #4254
Conversation
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
The error-path change is straightforward. I left one non-blocking suggestion to pin the new diagnostic in the existing missing-merge-ref test.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 18629, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
Model: gpt-5.6-sol with xhigh thinking.
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. Unless repository policy already allows L2 approval, grant it by mentioning @buildsworth-bk (see approval ceiling and L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only when repository policy or a verified grant allows it; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.
1a87354 to
5403188
Compare
ss1909
changed the title
5403188 to
6ad5bb3
Compare
There was a problem hiding this comment.
One supported checkout path still bypasses the new diagnostic; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 18638, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
refs/pull/N/merge only exists when GitHub can compute the speculative merge. When there's a real merge conflict the ref is never created and the fetch fails with a generic git error, which looks like transient flake rather than something the PR author needs to resolve. A-1711, A-1722
6ad5bb3 to
5ef69bd
Compare
Previous concerns have been addressed.
|
🤖 Docs Draft Bot: I've created a draft documentation PR based on this change: 📝 Docs PR: https://github.com/buildkite/docs-private/pull/2023 Please review the draft and make any necessary adjustments. |
1 similar comment
|
🤖 Docs Draft Bot: I've created a draft documentation PR based on this change: 📝 Docs PR: https://github.com/buildkite/docs-private/pull/2023 Please review the draft and make any necessary adjustments. |
2 participants
Description
refs/pull/N/mergeonly exists when GitHub can compute the speculative merge. When there's a real merge conflict the ref is never created and the fetch fails with a generic git error, which looks like transient flake rather than something the PR author needs to resolve. This fixes that by appending a clear hint when these errors surface.Two fetch paths hit this ref, so both are covered:
fetchSource(canonical checkout) — already fails fast on a missing merge ref; now also gets the hint.updateGitMirror(the--git-mirrors-pathmirror-update fetch) — previously returned the raw git error with no hint, and unconditionally retried for ~2m17s even for the merge refspec. Now it gets the same hint and skips the retry for the merge refspec, matchingfetchSource's existing rationale (a missing merge ref usually means a real conflict, not a transient issue worth retrying).Context
A-1711, A-1722
Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go tool gofumpt -extra -w .)Affiliation (optional, external contributors)
Disclosures / Credits