Commit `79155d1b2f`

79155d1b2f44b69708073e62a6a6f3b164cfc1fb

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

e2e/mergerecord_test.go +8 −10

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

e2e/mrweb_test.go +3 −4

		@@ -407,8 +407,8 @@ func (i instance) mrThreads(t testing.T, key, repo, n string) []mrThread {
407	407	return env.Data
408	408	}
409	409
410		// TestMRDiffEmptyExplained: a merge request whose head was fast-forwarded
411		// into the target outside the request shows why its diff is empty.
	410	// TestMRDiffEmptyExplained: a merge request opened on a head the target
	411	// already contains shows why its diff is empty.
412	412	func TestMRDiffEmptyExplained(t *testing.T) {
413	413	t.Parallel()
414	414	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
		@@ -431,11 +431,10 @@ func TestMRDiffEmptyExplained(t *testing.T) {
431	431	os.WriteFile(filepath.Join(dir, "f.txt"), []byte("one\n"), 0o644)
432	432	mustGit(t, dir, env, "add", ".")
433	433	mustGit(t, dir, env, "commit", "-q", "-m", "one")
434		mustGit(t, dir, env, "push", "-q", "origin", "feature")
	434	mustGit(t, dir, env, "push", "-q", "origin", "feature", "feature:main")
435	435	if _, errOut, code := inst.ssh(t, key, "", "mr", "create", "alice/app", "--source", "feature", "--target", "main", "--title", "one"); code != 0 {
436	436	t.Fatal(errOut)
437	437	}
438		mustGit(t, dir, env, "push", "-q", "origin", "feature:main")
439	438	_, body := inst.get(t, "/alice/app/mrs/1?view=diff")
440	439	if !strings.Contains(body, "No changes between the source and target.") \|\|
441	440	!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	1640	return c.fail(protocol.ExitFailure, "%v", err)
1641	1641	}
1642	1642	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA))
	1643	MarkLandedMRs(c.Store, dir, repo.ID, c.User.ID, mr.TargetRef, targetSHA, newSHA)
1643	1644	// The stack moves up: whatever targeted this branch now targets what
1644	1645	// it merged into, reviews intact, since that diff is the one they
1645	1646	// were of.

internal/control/refsupdated.go +44

		@@ -118,6 +118,50 @@ func RefsUpdated(st *store.Store, cfg config.Config, repoID, userID int64, scope
118	118	}
119	119	}
120	120	}
	121	// After every MR head above is refreshed, so a push carrying both a
	122	// source branch and its target compares against the new head.
	123	if pushedRepoErr == nil {
	124	dir := RepoDir(cfg.Server.Root, pushedRepo.OwnerName, pushedRepo.Name)
	125	for _, u := range updates {
	126	if branch, ok := cutHeads(u.Ref); ok && !u.IsDelete {
	127	MarkLandedMRs(st, dir, pushedRepo.ID, 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, dir string, repoID, userID int64, branch, old, new string) {
	138	mrs, err := st.UnmergedMRsByTarget(repoID, branch)
	139	if err != nil {
	140	slog.Error("post-receive: listing MRs by target", "err", err)
	141	return
	142	}
	143	for _, mr := range mrs {
	144	head, err := gitutil.ResolveRef(dir, mrHeadRef(mr.Number))
	145	if err != nil {
	146	continue
	147	}
	148	if landed, err := gitutil.IsAncestor(dir, head, new); err != nil \|\| !landed {
	149	continue
	150	}
	151	base := ""
	152	if !gitutil.ZeroSHA(old) {
	153	if was, err := gitutil.IsAncestor(dir, head, old); err != nil \|\| was {
	154	continue
	155	}
	156	base, _ = gitutil.MergeBase(dir, old, head)
	157	}
	158	if err := st.MarkMerged(mr.ID, base, head, userID, ""); err != nil {
	159	slog.Error("post-receive: recording landed MR", "mr", mr.Number, "err", err)
	160	continue
	161	}
	162	st.DequeueMerge(mr.ID)
	163	st.RecordEvent(repoID, userID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, head))
	164	}
121	165	}
122	166
123	167	// 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	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	306	// MarkMerged records the merge along with the target tip it landed on, so
286	307	// the MR's diff stays reconstructable after fast-forwards, and the commit
287	308	// it landed as, which release notes match against a range. actorID 0 and

Commit 79155d1b2f

Commit `79155d1b2f`