Commit 8c94b3c92a

8c94b3c92a3428a370bdd9a2c8241c6dfc9b3b78

parent: 4fdea28d08

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

cmc <hello@cleberg.net> · 2026-10-12 04:27 UTC

control: record MRs merged when their head lands on the target

A branch update from old to new marks merged each open or source_gone MR targeting it whose head is an ancestor of new and not of old, by the user who moved the branch. Runs from RefsUpdated and after mr merge, so an MR carried inside another one is recorded too. A head already on the target before the update is left to mr merge (#108).

Closes #330

Layout: unified · split

.gitbay/wiki/Users.org +6
@@ -685,6 +685,12 @@ editor, and the server's merge is its only writer. Creating the branch
685is still a push, since there is nothing to open a merge request against 685is still a push, since there is nothing to open a merge request against
686yet. 686yet.
687 687
688A merge request whose head reaches its target some other way — a direct
689push, or inside another merge request that is merged — is recorded as
690merged by whoever moved the branch, including one whose source branch
691is gone. One whose head was on the target before the update is left
692alone; =mr merge= on it records it as merged.
693
688A merge request whose target is another open merge request's source 694A merge request whose target is another open merge request's source
689branch is stacked on it: =mr create= says so, =mr show= carries 695branch is stacked on it: =mr create= says so, =mr show= carries
690=stacked_on= and =stacked=, and merging the lower one retargets the 696=stacked_on= and =stacked=, and merging the lower one retargets the
e2e/mergerecord_test.go +14 −5
@@ -8,9 +8,10 @@ import (
8) 8)
9 9
10// A merge request whose head the target already contains is recorded as 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, 11// merged rather than refused: it was opened after the branch landed, or a
12// or a merge moved the ref and then failed to record itself. Either way 12// merge moved the ref and then failed to record itself. Either way
13// refusing left the merge request open for good (#108). 13// refusing left the merge request open for good (#108). A push that lands
14// the head on an open merge request records it then (#330).
14func TestMergeRecordedWhenTargetContainsHead(t *testing.T) { 15func TestMergeRecordedWhenTargetContainsHead(t *testing.T) {
15 t.Parallel() 16 t.Parallel()
16 inst := startInstance(t) 17 inst := startInstance(t)
@@ -38,12 +39,20 @@ func TestMergeRecordedWhenTargetContainsHead(t *testing.T) {
38 } 39 }
39 // Merged by hand: main fast-forwards to the branch outside the forge. 40 // Merged by hand: main fast-forwards to the branch outside the forge.
40 mustGit(t, dir, env, "push", "-q", "origin", "feat:main") 41 mustGit(t, dir, env, "push", "-q", "origin", "feat:main")
42 show, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "1", "--json")
43 if !strings.Contains(show, `"state":"merged"`) || !strings.Contains(show, `"merged_by":"alice"`) {
44 t.Fatalf("push landing the head did not record the merge:\n%s", show)
45 }
41 46
42 out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", "--json") 47 // Opened after its head landed: mr merge records it.
48 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", "--source", "feat", "--target", "main", "--title", "again"); code != 0 {
49 t.Fatalf("mr create: %s", errOut)
50 }
51 out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "2", "--json")
43 if code != 0 || !strings.Contains(out, `"strategy":"recorded"`) { 52 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) 53 t.Fatalf("merge of an already-landed head: exit %d\n%s%s", code, out, errOut)
45 } 54 }
46 show, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "1", "--json") 55 show, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "2", "--json")
47 if !strings.Contains(show, `"state":"merged"`) || !strings.Contains(show, `"merged_by":"alice"`) { 56 if !strings.Contains(show, `"state":"merged"`) || !strings.Contains(show, `"merged_by":"alice"`) {
48 t.Fatalf("merge request not recorded as merged:\n%s", show) 57 t.Fatalf("merge request not recorded as merged:\n%s", show)
49 } 58 }
e2e/mrweb_test.go +1 −2
@@ -431,11 +431,10 @@ func TestMRDiffEmptyExplained(t *testing.T) {
431 os.WriteFile(filepath.Join(dir, "f.txt"), []byte("one\n"), 0o644) 431 os.WriteFile(filepath.Join(dir, "f.txt"), []byte("one\n"), 0o644)
432 mustGit(t, dir, env, "add", ".") 432 mustGit(t, dir, env, "add", ".")
433 mustGit(t, dir, env, "commit", "-q", "-m", "one") 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 if _, errOut, code := inst.ssh(t, key, "", "mr", "create", "alice/app", "--source", "feature", "--target", "main", "--title", "one"); code != 0 { 435 if _, errOut, code := inst.ssh(t, key, "", "mr", "create", "alice/app", "--source", "feature", "--target", "main", "--title", "one"); code != 0 {
436 t.Fatal(errOut) 436 t.Fatal(errOut)
437 } 437 }
438 mustGit(t, dir, env, "push", "-q", "origin", "feature:main")
439 _, body := inst.get(t, "/alice/app/mrs/1?view=diff") 438 _, body := inst.get(t, "/alice/app/mrs/1?view=diff")
440 if !strings.Contains(body, "No changes between the source and target.") || 439 if !strings.Contains(body, "No changes between the source and target.") ||
441 !strings.Contains(body, "already merged or fast-forwarded into <code>main</code>") { 440 !strings.Contains(body, "already merged or fast-forwarded into <code>main</code>") {
internal/control/landed.go added +65
@@ -0,0 +1,65 @@
1package control
2
3import (
4 "fmt"
5 "io"
6 "log/slog"
7
8 "gitbay.org/gitbay/internal/config"
9 "gitbay.org/gitbay/internal/gitutil"
10 "gitbay.org/gitbay/internal/store"
11)
12
13// RecordLandedMRs records as merged, by userID, each open or source_gone
14// merge request targeting branch whose head the update from old to new
15// brought onto it: commits that reached the target by a direct push, or
16// inside another merge request. A head already on the branch before the
17// update is left alone, so a merge request opened with nothing to merge
18// is not closed by an unrelated push; `mr merge` records that case (#108).
19func RecordLandedMRs(st *store.Store, cfg config.Config, repo store.Repo, userID int64, branch, old, new string) {
20 mrs, err := st.UnresolvedMRsByTarget(repo.ID, branch)
21 if err != nil {
22 slog.Error("landed MRs: listing", "repo", repo.Path(), "err", err)
23 return
24 }
25 dir := RepoDir(cfg.Server.Root, repo.OwnerName, repo.Name)
26 base := old
27 if gitutil.ZeroSHA(old) {
28 base = ""
29 }
30 var c *Ctx
31 for _, mr := range mrs {
32 if mr.HeadSHA == "" {
33 continue
34 }
35 if landed, err := gitutil.IsAncestor(dir, mr.HeadSHA, new); err != nil || !landed {
36 continue
37 }
38 if base != "" {
39 if before, err := gitutil.IsAncestor(dir, mr.HeadSHA, base); err != nil || before {
40 continue
41 }
42 }
43 if err := st.MarkMerged(mr.ID, base, userID, ""); err != nil {
44 slog.Error("landed MRs: recording", "repo", repo.Path(), "mr", mr.Number, "err", err)
45 continue
46 }
47 if mr.QueuedAt != "" {
48 st.DequeueMerge(mr.ID)
49 }
50 st.RecordEvent(repo.ID, userID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, new))
51 if c == nil {
52 u, err := st.UserByID(userID)
53 if err != nil {
54 continue
55 }
56 c = &Ctx{User: u, Store: st, Cfg: cfg, Stdin: emptyReader{}, Stdout: io.Discard, Stderr: io.Discard}
57 }
58 if parts, err := st.MRParticipants(mr.ID); err == nil {
59 notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number,
60 subject: mrSubject(repo, mr.Number, mr.Title),
61 action: fmt.Sprintf("merged !%d into %s (pushed)", mr.Number, branch),
62 path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)})
63 }
64 }
65}
internal/control/landed_test.go added +119
@@ -0,0 +1,119 @@
1package control
2
3import (
4 "os"
5 "path/filepath"
6 "strings"
7 "testing"
8
9 "gitbay.org/gitbay/internal/config"
10 "gitbay.org/gitbay/internal/policy"
11 "gitbay.org/gitbay/internal/protocol"
12 "gitbay.org/gitbay/internal/store"
13)
14
15// A merge request whose head reaches its target by a push, or inside
16// another merge request, is recorded as merged; one whose head was on the
17// target already, or that is not on it, is left open.
18func TestRecordLandedMRs(t *testing.T) {
19 st, repo, uid := newQueueTestRepo(t)
20 git := gitRunner(t)
21 root := t.TempDir()
22 src := filepath.Join(root, "src")
23 os.MkdirAll(src, 0o755)
24 git(root, "init", "-q", "-b", "main", "src")
25 commit := func(file string) string {
26 os.WriteFile(filepath.Join(src, file), []byte(file+"\n"), 0o644)
27 git(src, "add", ".")
28 git(src, "commit", "-q", "-m", file)
29 return strings.TrimSpace(git(src, "rev-parse", "HEAD"))
30 }
31 base := commit("base")
32 branch := func(name, from string, files ...string) string {
33 git(src, "checkout", "-q", "-b", name, from)
34 var head string
35 for _, f := range files {
36 head = commit(f)
37 }
38 return head
39 }
40 pushed := branch("pushed", base, "a")
41 gone := branch("gone", base, "b")
42 other := branch("other", base, "c")
43
44 dir := RepoDir(root, repo.OwnerName, repo.Name)
45 os.MkdirAll(filepath.Dir(dir), 0o755)
46 git(root, "clone", "-q", "--bare", src, dir)
47 git(dir, "symbolic-ref", "HEAD", "refs/heads/main")
48 open := func(source, head string) store.MR {
49 n, err := st.CreateMR(repo.ID, uid, repo.ID, source, "main", source, "", head, "md", false)
50 if err != nil {
51 t.Fatal(err)
52 }
53 git(dir, "update-ref", mrHeadRef(n), head)
54 mr, err := st.MRByNumber(repo.ID, n)
55 if err != nil {
56 t.Fatal(err)
57 }
58 return mr
59 }
60 mrPushed, mrEmpty, mrOther := open("pushed", pushed), open("empty", base), open("other", other)
61 mrGone := open("gone", gone)
62 st.SetMRState(mrGone.ID, "source_gone")
63
64 cfg := config.Default()
65 cfg.Server.Root = root
66 state := func(mr store.MR) store.MR {
67 t.Helper()
68 m, err := st.MRByID(mr.ID)
69 if err != nil {
70 t.Fatal(err)
71 }
72 return m
73 }
74 push := func(old, new string) {
75 git(dir, "update-ref", "refs/heads/main", new, old)
76 RefsUpdated(st, cfg, repo.ID, uid, "full", []policy.RefUpdate{{Ref: "refs/heads/main", Old: old, New: new}})
77 }
78
79 push(base, pushed)
80 if m := state(mrPushed); m.State != "merged" || m.MergedBase != base {
81 t.Errorf("pushed head: state %s, base %.7s", m.State, m.MergedBase)
82 }
83 if m := state(mrEmpty); m.State != "open" {
84 t.Errorf("head already on main before the push: %s", m.State)
85 }
86 if m := state(mrOther); m.State != "open" {
87 t.Errorf("head not on main: %s", m.State)
88 }
89
90 git(src, "checkout", "-q", "main")
91 git(src, "merge", "-q", "--ff-only", pushed)
92 git(src, "merge", "-q", "--no-ff", "-m", "merge gone", "gone")
93 merged := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
94 git(src, "push", "-q", dir, "main:refs/heads/tmp")
95 git(dir, "update-ref", "-d", "refs/heads/tmp")
96 push(pushed, merged)
97 if m := state(mrGone); m.State != "merged" {
98 t.Errorf("source_gone head merged in: %s", m.State)
99 }
100
101 // Merging outer fast-forwards main over inner's head too.
102 inner := branch("inner", merged, "d")
103 outer := branch("outer", inner, "e")
104 git(src, "push", "-q", dir, "inner", "outer")
105 mrInner, mrOuter := open("inner", inner), open("outer", outer)
106 c, errOut := pruneCtx(st, root, store.User{ID: uid, Username: "alice"})
107 if code := mergeMR(c, repo, mrOuter, "ff"); code != protocol.ExitOK {
108 t.Fatalf("merge outer: exit %d: %s", code, errOut.String())
109 }
110 if m := state(mrOuter); m.State != "merged" {
111 t.Errorf("outer: %s", m.State)
112 }
113 if m := state(mrInner); m.State != "merged" || m.MergedBase != merged {
114 t.Errorf("inner carried by outer: state %s, base %.7s", m.State, m.MergedBase)
115 }
116 if m := state(mrOther); m.State != "open" {
117 t.Errorf("unrelated MR after merging outer: %s", m.State)
118 }
119}
internal/control/mr.go +1
@@ -1674,6 +1674,7 @@ func mergeMR(c *Ctx, repo store.Repo, mr store.MR, strategy string) int {
1674 QueueBranchBuilds(c.Store, c.Cfg.Server.Root, c.Cfg.Server.SiteURL, 1674 QueueBranchBuilds(c.Store, c.Cfg.Server.Root, c.Cfg.Server.SiteURL,
1675 repo, c.User.ID, mr.TargetRef, targetSHA, newSHA, time.Now()) 1675 repo, c.User.ID, mr.TargetRef, targetSHA, newSHA, time.Now())
1676 c.Store.MarkMirrorsDirty(repo.ID, "push") 1676 c.Store.MarkMirrorsDirty(repo.ID, "push")
1677 RecordLandedMRs(c.Store, c.Cfg, repo, c.User.ID, mr.TargetRef, targetSHA, newSHA)
1677 if parts, err := c.Store.MRParticipants(mr.ID); err == nil { 1678 if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
1678 notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number, 1679 notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number,
1679 subject: mrSubject(repo, mr.Number, mr.Title), 1680 subject: mrSubject(repo, mr.Number, mr.Title),
internal/control/refsupdated.go +1
@@ -58,6 +58,7 @@ func RefsUpdated(st *store.Store, cfg config.Config, repoID, userID int64, scope
58 if pushedRepoErr == nil && !u.IsDelete { 58 if pushedRepoErr == nil && !u.IsDelete {
59 QueueBranchBuilds(st, cfg.Server.Root, cfg.Server.SiteURL, 59 QueueBranchBuilds(st, cfg.Server.Root, cfg.Server.SiteURL,
60 pushedRepo, userID, branch, u.Old, u.New, time.Now()) 60 pushedRepo, userID, branch, u.Old, u.New, time.Now())
61 RecordLandedMRs(st, cfg, pushedRepo, userID, branch, u.Old, u.New)
61 } 62 }
62 if u.IsForce { 63 if u.IsForce {
63 st.Audit(userID, "push.forced", map[string]any{ 64 st.Audit(userID, "push.forced", map[string]any{
internal/store/mrs.go +21
@@ -281,6 +281,27 @@ func (s *Store) OpenMRsBySource(sourceRepoID int64, sourceRef string) ([]MR, err
281 return out, rows.Err() 281 return out, rows.Err()
282} 282}
283 283
284// UnresolvedMRsByTarget returns the open and source_gone MRs in repoID
285// targeting targetRef, oldest first.
286func (s *Store) UnresolvedMRsByTarget(repoID int64, targetRef string) ([]MR, error) {
287 rows, err := s.DB.Query(
288 mrSelect+" WHERE m.repo_id = ? AND m.target_ref = ? AND m.state IN ('open','source_gone') ORDER BY m.number",
289 repoID, targetRef)
290 if err != nil {
291 return nil, err
292 }
293 defer rows.Close()
294 var out []MR
295 for rows.Next() {
296 m, err := scanMR(rows)
297 if err != nil {
298 return nil, err
299 }
300 out = append(out, m)
301 }
302 return out, rows.Err()
303}
304
284// MarkMerged records the merge along with the target tip it landed on, so 305// MarkMerged records the merge along with the target tip it landed on, so
285// the MR's diff stays reconstructable after fast-forwards. actorID 0 and an 306// the MR's diff stays reconstructable after fast-forwards. actorID 0 and an
286// empty at leave the merger unknown and stamp the current time, which is 307// empty at leave the merger unknown and stamp the current time, which is