Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

#25

Draw the merge control from the landing plan

merged · by keenan 13d ago · merged into main at 097b2110 by keenan 13d ago

keenan opened this 13d ago

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 main cleanly: readme.md — rebase or merge main before landing it” (the pre-existing “What would land” block, which does run merge-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:

  • Land prepares, 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 from RequestView), 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 Prepare with Conflicts = [readme.md], NewTip empty, and the base branch untouched;
  • a clean non-fast-forward Prepare describes the merge commit, names its parents in the order the gate expects, moves nothing — and the Land that follows produces the same NewTip, 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.

Discussion (0)

No discussion yet.