detached 8539d6f

fix(repo): serialize merge settings updates with merges

commit
8539d6f6ae842df4ebbfa6e5cfa108fb2e4f625e
author
brice · · 2026-10-20 16:31 UTC
parent
63e55ec
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
7 files · +175 -2
Unified
UnifiedSplit
M internal/app/forge.go +1 -1
@@ -375,7 +375,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e
375375 })
376376 f.MergeRequests.WithAutoMerge(f.AutoMerge)
377377 f.CI.SetAutoMergeTrigger(f.AutoMerge)
378- f.Repos.WithAutoMerge(f.AutoMerge)
378+ f.Repos.WithAutoMerge(f.AutoMerge).WithMergeGate(f.MergeRequests)
379379 f.Mergeability.WithAutoMerge(f.AutoMerge)
380380 f.Mergeability.Start()
381381 f.MergeRequests.WithVerifier(f.Signatures).WithMirror(f.Mirrors)
@@ -375,7 +375,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e
375 })375 })
376 f.MergeRequests.WithAutoMerge(f.AutoMerge)376 f.MergeRequests.WithAutoMerge(f.AutoMerge)
377 f.CI.SetAutoMergeTrigger(f.AutoMerge)377 f.CI.SetAutoMergeTrigger(f.AutoMerge)
378- f.Repos.WithAutoMerge(f.AutoMerge)378+ f.Repos.WithAutoMerge(f.AutoMerge).WithMergeGate(f.MergeRequests)
379 f.Mergeability.WithAutoMerge(f.AutoMerge)379 f.Mergeability.WithAutoMerge(f.AutoMerge)
380 f.Mergeability.Start()380 f.Mergeability.Start()
381 f.MergeRequests.WithVerifier(f.Signatures).WithMirror(f.Mirrors)381 f.MergeRequests.WithVerifier(f.Signatures).WithMirror(f.Mirrors)
M internal/ports/ci.go +12 -0
@@ -367,6 +367,18 @@ type PullGate interface {
367367 WhileOpen(ctx context.Context, repo domain.RepoName, number int64, fn func() error) error
368368 }
369369
370+// MergeGate serializes a change with the merges of a repository.
371+// *usecase.MergeRequests implements it.
372+// Lock order is merge lock → store (meta.Mutate / kv). Nothing holding a
373+// store transaction or a CI lock may take the merge lock. MergeRequests never
374+// calls Repos.Update.
375+type MergeGate interface {
376+ // WithMergeLock runs fn holding the repository merge lock (also taken by
377+ // Merge, Close and auto-merge). fn must be short and must not call the
378+ // merge request use cases.
379+ WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error
380+}
381+
370382 // PullGit is what the refs/pull/<n>/{head,merge} of the merge requests need
371383 // from Git. Every method returns domain.ErrNotFound when the repository has no
372384 // directory.
@@ -367,6 +367,18 @@ type PullGate interface {
367 WhileOpen(ctx context.Context, repo domain.RepoName, number int64, fn func() error) error367 WhileOpen(ctx context.Context, repo domain.RepoName, number int64, fn func() error) error
368 }368 }
369 369
370+// MergeGate serializes a change with the merges of a repository.
371+// *usecase.MergeRequests implements it.
372+// Lock order is merge lock → store (meta.Mutate / kv). Nothing holding a
373+// store transaction or a CI lock may take the merge lock. MergeRequests never
374+// calls Repos.Update.
375+type MergeGate interface {
376+ // WithMergeLock runs fn holding the repository merge lock (also taken by
377+ // Merge, Close and auto-merge). fn must be short and must not call the
378+ // merge request use cases.
379+ WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error
380+}
381+
370 // PullGit is what the refs/pull/<n>/{head,merge} of the merge requests need382 // PullGit is what the refs/pull/<n>/{head,merge} of the merge requests need
371 // from Git. Every method returns domain.ErrNotFound when the repository has no383 // from Git. Every method returns domain.ErrNotFound when the repository has no
372 // directory.384 // directory.
A internal/ports/mocks/MergeGate.go +111 -0
@@ -0,0 +1,111 @@
1+// Code generated by mockery; DO NOT EDIT.
2+// github.com/vektra/mockery
3+// template: testify
4+
5+package mocks
6+
7+import (
8+ "context"
9+
10+ "bnema.dev/ermite/internal/domain"
11+ mock "github.com/stretchr/testify/mock"
12+)
13+
14+// NewMergeGate creates a new instance of MergeGate. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
15+// The first argument is typically a *testing.T value.
16+func NewMergeGate(t interface {
17+ mock.TestingT
18+ Cleanup(func())
19+}) *MergeGate {
20+ if helper, ok := t.(interface{ Helper() }); ok {
21+ helper.Helper()
22+ }
23+
24+ mock := &MergeGate{}
25+ mock.Mock.Test(t)
26+
27+ t.Cleanup(func() {
28+ if helper, ok := t.(interface{ Helper() }); ok {
29+ helper.Helper()
30+ }
31+ mock.AssertExpectations(t)
32+ })
33+
34+ return mock
35+}
36+
37+// MergeGate is an autogenerated mock type for the MergeGate type
38+type MergeGate struct {
39+ mock.Mock
40+}
41+
42+type MergeGate_Expecter struct {
43+ mock *mock.Mock
44+}
45+
46+func (_m *MergeGate) EXPECT() *MergeGate_Expecter {
47+ return &MergeGate_Expecter{mock: &_m.Mock}
48+}
49+
50+// WithMergeLock provides a mock function for the type MergeGate
51+func (_mock *MergeGate) WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error {
52+ ret := _mock.Called(ctx, repo, fn)
53+
54+ if len(ret) == 0 {
55+ panic("no return value specified for WithMergeLock")
56+ }
57+
58+ var r0 error
59+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, func() error) error); ok {
60+ r0 = returnFunc(ctx, repo, fn)
61+ } else {
62+ r0 = ret.Error(0)
63+ }
64+ return r0
65+}
66+
67+// MergeGate_WithMergeLock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WithMergeLock'
68+type MergeGate_WithMergeLock_Call struct {
69+ *mock.Call
70+}
71+
72+// WithMergeLock is a helper method to define mock.On call
73+// - ctx context.Context
74+// - repo domain.RepoName
75+// - fn func() error
76+func (_e *MergeGate_Expecter) WithMergeLock(ctx any, repo any, fn any) *MergeGate_WithMergeLock_Call {
77+ return &MergeGate_WithMergeLock_Call{Call: _e.mock.On("WithMergeLock", ctx, repo, fn)}
78+}
79+
80+func (_c *MergeGate_WithMergeLock_Call) Run(run func(ctx context.Context, repo domain.RepoName, fn func() error)) *MergeGate_WithMergeLock_Call {
81+ _c.Call.Run(func(args mock.Arguments) {
82+ var arg0 context.Context
83+ if args[0] != nil {
84+ arg0 = args[0].(context.Context)
85+ }
86+ var arg1 domain.RepoName
87+ if args[1] != nil {
88+ arg1 = args[1].(domain.RepoName)
89+ }
90+ var arg2 func() error
91+ if args[2] != nil {
92+ arg2 = args[2].(func() error)
93+ }
94+ run(
95+ arg0,
96+ arg1,
97+ arg2,
98+ )
99+ })
100+ return _c
101+}
102+
103+func (_c *MergeGate_WithMergeLock_Call) Return(err error) *MergeGate_WithMergeLock_Call {
104+ _c.Call.Return(err)
105+ return _c
106+}
107+
108+func (_c *MergeGate_WithMergeLock_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, fn func() error) error) *MergeGate_WithMergeLock_Call {
109+ _c.Call.Return(run)
110+ return _c
111+}
@@ -0,0 +1,111 @@
1+// Code generated by mockery; DO NOT EDIT.
2+// github.com/vektra/mockery
3+// template: testify
4+
5+package mocks
6+
7+import (
8+ "context"
9+
10+ "bnema.dev/ermite/internal/domain"
11+ mock "github.com/stretchr/testify/mock"
12+)
13+
14+// NewMergeGate creates a new instance of MergeGate. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
15+// The first argument is typically a *testing.T value.
16+func NewMergeGate(t interface {
17+ mock.TestingT
18+ Cleanup(func())
19+}) *MergeGate {
20+ if helper, ok := t.(interface{ Helper() }); ok {
21+ helper.Helper()
22+ }
23+
24+ mock := &MergeGate{}
25+ mock.Mock.Test(t)
26+
27+ t.Cleanup(func() {
28+ if helper, ok := t.(interface{ Helper() }); ok {
29+ helper.Helper()
30+ }
31+ mock.AssertExpectations(t)
32+ })
33+
34+ return mock
35+}
36+
37+// MergeGate is an autogenerated mock type for the MergeGate type
38+type MergeGate struct {
39+ mock.Mock
40+}
41+
42+type MergeGate_Expecter struct {
43+ mock *mock.Mock
44+}
45+
46+func (_m *MergeGate) EXPECT() *MergeGate_Expecter {
47+ return &MergeGate_Expecter{mock: &_m.Mock}
48+}
49+
50+// WithMergeLock provides a mock function for the type MergeGate
51+func (_mock *MergeGate) WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error {
52+ ret := _mock.Called(ctx, repo, fn)
53+
54+ if len(ret) == 0 {
55+ panic("no return value specified for WithMergeLock")
56+ }
57+
58+ var r0 error
59+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, func() error) error); ok {
60+ r0 = returnFunc(ctx, repo, fn)
61+ } else {
62+ r0 = ret.Error(0)
63+ }
64+ return r0
65+}
66+
67+// MergeGate_WithMergeLock_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WithMergeLock'
68+type MergeGate_WithMergeLock_Call struct {
69+ *mock.Call
70+}
71+
72+// WithMergeLock is a helper method to define mock.On call
73+// - ctx context.Context
74+// - repo domain.RepoName
75+// - fn func() error
76+func (_e *MergeGate_Expecter) WithMergeLock(ctx any, repo any, fn any) *MergeGate_WithMergeLock_Call {
77+ return &MergeGate_WithMergeLock_Call{Call: _e.mock.On("WithMergeLock", ctx, repo, fn)}
78+}
79+
80+func (_c *MergeGate_WithMergeLock_Call) Run(run func(ctx context.Context, repo domain.RepoName, fn func() error)) *MergeGate_WithMergeLock_Call {
81+ _c.Call.Run(func(args mock.Arguments) {
82+ var arg0 context.Context
83+ if args[0] != nil {
84+ arg0 = args[0].(context.Context)
85+ }
86+ var arg1 domain.RepoName
87+ if args[1] != nil {
88+ arg1 = args[1].(domain.RepoName)
89+ }
90+ var arg2 func() error
91+ if args[2] != nil {
92+ arg2 = args[2].(func() error)
93+ }
94+ run(
95+ arg0,
96+ arg1,
97+ arg2,
98+ )
99+ })
100+ return _c
101+}
102+
103+func (_c *MergeGate_WithMergeLock_Call) Return(err error) *MergeGate_WithMergeLock_Call {
104+ _c.Call.Return(err)
105+ return _c
106+}
107+
108+func (_c *MergeGate_WithMergeLock_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, fn func() error) error) *MergeGate_WithMergeLock_Call {
109+ _c.Call.Return(run)
110+ return _c
111+}
M internal/usecase/automerge_test.go +24 -0
@@ -372,3 +372,27 @@ func TestEnableAutoMergeNeedsTheHeadTheCallerSaw(t *testing.T) {
372372 assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
373373 })
374374 }
375+
376+func TestMergeGateSettingsUpdateWaits(t *testing.T) {
377+ for _, params := range []ports.UpdateRepoParams{
378+ {AllowAutoMerge: ptr(false)},
379+ {AllowedMergeModes: ptr([]domain.MergeMode{domain.MergeModeSquash})},
380+ {FailingChecksBlock: ptr(false)},
381+ } {
382+ r, _, _ := autoRig(t)
383+ r.repos.WithMergeGate(r.mrs)
384+ entered, release, done := make(chan struct{}), make(chan struct{}), make(chan error, 1)
385+ go func() {
386+ done <- r.mrs.WithMergeLock(r.ctx, r.repo, func() error { close(entered); <-release; return nil })
387+ }()
388+ <-entered
389+ ctx, cancel := context.WithTimeout(r.ctx, 20*time.Millisecond)
390+ _, err := r.repos.Update(ctx, root, r.repo, params)
391+ cancel()
392+ require.ErrorIs(t, err, context.DeadlineExceeded)
393+ close(release)
394+ require.NoError(t, <-done)
395+ _, err = r.repos.Update(r.ctx, root, r.repo, params)
396+ require.NoError(t, err)
397+ }
398+}
@@ -372,3 +372,27 @@ func TestEnableAutoMergeNeedsTheHeadTheCallerSaw(t *testing.T) {
372 assert.Nil(t, storedAuto(t, r, mr).AutoMerge)372 assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
373 })373 })
374 }374 }
375+
376+func TestMergeGateSettingsUpdateWaits(t *testing.T) {
377+ for _, params := range []ports.UpdateRepoParams{
378+ {AllowAutoMerge: ptr(false)},
379+ {AllowedMergeModes: ptr([]domain.MergeMode{domain.MergeModeSquash})},
380+ {FailingChecksBlock: ptr(false)},
381+ } {
382+ r, _, _ := autoRig(t)
383+ r.repos.WithMergeGate(r.mrs)
384+ entered, release, done := make(chan struct{}), make(chan struct{}), make(chan error, 1)
385+ go func() {
386+ done <- r.mrs.WithMergeLock(r.ctx, r.repo, func() error { close(entered); <-release; return nil })
387+ }()
388+ <-entered
389+ ctx, cancel := context.WithTimeout(r.ctx, 20*time.Millisecond)
390+ _, err := r.repos.Update(ctx, root, r.repo, params)
391+ cancel()
392+ require.ErrorIs(t, err, context.DeadlineExceeded)
393+ close(release)
394+ require.NoError(t, <-done)
395+ _, err = r.repos.Update(r.ctx, root, r.repo, params)
396+ require.NoError(t, err)
397+ }
398+}
M internal/usecase/mergerequests.go +13 -0
@@ -149,6 +149,9 @@ func (m *MergeRequests) Wait(ctx context.Context) {
149149 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The
150150 // merges of one repository, and the creation of its merge requests, run one at
151151 // a time. The returned function releases the lock.
152+// Lock order is merge lock → store (meta.Mutate / kv). Nothing holding a
153+// store transaction or a CI lock may take this lock. MergeRequests never
154+// calls Repos.Update.
152155 func (m *MergeRequests) lockRepo(ctx context.Context, name domain.RepoName) (unlock func(), err error) {
153156 m.mu.Lock()
154157 ch, ok := m.locks[name]
@@ -1556,3 +1559,13 @@ func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, rep
15561559 }
15571560 return ports.PushSummary{Count: len(commits), More: more}, nil
15581561 }
1562+
1563+// WithMergeLock serializes fn with repository merges and closes.
1564+func (m *MergeRequests) WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error {
1565+ unlock, err := m.lockRepo(ctx, repo)
1566+ if err != nil {
1567+ return err
1568+ }
1569+ defer unlock()
1570+ return fn()
1571+}
@@ -149,6 +149,9 @@ func (m *MergeRequests) Wait(ctx context.Context) {
149 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The149 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The
150 // merges of one repository, and the creation of its merge requests, run one at150 // merges of one repository, and the creation of its merge requests, run one at
151 // a time. The returned function releases the lock.151 // a time. The returned function releases the lock.
152+// Lock order is merge lock → store (meta.Mutate / kv). Nothing holding a
153+// store transaction or a CI lock may take this lock. MergeRequests never
154+// calls Repos.Update.
152 func (m *MergeRequests) lockRepo(ctx context.Context, name domain.RepoName) (unlock func(), err error) {155 func (m *MergeRequests) lockRepo(ctx context.Context, name domain.RepoName) (unlock func(), err error) {
153 m.mu.Lock()156 m.mu.Lock()
154 ch, ok := m.locks[name]157 ch, ok := m.locks[name]
@@ -1556,3 +1559,13 @@ func (m *MergeRequests) PushSummary(ctx context.Context, p domain.Principal, rep
1556 }1559 }
1557 return ports.PushSummary{Count: len(commits), More: more}, nil1560 return ports.PushSummary{Count: len(commits), More: more}, nil
1558 }1561 }
1562+
1563+// WithMergeLock serializes fn with repository merges and closes.
1564+func (m *MergeRequests) WithMergeLock(ctx context.Context, repo domain.RepoName, fn func() error) error {
1565+ unlock, err := m.lockRepo(ctx, repo)
1566+ if err != nil {
1567+ return err
1568+ }
1569+ defer unlock()
1570+ return fn()
1571+}
M internal/usecase/repos.go +4 -0
@@ -23,6 +23,7 @@ type Repos struct {
2323 clock ports.Clock
2424 ci ports.CIRunCleaner // nil: no CI logs to remove
2525 autoMerge ports.AutoMergeTrigger
26+ mergeGate ports.MergeGate
2627 mirror ports.MirrorTrigger // nil: no outbound mirror
2728 }
2829
@@ -237,3 +238,6 @@ func (r *Repos) Reconcile(ctx context.Context) (ReconcileReport, error) {
237238
238239 // WithAutoMerge installs the settings-change trigger before serving requests.
239240 func (r *Repos) WithAutoMerge(t ports.AutoMergeTrigger) *Repos { r.autoMerge = t; return r }
241+
242+// WithMergeGate installs the settings-write gate before serving requests.
243+func (r *Repos) WithMergeGate(g ports.MergeGate) *Repos { r.mergeGate = g; return r }
@@ -23,6 +23,7 @@ type Repos struct {
23 clock ports.Clock23 clock ports.Clock
24 ci ports.CIRunCleaner // nil: no CI logs to remove24 ci ports.CIRunCleaner // nil: no CI logs to remove
25 autoMerge ports.AutoMergeTrigger25 autoMerge ports.AutoMergeTrigger
26+ mergeGate ports.MergeGate
26 mirror ports.MirrorTrigger // nil: no outbound mirror27 mirror ports.MirrorTrigger // nil: no outbound mirror
27 }28 }
28 29
@@ -237,3 +238,6 @@ func (r *Repos) Reconcile(ctx context.Context) (ReconcileReport, error) {
237 238
238 // WithAutoMerge installs the settings-change trigger before serving requests.239 // WithAutoMerge installs the settings-change trigger before serving requests.
239 func (r *Repos) WithAutoMerge(t ports.AutoMergeTrigger) *Repos { r.autoMerge = t; return r }240 func (r *Repos) WithAutoMerge(t ports.AutoMergeTrigger) *Repos { r.autoMerge = t; return r }
241+
242+// WithMergeGate installs the settings-write gate before serving requests.
243+func (r *Repos) WithMergeGate(g ports.MergeGate) *Repos { r.mergeGate = g; return r }
M internal/usecase/repos_write.go +10 -1
@@ -117,7 +117,16 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo
117117 // repository or an I/O failure.
118118 metaDone := false
119119 if params.Description != nil || params.Topics != nil || params.Visibility != nil || params.AllowedMergeModes != nil || params.FailingChecksBlock != nil || params.AllowAutoMerge != nil || params.Issues != nil {
120- if repo, err = r.updateMeta(ctx, name, params); err != nil {
120+ update := func() error {
121+ repo, err = r.updateMeta(ctx, name, params)
122+ return err
123+ }
124+ if r.mergeGate != nil && (params.AllowedMergeModes != nil || params.FailingChecksBlock != nil || params.AllowAutoMerge != nil) {
125+ err = r.mergeGate.WithMergeLock(ctx, name, update)
126+ } else {
127+ err = update()
128+ }
129+ if err != nil {
121130 return domain.Repo{}, err
122131 }
123132 metaDone = true
@@ -117,7 +117,16 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo
117 // repository or an I/O failure.117 // repository or an I/O failure.
118 metaDone := false118 metaDone := false
119 if params.Description != nil || params.Topics != nil || params.Visibility != nil || params.AllowedMergeModes != nil || params.FailingChecksBlock != nil || params.AllowAutoMerge != nil || params.Issues != nil {119 if params.Description != nil || params.Topics != nil || params.Visibility != nil || params.AllowedMergeModes != nil || params.FailingChecksBlock != nil || params.AllowAutoMerge != nil || params.Issues != nil {
120- if repo, err = r.updateMeta(ctx, name, params); err != nil {120+ update := func() error {
121+ repo, err = r.updateMeta(ctx, name, params)
122+ return err
123+ }
124+ if r.mergeGate != nil && (params.AllowedMergeModes != nil || params.FailingChecksBlock != nil || params.AllowAutoMerge != nil) {
125+ err = r.mergeGate.WithMergeLock(ctx, name, update)
126+ } else {
127+ err = update()
128+ }
129+ if err != nil {
121 return domain.Repo{}, err130 return domain.Repo{}, err
122 }131 }
123 metaDone = true132 metaDone = true

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 2

Tags 45

↑↓ moveenter openesc close