detached a39330a

fix(mr): safe async source deletion, single-run mergeability queue and domain-side merge-tree bounds

commit
a39330aa4be5a520fd7ba0564904d297edafccf9
author
brice · · 2026-10-19 13:59 UTC
committer
brice · · 2026-10-19 14:36 UTC
parent
f44d044
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
20 files · +670 -111
Unified
UnifiedSplit
M internal/adapters/gitcli/merge_test.go +7 -11
@@ -287,8 +287,8 @@ func TestMergeTree(t *testing.T) {
287287 res, err = g.MergeTree(ctx, m.f.Name, wa, wb)
288288 require.NoError(t, err)
289289 assert.False(t, res.Clean)
290- assert.Len(t, res.ConflictPaths, domain.MaxConflictPaths)
291- assert.True(t, res.Truncated)
290+ assert.Len(t, res.ConflictPaths, domain.MaxConflictPaths+5, "the adapter returns every path")
291+ assert.False(t, res.Truncated, "bounding is the domain's")
292292 }
293293
294294 func TestCountBehind(t *testing.T) {
@@ -298,23 +298,19 @@ func TestCountBehind(t *testing.T) {
298298 t.Cleanup(func() { _ = g.Close(ctx) })
299299
300300 // main has b that a-based head lacks.
301- n, capped, err := g.CountBehind(ctx, m.f.Name, m.a, m.mainTip, 10)
301+ n, err := g.CountBehind(ctx, m.f.Name, m.a, m.mainTip, 10)
302302 require.NoError(t, err)
303303 assert.Equal(t, 1, n)
304- assert.False(t, capped)
305304
306- n, capped, err = g.CountBehind(ctx, m.f.Name, m.featTip, m.mainTip, 10)
305+ n, err = g.CountBehind(ctx, m.f.Name, m.featTip, m.mainTip, 10)
307306 require.NoError(t, err)
308307 assert.Equal(t, 0, n)
309- assert.False(t, capped)
310308
311- // feat is 3 commits ahead of a: counting target=feat from head=a with cap 2.
312- n, capped, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 2)
309+ // feat is 3 commits ahead of a: the raw count stops at the limit.
310+ n, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 2)
313311 require.NoError(t, err)
314312 assert.Equal(t, 2, n)
315- assert.True(t, capped)
316- n, capped, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 3)
313+ n, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 4)
317314 require.NoError(t, err)
318315 assert.Equal(t, 3, n)
319- assert.False(t, capped)
320316 }
@@ -287,8 +287,8 @@ func TestMergeTree(t *testing.T) {
287 res, err = g.MergeTree(ctx, m.f.Name, wa, wb)287 res, err = g.MergeTree(ctx, m.f.Name, wa, wb)
288 require.NoError(t, err)288 require.NoError(t, err)
289 assert.False(t, res.Clean)289 assert.False(t, res.Clean)
290- assert.Len(t, res.ConflictPaths, domain.MaxConflictPaths)290+ assert.Len(t, res.ConflictPaths, domain.MaxConflictPaths+5, "the adapter returns every path")
291- assert.True(t, res.Truncated)291+ assert.False(t, res.Truncated, "bounding is the domain's")
292 }292 }
293 293
294 func TestCountBehind(t *testing.T) {294 func TestCountBehind(t *testing.T) {
@@ -298,23 +298,19 @@ func TestCountBehind(t *testing.T) {
298 t.Cleanup(func() { _ = g.Close(ctx) })298 t.Cleanup(func() { _ = g.Close(ctx) })
299 299
300 // main has b that a-based head lacks.300 // main has b that a-based head lacks.
301- n, capped, err := g.CountBehind(ctx, m.f.Name, m.a, m.mainTip, 10)301+ n, err := g.CountBehind(ctx, m.f.Name, m.a, m.mainTip, 10)
302 require.NoError(t, err)302 require.NoError(t, err)
303 assert.Equal(t, 1, n)303 assert.Equal(t, 1, n)
304- assert.False(t, capped)
305 304
306- n, capped, err = g.CountBehind(ctx, m.f.Name, m.featTip, m.mainTip, 10)305+ n, err = g.CountBehind(ctx, m.f.Name, m.featTip, m.mainTip, 10)
307 require.NoError(t, err)306 require.NoError(t, err)
308 assert.Equal(t, 0, n)307 assert.Equal(t, 0, n)
309- assert.False(t, capped)
310 308
311- // feat is 3 commits ahead of a: counting target=feat from head=a with cap 2.309+ // feat is 3 commits ahead of a: the raw count stops at the limit.
312- n, capped, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 2)310+ n, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 2)
313 require.NoError(t, err)311 require.NoError(t, err)
314 assert.Equal(t, 2, n)312 assert.Equal(t, 2, n)
315- assert.True(t, capped)313+ n, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 4)
316- n, capped, err = g.CountBehind(ctx, m.f.Name, m.a, m.featTip, 3)
317 require.NoError(t, err)314 require.NoError(t, err)
318 assert.Equal(t, 3, n)315 assert.Equal(t, 3, n)
319- assert.False(t, capped)
320 }316 }
M internal/adapters/gitcli/mergeability.go +47 -41
@@ -6,6 +6,7 @@ import (
66 "context"
77 "errors"
88 "fmt"
9+ "slices"
910 "strconv"
1011 "strings"
1112
@@ -16,66 +17,71 @@ import (
1617 // the object database and touches no ref or index. It prints the tree of the
1718 // result, then on a conflict (exit 1) the NUL-separated conflicted paths.
1819 // Commits with no common history do not merge: not clean, without paths.
19-//
20-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes and output to the clean/conflict answer of the port
2120 func (g *Git) MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error) {
22- dir, err := g.repoDir(repo)
23- if err != nil {
21+ shared, err := g.sharesHistory(ctx, repo, base, head)
22+ if err != nil || !shared {
2423 return domain.MergeTreeResult{}, err
2524 }
26- if _, err := g.MergeBase(ctx, repo, base, head); errors.Is(err, domain.ErrNotFound) {
27- return domain.MergeTreeResult{}, nil
28- } else if err != nil {
25+ out, conflict, err := g.mergeTreeOutput(ctx, repo, base, head)
26+ if err != nil {
2927 return domain.MergeTreeResult{}, err
3028 }
31- out, err := g.runCapture(ctx, nil, readArgs(dir, "merge-tree", "--write-tree", "-z", "--name-only", "--no-messages", "--end-of-options", base.String(), head.String())...)
32- conflict := exitCode(err) == 1
29+ return parseMergeTree(out, conflict)
30+}
31+
32+// mergeTreeOutput runs merge-tree and returns what it printed; conflict is its
33+// exit status 1.
34+func (g *Git) mergeTreeOutput(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (out string, conflict bool, err error) {
35+ dir, err := g.repoDir(repo)
36+ if err != nil {
37+ return "", false, err
38+ }
39+ out, err = g.runCapture(ctx, nil, readArgs(dir, "merge-tree", "--write-tree", "-z", "--name-only", "--no-messages", "--end-of-options", base.String(), head.String())...)
40+ conflict = exitCode(err) == 1
3341 if err != nil && !conflict {
34- return domain.MergeTreeResult{}, fmt.Errorf("merge %s into %s in %q: %w", head, base, repo, err)
42+ return "", false, fmt.Errorf("merge %s into %s in %q: %w", head, base, repo, err)
3543 }
36- fields := strings.Split(out, "\x00")
37- tree, perr := parseID(fields[0])
38- if perr != nil && !conflict {
39- return domain.MergeTreeResult{}, perr
44+ return out, conflict, nil
45+}
46+
47+// sharesHistory reports whether the commits have a merge base.
48+func (g *Git) sharesHistory(ctx context.Context, repo domain.RepoName, a, b domain.ObjectID) (bool, error) {
49+ _, err := g.MergeBase(ctx, repo, a, b)
50+ if errors.Is(err, domain.ErrNotFound) {
51+ return false, nil
4052 }
53+ return err == nil, err
54+}
55+
56+// parseMergeTree reads the output of merge-tree: the tree, then on a conflict
57+// the NUL-separated paths, all of them.
58+func parseMergeTree(out string, conflict bool) (domain.MergeTreeResult, error) {
59+ fields := strings.Split(out, "\x00")
60+ tree, err := parseID(fields[0])
4161 if !conflict {
42- return domain.MergeTreeResult{Tree: tree, Clean: true}, nil
62+ return domain.MergeTreeResult{Tree: tree, Clean: true}, err
4363 }
44- res := domain.MergeTreeResult{}
45- for _, p := range fields[1:] {
46- if p == "" {
47- break
48- }
49- if len(res.ConflictPaths) == domain.MaxConflictPaths {
50- res.Truncated = true
51- break
52- }
53- res.ConflictPaths = append(res.ConflictPaths, p)
64+ paths := fields[1:]
65+ if i := slices.Index(paths, ""); i >= 0 {
66+ paths = paths[:i]
5467 }
55- return res, nil
68+ return domain.MergeTreeResult{ConflictPaths: paths}, nil
5669 }
5770
58-// CountBehind runs `git rev-list --count --max-count=max+1 <target> ^<head>`.
59-//
60-//hexcheck:ignore suspicious-business-logic-in-adapter applies the cap of the port to the count git printed
61-func (g *Git) CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, max int) (int, bool, error) {
71+// CountBehind runs `git rev-list --count --max-count=<limit> <target> ^<head>`
72+// and returns the count git printed.
73+func (g *Git) CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, limit int) (int, error) {
6274 dir, err := g.repoDir(repo)
6375 if err != nil {
64- return 0, false, err
76+ return 0, err
6577 }
66- if max < 0 {
67- max = 0
68- }
69- out, err := g.run(ctx, readArgs(dir, "rev-list", "--count", "--max-count="+strconv.Itoa(max+1), target.String(), "^"+head.String(), "--")...)
78+ out, err := g.run(ctx, readArgs(dir, "rev-list", "--count", "--max-count="+strconv.Itoa(limit), target.String(), "^"+head.String(), "--")...)
7079 if err != nil {
71- return 0, false, fmt.Errorf("count behind %s in %q: %w", target, repo, err)
80+ return 0, fmt.Errorf("count behind %s in %q: %w", target, repo, err)
7281 }
7382 n, err := strconv.Atoi(strings.TrimSpace(out))
7483 if err != nil {
75- return 0, false, fmt.Errorf("git printed %q: %w", strings.TrimSpace(out), err)
76- }
77- if n > max {
78- return max, true, nil
84+ return 0, fmt.Errorf("git printed %q: %w", strings.TrimSpace(out), err)
7985 }
80- return n, false, nil
86+ return n, nil
8187 }
@@ -6,6 +6,7 @@ import (
6 "context"6 "context"
7 "errors"7 "errors"
8 "fmt"8 "fmt"
9+ "slices"
9 "strconv"10 "strconv"
10 "strings"11 "strings"
11 12
@@ -16,66 +17,71 @@ import (
16 // the object database and touches no ref or index. It prints the tree of the17 // the object database and touches no ref or index. It prints the tree of the
17 // result, then on a conflict (exit 1) the NUL-separated conflicted paths.18 // result, then on a conflict (exit 1) the NUL-separated conflicted paths.
18 // Commits with no common history do not merge: not clean, without paths.19 // Commits with no common history do not merge: not clean, without paths.
19-//
20-//hexcheck:ignore suspicious-business-logic-in-adapter maps git exit codes and output to the clean/conflict answer of the port
21 func (g *Git) MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error) {20 func (g *Git) MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error) {
22- dir, err := g.repoDir(repo)21+ shared, err := g.sharesHistory(ctx, repo, base, head)
23- if err != nil {22+ if err != nil || !shared {
24 return domain.MergeTreeResult{}, err23 return domain.MergeTreeResult{}, err
25 }24 }
26- if _, err := g.MergeBase(ctx, repo, base, head); errors.Is(err, domain.ErrNotFound) {25+ out, conflict, err := g.mergeTreeOutput(ctx, repo, base, head)
27- return domain.MergeTreeResult{}, nil26+ if err != nil {
28- } else if err != nil {
29 return domain.MergeTreeResult{}, err27 return domain.MergeTreeResult{}, err
30 }28 }
31- out, err := g.runCapture(ctx, nil, readArgs(dir, "merge-tree", "--write-tree", "-z", "--name-only", "--no-messages", "--end-of-options", base.String(), head.String())...)29+ return parseMergeTree(out, conflict)
32- conflict := exitCode(err) == 130+}
31+
32+// mergeTreeOutput runs merge-tree and returns what it printed; conflict is its
33+// exit status 1.
34+func (g *Git) mergeTreeOutput(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (out string, conflict bool, err error) {
35+ dir, err := g.repoDir(repo)
36+ if err != nil {
37+ return "", false, err
38+ }
39+ out, err = g.runCapture(ctx, nil, readArgs(dir, "merge-tree", "--write-tree", "-z", "--name-only", "--no-messages", "--end-of-options", base.String(), head.String())...)
40+ conflict = exitCode(err) == 1
33 if err != nil && !conflict {41 if err != nil && !conflict {
34- return domain.MergeTreeResult{}, fmt.Errorf("merge %s into %s in %q: %w", head, base, repo, err)42+ return "", false, fmt.Errorf("merge %s into %s in %q: %w", head, base, repo, err)
35 }43 }
36- fields := strings.Split(out, "\x00")44+ return out, conflict, nil
37- tree, perr := parseID(fields[0])45+}
38- if perr != nil && !conflict {46+
39- return domain.MergeTreeResult{}, perr47+// sharesHistory reports whether the commits have a merge base.
48+func (g *Git) sharesHistory(ctx context.Context, repo domain.RepoName, a, b domain.ObjectID) (bool, error) {
49+ _, err := g.MergeBase(ctx, repo, a, b)
50+ if errors.Is(err, domain.ErrNotFound) {
51+ return false, nil
40 }52 }
53+ return err == nil, err
54+}
55+
56+// parseMergeTree reads the output of merge-tree: the tree, then on a conflict
57+// the NUL-separated paths, all of them.
58+func parseMergeTree(out string, conflict bool) (domain.MergeTreeResult, error) {
59+ fields := strings.Split(out, "\x00")
60+ tree, err := parseID(fields[0])
41 if !conflict {61 if !conflict {
42- return domain.MergeTreeResult{Tree: tree, Clean: true}, nil62+ return domain.MergeTreeResult{Tree: tree, Clean: true}, err
43 }63 }
44- res := domain.MergeTreeResult{}64+ paths := fields[1:]
45- for _, p := range fields[1:] {65+ if i := slices.Index(paths, ""); i >= 0 {
46- if p == "" {66+ paths = paths[:i]
47- break
48- }
49- if len(res.ConflictPaths) == domain.MaxConflictPaths {
50- res.Truncated = true
51- break
52- }
53- res.ConflictPaths = append(res.ConflictPaths, p)
54 }67 }
55- return res, nil68+ return domain.MergeTreeResult{ConflictPaths: paths}, nil
56 }69 }
57 70
58-// CountBehind runs `git rev-list --count --max-count=max+1 <target> ^<head>`.71+// CountBehind runs `git rev-list --count --max-count=<limit> <target> ^<head>`
59-//72+// and returns the count git printed.
60-//hexcheck:ignore suspicious-business-logic-in-adapter applies the cap of the port to the count git printed73+func (g *Git) CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, limit int) (int, error) {
61-func (g *Git) CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, max int) (int, bool, error) {
62 dir, err := g.repoDir(repo)74 dir, err := g.repoDir(repo)
63 if err != nil {75 if err != nil {
64- return 0, false, err76+ return 0, err
65 }77 }
66- if max < 0 {78+ out, err := g.run(ctx, readArgs(dir, "rev-list", "--count", "--max-count="+strconv.Itoa(limit), target.String(), "^"+head.String(), "--")...)
67- max = 0
68- }
69- out, err := g.run(ctx, readArgs(dir, "rev-list", "--count", "--max-count="+strconv.Itoa(max+1), target.String(), "^"+head.String(), "--")...)
70 if err != nil {79 if err != nil {
71- return 0, false, fmt.Errorf("count behind %s in %q: %w", target, repo, err)80+ return 0, fmt.Errorf("count behind %s in %q: %w", target, repo, err)
72 }81 }
73 n, err := strconv.Atoi(strings.TrimSpace(out))82 n, err := strconv.Atoi(strings.TrimSpace(out))
74 if err != nil {83 if err != nil {
75- return 0, false, fmt.Errorf("git printed %q: %w", strings.TrimSpace(out), err)84+ return 0, fmt.Errorf("git printed %q: %w", strings.TrimSpace(out), err)
76- }
77- if n > max {
78- return max, true, nil
79 }85 }
80- return n, false, nil86+ return n, nil
81 }87 }
M internal/adapters/kvstore/ci_tip.go +29 -0
@@ -89,3 +89,32 @@ func (c *CI) DeleteRefTip(ctx context.Context, repo domain.RepoName, ref string)
8989 }
9090 return nil
9191 }
92+
93+// DeleteRefTipIf removes the record of a ref only when it still names sha.
94+func (c *CI) DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref, sha string) error {
95+ for range updateAttemptsTip {
96+ it, ok := c.db.Get(ctx, refTipKey(repo, ref))
97+ if !ok {
98+ return nil
99+ }
100+ var pb storagepb.CIRefTip
101+ if err := proto.Unmarshal(it.Value, &pb); err != nil {
102+ return fmt.Errorf("decode ref tip: %w", err)
103+ }
104+ if pb.GetSha() != sha {
105+ return nil // a newer push owns the record
106+ }
107+ var b kv.Batch
108+ b.RequireVersion(refTipKey(repo, ref), it.Version)
109+ b.Delete(refTipKey(repo, ref))
110+ _, err := c.db.Apply(ctx, &b)
111+ if errors.Is(err, kv.ErrConflict) {
112+ continue
113+ }
114+ if err != nil {
115+ return fmt.Errorf("delete ref tip: %w", err)
116+ }
117+ return nil
118+ }
119+ return fmt.Errorf("delete ref tip: %w", domain.ErrConflict)
120+}
@@ -89,3 +89,32 @@ func (c *CI) DeleteRefTip(ctx context.Context, repo domain.RepoName, ref string)
89 }89 }
90 return nil90 return nil
91 }91 }
92+
93+// DeleteRefTipIf removes the record of a ref only when it still names sha.
94+func (c *CI) DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref, sha string) error {
95+ for range updateAttemptsTip {
96+ it, ok := c.db.Get(ctx, refTipKey(repo, ref))
97+ if !ok {
98+ return nil
99+ }
100+ var pb storagepb.CIRefTip
101+ if err := proto.Unmarshal(it.Value, &pb); err != nil {
102+ return fmt.Errorf("decode ref tip: %w", err)
103+ }
104+ if pb.GetSha() != sha {
105+ return nil // a newer push owns the record
106+ }
107+ var b kv.Batch
108+ b.RequireVersion(refTipKey(repo, ref), it.Version)
109+ b.Delete(refTipKey(repo, ref))
110+ _, err := c.db.Apply(ctx, &b)
111+ if errors.Is(err, kv.ErrConflict) {
112+ continue
113+ }
114+ if err != nil {
115+ return fmt.Errorf("delete ref tip: %w", err)
116+ }
117+ return nil
118+ }
119+ return fmt.Errorf("delete ref tip: %w", domain.ErrConflict)
120+}
A internal/adapters/kvstore/ci_tip_test.go +34 -0
@@ -0,0 +1,34 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package kvstore_test
4+
5+import (
6+ "context"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "bnema.dev/ermite/internal/domain"
13+)
14+
15+func TestDeleteRefTipIfOnlyRemovesTheMatchingRecord(t *testing.T) {
16+ ctx := context.Background()
17+ s := openStore(t, t.TempDir())
18+ n := name(t, "proj")
19+ require.NoError(t, s.Create(ctx, domain.Repo{Name: n, Visibility: domain.VisibilityPrivate, CreatedAt: created}))
20+ ci := s.CI()
21+ ref := "refs/heads/feat"
22+
23+ require.NoError(t, ci.PutRefTip(ctx, domain.CIRefTip{Repo: n, Ref: ref, Sha: "aaa", Actor: "ada"}))
24+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "bbb"))
25+ got, err := ci.GetRefTip(ctx, n, ref)
26+ require.NoError(t, err)
27+ assert.Equal(t, "aaa", got.Sha, "a record of another object stays")
28+
29+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "aaa"))
30+ _, err = ci.GetRefTip(ctx, n, ref)
31+ assert.ErrorIs(t, err, domain.ErrNotFound)
32+
33+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "aaa"), "a missing record is not an error")
34+}
@@ -0,0 +1,34 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package kvstore_test
4+
5+import (
6+ "context"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "bnema.dev/ermite/internal/domain"
13+)
14+
15+func TestDeleteRefTipIfOnlyRemovesTheMatchingRecord(t *testing.T) {
16+ ctx := context.Background()
17+ s := openStore(t, t.TempDir())
18+ n := name(t, "proj")
19+ require.NoError(t, s.Create(ctx, domain.Repo{Name: n, Visibility: domain.VisibilityPrivate, CreatedAt: created}))
20+ ci := s.CI()
21+ ref := "refs/heads/feat"
22+
23+ require.NoError(t, ci.PutRefTip(ctx, domain.CIRefTip{Repo: n, Ref: ref, Sha: "aaa", Actor: "ada"}))
24+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "bbb"))
25+ got, err := ci.GetRefTip(ctx, n, ref)
26+ require.NoError(t, err)
27+ assert.Equal(t, "aaa", got.Sha, "a record of another object stays")
28+
29+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "aaa"))
30+ _, err = ci.GetRefTip(ctx, n, ref)
31+ assert.ErrorIs(t, err, domain.ErrNotFound)
32+
33+ require.NoError(t, ci.DeleteRefTipIf(ctx, n, ref, "aaa"), "a missing record is not an error")
34+}
M internal/app/ci_schedule_trust_test.go +6 -2
@@ -3,6 +3,7 @@
33 package app
44
55 import (
6+ "errors"
67 "strings"
78 "testing"
89 "time"
@@ -230,8 +231,11 @@ func TestMergeRecordsTheTargetAndDropsTheSourceRecord(t *testing.T) {
230231 }))
231232 require.NoError(t, err)
232233
233- _, err = tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/feat")
234- assert.ErrorIs(t, err, domain.ErrNotFound, "the deleted source branch has no record")
234+ // The deletion and the removal of the record run after the merge returned.
235+ c.waitFor("the deleted source branch has no record", func() bool {
236+ _, err := tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/feat")
237+ return errors.Is(err, domain.ErrNotFound)
238+ })
235239 main, err := tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/main")
236240 require.NoError(t, err)
237241 assert.Equal(t, c.ref("refs/heads/main"), main.Sha, "the merge recorded the new tip")
@@ -3,6 +3,7 @@
3 package app3 package app
4 4
5 import (5 import (
6+ "errors"
6 "strings"7 "strings"
7 "testing"8 "testing"
8 "time"9 "time"
@@ -230,8 +231,11 @@ func TestMergeRecordsTheTargetAndDropsTheSourceRecord(t *testing.T) {
230 }))231 }))
231 require.NoError(t, err)232 require.NoError(t, err)
232 233
233- _, err = tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/feat")234+ // The deletion and the removal of the record run after the merge returned.
234- assert.ErrorIs(t, err, domain.ErrNotFound, "the deleted source branch has no record")235+ c.waitFor("the deleted source branch has no record", func() bool {
236+ _, err := tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/feat")
237+ return errors.Is(err, domain.ErrNotFound)
238+ })
235 main, err := tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/main")239 main, err := tips.GetRefTip(c.ctx, c.name("proj"), "refs/heads/main")
236 require.NoError(t, err)240 require.NoError(t, err)
237 assert.Equal(t, c.ref("refs/heads/main"), main.Sha, "the merge recorded the new tip")241 assert.Equal(t, c.ref("refs/heads/main"), main.Sha, "the merge recorded the new tip")
M internal/domain/mergerequest.go +24 -0
@@ -535,3 +535,27 @@ type MergeTreeResult struct {
535535 ConflictPaths []string
536536 Truncated bool
537537 }
538+
539+// Bounded returns r with at most MaxConflictPaths conflicted paths, Truncated
540+// set when some were dropped. Adapters return the raw result; the bound is
541+// the domain's.
542+func (r MergeTreeResult) Bounded() MergeTreeResult {
543+ if len(r.ConflictPaths) > MaxConflictPaths {
544+ r.ConflictPaths = r.ConflictPaths[:MaxConflictPaths:MaxConflictPaths]
545+ r.Truncated = true
546+ }
547+ return r
548+}
549+
550+// BehindLimit is the limit to give a count of commits behind so that
551+// BoundBehind can tell whether MaxBehindCount was exceeded.
552+const BehindLimit = MaxBehindCount + 1
553+
554+// BoundBehind caps a raw count of commits behind (counted up to BehindLimit)
555+// at MaxBehindCount; capped is true when the count was over it.
556+func BoundBehind(n int) (count int, capped bool) {
557+ if n > MaxBehindCount {
558+ return MaxBehindCount, true
559+ }
560+ return n, false
561+}
@@ -535,3 +535,27 @@ type MergeTreeResult struct {
535 ConflictPaths []string535 ConflictPaths []string
536 Truncated bool536 Truncated bool
537 }537 }
538+
539+// Bounded returns r with at most MaxConflictPaths conflicted paths, Truncated
540+// set when some were dropped. Adapters return the raw result; the bound is
541+// the domain's.
542+func (r MergeTreeResult) Bounded() MergeTreeResult {
543+ if len(r.ConflictPaths) > MaxConflictPaths {
544+ r.ConflictPaths = r.ConflictPaths[:MaxConflictPaths:MaxConflictPaths]
545+ r.Truncated = true
546+ }
547+ return r
548+}
549+
550+// BehindLimit is the limit to give a count of commits behind so that
551+// BoundBehind can tell whether MaxBehindCount was exceeded.
552+const BehindLimit = MaxBehindCount + 1
553+
554+// BoundBehind caps a raw count of commits behind (counted up to BehindLimit)
555+// at MaxBehindCount; capped is true when the count was over it.
556+func BoundBehind(n int) (count int, capped bool) {
557+ if n > MaxBehindCount {
558+ return MaxBehindCount, true
559+ }
560+ return n, false
561+}
A internal/domain/mergetree_bound_test.go +35 -0
@@ -0,0 +1,35 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package domain_test
4+
5+import (
6+ "strconv"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+
11+ "bnema.dev/ermite/internal/domain"
12+)
13+
14+func TestMergeTreeResultBounded(t *testing.T) {
15+ var paths []string
16+ for i := 0; i < domain.MaxConflictPaths+2; i++ {
17+ paths = append(paths, strconv.Itoa(i))
18+ }
19+ got := domain.MergeTreeResult{ConflictPaths: paths}.Bounded()
20+ assert.Len(t, got.ConflictPaths, domain.MaxConflictPaths)
21+ assert.True(t, got.Truncated)
22+
23+ got = domain.MergeTreeResult{ConflictPaths: paths[:domain.MaxConflictPaths]}.Bounded()
24+ assert.Len(t, got.ConflictPaths, domain.MaxConflictPaths)
25+ assert.False(t, got.Truncated)
26+}
27+
28+func TestBoundBehind(t *testing.T) {
29+ n, capped := domain.BoundBehind(domain.MaxBehindCount)
30+ assert.Equal(t, domain.MaxBehindCount, n)
31+ assert.False(t, capped)
32+ n, capped = domain.BoundBehind(domain.BehindLimit)
33+ assert.Equal(t, domain.MaxBehindCount, n)
34+ assert.True(t, capped)
35+}
@@ -0,0 +1,35 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package domain_test
4+
5+import (
6+ "strconv"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+
11+ "bnema.dev/ermite/internal/domain"
12+)
13+
14+func TestMergeTreeResultBounded(t *testing.T) {
15+ var paths []string
16+ for i := 0; i < domain.MaxConflictPaths+2; i++ {
17+ paths = append(paths, strconv.Itoa(i))
18+ }
19+ got := domain.MergeTreeResult{ConflictPaths: paths}.Bounded()
20+ assert.Len(t, got.ConflictPaths, domain.MaxConflictPaths)
21+ assert.True(t, got.Truncated)
22+
23+ got = domain.MergeTreeResult{ConflictPaths: paths[:domain.MaxConflictPaths]}.Bounded()
24+ assert.Len(t, got.ConflictPaths, domain.MaxConflictPaths)
25+ assert.False(t, got.Truncated)
26+}
27+
28+func TestBoundBehind(t *testing.T) {
29+ n, capped := domain.BoundBehind(domain.MaxBehindCount)
30+ assert.Equal(t, domain.MaxBehindCount, n)
31+ assert.False(t, capped)
32+ n, capped = domain.BoundBehind(domain.BehindLimit)
33+ assert.Equal(t, domain.MaxBehindCount, n)
34+ assert.True(t, capped)
35+}
M internal/ports/ci.go +3 -0
@@ -93,6 +93,9 @@ type CIRefTipStore interface {
9393 GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error)
9494 // DeleteRefTip removes the record of a ref; a missing one is not an error.
9595 DeleteRefTip(ctx context.Context, repo domain.RepoName, ref string) error
96+ // DeleteRefTipIf removes the record of a ref only when it still names
97+ // sha; a missing or different one is left alone and is not an error.
98+ DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref, sha string) error
9699 }
97100
98101 // CIWorkflowStore keeps, per repository, the workflow files that are disabled
@@ -93,6 +93,9 @@ type CIRefTipStore interface {
93 GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error)93 GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error)
94 // DeleteRefTip removes the record of a ref; a missing one is not an error.94 // DeleteRefTip removes the record of a ref; a missing one is not an error.
95 DeleteRefTip(ctx context.Context, repo domain.RepoName, ref string) error95 DeleteRefTip(ctx context.Context, repo domain.RepoName, ref string) error
96+ // DeleteRefTipIf removes the record of a ref only when it still names
97+ // sha; a missing or different one is left alone and is not an error.
98+ DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref, sha string) error
96 }99 }
97 100
98 // CIWorkflowStore keeps, per repository, the workflow files that are disabled101 // CIWorkflowStore keeps, per repository, the workflow files that are disabled
M internal/ports/mocks/CIRefTipStore.go +69 -0
@@ -110,6 +110,75 @@ func (_c *CIRefTipStore_DeleteRefTip_Call) RunAndReturn(run func(ctx context.Con
110110 return _c
111111 }
112112
113+// DeleteRefTipIf provides a mock function for the type CIRefTipStore
114+func (_mock *CIRefTipStore) DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref string, sha string) error {
115+ ret := _mock.Called(ctx, repo, ref, sha)
116+
117+ if len(ret) == 0 {
118+ panic("no return value specified for DeleteRefTipIf")
119+ }
120+
121+ var r0 error
122+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, string, string) error); ok {
123+ r0 = returnFunc(ctx, repo, ref, sha)
124+ } else {
125+ r0 = ret.Error(0)
126+ }
127+ return r0
128+}
129+
130+// CIRefTipStore_DeleteRefTipIf_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteRefTipIf'
131+type CIRefTipStore_DeleteRefTipIf_Call struct {
132+ *mock.Call
133+}
134+
135+// DeleteRefTipIf is a helper method to define mock.On call
136+// - ctx context.Context
137+// - repo domain.RepoName
138+// - ref string
139+// - sha string
140+func (_e *CIRefTipStore_Expecter) DeleteRefTipIf(ctx any, repo any, ref any, sha any) *CIRefTipStore_DeleteRefTipIf_Call {
141+ return &CIRefTipStore_DeleteRefTipIf_Call{Call: _e.mock.On("DeleteRefTipIf", ctx, repo, ref, sha)}
142+}
143+
144+func (_c *CIRefTipStore_DeleteRefTipIf_Call) Run(run func(ctx context.Context, repo domain.RepoName, ref string, sha string)) *CIRefTipStore_DeleteRefTipIf_Call {
145+ _c.Call.Run(func(args mock.Arguments) {
146+ var arg0 context.Context
147+ if args[0] != nil {
148+ arg0 = args[0].(context.Context)
149+ }
150+ var arg1 domain.RepoName
151+ if args[1] != nil {
152+ arg1 = args[1].(domain.RepoName)
153+ }
154+ var arg2 string
155+ if args[2] != nil {
156+ arg2 = args[2].(string)
157+ }
158+ var arg3 string
159+ if args[3] != nil {
160+ arg3 = args[3].(string)
161+ }
162+ run(
163+ arg0,
164+ arg1,
165+ arg2,
166+ arg3,
167+ )
168+ })
169+ return _c
170+}
171+
172+func (_c *CIRefTipStore_DeleteRefTipIf_Call) Return(err error) *CIRefTipStore_DeleteRefTipIf_Call {
173+ _c.Call.Return(err)
174+ return _c
175+}
176+
177+func (_c *CIRefTipStore_DeleteRefTipIf_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, ref string, sha string) error) *CIRefTipStore_DeleteRefTipIf_Call {
178+ _c.Call.Return(run)
179+ return _c
180+}
181+
113182 // GetRefTip provides a mock function for the type CIRefTipStore
114183 func (_mock *CIRefTipStore) GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error) {
115184 ret := _mock.Called(ctx, repo, ref)
@@ -110,6 +110,75 @@ func (_c *CIRefTipStore_DeleteRefTip_Call) RunAndReturn(run func(ctx context.Con
110 return _c110 return _c
111 }111 }
112 112
113+// DeleteRefTipIf provides a mock function for the type CIRefTipStore
114+func (_mock *CIRefTipStore) DeleteRefTipIf(ctx context.Context, repo domain.RepoName, ref string, sha string) error {
115+ ret := _mock.Called(ctx, repo, ref, sha)
116+
117+ if len(ret) == 0 {
118+ panic("no return value specified for DeleteRefTipIf")
119+ }
120+
121+ var r0 error
122+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, string, string) error); ok {
123+ r0 = returnFunc(ctx, repo, ref, sha)
124+ } else {
125+ r0 = ret.Error(0)
126+ }
127+ return r0
128+}
129+
130+// CIRefTipStore_DeleteRefTipIf_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteRefTipIf'
131+type CIRefTipStore_DeleteRefTipIf_Call struct {
132+ *mock.Call
133+}
134+
135+// DeleteRefTipIf is a helper method to define mock.On call
136+// - ctx context.Context
137+// - repo domain.RepoName
138+// - ref string
139+// - sha string
140+func (_e *CIRefTipStore_Expecter) DeleteRefTipIf(ctx any, repo any, ref any, sha any) *CIRefTipStore_DeleteRefTipIf_Call {
141+ return &CIRefTipStore_DeleteRefTipIf_Call{Call: _e.mock.On("DeleteRefTipIf", ctx, repo, ref, sha)}
142+}
143+
144+func (_c *CIRefTipStore_DeleteRefTipIf_Call) Run(run func(ctx context.Context, repo domain.RepoName, ref string, sha string)) *CIRefTipStore_DeleteRefTipIf_Call {
145+ _c.Call.Run(func(args mock.Arguments) {
146+ var arg0 context.Context
147+ if args[0] != nil {
148+ arg0 = args[0].(context.Context)
149+ }
150+ var arg1 domain.RepoName
151+ if args[1] != nil {
152+ arg1 = args[1].(domain.RepoName)
153+ }
154+ var arg2 string
155+ if args[2] != nil {
156+ arg2 = args[2].(string)
157+ }
158+ var arg3 string
159+ if args[3] != nil {
160+ arg3 = args[3].(string)
161+ }
162+ run(
163+ arg0,
164+ arg1,
165+ arg2,
166+ arg3,
167+ )
168+ })
169+ return _c
170+}
171+
172+func (_c *CIRefTipStore_DeleteRefTipIf_Call) Return(err error) *CIRefTipStore_DeleteRefTipIf_Call {
173+ _c.Call.Return(err)
174+ return _c
175+}
176+
177+func (_c *CIRefTipStore_DeleteRefTipIf_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, ref string, sha string) error) *CIRefTipStore_DeleteRefTipIf_Call {
178+ _c.Call.Return(run)
179+ return _c
180+}
181+
113 // GetRefTip provides a mock function for the type CIRefTipStore182 // GetRefTip provides a mock function for the type CIRefTipStore
114 func (_mock *CIRefTipStore) GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error) {183 func (_mock *CIRefTipStore) GetRefTip(ctx context.Context, repo domain.RepoName, ref string) (domain.CIRefTip, error) {
115 ret := _mock.Called(ctx, repo, ref)184 ret := _mock.Called(ctx, repo, ref)
M internal/ports/mocks/MergeGit.go +17 -23
@@ -192,35 +192,29 @@ func (_c *MergeGit_CommitTree_Call) RunAndReturn(run func(ctx context.Context, r
192192 }
193193
194194 // CountBehind provides a mock function for the type MergeGit
195-func (_mock *MergeGit) CountBehind(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int) (int, bool, error) {
196- ret := _mock.Called(ctx, repo, head, target, max)
195+func (_mock *MergeGit) CountBehind(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int) (int, error) {
196+ ret := _mock.Called(ctx, repo, head, target, limit)
197197
198198 if len(ret) == 0 {
199199 panic("no return value specified for CountBehind")
200200 }
201201
202202 var r0 int
203- var r1 bool
204- var r2 error
205- if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) (int, bool, error)); ok {
206- return returnFunc(ctx, repo, head, target, max)
203+ var r1 error
204+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) (int, error)); ok {
205+ return returnFunc(ctx, repo, head, target, limit)
207206 }
208207 if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) int); ok {
209- r0 = returnFunc(ctx, repo, head, target, max)
208+ r0 = returnFunc(ctx, repo, head, target, limit)
210209 } else {
211210 r0 = ret.Get(0).(int)
212211 }
213- if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) bool); ok {
214- r1 = returnFunc(ctx, repo, head, target, max)
212+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) error); ok {
213+ r1 = returnFunc(ctx, repo, head, target, limit)
215214 } else {
216- r1 = ret.Get(1).(bool)
217- }
218- if returnFunc, ok := ret.Get(2).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) error); ok {
219- r2 = returnFunc(ctx, repo, head, target, max)
220- } else {
221- r2 = ret.Error(2)
215+ r1 = ret.Error(1)
222216 }
223- return r0, r1, r2
217+ return r0, r1
224218 }
225219
226220 // MergeGit_CountBehind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CountBehind'
@@ -233,12 +227,12 @@ type MergeGit_CountBehind_Call struct {
233227 // - repo domain.RepoName
234228 // - head domain.ObjectID
235229 // - target domain.ObjectID
236-// - max int
237-func (_e *MergeGit_Expecter) CountBehind(ctx any, repo any, head any, target any, max any) *MergeGit_CountBehind_Call {
238- return &MergeGit_CountBehind_Call{Call: _e.mock.On("CountBehind", ctx, repo, head, target, max)}
230+// - limit int
231+func (_e *MergeGit_Expecter) CountBehind(ctx any, repo any, head any, target any, limit any) *MergeGit_CountBehind_Call {
232+ return &MergeGit_CountBehind_Call{Call: _e.mock.On("CountBehind", ctx, repo, head, target, limit)}
239233 }
240234
241-func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int)) *MergeGit_CountBehind_Call {
235+func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int)) *MergeGit_CountBehind_Call {
242236 _c.Call.Run(func(args mock.Arguments) {
243237 var arg0 context.Context
244238 if args[0] != nil {
@@ -271,12 +265,12 @@ func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo doma
271265 return _c
272266 }
273267
274-func (_c *MergeGit_CountBehind_Call) Return(n int, capped bool, err error) *MergeGit_CountBehind_Call {
275- _c.Call.Return(n, capped, err)
268+func (_c *MergeGit_CountBehind_Call) Return(n int, err error) *MergeGit_CountBehind_Call {
269+ _c.Call.Return(n, err)
276270 return _c
277271 }
278272
279-func (_c *MergeGit_CountBehind_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int) (int, bool, error)) *MergeGit_CountBehind_Call {
273+func (_c *MergeGit_CountBehind_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int) (int, error)) *MergeGit_CountBehind_Call {
280274 _c.Call.Return(run)
281275 return _c
282276 }
@@ -192,35 +192,29 @@ func (_c *MergeGit_CommitTree_Call) RunAndReturn(run func(ctx context.Context, r
192 }192 }
193 193
194 // CountBehind provides a mock function for the type MergeGit194 // CountBehind provides a mock function for the type MergeGit
195-func (_mock *MergeGit) CountBehind(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int) (int, bool, error) {195+func (_mock *MergeGit) CountBehind(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int) (int, error) {
196- ret := _mock.Called(ctx, repo, head, target, max)196+ ret := _mock.Called(ctx, repo, head, target, limit)
197 197
198 if len(ret) == 0 {198 if len(ret) == 0 {
199 panic("no return value specified for CountBehind")199 panic("no return value specified for CountBehind")
200 }200 }
201 201
202 var r0 int202 var r0 int
203- var r1 bool203+ var r1 error
204- var r2 error204+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) (int, error)); ok {
205- if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) (int, bool, error)); ok {205+ return returnFunc(ctx, repo, head, target, limit)
206- return returnFunc(ctx, repo, head, target, max)
207 }206 }
208 if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) int); ok {207 if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) int); ok {
209- r0 = returnFunc(ctx, repo, head, target, max)208+ r0 = returnFunc(ctx, repo, head, target, limit)
210 } else {209 } else {
211 r0 = ret.Get(0).(int)210 r0 = ret.Get(0).(int)
212 }211 }
213- if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) bool); ok {212+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) error); ok {
214- r1 = returnFunc(ctx, repo, head, target, max)213+ r1 = returnFunc(ctx, repo, head, target, limit)
215 } else {214 } else {
216- r1 = ret.Get(1).(bool)215+ r1 = ret.Error(1)
217- }
218- if returnFunc, ok := ret.Get(2).(func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID, int) error); ok {
219- r2 = returnFunc(ctx, repo, head, target, max)
220- } else {
221- r2 = ret.Error(2)
222 }216 }
223- return r0, r1, r2217+ return r0, r1
224 }218 }
225 219
226 // MergeGit_CountBehind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CountBehind'220 // MergeGit_CountBehind_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CountBehind'
@@ -233,12 +227,12 @@ type MergeGit_CountBehind_Call struct {
233 // - repo domain.RepoName227 // - repo domain.RepoName
234 // - head domain.ObjectID228 // - head domain.ObjectID
235 // - target domain.ObjectID229 // - target domain.ObjectID
236-// - max int230+// - limit int
237-func (_e *MergeGit_Expecter) CountBehind(ctx any, repo any, head any, target any, max any) *MergeGit_CountBehind_Call {231+func (_e *MergeGit_Expecter) CountBehind(ctx any, repo any, head any, target any, limit any) *MergeGit_CountBehind_Call {
238- return &MergeGit_CountBehind_Call{Call: _e.mock.On("CountBehind", ctx, repo, head, target, max)}232+ return &MergeGit_CountBehind_Call{Call: _e.mock.On("CountBehind", ctx, repo, head, target, limit)}
239 }233 }
240 234
241-func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int)) *MergeGit_CountBehind_Call {235+func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int)) *MergeGit_CountBehind_Call {
242 _c.Call.Run(func(args mock.Arguments) {236 _c.Call.Run(func(args mock.Arguments) {
243 var arg0 context.Context237 var arg0 context.Context
244 if args[0] != nil {238 if args[0] != nil {
@@ -271,12 +265,12 @@ func (_c *MergeGit_CountBehind_Call) Run(run func(ctx context.Context, repo doma
271 return _c265 return _c
272 }266 }
273 267
274-func (_c *MergeGit_CountBehind_Call) Return(n int, capped bool, err error) *MergeGit_CountBehind_Call {268+func (_c *MergeGit_CountBehind_Call) Return(n int, err error) *MergeGit_CountBehind_Call {
275- _c.Call.Return(n, capped, err)269+ _c.Call.Return(n, err)
276 return _c270 return _c
277 }271 }
278 272
279-func (_c *MergeGit_CountBehind_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, max int) (int, bool, error)) *MergeGit_CountBehind_Call {273+func (_c *MergeGit_CountBehind_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, head domain.ObjectID, target domain.ObjectID, limit int) (int, error)) *MergeGit_CountBehind_Call {
280 _c.Call.Return(run)274 _c.Call.Return(run)
281 return _c275 return _c
282 }276 }
M internal/ports/ports.go +6 -4
@@ -425,11 +425,13 @@ type MergeGit interface {
425425 DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error
426426 // MergeTree merges head into base without creating a commit. When the
427427 // merge is not clean (or the commits share no history) Clean is false and
428- // ConflictPaths lists at most domain.MaxConflictPaths paths.
428+ // ConflictPaths lists every conflicted path, unbounded: the caller bounds
429+ // them (MergeTreeResult.Bounded).
429430 MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error)
430- // CountBehind counts the commits of target that head lacks, at most max;
431- // capped is true when there are more.
432- CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, max int) (n int, capped bool, err error)
431+ // CountBehind counts the commits of target that head lacks, stopping at
432+ // limit: the raw count, at most limit. The caller interprets it
433+ // (domain.BoundBehind).
434+ CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, limit int) (n int, err error)
433435 }
434436
435437 // MergeabilityTrigger queues mergeability checks. It only queues the work and
@@ -425,11 +425,13 @@ type MergeGit interface {
425 DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error425 DeleteBranch(ctx context.Context, repo domain.RepoName, branch string, old domain.ObjectID) error
426 // MergeTree merges head into base without creating a commit. When the426 // MergeTree merges head into base without creating a commit. When the
427 // merge is not clean (or the commits share no history) Clean is false and427 // merge is not clean (or the commits share no history) Clean is false and
428- // ConflictPaths lists at most domain.MaxConflictPaths paths.428+ // ConflictPaths lists every conflicted path, unbounded: the caller bounds
429+ // them (MergeTreeResult.Bounded).
429 MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error)430 MergeTree(ctx context.Context, repo domain.RepoName, base, head domain.ObjectID) (domain.MergeTreeResult, error)
430- // CountBehind counts the commits of target that head lacks, at most max;431+ // CountBehind counts the commits of target that head lacks, stopping at
431- // capped is true when there are more.432+ // limit: the raw count, at most limit. The caller interprets it
432- CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, max int) (n int, capped bool, err error)433+ // (domain.BoundBehind).
434+ CountBehind(ctx context.Context, repo domain.RepoName, head, target domain.ObjectID, limit int) (n int, err error)
433 }435 }
434 436
435 // MergeabilityTrigger queues mergeability checks. It only queues the work and437 // MergeabilityTrigger queues mergeability checks. It only queues the work and
M internal/usecase/ci_unit_test.go +1 -0
@@ -54,6 +54,7 @@ func newMergeUnit(t *testing.T, block bool) *mergeUnit {
5454 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "feat").Return(m.head, nil)
5555 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "main").Return(tip, nil)
5656 u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, tip, m.head).Return(true, nil)
57+ u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, m.head, tip).Return(false, nil)
5758 return m
5859 }
5960
@@ -54,6 +54,7 @@ func newMergeUnit(t *testing.T, block bool) *mergeUnit {
54 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "feat").Return(m.head, nil)54 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "feat").Return(m.head, nil)
55 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "main").Return(tip, nil)55 u.git.EXPECT().BranchHead(mock.Anything, u.repo.Name, "main").Return(tip, nil)
56 u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, tip, m.head).Return(true, nil)56 u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, tip, m.head).Return(true, nil)
57+ u.git.EXPECT().IsAncestor(mock.Anything, u.repo.Name, m.head, tip).Return(false, nil)
57 return m58 return m
58 }59 }
59 60
M internal/usecase/export_test.go +4 -0
@@ -161,3 +161,7 @@ func (s *IssueSubmissions) FillSpent(n int, exp time.Time) {
161161
162162 // MaxSpent is the bound of the spent set.
163163 const MaxSpent = maxSpent
164+
165+// SetAfterMergeGate makes the post-merge goroutine of m run f before anything
166+// else, to place a test between a merge and its side effects.
167+func (m *MergeRequests) SetAfterMergeGate(f func()) { m.afterGate = f }
@@ -161,3 +161,7 @@ func (s *IssueSubmissions) FillSpent(n int, exp time.Time) {
161 161
162 // MaxSpent is the bound of the spent set.162 // MaxSpent is the bound of the spent set.
163 const MaxSpent = maxSpent163 const MaxSpent = maxSpent
164+
165+// SetAfterMergeGate makes the post-merge goroutine of m run f before anything
166+// else, to place a test between a merge and its side effects.
167+func (m *MergeRequests) SetAfterMergeGate(f func()) { m.afterGate = f }
M internal/usecase/mergeability.go +7 -2
@@ -252,7 +252,9 @@ func (s *Mergeability) check(k mergeKey) {
252252 return
253253 }
254254 if prev.IsFor(tip, head) {
255- s.q.takeSeen(k)
255+ if s.q.takeSeen(k) {
256+ s.publish(ctx, prev) // a reader saw "checking" before this record
257+ }
256258 return
257259 }
258260 res, err := s.compute(ctx, k, tip, head)
@@ -328,13 +330,16 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain
328330 }
329331 if !tree.Clean {
330332 m.State = domain.MergeabilityConflict
333+ tree = tree.Bounded()
331334 m.ConflictPaths, m.ConflictPathsTruncated = tree.ConflictPaths, tree.Truncated
332335 return m, nil
333336 }
334337 m.State = domain.MergeabilityBehind
335- if m.Behind, m.BehindCapped, err = s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.MaxBehindCount); err != nil {
338+ n, err := s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.BehindLimit)
339+ if err != nil {
336340 return m, err
337341 }
342+ m.Behind, m.BehindCapped = domain.BoundBehind(n)
338343 return m, nil
339344 }
340345
@@ -252,7 +252,9 @@ func (s *Mergeability) check(k mergeKey) {
252 return252 return
253 }253 }
254 if prev.IsFor(tip, head) {254 if prev.IsFor(tip, head) {
255- s.q.takeSeen(k)255+ if s.q.takeSeen(k) {
256+ s.publish(ctx, prev) // a reader saw "checking" before this record
257+ }
256 return258 return
257 }259 }
258 res, err := s.compute(ctx, k, tip, head)260 res, err := s.compute(ctx, k, tip, head)
@@ -328,13 +330,16 @@ func (s *Mergeability) compute(ctx context.Context, k mergeKey, tip, head domain
328 }330 }
329 if !tree.Clean {331 if !tree.Clean {
330 m.State = domain.MergeabilityConflict332 m.State = domain.MergeabilityConflict
333+ tree = tree.Bounded()
331 m.ConflictPaths, m.ConflictPathsTruncated = tree.ConflictPaths, tree.Truncated334 m.ConflictPaths, m.ConflictPathsTruncated = tree.ConflictPaths, tree.Truncated
332 return m, nil335 return m, nil
333 }336 }
334 m.State = domain.MergeabilityBehind337 m.State = domain.MergeabilityBehind
335- if m.Behind, m.BehindCapped, err = s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.MaxBehindCount); err != nil {338+ n, err := s.cfg.Git.CountBehind(ctx, k.repo, head, tip, domain.BehindLimit)
339+ if err != nil {
336 return m, err340 return m, err
337 }341 }
342+ m.Behind, m.BehindCapped = domain.BoundBehind(n)
338 return m, nil343 return m, nil
339 }344 }
340 345
M internal/usecase/mergeability_queue.go +10 -3
@@ -84,14 +84,21 @@ func (q *mergeQueue) markDirtyLocked(repo domain.RepoName) {
8484 func (q *mergeQueue) next() (key *mergeKey, dirty *domain.RepoName) {
8585 q.mu.Lock()
8686 defer q.mu.Unlock()
87- switch {
88- case len(q.order) > 0:
87+ for key == nil && len(q.order) > 0 {
8988 k := q.order[0]
9089 q.order = q.order[1:]
90+ if !q.pending[k] {
91+ continue // claimed meanwhile by tryRun or an earlier entry
92+ }
9193 delete(q.pending, k)
94+ if q.running[k] {
95+ q.again[k] = true // never run a key concurrently
96+ continue
97+ }
9298 q.running[k] = true
9399 key = &k
94- case len(q.dirtyOrder) > 0:
100+ }
101+ if key == nil && len(q.dirtyOrder) > 0 {
95102 r := q.dirtyOrder[0]
96103 q.dirtyOrder = q.dirtyOrder[1:]
97104 delete(q.dirty, r)
@@ -84,14 +84,21 @@ func (q *mergeQueue) markDirtyLocked(repo domain.RepoName) {
84 func (q *mergeQueue) next() (key *mergeKey, dirty *domain.RepoName) {84 func (q *mergeQueue) next() (key *mergeKey, dirty *domain.RepoName) {
85 q.mu.Lock()85 q.mu.Lock()
86 defer q.mu.Unlock()86 defer q.mu.Unlock()
87- switch {87+ for key == nil && len(q.order) > 0 {
88- case len(q.order) > 0:
89 k := q.order[0]88 k := q.order[0]
90 q.order = q.order[1:]89 q.order = q.order[1:]
90+ if !q.pending[k] {
91+ continue // claimed meanwhile by tryRun or an earlier entry
92+ }
91 delete(q.pending, k)93 delete(q.pending, k)
94+ if q.running[k] {
95+ q.again[k] = true // never run a key concurrently
96+ continue
97+ }
92 q.running[k] = true98 q.running[k] = true
93 key = &k99 key = &k
94- case len(q.dirtyOrder) > 0:100+ }
101+ if key == nil && len(q.dirtyOrder) > 0 {
95 r := q.dirtyOrder[0]102 r := q.dirtyOrder[0]
96 q.dirtyOrder = q.dirtyOrder[1:]103 q.dirtyOrder = q.dirtyOrder[1:]
97 delete(q.dirty, r)104 delete(q.dirty, r)
A internal/usecase/mergeability_queue_test.go +48 -0
@@ -0,0 +1,48 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase
4+
5+import (
6+ "testing"
7+
8+ "github.com/stretchr/testify/assert"
9+ "github.com/stretchr/testify/require"
10+
11+ "bnema.dev/ermite/internal/domain"
12+)
13+
14+func TestMergeQueueNextSkipsAClaimedKey(t *testing.T) {
15+ repo, err := domain.ParseRepoName("proj")
16+ require.NoError(t, err)
17+ k := mergeKey{repo, 1}
18+ q := newMergeQueue(8)
19+
20+ q.add(k) // pending, in order
21+ require.True(t, q.tryRun(k)) // a rescan claims it: no longer pending, still in order
22+ key, dirty := q.next()
23+ assert.Nil(t, key, "a claimed key is not run again by a worker")
24+ assert.Nil(t, dirty)
25+ assert.True(t, q.running[k], "the rescan's marker is intact")
26+
27+ // Queued again while it runs: one more pass, still no concurrent run.
28+ q.add(k)
29+ key, _ = q.next()
30+ assert.Nil(t, key)
31+ assert.True(t, q.finish(k), "another pass is due")
32+ assert.False(t, q.finish(k))
33+ assert.False(t, q.running[k])
34+}
35+
36+func TestMergeQueueNextMarksAgainForARunningKey(t *testing.T) {
37+ repo, err := domain.ParseRepoName("proj")
38+ require.NoError(t, err)
39+ k := mergeKey{repo, 1}
40+ q := newMergeQueue(8)
41+
42+ // A stale order entry for a key that is pending again and running.
43+ q.pending[k], q.order, q.running[k] = true, []mergeKey{k}, true
44+ key, _ := q.next()
45+ assert.Nil(t, key)
46+ assert.True(t, q.again[k])
47+ assert.False(t, q.pending[k])
48+}
@@ -0,0 +1,48 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase
4+
5+import (
6+ "testing"
7+
8+ "github.com/stretchr/testify/assert"
9+ "github.com/stretchr/testify/require"
10+
11+ "bnema.dev/ermite/internal/domain"
12+)
13+
14+func TestMergeQueueNextSkipsAClaimedKey(t *testing.T) {
15+ repo, err := domain.ParseRepoName("proj")
16+ require.NoError(t, err)
17+ k := mergeKey{repo, 1}
18+ q := newMergeQueue(8)
19+
20+ q.add(k) // pending, in order
21+ require.True(t, q.tryRun(k)) // a rescan claims it: no longer pending, still in order
22+ key, dirty := q.next()
23+ assert.Nil(t, key, "a claimed key is not run again by a worker")
24+ assert.Nil(t, dirty)
25+ assert.True(t, q.running[k], "the rescan's marker is intact")
26+
27+ // Queued again while it runs: one more pass, still no concurrent run.
28+ q.add(k)
29+ key, _ = q.next()
30+ assert.Nil(t, key)
31+ assert.True(t, q.finish(k), "another pass is due")
32+ assert.False(t, q.finish(k))
33+ assert.False(t, q.running[k])
34+}
35+
36+func TestMergeQueueNextMarksAgainForARunningKey(t *testing.T) {
37+ repo, err := domain.ParseRepoName("proj")
38+ require.NoError(t, err)
39+ k := mergeKey{repo, 1}
40+ q := newMergeQueue(8)
41+
42+ // A stale order entry for a key that is pending again and running.
43+ q.pending[k], q.order, q.running[k] = true, []mergeKey{k}, true
44+ key, _ := q.next()
45+ assert.Nil(t, key)
46+ assert.True(t, q.again[k])
47+ assert.False(t, q.pending[k])
48+}
M internal/usecase/mergeability_test.go +123 -8
@@ -76,12 +76,23 @@ func (r *mergeRig) events() []domain.MergeRequestMergeability {
7676 }
7777 }
7878
79-func (r *mergeRig) push2(branch, path, content string) domain.ObjectID {
79+// awaitEvents receives mergeability events until n arrived, or fails after a
80+// bound: the workers publish after they stored the record the tests wait for.
81+func (r *mergeRig) awaitEvents(n int) []domain.MergeRequestMergeability {
8082 r.t.Helper()
81- r.sh("checkout", "-q", branch)
82- id := r.commit(author, path, content, "edit "+path)
83- r.push(branch)
84- return id
83+ var out []domain.MergeRequestMergeability
84+ deadline := time.After(10 * time.Second)
85+ for len(out) < n {
86+ select {
87+ case ev := <-r.mev:
88+ if m, ok := ev.(domain.MergeRequestMergeability); ok {
89+ out = append(out, m)
90+ }
91+ case <-deadline:
92+ r.t.Fatalf("got %d mergeability events, want %d", len(out), n)
93+ }
94+ }
95+ return out
8596 }
8697
8798 func TestMergeabilityStatesEndToEnd(t *testing.T) {
@@ -111,8 +122,7 @@ func TestMergeabilityStatesEndToEnd(t *testing.T) {
111122 m = r.waitState(mr.Number, domain.MergeabilityConflict, main3, head)
112123 assert.Equal(t, []string{"a.txt"}, m.ConflictPaths)
113124
114- got := r.events()
115- require.Len(t, got, 3)
125+ got := r.awaitEvents(3)
116126 for i, s := range []domain.MergeabilityState{domain.MergeabilityUpToDate, domain.MergeabilityBehind, domain.MergeabilityConflict} {
117127 assert.Equal(t, s, got[i].Mergeability.State)
118128 assert.Equal(t, mr.Number, got[i].Number)
@@ -331,7 +341,7 @@ func TestMergeabilityDedupsTriggers(t *testing.T) {
331341 }
332342 r.svc.Start()
333343 require.Eventually(t, func() bool { return computes.Load() >= 1 }, 5*time.Second, 5*time.Millisecond)
334- time.Sleep(50 * time.Millisecond)
344+ r.svc.Close(context.Background()) // the workers have each looked at the queue once more
335345 assert.EqualValues(t, 1, computes.Load())
336346 }
337347
@@ -345,3 +355,108 @@ func TestMergeabilityNoWriteForClosedRequest(t *testing.T) {
345355 r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile
346356 r.svc.Check(repo, 7) // no event, no further write
347357 }
358+
359+// A reader that saw a stale record marks the request seen and queues it; the
360+// worker finds the record already current (stored and published by an earlier
361+// check) and must still publish it, as the reader was told "checking".
362+func TestMergeabilityFastPathPublishesWhenAReaderSawChecking(t *testing.T) {
363+ repo := name(t, "proj")
364+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
365+ mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen, HeadID: head}
366+ rec := domain.Mergeability{Repo: repo, Number: 7, State: domain.MergeabilityUpToDate, TargetID: tip, HeadID: head, CheckedAt: now}
367+ store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t)
368+ store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil)
369+ store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Once() // the reader's read: stale
370+ store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(rec, nil) // the worker has stored it since
371+ meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil)
372+ git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
373+ git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
374+ var published []domain.Event
375+ bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(_ context.Context, ev domain.Event) { published = append(published, ev) }).Return().Once()
376+ svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}})
377+ t.Cleanup(func() { svc.Close(context.Background()) })
378+
379+ got := svc.Read(context.Background(), mr, tip)
380+ assert.Equal(t, domain.MergeabilityChecking, got.State)
381+ svc.Check(repo, 7)
382+ require.Len(t, published, 1, "the current record is published for the reader")
383+ assert.Equal(t, domain.MergeabilityUpToDate, published[0].(domain.MergeRequestMergeability).Mergeability.State)
384+
385+ svc.Check(repo, 7) // nobody saw "checking" since: nothing more
386+ assert.Len(t, published, 1)
387+}
388+
389+// A rescan claims a key that is also pending; a worker must then not run it
390+// concurrently (GO-001).
391+func TestMergeabilityRescanAndWorkersNeverRunAKeyConcurrently(t *testing.T) {
392+ r := newMockRig(t)
393+ repo := r.mr.Repo
394+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
395+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
396+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
397+ var inflight, peak atomic.Int32
398+ entered, release := make(chan struct{}, 8), make(chan struct{})
399+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn(
400+ func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) {
401+ n := inflight.Add(1)
402+ for p := peak.Load(); n > p && !peak.CompareAndSwap(p, n); p = peak.Load() {
403+ }
404+ entered <- struct{}{}
405+ <-release
406+ inflight.Add(-1)
407+ return true, nil
408+ })
409+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil)
410+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
411+
412+ r.svc.OnOpened(context.Background(), r.mr) // pending
413+ done := make(chan struct{})
414+ go func() { r.svc.Check(repo, 7); close(done) }() // claims it, as a rescan does
415+ <-entered
416+ r.svc.Start() // two workers
417+ r.svc.Close(context.Background())
418+ close(release)
419+ <-done
420+ assert.EqualValues(t, 1, peak.Load(), "one check of the key at a time")
421+}
422+
423+func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) {
424+ t.Run("conflict paths", func(t *testing.T) {
425+ r := newMockRig(t)
426+ repo := r.mr.Repo
427+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
428+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
429+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
430+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(false, nil)
431+ var paths []string
432+ for i := 0; i < domain.MaxConflictPaths+3; i++ {
433+ paths = append(paths, strings.Repeat("p", i+1))
434+ }
435+ r.git.EXPECT().MergeTree(mock.Anything, repo, tip, head).Return(domain.MergeTreeResult{ConflictPaths: paths}, nil)
436+ var put domain.Mergeability
437+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn(
438+ func(_ context.Context, m domain.Mergeability) error { put = m; return nil })
439+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
440+ r.svc.Check(repo, 7)
441+ assert.Equal(t, domain.MergeabilityConflict, put.State)
442+ assert.Len(t, put.ConflictPaths, domain.MaxConflictPaths)
443+ assert.True(t, put.ConflictPathsTruncated)
444+ })
445+ t.Run("behind", func(t *testing.T) {
446+ r := newMockRig(t)
447+ repo := r.mr.Repo
448+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
449+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
450+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
451+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(false, nil)
452+ r.git.EXPECT().MergeTree(mock.Anything, repo, tip, head).Return(domain.MergeTreeResult{Clean: true}, nil)
453+ r.git.EXPECT().CountBehind(mock.Anything, repo, head, tip, domain.MaxBehindCount+1).Return(domain.MaxBehindCount+1, nil)
454+ var put domain.Mergeability
455+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn(
456+ func(_ context.Context, m domain.Mergeability) error { put = m; return nil })
457+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
458+ r.svc.Check(repo, 7)
459+ assert.Equal(t, domain.MaxBehindCount, put.Behind)
460+ assert.True(t, put.BehindCapped)
461+ })
462+}
@@ -76,12 +76,23 @@ func (r *mergeRig) events() []domain.MergeRequestMergeability {
76 }76 }
77 }77 }
78 78
79-func (r *mergeRig) push2(branch, path, content string) domain.ObjectID {79+// awaitEvents receives mergeability events until n arrived, or fails after a
80+// bound: the workers publish after they stored the record the tests wait for.
81+func (r *mergeRig) awaitEvents(n int) []domain.MergeRequestMergeability {
80 r.t.Helper()82 r.t.Helper()
81- r.sh("checkout", "-q", branch)83+ var out []domain.MergeRequestMergeability
82- id := r.commit(author, path, content, "edit "+path)84+ deadline := time.After(10 * time.Second)
83- r.push(branch)85+ for len(out) < n {
84- return id86+ select {
87+ case ev := <-r.mev:
88+ if m, ok := ev.(domain.MergeRequestMergeability); ok {
89+ out = append(out, m)
90+ }
91+ case <-deadline:
92+ r.t.Fatalf("got %d mergeability events, want %d", len(out), n)
93+ }
94+ }
95+ return out
85 }96 }
86 97
87 func TestMergeabilityStatesEndToEnd(t *testing.T) {98 func TestMergeabilityStatesEndToEnd(t *testing.T) {
@@ -111,8 +122,7 @@ func TestMergeabilityStatesEndToEnd(t *testing.T) {
111 m = r.waitState(mr.Number, domain.MergeabilityConflict, main3, head)122 m = r.waitState(mr.Number, domain.MergeabilityConflict, main3, head)
112 assert.Equal(t, []string{"a.txt"}, m.ConflictPaths)123 assert.Equal(t, []string{"a.txt"}, m.ConflictPaths)
113 124
114- got := r.events()125+ got := r.awaitEvents(3)
115- require.Len(t, got, 3)
116 for i, s := range []domain.MergeabilityState{domain.MergeabilityUpToDate, domain.MergeabilityBehind, domain.MergeabilityConflict} {126 for i, s := range []domain.MergeabilityState{domain.MergeabilityUpToDate, domain.MergeabilityBehind, domain.MergeabilityConflict} {
117 assert.Equal(t, s, got[i].Mergeability.State)127 assert.Equal(t, s, got[i].Mergeability.State)
118 assert.Equal(t, mr.Number, got[i].Number)128 assert.Equal(t, mr.Number, got[i].Number)
@@ -331,7 +341,7 @@ func TestMergeabilityDedupsTriggers(t *testing.T) {
331 }341 }
332 r.svc.Start()342 r.svc.Start()
333 require.Eventually(t, func() bool { return computes.Load() >= 1 }, 5*time.Second, 5*time.Millisecond)343 require.Eventually(t, func() bool { return computes.Load() >= 1 }, 5*time.Second, 5*time.Millisecond)
334- time.Sleep(50 * time.Millisecond)344+ r.svc.Close(context.Background()) // the workers have each looked at the queue once more
335 assert.EqualValues(t, 1, computes.Load())345 assert.EqualValues(t, 1, computes.Load())
336 }346 }
337 347
@@ -345,3 +355,108 @@ func TestMergeabilityNoWriteForClosedRequest(t *testing.T) {
345 r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile355 r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() // closed meanwhile
346 r.svc.Check(repo, 7) // no event, no further write356 r.svc.Check(repo, 7) // no event, no further write
347 }357 }
358+
359+// A reader that saw a stale record marks the request seen and queues it; the
360+// worker finds the record already current (stored and published by an earlier
361+// check) and must still publish it, as the reader was told "checking".
362+func TestMergeabilityFastPathPublishesWhenAReaderSawChecking(t *testing.T) {
363+ repo := name(t, "proj")
364+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
365+ mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen, HeadID: head}
366+ rec := domain.Mergeability{Repo: repo, Number: 7, State: domain.MergeabilityUpToDate, TargetID: tip, HeadID: head, CheckedAt: now}
367+ store, meta, git, bus := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t), mocks.NewEventBus(t)
368+ store.EXPECT().Get(mock.Anything, repo, int64(7)).Return(mr, nil)
369+ store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound).Once() // the reader's read: stale
370+ store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(rec, nil) // the worker has stored it since
371+ meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil)
372+ git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
373+ git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
374+ var published []domain.Event
375+ bus.EXPECT().Publish(mock.Anything, mock.Anything).Run(func(_ context.Context, ev domain.Event) { published = append(published, ev) }).Return().Once()
376+ svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{Store: store, Meta: meta, Git: git, Bus: bus, Clock: fixedClock{}})
377+ t.Cleanup(func() { svc.Close(context.Background()) })
378+
379+ got := svc.Read(context.Background(), mr, tip)
380+ assert.Equal(t, domain.MergeabilityChecking, got.State)
381+ svc.Check(repo, 7)
382+ require.Len(t, published, 1, "the current record is published for the reader")
383+ assert.Equal(t, domain.MergeabilityUpToDate, published[0].(domain.MergeRequestMergeability).Mergeability.State)
384+
385+ svc.Check(repo, 7) // nobody saw "checking" since: nothing more
386+ assert.Len(t, published, 1)
387+}
388+
389+// A rescan claims a key that is also pending; a worker must then not run it
390+// concurrently (GO-001).
391+func TestMergeabilityRescanAndWorkersNeverRunAKeyConcurrently(t *testing.T) {
392+ r := newMockRig(t)
393+ repo := r.mr.Repo
394+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
395+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
396+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
397+ var inflight, peak atomic.Int32
398+ entered, release := make(chan struct{}, 8), make(chan struct{})
399+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).RunAndReturn(
400+ func(context.Context, domain.RepoName, domain.ObjectID, domain.ObjectID) (bool, error) {
401+ n := inflight.Add(1)
402+ for p := peak.Load(); n > p && !peak.CompareAndSwap(p, n); p = peak.Load() {
403+ }
404+ entered <- struct{}{}
405+ <-release
406+ inflight.Add(-1)
407+ return true, nil
408+ })
409+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(nil)
410+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
411+
412+ r.svc.OnOpened(context.Background(), r.mr) // pending
413+ done := make(chan struct{})
414+ go func() { r.svc.Check(repo, 7); close(done) }() // claims it, as a rescan does
415+ <-entered
416+ r.svc.Start() // two workers
417+ r.svc.Close(context.Background())
418+ close(release)
419+ <-done
420+ assert.EqualValues(t, 1, peak.Load(), "one check of the key at a time")
421+}
422+
423+func TestMergeabilityBoundsConflictPathsAndBehind(t *testing.T) {
424+ t.Run("conflict paths", func(t *testing.T) {
425+ r := newMockRig(t)
426+ repo := r.mr.Repo
427+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
428+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
429+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
430+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(false, nil)
431+ var paths []string
432+ for i := 0; i < domain.MaxConflictPaths+3; i++ {
433+ paths = append(paths, strings.Repeat("p", i+1))
434+ }
435+ r.git.EXPECT().MergeTree(mock.Anything, repo, tip, head).Return(domain.MergeTreeResult{ConflictPaths: paths}, nil)
436+ var put domain.Mergeability
437+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn(
438+ func(_ context.Context, m domain.Mergeability) error { put = m; return nil })
439+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
440+ r.svc.Check(repo, 7)
441+ assert.Equal(t, domain.MergeabilityConflict, put.State)
442+ assert.Len(t, put.ConflictPaths, domain.MaxConflictPaths)
443+ assert.True(t, put.ConflictPathsTruncated)
444+ })
445+ t.Run("behind", func(t *testing.T) {
446+ r := newMockRig(t)
447+ repo := r.mr.Repo
448+ head, tip := fakeOID(t, "a"), fakeOID(t, "b")
449+ r.git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil)
450+ r.git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil)
451+ r.git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(false, nil)
452+ r.git.EXPECT().MergeTree(mock.Anything, repo, tip, head).Return(domain.MergeTreeResult{Clean: true}, nil)
453+ r.git.EXPECT().CountBehind(mock.Anything, repo, head, tip, domain.MaxBehindCount+1).Return(domain.MaxBehindCount+1, nil)
454+ var put domain.Mergeability
455+ r.store.EXPECT().PutMergeability(mock.Anything, mock.Anything).RunAndReturn(
456+ func(_ context.Context, m domain.Mergeability) error { put = m; return nil })
457+ r.bus.EXPECT().Publish(mock.Anything, mock.Anything).Return()
458+ r.svc.Check(repo, 7)
459+ assert.Equal(t, domain.MaxBehindCount, put.Behind)
460+ assert.True(t, put.BehindCapped)
461+ })
462+}
M internal/usecase/mergerequests.go +53 -17
@@ -8,6 +8,7 @@ import (
88 "errors"
99 "fmt"
1010 "math"
11+ "runtime/debug"
1112 "slices"
1213 "strconv"
1314 "strings"
@@ -63,6 +64,8 @@ type MergeRequests struct {
6364
6465 // after tracks the post-merge side effects running off the request path.
6566 after sync.WaitGroup
67+ // afterGate, when set by a test, runs first in the post-merge goroutine.
68+ afterGate func()
6669 }
6770
6871 var (
@@ -948,6 +951,7 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m
948951 if err != nil {
949952 return domain.ObjectID{}, err
950953 }
954+ res = res.Bounded()
951955 if !res.Clean {
952956 what := "no common history"
953957 if len(res.ConflictPaths) > 0 {
@@ -1045,6 +1049,12 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
10451049 case tip == head:
10461050 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
10471051 }
1052+ // A head behind the target (or at it) has nothing to merge, in any mode.
1053+ if behind, err := m.git.IsAncestor(ctx, name, head, tip); err != nil {
1054+ return domain.MergeRequest{}, err
1055+ } else if behind {
1056+ return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already contains %s", domain.ErrPreconditionFailed, mr.Target, head)
1057+ }
10481058 tree, err := m.checkMode(ctx, mr, mode, tip, head)
10491059 if err != nil {
10501060 return domain.MergeRequest{}, err
@@ -1081,6 +1091,16 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
10811091 m.after.Add(1)
10821092 go func() {
10831093 defer m.after.Done()
1094+ defer func() {
1095+ if r := recover(); r != nil {
1096+ log := zerowrap.FromCtx(bg)
1097+ log.Error().Str("repo", name.String()).Int64("number", number).
1098+ Str("panic", fmt.Sprint(r)).Bytes("stack", debug.Stack()).Msg("post-merge side effects panicked")
1099+ }
1100+ }()
1101+ if m.afterGate != nil {
1102+ m.afterGate()
1103+ }
10841104 m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
10851105 }()
10861106 return mr, nil
@@ -1091,24 +1111,9 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
10911111 // new one.
10921112 func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
10931113 name := repo.Name
1094- log := zerowrap.FromCtx(ctx)
10951114 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
1096- if deleteSource {
1097- // Who pushed the source goes first (the branch is gone either way for a
1098- // schedule; a kept branch merely has no record: not trusted).
1099- del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
1100- if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
1101- log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
1102- }
1103- switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
1104- case err != nil:
1105- log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1106- Msg("source branch kept after the merge")
1107- default:
1108- updates = append(updates, del)
1109- log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1110- Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1111- }
1115+ if deleteSource && m.deleteSource(ctx, p, name, mr, head) {
1116+ updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})
11121117 }
11131118 at := mr.UpdatedAt
11141119 m.bus.Publish(ctx, domain.MergeRequestMerged{MergeRequest: mr, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
@@ -1129,6 +1134,37 @@ func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo
11291134 }
11301135 }
11311136
1137+// deleteSource deletes the source branch of a merged request if it is still
1138+// head and still unused, and reports whether it did. The eligibility is checked
1139+// again and the branch deleted in one critical section of the merge lock: a
1140+// request opened on the branch since the merge, or a push to it, keeps it. The
1141+// record of who pushed the branch goes only after a deletion, and only if it
1142+// still names head.
1143+func (m *MergeRequests) deleteSource(ctx context.Context, p domain.Principal, name domain.RepoName, mr domain.MergeRequest, head domain.ObjectID) bool {
1144+ log := zerowrap.FromCtx(ctx).With().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).Logger()
1145+ unlock, err := m.lockRepo(ctx, name)
1146+ if err != nil {
1147+ log.Warn().Err(err).Msg("source branch kept after the merge")
1148+ return false
1149+ }
1150+ defer unlock()
1151+ if err := m.checkDeletable(ctx, name, mr); err != nil {
1152+ log.Warn().Err(err).Msg("source branch kept after the merge")
1153+ return false
1154+ }
1155+ if err := m.git.DeleteBranch(ctx, name, mr.Source, head); err != nil {
1156+ log.Warn().Err(err).Msg("source branch kept after the merge")
1157+ return false
1158+ }
1159+ if m.tips != nil {
1160+ if err := m.tips.DeleteRefTipIf(ctx, name, "refs/heads/"+mr.Source, head.String()); err != nil && !errors.Is(err, domain.ErrNotFound) {
1161+ log.Warn().Err(err).Msg("the record of the source branch was kept")
1162+ }
1163+ }
1164+ log.Info().Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1165+ return true
1166+}
1167+
11321168 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a
11331169 // close or a merge of the request cannot complete in the middle of it, and not
11341170 // at all when the request is no longer open.
@@ -8,6 +8,7 @@ import (
8 "errors"8 "errors"
9 "fmt"9 "fmt"
10 "math"10 "math"
11+ "runtime/debug"
11 "slices"12 "slices"
12 "strconv"13 "strconv"
13 "strings"14 "strings"
@@ -63,6 +64,8 @@ type MergeRequests struct {
63 64
64 // after tracks the post-merge side effects running off the request path.65 // after tracks the post-merge side effects running off the request path.
65 after sync.WaitGroup66 after sync.WaitGroup
67+ // afterGate, when set by a test, runs first in the post-merge goroutine.
68+ afterGate func()
66 }69 }
67 70
68 var (71 var (
@@ -948,6 +951,7 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m
948 if err != nil {951 if err != nil {
949 return domain.ObjectID{}, err952 return domain.ObjectID{}, err
950 }953 }
954+ res = res.Bounded()
951 if !res.Clean {955 if !res.Clean {
952 what := "no common history"956 what := "no common history"
953 if len(res.ConflictPaths) > 0 {957 if len(res.ConflictPaths) > 0 {
@@ -1045,6 +1049,12 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
1045 case tip == head:1049 case tip == head:
1046 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)1050 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
1047 }1051 }
1052+ // A head behind the target (or at it) has nothing to merge, in any mode.
1053+ if behind, err := m.git.IsAncestor(ctx, name, head, tip); err != nil {
1054+ return domain.MergeRequest{}, err
1055+ } else if behind {
1056+ return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already contains %s", domain.ErrPreconditionFailed, mr.Target, head)
1057+ }
1048 tree, err := m.checkMode(ctx, mr, mode, tip, head)1058 tree, err := m.checkMode(ctx, mr, mode, tip, head)
1049 if err != nil {1059 if err != nil {
1050 return domain.MergeRequest{}, err1060 return domain.MergeRequest{}, err
@@ -1081,6 +1091,16 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
1081 m.after.Add(1)1091 m.after.Add(1)
1082 go func() {1092 go func() {
1083 defer m.after.Done()1093 defer m.after.Done()
1094+ defer func() {
1095+ if r := recover(); r != nil {
1096+ log := zerowrap.FromCtx(bg)
1097+ log.Error().Str("repo", name.String()).Int64("number", number).
1098+ Str("panic", fmt.Sprint(r)).Bytes("stack", debug.Stack()).Msg("post-merge side effects panicked")
1099+ }
1100+ }()
1101+ if m.afterGate != nil {
1102+ m.afterGate()
1103+ }
1084 m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)1104 m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
1085 }()1105 }()
1086 return mr, nil1106 return mr, nil
@@ -1091,24 +1111,9 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
1091 // new one.1111 // new one.
1092 func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {1112 func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
1093 name := repo.Name1113 name := repo.Name
1094- log := zerowrap.FromCtx(ctx)
1095 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}1114 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
1096- if deleteSource {1115+ if deleteSource && m.deleteSource(ctx, p, name, mr, head) {
1097- // Who pushed the source goes first (the branch is gone either way for a1116+ updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})
1098- // schedule; a kept branch merely has no record: not trusted).
1099- del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
1100- if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
1101- log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
1102- }
1103- switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
1104- case err != nil:
1105- log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1106- Msg("source branch kept after the merge")
1107- default:
1108- updates = append(updates, del)
1109- log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1110- Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1111- }
1112 }1117 }
1113 at := mr.UpdatedAt1118 at := mr.UpdatedAt
1114 m.bus.Publish(ctx, domain.MergeRequestMerged{MergeRequest: mr, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})1119 m.bus.Publish(ctx, domain.MergeRequestMerged{MergeRequest: mr, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
@@ -1129,6 +1134,37 @@ func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo
1129 }1134 }
1130 }1135 }
1131 1136
1137+// deleteSource deletes the source branch of a merged request if it is still
1138+// head and still unused, and reports whether it did. The eligibility is checked
1139+// again and the branch deleted in one critical section of the merge lock: a
1140+// request opened on the branch since the merge, or a push to it, keeps it. The
1141+// record of who pushed the branch goes only after a deletion, and only if it
1142+// still names head.
1143+func (m *MergeRequests) deleteSource(ctx context.Context, p domain.Principal, name domain.RepoName, mr domain.MergeRequest, head domain.ObjectID) bool {
1144+ log := zerowrap.FromCtx(ctx).With().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).Logger()
1145+ unlock, err := m.lockRepo(ctx, name)
1146+ if err != nil {
1147+ log.Warn().Err(err).Msg("source branch kept after the merge")
1148+ return false
1149+ }
1150+ defer unlock()
1151+ if err := m.checkDeletable(ctx, name, mr); err != nil {
1152+ log.Warn().Err(err).Msg("source branch kept after the merge")
1153+ return false
1154+ }
1155+ if err := m.git.DeleteBranch(ctx, name, mr.Source, head); err != nil {
1156+ log.Warn().Err(err).Msg("source branch kept after the merge")
1157+ return false
1158+ }
1159+ if m.tips != nil {
1160+ if err := m.tips.DeleteRefTipIf(ctx, name, "refs/heads/"+mr.Source, head.String()); err != nil && !errors.Is(err, domain.ErrNotFound) {
1161+ log.Warn().Err(err).Msg("the record of the source branch was kept")
1162+ }
1163+ }
1164+ log.Info().Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1165+ return true
1166+}
1167+
1132 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a1168 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a
1133 // close or a merge of the request cannot complete in the middle of it, and not1169 // close or a merge of the request cannot complete in the middle of it, and not
1134 // at all when the request is no longer open.1170 // at all when the request is no longer open.
M internal/usecase/mergerequests_branches_test.go +1 -0
@@ -67,6 +67,7 @@ func TestMergeWithBranchLimitedToken(t *testing.T) {
6767 res, err := r.mrs.Merge(r.ctx, branchMerger, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
6868 require.NoError(t, err)
6969 assert.True(t, res.SourceDeleted)
70+ r.mrs.Wait(r.ctx) // the deletion runs after Merge returned
7071 assert.Equal(t, head, r.branchHead("agent/claude/base"))
7172 _, err = r.git.BranchHead(r.ctx, r.repo, "agent/claude/feat")
7273 assert.ErrorIs(t, err, domain.ErrNotFound)
@@ -67,6 +67,7 @@ func TestMergeWithBranchLimitedToken(t *testing.T) {
67 res, err := r.mrs.Merge(r.ctx, branchMerger, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})67 res, err := r.mrs.Merge(r.ctx, branchMerger, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
68 require.NoError(t, err)68 require.NoError(t, err)
69 assert.True(t, res.SourceDeleted)69 assert.True(t, res.SourceDeleted)
70+ r.mrs.Wait(r.ctx) // the deletion runs after Merge returned
70 assert.Equal(t, head, r.branchHead("agent/claude/base"))71 assert.Equal(t, head, r.branchHead("agent/claude/base"))
71 _, err = r.git.BranchHead(r.ctx, r.repo, "agent/claude/feat")72 _, err = r.git.BranchHead(r.ctx, r.repo, "agent/claude/feat")
72 assert.ErrorIs(t, err, domain.ErrNotFound)73 assert.ErrorIs(t, err, domain.ErrNotFound)
M internal/usecase/mergerequests_delete_test.go +146 -0
@@ -3,8 +3,13 @@
33 package usecase_test
44
55 import (
6+ "bytes"
67 "context"
8+ "sync"
79 "testing"
10+ "time"
11+
12+ "github.com/bnema/zerowrap"
813
914 "github.com/stretchr/testify/assert"
1015 "github.com/stretchr/testify/require"
@@ -158,6 +163,7 @@ func TestCreateAfterAMergeDeletedTheBranch(t *testing.T) {
158163 mr := r.open("feat")
159164 _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
160165 require.NoError(t, err)
166+ r.mrs.Wait(r.ctx) // the deletion runs after Merge returned
161167 _, err = r.mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "late", Source: "feat"})
162168 require.ErrorIs(t, err, domain.ErrInvalid)
163169 assert.Contains(t, fieldsOf(t, err), "source_branch")
@@ -202,3 +208,143 @@ func TestMergeKeepsASourceThatMovedAfterTheMerge(t *testing.T) {
202208 require.Len(t, evs, 2)
203209 assert.Len(t, evs[1].(domain.PushEvent).Updates, 1, "only the target moved")
204210 }
211+
212+// afterMergeRig builds merge request use cases over the rig with ref-tip
213+// records, whose post-merge goroutine waits for the returned release function.
214+func (r *mrRig) afterMergeRig(extra func(*usecase.MergeRequestsConfig)) (mrs *usecase.MergeRequests, release func()) {
215+ gate := make(chan struct{})
216+ cfg := usecase.MergeRequestsConfig{
217+ Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Signer: nil,
218+ Committer: domain.Identity{Name: "ermite", Email: "ermite@git.example.org"},
219+ Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, RefTips: r.store.CI(),
220+ }
221+ if extra != nil {
222+ extra(&cfg)
223+ }
224+ mrs = usecase.NewMergeRequests(cfg)
225+ mrs.SetAfterMergeGate(func() { <-gate })
226+ var once sync.Once
227+ return mrs, func() { once.Do(func() { close(gate) }) }
228+}
229+
230+// A request opened on the source after the merge, before the deletion runs,
231+// keeps the branch (GO-002).
232+func TestMergeKeepsTheSourceWhenAnotherRequestOpensOnItMeanwhile(t *testing.T) {
233+ t.Parallel()
234+ r := newMRRig(t)
235+ r.seed()
236+ head := r.feature("feat")
237+ mr := r.open("feat")
238+ r.sh("checkout", "-q", "-B", "release", "main")
239+ r.push("release")
240+ mrs, release := r.afterMergeRig(nil)
241+ defer release()
242+
243+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
244+ require.NoError(t, err)
245+ _, err = mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "to release", Source: "feat", Target: "release"})
246+ require.NoError(t, err, "the merge released the lock; the deletion has not run")
247+ release()
248+ mrs.Wait(r.ctx)
249+ assert.Equal(t, head, r.branchHead("feat"), "the branch is kept")
250+}
251+
252+// A push that moves the source after the merge keeps both the branch and the
253+// record of who pushed it (GO-003).
254+func TestMergeKeepsTheSourceAndItsRecordWhenAPushMovesItMeanwhile(t *testing.T) {
255+ t.Parallel()
256+ r := newMRRig(t)
257+ r.seed()
258+ head := r.feature("feat")
259+ mr := r.open("feat")
260+ mrs, release := r.afterMergeRig(nil)
261+ defer release()
262+
263+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
264+ require.NoError(t, err)
265+ r.sh("checkout", "-q", "feat")
266+ moved := r.commit(author, "later.txt", "later\n", "pushed meanwhile")
267+ r.push("feat")
268+ require.NoError(t, r.store.CI().PutRefTip(r.ctx, domain.CIRefTip{Repo: r.repo, Ref: "refs/heads/feat", Sha: moved.String(), Actor: "ada"}))
269+ release()
270+ mrs.Wait(r.ctx)
271+
272+ assert.Equal(t, moved, r.branchHead("feat"), "the branch is kept")
273+ tip, err := r.store.CI().GetRefTip(r.ctx, r.repo, "refs/heads/feat")
274+ require.NoError(t, err, "the record of the new push is kept")
275+ assert.Equal(t, moved.String(), tip.Sha)
276+}
277+
278+// The record of the source goes with the branch, once it is deleted.
279+func TestMergeRemovesTheSourceRecordAfterDeletingIt(t *testing.T) {
280+ t.Parallel()
281+ r := newMRRig(t)
282+ r.seed()
283+ head := r.feature("feat")
284+ mr := r.open("feat")
285+ mrs, release := r.afterMergeRig(nil)
286+ release()
287+ require.NoError(t, r.store.CI().PutRefTip(r.ctx, domain.CIRefTip{Repo: r.repo, Ref: "refs/heads/feat", Sha: head.String(), Actor: "ada"}))
288+
289+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
290+ require.NoError(t, err)
291+ mrs.Wait(r.ctx)
292+ _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
293+ require.ErrorIs(t, err, domain.ErrNotFound)
294+ _, err = r.store.CI().GetRefTip(r.ctx, r.repo, "refs/heads/feat")
295+ assert.ErrorIs(t, err, domain.ErrNotFound)
296+}
297+
298+// panicHooks is a CI trigger whose merge hook panics.
299+type panicHooks struct{}
300+
301+func (panicHooks) OnPush(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {}
302+func (panicHooks) OnMerge(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {
303+ panic("boom from ci")
304+}
305+
306+func TestMergeLogsAPanickingSideEffectAndWaitReturns(t *testing.T) {
307+ t.Parallel()
308+ r := newMRRig(t)
309+ r.seed()
310+ head := r.feature("feat")
311+ mr := r.open("feat")
312+ mrs, release := r.afterMergeRig(func(c *usecase.MergeRequestsConfig) { c.CI = panicHooks{} })
313+ release()
314+ var logs bytes.Buffer
315+ ctx := zerowrap.WithCtx(r.ctx, zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: &logs}))
316+
317+ _, err := mrs.Merge(ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
318+ require.NoError(t, err)
319+ waited := make(chan struct{})
320+ go func() { mrs.Wait(context.Background()); close(waited) }()
321+ select {
322+ case <-waited:
323+ case <-time.After(10 * time.Second):
324+ t.Fatal("Wait did not return after a panicking side effect")
325+ }
326+ assert.Contains(t, logs.String(), "post-merge side effects panicked")
327+ assert.Contains(t, logs.String(), "boom from ci")
328+}
329+
330+// A head behind the target has nothing to merge, whatever the mode (GO-007).
331+func TestMergeRefusesAHeadBehindTheTarget(t *testing.T) {
332+ t.Parallel()
333+ for _, mode := range []domain.MergeMode{domain.MergeModeSquash, domain.MergeModeMerge, domain.MergeModeFastForward} {
334+ t.Run(string(mode), func(t *testing.T) {
335+ t.Parallel()
336+ r := newMRRig(t)
337+ main := r.seed()
338+ r.sh("checkout", "-q", "-B", "old", "main~1")
339+ r.push("old")
340+ old := r.branchHead("old")
341+ require.NotEqual(t, main, old)
342+ mr := r.open("old")
343+
344+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{Mode: mode, HeadID: old})
345+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
346+ assert.Contains(t, err.Error(), "nothing to merge")
347+ assert.Equal(t, main, r.branchHead("main"), "no commit was made")
348+ })
349+ }
350+}
@@ -3,8 +3,13 @@
3 package usecase_test3 package usecase_test
4 4
5 import (5 import (
6+ "bytes"
6 "context"7 "context"
8+ "sync"
7 "testing"9 "testing"
10+ "time"
11+
12+ "github.com/bnema/zerowrap"
8 13
9 "github.com/stretchr/testify/assert"14 "github.com/stretchr/testify/assert"
10 "github.com/stretchr/testify/require"15 "github.com/stretchr/testify/require"
@@ -158,6 +163,7 @@ func TestCreateAfterAMergeDeletedTheBranch(t *testing.T) {
158 mr := r.open("feat")163 mr := r.open("feat")
159 _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})164 _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
160 require.NoError(t, err)165 require.NoError(t, err)
166+ r.mrs.Wait(r.ctx) // the deletion runs after Merge returned
161 _, err = r.mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "late", Source: "feat"})167 _, err = r.mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "late", Source: "feat"})
162 require.ErrorIs(t, err, domain.ErrInvalid)168 require.ErrorIs(t, err, domain.ErrInvalid)
163 assert.Contains(t, fieldsOf(t, err), "source_branch")169 assert.Contains(t, fieldsOf(t, err), "source_branch")
@@ -202,3 +208,143 @@ func TestMergeKeepsASourceThatMovedAfterTheMerge(t *testing.T) {
202 require.Len(t, evs, 2)208 require.Len(t, evs, 2)
203 assert.Len(t, evs[1].(domain.PushEvent).Updates, 1, "only the target moved")209 assert.Len(t, evs[1].(domain.PushEvent).Updates, 1, "only the target moved")
204 }210 }
211+
212+// afterMergeRig builds merge request use cases over the rig with ref-tip
213+// records, whose post-merge goroutine waits for the returned release function.
214+func (r *mrRig) afterMergeRig(extra func(*usecase.MergeRequestsConfig)) (mrs *usecase.MergeRequests, release func()) {
215+ gate := make(chan struct{})
216+ cfg := usecase.MergeRequestsConfig{
217+ Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Signer: nil,
218+ Committer: domain.Identity{Name: "ermite", Email: "ermite@git.example.org"},
219+ Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, RefTips: r.store.CI(),
220+ }
221+ if extra != nil {
222+ extra(&cfg)
223+ }
224+ mrs = usecase.NewMergeRequests(cfg)
225+ mrs.SetAfterMergeGate(func() { <-gate })
226+ var once sync.Once
227+ return mrs, func() { once.Do(func() { close(gate) }) }
228+}
229+
230+// A request opened on the source after the merge, before the deletion runs,
231+// keeps the branch (GO-002).
232+func TestMergeKeepsTheSourceWhenAnotherRequestOpensOnItMeanwhile(t *testing.T) {
233+ t.Parallel()
234+ r := newMRRig(t)
235+ r.seed()
236+ head := r.feature("feat")
237+ mr := r.open("feat")
238+ r.sh("checkout", "-q", "-B", "release", "main")
239+ r.push("release")
240+ mrs, release := r.afterMergeRig(nil)
241+ defer release()
242+
243+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
244+ require.NoError(t, err)
245+ _, err = mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "to release", Source: "feat", Target: "release"})
246+ require.NoError(t, err, "the merge released the lock; the deletion has not run")
247+ release()
248+ mrs.Wait(r.ctx)
249+ assert.Equal(t, head, r.branchHead("feat"), "the branch is kept")
250+}
251+
252+// A push that moves the source after the merge keeps both the branch and the
253+// record of who pushed it (GO-003).
254+func TestMergeKeepsTheSourceAndItsRecordWhenAPushMovesItMeanwhile(t *testing.T) {
255+ t.Parallel()
256+ r := newMRRig(t)
257+ r.seed()
258+ head := r.feature("feat")
259+ mr := r.open("feat")
260+ mrs, release := r.afterMergeRig(nil)
261+ defer release()
262+
263+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
264+ require.NoError(t, err)
265+ r.sh("checkout", "-q", "feat")
266+ moved := r.commit(author, "later.txt", "later\n", "pushed meanwhile")
267+ r.push("feat")
268+ require.NoError(t, r.store.CI().PutRefTip(r.ctx, domain.CIRefTip{Repo: r.repo, Ref: "refs/heads/feat", Sha: moved.String(), Actor: "ada"}))
269+ release()
270+ mrs.Wait(r.ctx)
271+
272+ assert.Equal(t, moved, r.branchHead("feat"), "the branch is kept")
273+ tip, err := r.store.CI().GetRefTip(r.ctx, r.repo, "refs/heads/feat")
274+ require.NoError(t, err, "the record of the new push is kept")
275+ assert.Equal(t, moved.String(), tip.Sha)
276+}
277+
278+// The record of the source goes with the branch, once it is deleted.
279+func TestMergeRemovesTheSourceRecordAfterDeletingIt(t *testing.T) {
280+ t.Parallel()
281+ r := newMRRig(t)
282+ r.seed()
283+ head := r.feature("feat")
284+ mr := r.open("feat")
285+ mrs, release := r.afterMergeRig(nil)
286+ release()
287+ require.NoError(t, r.store.CI().PutRefTip(r.ctx, domain.CIRefTip{Repo: r.repo, Ref: "refs/heads/feat", Sha: head.String(), Actor: "ada"}))
288+
289+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
290+ require.NoError(t, err)
291+ mrs.Wait(r.ctx)
292+ _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
293+ require.ErrorIs(t, err, domain.ErrNotFound)
294+ _, err = r.store.CI().GetRefTip(r.ctx, r.repo, "refs/heads/feat")
295+ assert.ErrorIs(t, err, domain.ErrNotFound)
296+}
297+
298+// panicHooks is a CI trigger whose merge hook panics.
299+type panicHooks struct{}
300+
301+func (panicHooks) OnPush(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {}
302+func (panicHooks) OnMerge(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {
303+ panic("boom from ci")
304+}
305+
306+func TestMergeLogsAPanickingSideEffectAndWaitReturns(t *testing.T) {
307+ t.Parallel()
308+ r := newMRRig(t)
309+ r.seed()
310+ head := r.feature("feat")
311+ mr := r.open("feat")
312+ mrs, release := r.afterMergeRig(func(c *usecase.MergeRequestsConfig) { c.CI = panicHooks{} })
313+ release()
314+ var logs bytes.Buffer
315+ ctx := zerowrap.WithCtx(r.ctx, zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: &logs}))
316+
317+ _, err := mrs.Merge(ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
318+ require.NoError(t, err)
319+ waited := make(chan struct{})
320+ go func() { mrs.Wait(context.Background()); close(waited) }()
321+ select {
322+ case <-waited:
323+ case <-time.After(10 * time.Second):
324+ t.Fatal("Wait did not return after a panicking side effect")
325+ }
326+ assert.Contains(t, logs.String(), "post-merge side effects panicked")
327+ assert.Contains(t, logs.String(), "boom from ci")
328+}
329+
330+// A head behind the target has nothing to merge, whatever the mode (GO-007).
331+func TestMergeRefusesAHeadBehindTheTarget(t *testing.T) {
332+ t.Parallel()
333+ for _, mode := range []domain.MergeMode{domain.MergeModeSquash, domain.MergeModeMerge, domain.MergeModeFastForward} {
334+ t.Run(string(mode), func(t *testing.T) {
335+ t.Parallel()
336+ r := newMRRig(t)
337+ main := r.seed()
338+ r.sh("checkout", "-q", "-B", "old", "main~1")
339+ r.push("old")
340+ old := r.branchHead("old")
341+ require.NotEqual(t, main, old)
342+ mr := r.open("old")
343+
344+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{Mode: mode, HeadID: old})
345+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
346+ assert.Contains(t, err.Error(), "nothing to merge")
347+ assert.Equal(t, main, r.branchHead("main"), "no commit was made")
348+ })
349+ }
350+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 4

Tags 45

↑↓ moveenter openesc close