Part of #01M5XF4XWHVMMCQEYENA8X32GK. This change is the landing half: a
server-side merge with the merged status. vole change merge follows in its own
change request, because the CLI has no access to the repository’s ACL or the run
queue and therefore needs a verb on the SSH channel — a transport worth reviewing
on its own rather than buried in this one.
Why
Landing was “merge locally, push the base branch”. Nothing recorded that a request
had landed: requests/<ulid>/request.md carries state: open|merged|closed, the
reader accepted merged and the activity feed rendered “merged change request”, but
no code path ever wrote it. A landed request and an abandoned one looked identical.
The stronger reason is the gate. Approval and check enforcement lives only in the
pre-receive hook (server/hook/hook.go), which sees pushes. Anything the server does
itself moves the ref without the hook, so a merge button either re-runs that rule or
quietly becomes the easiest way to land an unapproved change.
What changed
forge/landing(new): the gate moved out of the hook.Gate.Checkis what both callers ask — unprotected ref needs nothing, an admin keeps the bypass, and everyone else needs an open request whose head is exactly the commit being landed, with the required checks passing for that head. The check query (CheckGaps, formerlyserver/hook/checkGaps) moved with it, unchanged in wording, so a refusal reads the same whether a push or a merge met it.landing.Land: fast-forward when the base is an ancestor of the reviewed head (the tested SHA is the landed SHA); otherwise a two-parent merge commit, the only other shapeFindApprovedMatchaccepts, so the reviewed head stays a parent. Conflicts are refused and named, never resolved. The ref moves byUpdateRefCAS, so a concurrent push loses cleanly and the landing retries; a closed request is refused.- Ordering: ref first, request second. A crash between them leaves a landed head and an open request, and re-running lands nothing and only writes the mark — idempotent, keyed on “the head is already in the base”.
metadata:merged_head,merged_by,merged_aton a request, written byMarkMergedin the same commit as the state. The reader refusesstate: mergedwithout a full commit id, refuses those fields on a request that is not merged, andSetRequestStateno longer acceptsmergedat all — it has to travel with the commit.- Web:
POST /r/{name}/requests/{id}/mergecalls the sameLand. The button is drawn from a preflight ofGate.Check, so it appears only when the landing would succeed, and when it does not the page says why (missing approval, failing check, conflict). The header shows “merged intomainat<sha>by<account>”. core/git:MergeTreereturns the tree id as well as the conflicts (a merge commit needs it), andCommitMergeTreewrites a two-parent commit.
Verification
vole workflow run verify in the pinned image: gofmt, vet, build, govulncheck
(0 reachable), packaging, links, and go test -race ./... all pass.
New tests:
forge/landing: fast-forward lands the reviewed head and recordsmerged_head/merged_by; no approval is refused and moves nothing; a moved base produces a merge commit whose parents are[base tip, reviewed head]; a conflict is refused and names the path; a second landing reports already-landed and keeps the original head; a closed request is refused.metadata: a merged request must name a full commit id,merged_byand an RFC3339merged_at; those fields are refused on a non-merged request; andSetRequestState("merged")is refused.
The hook’s own tests still pass unchanged, which is the point of the extraction: the push path behaves as before, and it now shares its rule with the merge path.
Not in this change
vole change merge <id>. The operation it needs is here; what is missing is a way
for a CLI process to reach it, since ACL rules and run results live in the instance,
not in the clone. That verb and its client are the next change against the same
issue.
Acceptance against the issue
- One gate, used by the hook and the merge operation
- Fast-forward lands the tested SHA; otherwise a two-parent merge of the approved head
- A merge with no approval or a failing check is refused and changes nothing
- Conflicts refused with the paths
- Two concurrent landings: one wins the CAS, the loser reports the move
-
state: mergedis only writable with a commit that backs it - The web control is drawn from the gate, so it cannot offer a landing the handler would refuse
-
vole change merge— next change