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.

#22

Land a change request on the server instead of by pushing

merged · by keenan 13d ago · merged into main at 38b30251 by keenan 13d ago

keenan opened this 13d ago

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.Check is 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, formerly server/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 shape FindApprovedMatch accepts, so the reviewed head stays a parent. Conflicts are refused and named, never resolved. The ref moves by UpdateRefCAS, 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_at on a request, written by MarkMerged in the same commit as the state. The reader refuses state: merged without a full commit id, refuses those fields on a request that is not merged, and SetRequestState no longer accepts merged at all — it has to travel with the commit.
  • Web: POST /r/{name}/requests/{id}/merge calls the same Land. The button is drawn from a preflight of Gate.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 into main at <sha> by <account> ”.
  • core/git: MergeTree returns the tree id as well as the conflicts (a merge commit needs it), and CommitMergeTree writes 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 records merged_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_by and an RFC3339 merged_at; those fields are refused on a non-merged request; and SetRequestState("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: merged is 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

Discussion (0)

No discussion yet.