Commit `aa92532762`

aa9253276246e4345be8e0298619e174b9c6dea7

parent: b55f4c670c

Verified · cmc ci/build: success ci/test: success

cmc <hello@cleberg.net> · 2026-10-16 04:33 UTC

mr: record merge requests whose head lands by push

When a branch moves from old to new, each open or source_gone merge
request targeting it whose head is an ancestor of new and not of old is
marked merged by the actor, dequeued, and gets an mr.merged event.
merged_base is merge-base(old, head). Runs at the end of RefsUpdated,
after MR heads are refreshed, and after mr merge moves the target.

TestMRDiffEmptyExplained now opens its MR after the push, since a
fast-forward outside the MR records it as merged.

Closes #330

Layout: unified · split

.gitbay/wiki/Users.org +6

 is still a push, since there is nothing to open a merge request against
 yet.
+A merge request whose head reaches its target some other way — a direct
+push, or inside another merge request that is merged — is recorded as
+merged by whoever moved the branch, including one whose source branch
+is gone. One whose head was on the target before the update is left
+alone; =mr merge= on it records it as merged.
 A merge request whose target is another open merge request's source
 branch is stacked on it: =mr create= says so, =mr show= carries
 =stacked_on= and =stacked=, and merging the lower one retargets the

e2e/mergerecord_test.go +8 −10

 	"testing"
 )
-// A merge request whose head the target already contains is recorded as
+// A merge request whose head is pushed onto its target by hand is
-// merged rather than refused: the branch was merged by hand and pushed,
+// recorded as merged by the push (#330); mr merge on it then reports the
-// or a merge moved the ref and then failed to record itself. Either way
+// state rather than merging again.
-// refusing left the merge request open for good (#108).
+func TestMergeRecordedWhenPushLandsHead(t *testing.T) {
-func TestMergeRecordedWhenTargetContainsHead(t *testing.T) {
 	t.Parallel()
 	inst := startInstance(t)
 	aliceKey := inst.newKey(t, "alice")
 	// Merged by hand: main fast-forwards to the branch outside the forge.
 	mustGit(t, dir, env, "push", "-q", "origin", "feat:main")
-	out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", "--json")
-	if code != 0 || !strings.Contains(out, `"strategy":"recorded"`) {
-		t.Fatalf("merge of an already-landed head: exit %d\n%s%s", code, out, errOut)
-	}
 	show, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "1", "--json")
 	if !strings.Contains(show, `"state":"merged"`) || !strings.Contains(show, `"merged_by":"alice"`) {
-		t.Fatalf("merge request not recorded as merged:\n%s", show)
+		t.Fatalf("merge request not recorded as merged by the push:\n%s", show)
+	}
+	if out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1"); code == 0 || !strings.Contains(errOut, "is merged") {
+		t.Fatalf("merge of a merged MR: exit %d\n%s%s", code, out, errOut)
 	}
 }

e2e/mrweb_test.go +3 −4

 	return env.Data
 }
-// TestMRDiffEmptyExplained: a merge request whose head was fast-forwarded
+// TestMRDiffEmptyExplained: a merge request opened on a head the target
-// into the target outside the request shows why its diff is empty.
+// already contains shows why its diff is empty.
 func TestMRDiffEmptyExplained(t *testing.T) {
 	t.Parallel()
 	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
 	os.WriteFile(filepath.Join(dir, "f.txt"), []byte("one\n"), 0o644)
 	mustGit(t, dir, env, "add", ".")
 	mustGit(t, dir, env, "commit", "-q", "-m", "one")
-	mustGit(t, dir, env, "push", "-q", "origin", "feature")
+	mustGit(t, dir, env, "push", "-q", "origin", "feature", "feature:main")
 	if _, errOut, code := inst.ssh(t, key, "", "mr", "create", "alice/app", "--source", "feature", "--target", "main", "--title", "one"); code != 0 {
 		t.Fatal(errOut)
 	}
-	mustGit(t, dir, env, "push", "-q", "origin", "feature:main")
 	_, body := inst.get(t, "/alice/app/mrs/1?view=diff")
 	if !strings.Contains(body, "No changes between the source and target.") ||
 		!strings.Contains(body, "already merged or fast-forwarded into <code>main</code>") {

internal/control/landed_test.go added +85

@@ -0,0 +1,85 @@
	1	package control
	2
	3	import (
	4	"strings"
	5	"testing"
	6
	7	"gitbay.org/gitbay/internal/config"
	8	"gitbay.org/gitbay/internal/policy"
	9	)
	10
	11	// A push that puts a merge request's head on its target records the
	12	// merge request as merged by the pusher (#330).
	13	func TestPushLandingTheHeadMarksMRMerged(t *testing.T) {
	14	f := newQueueFixture(t, nil)
	15	f.branch("main", f.headSHA)
	16	var cfg config.Config
	17	cfg.Server.Root = f.root
	18	RefsUpdated(f.st, cfg, f.repo.ID, f.alice.ID, "full", []policy.RefUpdate{
	19	{Ref: "refs/heads/main", Old: f.targetSH, New: f.headSHA}})
	20
	21	mr := f.mr()
	22	if mr.State != "merged" \|\| mr.MergedBy != "alice" \|\| mr.MergedSHA != f.headSHA \|\| mr.MergedBase != f.targetSH {
	23	t.Fatalf("MR = state %s by %q sha %q base %q, want merged by alice at %s on %s",
	24	mr.State, mr.MergedBy, mr.MergedSHA, mr.MergedBase, f.headSHA, f.targetSH)
	25	}
	26	}
	27
	28	// A source_gone merge request whose head lands inside another merge
	29	// request's merge is recorded as merged with it (#330).
	30	func TestMergeCarryingAnotherHeadMarksItMerged(t *testing.T) {
	31	f := newQueueFixture(t, nil)
	32	f.git(f.src, "checkout", "-q", "-b", "next", "feature")
	33	f.write("next.txt", "z\n")
	34	f.git(f.src, "add", ".")
	35	f.git(f.src, "commit", "-q", "-m", "next")
	36	next := strings.TrimSpace(f.git(f.src, "rev-parse", "HEAD"))
	37	f.git(f.dir, "fetch", "-q", f.src, "refs/heads/next:refs/heads/next")
	38	if _, err := f.st.CreateMR(f.repo.ID, f.alice.ID, f.repo.ID, "next", "main", "next", "", next, "md", false); err != nil {
	39	t.Fatal(err)
	40	}
	41	f.git(f.dir, "update-ref", mrHeadRef(2), next)
	42	if err := f.st.SetMRState(f.mr().ID, "source_gone"); err != nil {
	43	t.Fatal(err)
	44	}
	45
	46	f.mustRun(f.alice, "mr", "merge", f.repo.Path(), "2", "--strategy", "ff")
	47	if mr := f.mr(); mr.State != "merged" \|\| mr.MergedSHA != f.headSHA {
	48	t.Fatalf("!1 = state %s sha %q, want merged at %s", mr.State, mr.MergedSHA, f.headSHA)
	49	}
	50	}
	51
	52	// A head already on the target before the push is left alone: the push
	53	// did not land it.
	54	func TestPushLeavesHeadAlreadyOnTarget(t *testing.T) {
	55	f := newQueueFixture(t, nil)
	56	f.branch("main", f.headSHA)
	57	var cfg config.Config
	58	cfg.Server.Root = f.root
	59	f.git(f.src, "checkout", "-q", "feature")
	60	f.write("more.txt", "m\n")
	61	f.git(f.src, "add", ".")
	62	f.git(f.src, "commit", "-q", "-m", "more")
	63	more := strings.TrimSpace(f.git(f.src, "rev-parse", "HEAD"))
	64	f.git(f.dir, "fetch", "-q", f.src, "refs/heads/feature:refs/heads/main")
	65	RefsUpdated(f.st, cfg, f.repo.ID, f.alice.ID, "full", []policy.RefUpdate{
	66	{Ref: "refs/heads/main", Old: f.headSHA, New: more}})
	67
	68	if mr := f.mr(); mr.State != "open" {
	69	t.Fatalf("MR = state %s, want open", mr.State)
	70	}
	71	}
	72
	73	// mr merge still records a head that reached the target without
	74	// post-receive seeing it (#108).
	75	func TestMergeRecordsHeadAlreadyOnTarget(t *testing.T) {
	76	f := newQueueFixture(t, nil)
	77	f.branch("main", f.headSHA)
	78	out := f.mustRun(f.alice, "mr", "merge", f.repo.Path(), "1", "--json")
	79	if !strings.Contains(out, `"strategy":"recorded"`) {
	80	t.Fatalf("merge of an already-landed head:\n%s", out)
	81	}
	82	if mr := f.mr(); mr.State != "merged" \|\| mr.MergedBy != "alice" {
	83	t.Fatalf("MR = state %s by %q, want merged by alice", mr.State, mr.MergedBy)
	84	}
	85	}

internal/control/mr.go +1

	@@ -1640,6 +1640,7 @@ func mergeMR(c *Ctx, repo store.Repo, mr store.MR, strategy string) int {
1640	return c.fail(protocol.ExitFailure, "%v", err)	1640	return c.fail(protocol.ExitFailure, "%v", err)
1641	}	1641	}
1642	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA))	1642	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA))
		1643	MarkLandedMRs(c.Store, c.Cfg, repo, c.User.ID, mr.TargetRef, targetSHA, newSHA)
1643	// The stack moves up: whatever targeted this branch now targets what	1644	// The stack moves up: whatever targeted this branch now targets what
1644	// it merged into, reviews intact, since that diff is the one they	1645	// it merged into, reviews intact, since that diff is the one they
1645	// were of.	1646	// were of.

internal/control/refsupdated.go +59

	@@ -3,6 +3,7 @@ package control
3	import (	3	import (
4	"encoding/json"	4	"encoding/json"
5	"fmt"	5	"fmt"
		6	"io"
6	"log/slog"	7	"log/slog"
7	"path"	8	"path"
8	"strings"	9	"strings"
	@@ -118,6 +119,64 @@ func RefsUpdated(st *store.Store, cfg config.Config, repoID, userID int64, scope
118	}	119	}
119	}	120	}
120	}	121	}
		122	// After every MR head above is refreshed, so a push carrying both a
		123	// source branch and its target compares against the new head.
		124	if pushedRepoErr == nil {
		125	for _, u := range updates {
		126	if branch, ok := cutHeads(u.Ref); ok && !u.IsDelete {
		127	MarkLandedMRs(st, cfg, pushedRepo, userID, branch, u.Old, u.New)
		128	}
		129	}
		130	}
		131	}
		132
		133	// MarkLandedMRs records as merged each open or source_gone merge request
		134	// into branch whose head the move from old to new brought onto it: a
		135	// direct push, or the head riding in with another merge request (#330).
		136	// A head already on old is left alone.
		137	func MarkLandedMRs(st *store.Store, cfg config.Config, repo store.Repo, userID int64, branch, old, new string) {
		138	mrs, err := st.UnmergedMRsByTarget(repo.ID, branch)
		139	if err != nil {
		140	slog.Error("post-receive: listing MRs by target", "err", err)
		141	return
		142	}
		143	dir := RepoDir(cfg.Server.Root, repo.OwnerName, repo.Name)
		144	var c *Ctx
		145	for _, mr := range mrs {
		146	head, err := gitutil.ResolveRef(dir, mrHeadRef(mr.Number))
		147	if err != nil {
		148	continue
		149	}
		150	if landed, err := gitutil.IsAncestor(dir, head, new); err != nil \|\| !landed {
		151	continue
		152	}
		153	base := ""
		154	if !gitutil.ZeroSHA(old) {
		155	if was, err := gitutil.IsAncestor(dir, head, old); err != nil \|\| was {
		156	continue
		157	}
		158	base, _ = gitutil.MergeBase(dir, old, head)
		159	}
		160	if err := st.MarkMerged(mr.ID, base, head, userID, ""); err != nil {
		161	slog.Error("post-receive: recording landed MR", "mr", mr.Number, "err", err)
		162	continue
		163	}
		164	st.DequeueMerge(mr.ID)
		165	st.RecordEvent(repo.ID, userID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, head))
		166	if c == nil {
		167	u, err := st.UserByID(userID)
		168	if err != nil {
		169	continue
		170	}
		171	c = &Ctx{User: u, Store: st, Cfg: cfg, Stdin: emptyReader{}, Stdout: io.Discard, Stderr: io.Discard}
		172	}
		173	if parts, err := st.MRParticipants(mr.ID); err == nil {
		174	notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number,
		175	subject: mrSubject(repo, mr.Number, mr.Title),
		176	action: fmt.Sprintf("merged !%d into %s (pushed)", mr.Number, branch),
		177	path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)})
		178	}
		179	}
121	}	180	}
122		181
123	// adoptDefaultBranch moves an unborn HEAD to the first branch a push	182	// adoptDefaultBranch moves an unborn HEAD to the first branch a push

internal/store/mrs.go +21

	@@ -282,6 +282,27 @@ func (s *Store) OpenMRsBySource(sourceRepoID int64, sourceRef string) ([]MR, err
282	return out, rows.Err()	282	return out, rows.Err()
283	}	283	}
284		284
		285	// UnmergedMRsByTarget returns the open and source_gone MRs in repoID
		286	// targeting targetRef, oldest first.
		287	func (s *Store) UnmergedMRsByTarget(repoID int64, targetRef string) ([]MR, error) {
		288	rows, err := s.DB.Query(
		289	mrSelect+" WHERE m.repo_id = ? AND m.target_ref = ? AND m.state IN ('open','source_gone') ORDER BY m.number",
		290	repoID, targetRef)
		291	if err != nil {
		292	return nil, err
		293	}
		294	defer rows.Close()
		295	var out []MR
		296	for rows.Next() {
		297	m, err := scanMR(rows)
		298	if err != nil {
		299	return nil, err
		300	}
		301	out = append(out, m)
		302	}
		303	return out, rows.Err()
		304	}
		305
285	// MarkMerged records the merge along with the target tip it landed on, so	306	// MarkMerged records the merge along with the target tip it landed on, so
286	// the MR's diff stays reconstructable after fast-forwards, and the commit	307	// the MR's diff stays reconstructable after fast-forwards, and the commit
287	// it landed as, which release notes match against a range. actorID 0 and	308	// it landed as, which release notes match against a range. actorID 0 and

Commit aa92532762

Commit `aa92532762`