Follow-up to #01M5XF4XWHVMMCQEYENA8X32GK (the merged halves are #01M5XFEHF38KDXD21TZVJS15GS and #01M5XFTNYNSBWAP2BECH22V2M1). This fixes a hole in the first one.
The bug
The merge control was drawn from Gate.Check, which answers two questions —
approvals and required checks. It never looks at the tree. So a request that no
longer merges into its base rendered:
- “This branch no longer merges into
maincleanly:readme.md— rebase or mergemainbefore landing it” (the pre-existing “What would land” block, which does runmerge-tree), and - a Merge button, because the gate was satisfied.
Clicking it ran the landing, merge-tree refused, and the handler answered with
http.Error(..., 409) — a bare text/plain page. The paths were in that sentence,
but the review, the diff and the styled conflict block were gone, for a button the
page had already told the reader was hopeless.
That also failed the acceptance criterion the first change claimed: “the button appears only when the CLI command would succeed.”
The fix
One decision, three callers. landing.Prepare works out what a landing would do —
fast-forward, merge commit, already landed, or the list of conflicting paths — and
refuses exactly what Land refuses:
Landprepares, then moves the ref and marks the request. Its retry loop now recomputes the plan against the new tip after a lost CAS, rather than rebuilding it inline.- The page prepares to decide whether to draw the Merge button. A conflict is reported in the plan rather than as an error, because the paths are already listed by “What would land”: the sidebar stays quiet and the button stays away.
- A refused merge re-renders the request page with the message beside the conflict
block (
renderRequestPage, extracted fromRequestView), so the reader keeps the review instead of getting one line of text.
Prepare is not quite pure: a non-fast-forward landing needs the merge commit to
exist before the gate can be asked, because FindApprovedMatch matches a commit whose
parents are the base tip and the approved head. So it writes one unreferenced commit
object and moves nothing; git collects it. That is stated in the type’s doc comment
rather than hidden.
Verification
vole workflow run verify in the pinned image: gofmt, vet, build, govulncheck
(0 reachable), packaging, links, go test -race ./... — all pass.
New tests in forge/landing:
- a conflicting landing is reported by
PreparewithConflicts = [readme.md],NewTipempty, and the base branch untouched; - a clean non-fast-forward
Preparedescribes the merge commit, names its parents in the order the gate expects, moves nothing — and theLandthat follows produces the sameNewTip, which is the property that makes the control trustworthy.
The existing nine landing tests and the hook’s suite still pass, since Land’s
behaviour is unchanged; only the decision moved.