detached 84db480

perf(mr): skip auto-merge scans of repositories without a pending auto-merge, read checks once under the merge lock and keep the manual pending-checks error

commit
84db48001f7d781763133b288d6a155ddc21ef02
author
brice · · 2026-10-20 17:15 UTC
parent
b6e9374
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +154 -85
M internal/app/forge.go +1 -1
@@ -371,7 +371,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e
371371 })
372372 f.AutoMerge = usecase.NewAutoMerge(ctx, usecase.AutoMergeConfig{
373373 MRs: f.MergeRequests, Store: store.MergeRequests(), Meta: store, Git: git,
374- Checks: f.CI, Activity: f.CI, Tokens: store.Tokens(), Bus: f.Bus, Clock: clock,
374+ Checks: f.CI, Activity: f.CI, Tokens: store.Tokens(), Clock: clock,
375375 })
376376 f.MergeRequests.WithAutoMerge(f.AutoMerge)
377377 f.CI.SetAutoMergeTrigger(f.AutoMerge)
@@ -371,7 +371,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e
371 })371 })
372 f.AutoMerge = usecase.NewAutoMerge(ctx, usecase.AutoMergeConfig{372 f.AutoMerge = usecase.NewAutoMerge(ctx, usecase.AutoMergeConfig{
373 MRs: f.MergeRequests, Store: store.MergeRequests(), Meta: store, Git: git,373 MRs: f.MergeRequests, Store: store.MergeRequests(), Meta: store, Git: git,
374- Checks: f.CI, Activity: f.CI, Tokens: store.Tokens(), Bus: f.Bus, Clock: clock,374+ Checks: f.CI, Activity: f.CI, Tokens: store.Tokens(), Clock: clock,
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)
M internal/ports/ci.go +1 -1
@@ -914,5 +914,5 @@ type CIJobSettings interface {
914914 type CIActivity interface {
915915 // Settled reports that no push trigger of the repository and no pull event
916916 // of the merge request is queued or being planned.
917- Settled(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool
917+ Settled(ctx context.Context, repo domain.RepoName, number int64) bool
918918 }
@@ -914,5 +914,5 @@ type CIJobSettings interface {
914 type CIActivity interface {914 type CIActivity interface {
915 // Settled reports that no push trigger of the repository and no pull event915 // Settled reports that no push trigger of the repository and no pull event
916 // of the merge request is queued or being planned.916 // of the merge request is queued or being planned.
917- Settled(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool917+ Settled(ctx context.Context, repo domain.RepoName, number int64) bool
918 }918 }
M internal/ports/mocks/AutoMergeTrigger.go +46 -0
@@ -47,6 +47,52 @@ func (_m *AutoMergeTrigger) EXPECT() *AutoMergeTrigger_Expecter {
4747 return &AutoMergeTrigger_Expecter{mock: &_m.Mock}
4848 }
4949
50+// OnAutoMergeEnabled provides a mock function for the type AutoMergeTrigger
51+func (_mock *AutoMergeTrigger) OnAutoMergeEnabled(ctx context.Context, repo domain.RepoName) {
52+ _mock.Called(ctx, repo)
53+ return
54+}
55+
56+// AutoMergeTrigger_OnAutoMergeEnabled_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OnAutoMergeEnabled'
57+type AutoMergeTrigger_OnAutoMergeEnabled_Call struct {
58+ *mock.Call
59+}
60+
61+// OnAutoMergeEnabled is a helper method to define mock.On call
62+// - ctx context.Context
63+// - repo domain.RepoName
64+func (_e *AutoMergeTrigger_Expecter) OnAutoMergeEnabled(ctx any, repo any) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
65+ return &AutoMergeTrigger_OnAutoMergeEnabled_Call{Call: _e.mock.On("OnAutoMergeEnabled", ctx, repo)}
66+}
67+
68+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) Run(run func(ctx context.Context, repo domain.RepoName)) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
69+ _c.Call.Run(func(args mock.Arguments) {
70+ var arg0 context.Context
71+ if args[0] != nil {
72+ arg0 = args[0].(context.Context)
73+ }
74+ var arg1 domain.RepoName
75+ if args[1] != nil {
76+ arg1 = args[1].(domain.RepoName)
77+ }
78+ run(
79+ arg0,
80+ arg1,
81+ )
82+ })
83+ return _c
84+}
85+
86+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) Return() *AutoMergeTrigger_OnAutoMergeEnabled_Call {
87+ _c.Call.Return()
88+ return _c
89+}
90+
91+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName)) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
92+ _c.Run(run)
93+ return _c
94+}
95+
5096 // OnRepoChanged provides a mock function for the type AutoMergeTrigger
5197 func (_mock *AutoMergeTrigger) OnRepoChanged(ctx context.Context, repo domain.RepoName) {
5298 _mock.Called(ctx, repo)
@@ -47,6 +47,52 @@ func (_m *AutoMergeTrigger) EXPECT() *AutoMergeTrigger_Expecter {
47 return &AutoMergeTrigger_Expecter{mock: &_m.Mock}47 return &AutoMergeTrigger_Expecter{mock: &_m.Mock}
48 }48 }
49 49
50+// OnAutoMergeEnabled provides a mock function for the type AutoMergeTrigger
51+func (_mock *AutoMergeTrigger) OnAutoMergeEnabled(ctx context.Context, repo domain.RepoName) {
52+ _mock.Called(ctx, repo)
53+ return
54+}
55+
56+// AutoMergeTrigger_OnAutoMergeEnabled_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OnAutoMergeEnabled'
57+type AutoMergeTrigger_OnAutoMergeEnabled_Call struct {
58+ *mock.Call
59+}
60+
61+// OnAutoMergeEnabled is a helper method to define mock.On call
62+// - ctx context.Context
63+// - repo domain.RepoName
64+func (_e *AutoMergeTrigger_Expecter) OnAutoMergeEnabled(ctx any, repo any) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
65+ return &AutoMergeTrigger_OnAutoMergeEnabled_Call{Call: _e.mock.On("OnAutoMergeEnabled", ctx, repo)}
66+}
67+
68+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) Run(run func(ctx context.Context, repo domain.RepoName)) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
69+ _c.Call.Run(func(args mock.Arguments) {
70+ var arg0 context.Context
71+ if args[0] != nil {
72+ arg0 = args[0].(context.Context)
73+ }
74+ var arg1 domain.RepoName
75+ if args[1] != nil {
76+ arg1 = args[1].(domain.RepoName)
77+ }
78+ run(
79+ arg0,
80+ arg1,
81+ )
82+ })
83+ return _c
84+}
85+
86+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) Return() *AutoMergeTrigger_OnAutoMergeEnabled_Call {
87+ _c.Call.Return()
88+ return _c
89+}
90+
91+func (_c *AutoMergeTrigger_OnAutoMergeEnabled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName)) *AutoMergeTrigger_OnAutoMergeEnabled_Call {
92+ _c.Run(run)
93+ return _c
94+}
95+
50 // OnRepoChanged provides a mock function for the type AutoMergeTrigger96 // OnRepoChanged provides a mock function for the type AutoMergeTrigger
51 func (_mock *AutoMergeTrigger) OnRepoChanged(ctx context.Context, repo domain.RepoName) {97 func (_mock *AutoMergeTrigger) OnRepoChanged(ctx context.Context, repo domain.RepoName) {
52 _mock.Called(ctx, repo)98 _mock.Called(ctx, repo)
M internal/ports/mocks/CIActivity.go +8 -20
@@ -48,16 +48,16 @@ func (_m *CIActivity) EXPECT() *CIActivity_Expecter {
4848 }
4949
5050 // Settled provides a mock function for the type CIActivity
51-func (_mock *CIActivity) Settled(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool {
52- ret := _mock.Called(ctx, repo, number, source, head)
51+func (_mock *CIActivity) Settled(ctx context.Context, repo domain.RepoName, number int64) bool {
52+ ret := _mock.Called(ctx, repo, number)
5353
5454 if len(ret) == 0 {
5555 panic("no return value specified for Settled")
5656 }
5757
5858 var r0 bool
59- if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, int64, string, domain.ObjectID) bool); ok {
60- r0 = returnFunc(ctx, repo, number, source, head)
59+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, int64) bool); ok {
60+ r0 = returnFunc(ctx, repo, number)
6161 } else {
6262 r0 = ret.Get(0).(bool)
6363 }
@@ -73,13 +73,11 @@ type CIActivity_Settled_Call struct {
7373 // - ctx context.Context
7474 // - repo domain.RepoName
7575 // - number int64
76-// - source string
77-// - head domain.ObjectID
78-func (_e *CIActivity_Expecter) Settled(ctx any, repo any, number any, source any, head any) *CIActivity_Settled_Call {
79- return &CIActivity_Settled_Call{Call: _e.mock.On("Settled", ctx, repo, number, source, head)}
76+func (_e *CIActivity_Expecter) Settled(ctx any, repo any, number any) *CIActivity_Settled_Call {
77+ return &CIActivity_Settled_Call{Call: _e.mock.On("Settled", ctx, repo, number)}
8078 }
8179
82-func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID)) *CIActivity_Settled_Call {
80+func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain.RepoName, number int64)) *CIActivity_Settled_Call {
8381 _c.Call.Run(func(args mock.Arguments) {
8482 var arg0 context.Context
8583 if args[0] != nil {
@@ -93,20 +91,10 @@ func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain
9391 if args[2] != nil {
9492 arg2 = args[2].(int64)
9593 }
96- var arg3 string
97- if args[3] != nil {
98- arg3 = args[3].(string)
99- }
100- var arg4 domain.ObjectID
101- if args[4] != nil {
102- arg4 = args[4].(domain.ObjectID)
103- }
10494 run(
10595 arg0,
10696 arg1,
10797 arg2,
108- arg3,
109- arg4,
11098 )
11199 })
112100 return _c
@@ -117,7 +105,7 @@ func (_c *CIActivity_Settled_Call) Return(b bool) *CIActivity_Settled_Call {
117105 return _c
118106 }
119107
120-func (_c *CIActivity_Settled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool) *CIActivity_Settled_Call {
108+func (_c *CIActivity_Settled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, number int64) bool) *CIActivity_Settled_Call {
121109 _c.Call.Return(run)
122110 return _c
123111 }
@@ -48,16 +48,16 @@ func (_m *CIActivity) EXPECT() *CIActivity_Expecter {
48 }48 }
49 49
50 // Settled provides a mock function for the type CIActivity50 // Settled provides a mock function for the type CIActivity
51-func (_mock *CIActivity) Settled(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool {51+func (_mock *CIActivity) Settled(ctx context.Context, repo domain.RepoName, number int64) bool {
52- ret := _mock.Called(ctx, repo, number, source, head)52+ ret := _mock.Called(ctx, repo, number)
53 53
54 if len(ret) == 0 {54 if len(ret) == 0 {
55 panic("no return value specified for Settled")55 panic("no return value specified for Settled")
56 }56 }
57 57
58 var r0 bool58 var r0 bool
59- if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, int64, string, domain.ObjectID) bool); ok {59+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.RepoName, int64) bool); ok {
60- r0 = returnFunc(ctx, repo, number, source, head)60+ r0 = returnFunc(ctx, repo, number)
61 } else {61 } else {
62 r0 = ret.Get(0).(bool)62 r0 = ret.Get(0).(bool)
63 }63 }
@@ -73,13 +73,11 @@ type CIActivity_Settled_Call struct {
73 // - ctx context.Context73 // - ctx context.Context
74 // - repo domain.RepoName74 // - repo domain.RepoName
75 // - number int6475 // - number int64
76-// - source string76+func (_e *CIActivity_Expecter) Settled(ctx any, repo any, number any) *CIActivity_Settled_Call {
77-// - head domain.ObjectID77+ return &CIActivity_Settled_Call{Call: _e.mock.On("Settled", ctx, repo, number)}
78-func (_e *CIActivity_Expecter) Settled(ctx any, repo any, number any, source any, head any) *CIActivity_Settled_Call {
79- return &CIActivity_Settled_Call{Call: _e.mock.On("Settled", ctx, repo, number, source, head)}
80 }78 }
81 79
82-func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID)) *CIActivity_Settled_Call {80+func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain.RepoName, number int64)) *CIActivity_Settled_Call {
83 _c.Call.Run(func(args mock.Arguments) {81 _c.Call.Run(func(args mock.Arguments) {
84 var arg0 context.Context82 var arg0 context.Context
85 if args[0] != nil {83 if args[0] != nil {
@@ -93,20 +91,10 @@ func (_c *CIActivity_Settled_Call) Run(run func(ctx context.Context, repo domain
93 if args[2] != nil {91 if args[2] != nil {
94 arg2 = args[2].(int64)92 arg2 = args[2].(int64)
95 }93 }
96- var arg3 string
97- if args[3] != nil {
98- arg3 = args[3].(string)
99- }
100- var arg4 domain.ObjectID
101- if args[4] != nil {
102- arg4 = args[4].(domain.ObjectID)
103- }
104 run(94 run(
105 arg0,95 arg0,
106 arg1,96 arg1,
107 arg2,97 arg2,
108- arg3,
109- arg4,
110 )98 )
111 })99 })
112 return _c100 return _c
@@ -117,7 +105,7 @@ func (_c *CIActivity_Settled_Call) Return(b bool) *CIActivity_Settled_Call {
117 return _c105 return _c
118 }106 }
119 107
120-func (_c *CIActivity_Settled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, number int64, source string, head domain.ObjectID) bool) *CIActivity_Settled_Call {108+func (_c *CIActivity_Settled_Call) RunAndReturn(run func(ctx context.Context, repo domain.RepoName, number int64) bool) *CIActivity_Settled_Call {
121 _c.Call.Return(run)109 _c.Call.Return(run)
122 return _c110 return _c
123 }111 }
M internal/ports/ports.go +1 -0
@@ -441,6 +441,7 @@ type MergeGit interface {
441441 // again. It only marks the repository and returns.
442442 type AutoMergeTrigger interface {
443443 OnRepoChanged(ctx context.Context, repo domain.RepoName)
444+ OnAutoMergeEnabled(ctx context.Context, repo domain.RepoName)
444445 }
445446
446447 // MergeabilityTrigger queues mergeability checks. It only queues the work and
@@ -441,6 +441,7 @@ type MergeGit interface {
441 // again. It only marks the repository and returns.441 // again. It only marks the repository and returns.
442 type AutoMergeTrigger interface {442 type AutoMergeTrigger interface {
443 OnRepoChanged(ctx context.Context, repo domain.RepoName)443 OnRepoChanged(ctx context.Context, repo domain.RepoName)
444+ OnAutoMergeEnabled(ctx context.Context, repo domain.RepoName)
444 }445 }
445 446
446 // MergeabilityTrigger queues mergeability checks. It only queues the work and447 // MergeabilityTrigger queues mergeability checks. It only queues the work and
M internal/usecase/automerge.go +46 -16
@@ -24,7 +24,6 @@ type AutoMergeConfig struct {
2424 Checks ports.CIChecksReader
2525 Activity ports.CIActivity
2626 Tokens ports.TokenStore
27- Bus ports.EventBus
2827 Clock ports.Clock
2928 After func(time.Duration) <-chan time.Time
3029 }
@@ -37,6 +36,7 @@ type AutoMerge struct {
3736 cancel context.CancelFunc
3837 wg sync.WaitGroup
3938 mu sync.Mutex
39+ pending map[domain.RepoName]bool
4040 dirty map[domain.RepoName]bool
4141 order []domain.RepoName
4242 running, again map[domain.RepoName]bool
@@ -58,7 +58,7 @@ func NewAutoMerge(ctx context.Context, cfg AutoMergeConfig) *AutoMerge {
5858 }
5959 ctx, cancel := context.WithCancel(context.WithoutCancel(ctx))
6060 return &AutoMerge{cfg: cfg, ctx: ctx, cancel: cancel,
61- dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1),
61+ pending: map[domain.RepoName]bool{}, dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1),
6262 retries: map[domain.RepoName]int{}, scheduled: map[domain.RepoName]bool{}, retryKick: make(chan struct{}, 1)}
6363 }
6464
@@ -76,7 +76,7 @@ func (s *AutoMerge) Start() {
7676 if err == nil {
7777 for _, repo := range repos {
7878 if repo.AutoMerge != nil {
79- s.markDirty(repo.Repo)
79+ s.OnAutoMergeEnabled(s.ctx, repo.Repo)
8080 }
8181 }
8282 return
@@ -114,6 +114,10 @@ func (s *AutoMerge) signal() {
114114
115115 func (s *AutoMerge) markDirty(repo domain.RepoName) {
116116 s.mu.Lock()
117+ if !s.pending[repo] {
118+ s.mu.Unlock()
119+ return
120+ }
117121 if s.running[repo] {
118122 s.again[repo] = true
119123 } else if !s.dirty[repo] {
@@ -172,6 +176,17 @@ func (s *AutoMerge) loop() {
172176 // OnRepoChanged marks and wakes only; it never merges on the caller's stack.
173177 func (s *AutoMerge) OnRepoChanged(_ context.Context, repo domain.RepoName) { s.markDirty(repo) }
174178
179+// OnAutoMergeEnabled remembers an intent before queuing evaluation.
180+func (s *AutoMerge) OnAutoMergeEnabled(_ context.Context, repo domain.RepoName) {
181+ s.mu.Lock()
182+ s.pending[repo] = true
183+ if s.running[repo] {
184+ s.again[repo] = true
185+ }
186+ s.mu.Unlock()
187+ s.markDirty(repo)
188+}
189+
175190 func (s *AutoMerge) evaluate(repo domain.RepoName) bool {
176191 err := s.evaluateRepo(repo)
177192 if err != nil && s.ctx.Err() == nil {
@@ -193,6 +208,16 @@ func (s *AutoMerge) evaluateRepo(repo domain.RepoName) error {
193208 if err != nil {
194209 return err
195210 }
211+ hasIntent := slices.ContainsFunc(mrs, func(mr domain.MergeRequest) bool { return mr.AutoMerge != nil })
212+ if !hasIntent {
213+ s.mu.Lock()
214+ // An enable arriving during this scan requests another pass. Do not
215+ // erase its mark based on the older store snapshot.
216+ if !s.again[repo] {
217+ delete(s.pending, repo)
218+ }
219+ s.mu.Unlock()
220+ }
196221 var failures []error
197222 for _, mr := range mrs {
198223 if ctx.Err() != nil {
@@ -231,29 +256,23 @@ func (s *AutoMerge) evaluateMR(ctx context.Context, repo domain.Repo, mr domain.
231256 // An enabler that can no longer merge cancels at once, even while CI is
232257 // pending: the intent must not stay shown as enabled. This check only
233258 // cancels; the merge itself resolves the enabler again under the merge lock.
234- if fresh, valid, err := principalOfPusher(ctx, s.cfg.Tokens, intent.EnabledBy, s.cfg.Clock.Now()); err != nil {
259+ if _, err := enablerCanMerge(ctx, s.cfg.Tokens, intent, mr, s.cfg.Clock.Now()); err != nil {
260+ if errors.Is(err, domain.ErrForbidden) {
261+ return cancel(autoMergeReason(err))
262+ }
235263 return err
236- } else if !valid || !fresh.Allows(domain.ScopeMRMerge, mr.Repo) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
237- return cancel("the principal that enabled auto-merge can no longer merge " + mr.Target)
238264 }
239- if s.cfg.Activity != nil && !s.cfg.Activity.Settled(ctx, mr.Repo, mr.Number, mr.Source, head) {
265+ if s.cfg.Activity != nil && !s.cfg.Activity.Settled(ctx, mr.Repo, mr.Number) {
240266 return nil
241267 }
242268 checks, err := s.cfg.Checks.CommitChecks(ctx, mr.Repo, head.String())
243269 if err != nil {
244270 return err
245271 }
246- passed := false
247- for _, check := range checks {
248- if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
249- passed = true
250- break
251- }
252- }
253- if !passed {
272+ if !hasSuccessfulCheck(checks) {
254273 return nil
255274 }
256- _, err = s.cfg.MRs.mergeAs(ctx, domain.Principal{}, mr.Repo, mr.Number, ports.MergeParams{Mode: intent.Mode, HeadID: intent.HeadID}, mergeOpts{auto: true, intent: intent, enabler: intent.EnabledBy, tokens: s.cfg.Tokens, activity: s.cfg.Activity})
275+ _, err = s.cfg.MRs.mergeAs(ctx, domain.Principal{}, mr.Repo, mr.Number, ports.MergeParams{Mode: intent.Mode, HeadID: intent.HeadID}, mergeOpts{auto: &autoMergeOpts{intent: intent, tokens: s.cfg.Tokens, activity: s.cfg.Activity}})
257276 switch {
258277 case err == nil, errors.Is(err, errAutoMergeWait):
259278 return nil
@@ -381,3 +400,14 @@ func (s *AutoMerge) retryDone(key domain.RepoName) {
381400 }
382401 s.retryMu.Unlock()
383402 }
403+
404+func enablerCanMerge(ctx context.Context, tokens ports.TokenStore, intent *domain.AutoMerge, mr domain.MergeRequest, now time.Time) (domain.Principal, error) {
405+ fresh, valid, err := principalOfPusher(ctx, tokens, intent.EnabledBy, now)
406+ if err != nil {
407+ return domain.Principal{}, err
408+ }
409+ if !valid || !fresh.Allows(domain.ScopeMRMerge, mr.Repo) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
410+ return domain.Principal{}, fmt.Errorf("%w: the principal that enabled auto-merge can no longer merge %s", domain.ErrForbidden, mr.Target)
411+ }
412+ return fresh, nil
413+}
@@ -24,7 +24,6 @@ type AutoMergeConfig struct {
24 Checks ports.CIChecksReader24 Checks ports.CIChecksReader
25 Activity ports.CIActivity25 Activity ports.CIActivity
26 Tokens ports.TokenStore26 Tokens ports.TokenStore
27- Bus ports.EventBus
28 Clock ports.Clock27 Clock ports.Clock
29 After func(time.Duration) <-chan time.Time28 After func(time.Duration) <-chan time.Time
30 }29 }
@@ -37,6 +36,7 @@ type AutoMerge struct {
37 cancel context.CancelFunc36 cancel context.CancelFunc
38 wg sync.WaitGroup37 wg sync.WaitGroup
39 mu sync.Mutex38 mu sync.Mutex
39+ pending map[domain.RepoName]bool
40 dirty map[domain.RepoName]bool40 dirty map[domain.RepoName]bool
41 order []domain.RepoName41 order []domain.RepoName
42 running, again map[domain.RepoName]bool42 running, again map[domain.RepoName]bool
@@ -58,7 +58,7 @@ func NewAutoMerge(ctx context.Context, cfg AutoMergeConfig) *AutoMerge {
58 }58 }
59 ctx, cancel := context.WithCancel(context.WithoutCancel(ctx))59 ctx, cancel := context.WithCancel(context.WithoutCancel(ctx))
60 return &AutoMerge{cfg: cfg, ctx: ctx, cancel: cancel,60 return &AutoMerge{cfg: cfg, ctx: ctx, cancel: cancel,
61- dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1),61+ pending: map[domain.RepoName]bool{}, dirty: map[domain.RepoName]bool{}, running: map[domain.RepoName]bool{}, again: map[domain.RepoName]bool{}, wake: make(chan struct{}, 1),
62 retries: map[domain.RepoName]int{}, scheduled: map[domain.RepoName]bool{}, retryKick: make(chan struct{}, 1)}62 retries: map[domain.RepoName]int{}, scheduled: map[domain.RepoName]bool{}, retryKick: make(chan struct{}, 1)}
63 }63 }
64 64
@@ -76,7 +76,7 @@ func (s *AutoMerge) Start() {
76 if err == nil {76 if err == nil {
77 for _, repo := range repos {77 for _, repo := range repos {
78 if repo.AutoMerge != nil {78 if repo.AutoMerge != nil {
79- s.markDirty(repo.Repo)79+ s.OnAutoMergeEnabled(s.ctx, repo.Repo)
80 }80 }
81 }81 }
82 return82 return
@@ -114,6 +114,10 @@ func (s *AutoMerge) signal() {
114 114
115 func (s *AutoMerge) markDirty(repo domain.RepoName) {115 func (s *AutoMerge) markDirty(repo domain.RepoName) {
116 s.mu.Lock()116 s.mu.Lock()
117+ if !s.pending[repo] {
118+ s.mu.Unlock()
119+ return
120+ }
117 if s.running[repo] {121 if s.running[repo] {
118 s.again[repo] = true122 s.again[repo] = true
119 } else if !s.dirty[repo] {123 } else if !s.dirty[repo] {
@@ -172,6 +176,17 @@ func (s *AutoMerge) loop() {
172 // OnRepoChanged marks and wakes only; it never merges on the caller's stack.176 // OnRepoChanged marks and wakes only; it never merges on the caller's stack.
173 func (s *AutoMerge) OnRepoChanged(_ context.Context, repo domain.RepoName) { s.markDirty(repo) }177 func (s *AutoMerge) OnRepoChanged(_ context.Context, repo domain.RepoName) { s.markDirty(repo) }
174 178
179+// OnAutoMergeEnabled remembers an intent before queuing evaluation.
180+func (s *AutoMerge) OnAutoMergeEnabled(_ context.Context, repo domain.RepoName) {
181+ s.mu.Lock()
182+ s.pending[repo] = true
183+ if s.running[repo] {
184+ s.again[repo] = true
185+ }
186+ s.mu.Unlock()
187+ s.markDirty(repo)
188+}
189+
175 func (s *AutoMerge) evaluate(repo domain.RepoName) bool {190 func (s *AutoMerge) evaluate(repo domain.RepoName) bool {
176 err := s.evaluateRepo(repo)191 err := s.evaluateRepo(repo)
177 if err != nil && s.ctx.Err() == nil {192 if err != nil && s.ctx.Err() == nil {
@@ -193,6 +208,16 @@ func (s *AutoMerge) evaluateRepo(repo domain.RepoName) error {
193 if err != nil {208 if err != nil {
194 return err209 return err
195 }210 }
211+ hasIntent := slices.ContainsFunc(mrs, func(mr domain.MergeRequest) bool { return mr.AutoMerge != nil })
212+ if !hasIntent {
213+ s.mu.Lock()
214+ // An enable arriving during this scan requests another pass. Do not
215+ // erase its mark based on the older store snapshot.
216+ if !s.again[repo] {
217+ delete(s.pending, repo)
218+ }
219+ s.mu.Unlock()
220+ }
196 var failures []error221 var failures []error
197 for _, mr := range mrs {222 for _, mr := range mrs {
198 if ctx.Err() != nil {223 if ctx.Err() != nil {
@@ -231,29 +256,23 @@ func (s *AutoMerge) evaluateMR(ctx context.Context, repo domain.Repo, mr domain.
231 // An enabler that can no longer merge cancels at once, even while CI is256 // An enabler that can no longer merge cancels at once, even while CI is
232 // pending: the intent must not stay shown as enabled. This check only257 // pending: the intent must not stay shown as enabled. This check only
233 // cancels; the merge itself resolves the enabler again under the merge lock.258 // cancels; the merge itself resolves the enabler again under the merge lock.
234- if fresh, valid, err := principalOfPusher(ctx, s.cfg.Tokens, intent.EnabledBy, s.cfg.Clock.Now()); err != nil {259+ if _, err := enablerCanMerge(ctx, s.cfg.Tokens, intent, mr, s.cfg.Clock.Now()); err != nil {
260+ if errors.Is(err, domain.ErrForbidden) {
261+ return cancel(autoMergeReason(err))
262+ }
235 return err263 return err
236- } else if !valid || !fresh.Allows(domain.ScopeMRMerge, mr.Repo) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
237- return cancel("the principal that enabled auto-merge can no longer merge " + mr.Target)
238 }264 }
239- if s.cfg.Activity != nil && !s.cfg.Activity.Settled(ctx, mr.Repo, mr.Number, mr.Source, head) {265+ if s.cfg.Activity != nil && !s.cfg.Activity.Settled(ctx, mr.Repo, mr.Number) {
240 return nil266 return nil
241 }267 }
242 checks, err := s.cfg.Checks.CommitChecks(ctx, mr.Repo, head.String())268 checks, err := s.cfg.Checks.CommitChecks(ctx, mr.Repo, head.String())
243 if err != nil {269 if err != nil {
244 return err270 return err
245 }271 }
246- passed := false272+ if !hasSuccessfulCheck(checks) {
247- for _, check := range checks {
248- if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
249- passed = true
250- break
251- }
252- }
253- if !passed {
254 return nil273 return nil
255 }274 }
256- _, err = s.cfg.MRs.mergeAs(ctx, domain.Principal{}, mr.Repo, mr.Number, ports.MergeParams{Mode: intent.Mode, HeadID: intent.HeadID}, mergeOpts{auto: true, intent: intent, enabler: intent.EnabledBy, tokens: s.cfg.Tokens, activity: s.cfg.Activity})275+ _, err = s.cfg.MRs.mergeAs(ctx, domain.Principal{}, mr.Repo, mr.Number, ports.MergeParams{Mode: intent.Mode, HeadID: intent.HeadID}, mergeOpts{auto: &autoMergeOpts{intent: intent, tokens: s.cfg.Tokens, activity: s.cfg.Activity}})
257 switch {276 switch {
258 case err == nil, errors.Is(err, errAutoMergeWait):277 case err == nil, errors.Is(err, errAutoMergeWait):
259 return nil278 return nil
@@ -381,3 +400,14 @@ func (s *AutoMerge) retryDone(key domain.RepoName) {
381 }400 }
382 s.retryMu.Unlock()401 s.retryMu.Unlock()
383 }402 }
403+
404+func enablerCanMerge(ctx context.Context, tokens ports.TokenStore, intent *domain.AutoMerge, mr domain.MergeRequest, now time.Time) (domain.Principal, error) {
405+ fresh, valid, err := principalOfPusher(ctx, tokens, intent.EnabledBy, now)
406+ if err != nil {
407+ return domain.Principal{}, err
408+ }
409+ if !valid || !fresh.Allows(domain.ScopeMRMerge, mr.Repo) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
410+ return domain.Principal{}, fmt.Errorf("%w: the principal that enabled auto-merge can no longer merge %s", domain.ErrForbidden, mr.Target)
411+ }
412+ return fresh, nil
413+}
M internal/usecase/ci.go +3 -3
@@ -774,9 +774,9 @@ func (c *CI) kickAutoMerge(ctx context.Context, repo domain.RepoName) {
774774
775775 // Settled reports whether all push planning and pull activities have finished.
776776 // Workflow-run events, schedules and dispatches are intentionally not counted.
777-// Source and head are reserved for finer-grained accounting; repo-wide pushes
778-// conservatively delay requests even when an unrelated branch is updated.
779-func (c *CI) Settled(_ context.Context, repo domain.RepoName, number int64, _ string, _ domain.ObjectID) bool {
777+// Repo-wide pushes conservatively delay requests even when an unrelated
778+// branch is updated.
779+func (c *CI) Settled(_ context.Context, repo domain.RepoName, number int64) bool {
780780 c.mu.Lock()
781781 defer c.mu.Unlock()
782782 return c.pushes[repo] == 0 && c.pulls[pullKey{repo: repo, number: number}] == 0
@@ -774,9 +774,9 @@ func (c *CI) kickAutoMerge(ctx context.Context, repo domain.RepoName) {
774 774
775 // Settled reports whether all push planning and pull activities have finished.775 // Settled reports whether all push planning and pull activities have finished.
776 // Workflow-run events, schedules and dispatches are intentionally not counted.776 // Workflow-run events, schedules and dispatches are intentionally not counted.
777-// Source and head are reserved for finer-grained accounting; repo-wide pushes777+// Repo-wide pushes conservatively delay requests even when an unrelated
778-// conservatively delay requests even when an unrelated branch is updated.778+// branch is updated.
779-func (c *CI) Settled(_ context.Context, repo domain.RepoName, number int64, _ string, _ domain.ObjectID) bool {779+func (c *CI) Settled(_ context.Context, repo domain.RepoName, number int64) bool {
780 c.mu.Lock()780 c.mu.Lock()
781 defer c.mu.Unlock()781 defer c.mu.Unlock()
782 return c.pushes[repo] == 0 && c.pulls[pullKey{repo: repo, number: number}] == 0782 return c.pushes[repo] == 0 && c.pulls[pullKey{repo: repo, number: number}] == 0
M internal/usecase/mergeability.go +5 -3
@@ -236,6 +236,7 @@ func (s *Mergeability) reconcile(repo domain.RepoName) bool {
236236 return false
237237 }
238238 ok := true
239+ changed := false
239240 for _, mr := range mrs {
240241 if ctx.Err() != nil {
241242 return true
@@ -276,9 +277,10 @@ func (s *Mergeability) reconcile(repo domain.RepoName) bool {
276277 Repo: repo, Number: mr.Number, Mergeability: res, At: res.CheckedAt,
277278 Visibility: meta.Visibility, CreatedAt: meta.CreatedAt,
278279 })
279- if s.cfg.AutoMerge != nil {
280- s.cfg.AutoMerge.OnRepoChanged(ctx, repo)
281- }
280+ changed = true
281+ }
282+ if changed && s.cfg.AutoMerge != nil {
283+ s.cfg.AutoMerge.OnRepoChanged(ctx, repo)
282284 }
283285 return ok
284286 }
@@ -236,6 +236,7 @@ func (s *Mergeability) reconcile(repo domain.RepoName) bool {
236 return false236 return false
237 }237 }
238 ok := true238 ok := true
239+ changed := false
239 for _, mr := range mrs {240 for _, mr := range mrs {
240 if ctx.Err() != nil {241 if ctx.Err() != nil {
241 return true242 return true
@@ -276,9 +277,10 @@ func (s *Mergeability) reconcile(repo domain.RepoName) bool {
276 Repo: repo, Number: mr.Number, Mergeability: res, At: res.CheckedAt,277 Repo: repo, Number: mr.Number, Mergeability: res, At: res.CheckedAt,
277 Visibility: meta.Visibility, CreatedAt: meta.CreatedAt,278 Visibility: meta.Visibility, CreatedAt: meta.CreatedAt,
278 })279 })
279- if s.cfg.AutoMerge != nil {280+ changed = true
280- s.cfg.AutoMerge.OnRepoChanged(ctx, repo)281+ }
281- }282+ if changed && s.cfg.AutoMerge != nil {
283+ s.cfg.AutoMerge.OnRepoChanged(ctx, repo)
282 }284 }
283 return ok285 return ok
284 }286 }
M internal/usecase/mergerequests.go +42 -40
@@ -1041,9 +1041,11 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m
10411041 }
10421042
10431043 type mergeOpts struct {
1044- auto bool
1044+ auto *autoMergeOpts
1045+}
1046+
1047+type autoMergeOpts struct {
10451048 intent *domain.AutoMerge
1046- enabler domain.CIPusher
10471049 tokens ports.TokenStore
10481050 activity ports.CIActivity
10491051 }
@@ -1051,7 +1053,7 @@ type mergeOpts struct {
10511053 var errAutoMergeWait = errors.New("auto-merge is waiting for checks")
10521054
10531055 func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams, opts mergeOpts) (domain.MergeRequest, error) {
1054- if !opts.auto {
1056+ if opts.auto == nil {
10551057 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {
10561058 return domain.MergeRequest{}, err
10571059 }
@@ -1083,18 +1085,14 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
10831085 if err != nil {
10841086 return domain.MergeRequest{}, err
10851087 }
1086- if opts.auto && (mr.AutoMerge == nil || opts.intent == nil || *mr.AutoMerge != *opts.intent) {
1088+ if opts.auto != nil && (mr.AutoMerge == nil || opts.auto.intent == nil || *mr.AutoMerge != *opts.auto.intent) {
10871089 return domain.MergeRequest{}, errAutoMergeWait
10881090 }
1089- if opts.auto {
1090- fresh, valid, err := principalOfPusher(ctx, opts.tokens, opts.enabler, m.clock.Now())
1091+ if opts.auto != nil {
1092+ p, err = enablerCanMerge(ctx, opts.auto.tokens, opts.auto.intent, mr, m.clock.Now())
10911093 if err != nil {
10921094 return domain.MergeRequest{}, err
10931095 }
1094- if !valid || !fresh.Allows(domain.ScopeMRMerge, name) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
1095- return domain.MergeRequest{}, fmt.Errorf("%w: the principal that enabled auto-merge can no longer merge %s", domain.ErrForbidden, mr.Target)
1096- }
1097- p = fresh
10981096 }
10991097 // A merge moves the target and may delete the source: a token limited to
11001098 // branch prefixes may do so only within them.
@@ -1147,40 +1145,31 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
11471145 } else if behind {
11481146 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already contains %s", domain.ErrPreconditionFailed, mr.Target, head)
11491147 }
1150- if opts.auto && opts.activity != nil && !opts.activity.Settled(ctx, name, number, mr.Source, head) {
1148+ if opts.auto != nil && opts.auto.activity != nil && !opts.auto.activity.Settled(ctx, name, number) {
11511149 return domain.MergeRequest{}, errAutoMergeWait
11521150 }
11531151 tree, err := m.checkMode(ctx, mr, mode, tip, head)
11541152 if err != nil {
1155- if opts.auto && errors.Is(err, domain.ErrPreconditionFailed) {
1153+ if opts.auto != nil && errors.Is(err, domain.ErrPreconditionFailed) {
11561154 return domain.MergeRequest{}, errAutoMergeWait
11571155 }
11581156 return domain.MergeRequest{}, err
11591157 }
1160- if opts.auto && !settings.AllowAutoMerge {
1158+ if opts.auto != nil && !settings.AllowAutoMerge {
11611159 return domain.MergeRequest{}, fmt.Errorf("%w: auto-merge is off in %s", domain.ErrPreconditionFailed, name)
11621160 }
1163- if settings.FailingChecksBlock || opts.auto {
1164- if err := m.checksAllow(ctx, name, head); err != nil {
1165- if opts.auto && errors.Is(err, domain.ErrPreconditionFailed) {
1166- return domain.MergeRequest{}, errAutoMergeWait
1167- }
1168- return domain.MergeRequest{}, err
1161+ if settings.FailingChecksBlock || opts.auto != nil {
1162+ checks, err := m.readMergeChecks(ctx, name, head)
1163+ if err == nil {
1164+ err = checksPolicy(checks)
11691165 }
1170- }
1171- if opts.auto {
1172- checks, err := m.checks.CommitChecks(ctx, name, head.String())
11731166 if err != nil {
1174- return domain.MergeRequest{}, err
1175- }
1176- passed := false
1177- for _, check := range checks {
1178- if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
1179- passed = true
1180- break
1167+ if opts.auto != nil && errors.Is(err, domain.ErrPreconditionFailed) {
1168+ return domain.MergeRequest{}, errAutoMergeWait
11811169 }
1170+ return domain.MergeRequest{}, err
11821171 }
1183- if !passed {
1172+ if opts.auto != nil && !hasSuccessfulCheck(checks) {
11841173 return domain.MergeRequest{}, errAutoMergeWait
11851174 }
11861175 }
@@ -1195,7 +1184,7 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
11951184 return domain.MergeRequest{}, err
11961185 }
11971186 m.recordPush(ctx, name) // the target moved, whatever happens next
1198- mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged, opts.auto)
1187+ mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged, opts.auto != nil)
11991188 if err != nil {
12001189 return domain.MergeRequest{}, err
12011190 }
@@ -1307,21 +1296,34 @@ func (m *MergeRequests) WhileOpen(ctx context.Context, name domain.RepoName, num
13071296 return fn()
13081297 }
13091298
1310-// checksAllow is the merge rule of FailingChecksBlock: every check of the
1311-// head must have ended as success, skipped or neutral. A check still queued or
1312-// running blocks too (the merge could land code that then fails), and a head
1313-// without any check merges.
1314-func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, head domain.ObjectID) error {
1299+// readMergeChecks reads one snapshot, classifying an incomplete result as a
1300+// failed precondition rather than treating it as a passing set of checks.
1301+func (m *MergeRequests) readMergeChecks(ctx context.Context, name domain.RepoName, head domain.ObjectID) ([]domain.CICheck, error) {
13151302 if m.checks == nil {
1316- return nil
1303+ return nil, nil
13171304 }
13181305 checks, err := m.checks.CommitChecks(ctx, name, head.String())
13191306 if errors.Is(err, domain.ErrResourceExhausted) {
1320- return fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)
1307+ return nil, fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)
13211308 }
13221309 if err != nil {
1323- return fmt.Errorf("read the checks of %s: %w", head, err)
1310+ return nil, fmt.Errorf("read the checks of %s: %w", head, err)
13241311 }
1312+ return checks, nil
1313+}
1314+
1315+func hasSuccessfulCheck(checks []domain.CICheck) bool {
1316+ for _, check := range checks {
1317+ if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
1318+ return true
1319+ }
1320+ }
1321+ return false
1322+}
1323+
1324+// checksPolicy requires every check to have ended as success, skipped or
1325+// neutral. Queued and running checks block too; an empty set is allowed.
1326+func checksPolicy(checks []domain.CICheck) error {
13251327 var failing, pending []string
13261328 for _, c := range checks {
13271329 switch {
@@ -1335,7 +1337,7 @@ func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, h
13351337 case len(failing) > 0:
13361338 return fmt.Errorf("%w: failing checks block the merge: %s (turn off failing_checks_block to merge anyway)", domain.ErrPreconditionFailed, strings.Join(failing, ", "))
13371339 case len(pending) > 0:
1338- return fmt.Errorf("%w: %w: checks have not finished: %s", domain.ErrPreconditionFailed, errAutoMergeWait, strings.Join(pending, ", "))
1340+ return fmt.Errorf("%w: checks have not finished: %s", domain.ErrPreconditionFailed, strings.Join(pending, ", "))
13391341 }
13401342 return nil
13411343 }
@@ -1041,9 +1041,11 @@ func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, m
1041 }1041 }
1042 1042
1043 type mergeOpts struct {1043 type mergeOpts struct {
1044- auto bool1044+ auto *autoMergeOpts
1045+}
1046+
1047+type autoMergeOpts struct {
1045 intent *domain.AutoMerge1048 intent *domain.AutoMerge
1046- enabler domain.CIPusher
1047 tokens ports.TokenStore1049 tokens ports.TokenStore
1048 activity ports.CIActivity1050 activity ports.CIActivity
1049 }1051 }
@@ -1051,7 +1053,7 @@ type mergeOpts struct {
1051 var errAutoMergeWait = errors.New("auto-merge is waiting for checks")1053 var errAutoMergeWait = errors.New("auto-merge is waiting for checks")
1052 1054
1053 func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams, opts mergeOpts) (domain.MergeRequest, error) {1055 func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams, opts mergeOpts) (domain.MergeRequest, error) {
1054- if !opts.auto {1056+ if opts.auto == nil {
1055 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {1057 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {
1056 return domain.MergeRequest{}, err1058 return domain.MergeRequest{}, err
1057 }1059 }
@@ -1083,18 +1085,14 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
1083 if err != nil {1085 if err != nil {
1084 return domain.MergeRequest{}, err1086 return domain.MergeRequest{}, err
1085 }1087 }
1086- if opts.auto && (mr.AutoMerge == nil || opts.intent == nil || *mr.AutoMerge != *opts.intent) {1088+ if opts.auto != nil && (mr.AutoMerge == nil || opts.auto.intent == nil || *mr.AutoMerge != *opts.auto.intent) {
1087 return domain.MergeRequest{}, errAutoMergeWait1089 return domain.MergeRequest{}, errAutoMergeWait
1088 }1090 }
1089- if opts.auto {1091+ if opts.auto != nil {
1090- fresh, valid, err := principalOfPusher(ctx, opts.tokens, opts.enabler, m.clock.Now())1092+ p, err = enablerCanMerge(ctx, opts.auto.tokens, opts.auto.intent, mr, m.clock.Now())
1091 if err != nil {1093 if err != nil {
1092 return domain.MergeRequest{}, err1094 return domain.MergeRequest{}, err
1093 }1095 }
1094- if !valid || !fresh.Allows(domain.ScopeMRMerge, name) || !fresh.AllowsRef("refs/heads/"+mr.Target) {
1095- return domain.MergeRequest{}, fmt.Errorf("%w: the principal that enabled auto-merge can no longer merge %s", domain.ErrForbidden, mr.Target)
1096- }
1097- p = fresh
1098 }1096 }
1099 // A merge moves the target and may delete the source: a token limited to1097 // A merge moves the target and may delete the source: a token limited to
1100 // branch prefixes may do so only within them.1098 // branch prefixes may do so only within them.
@@ -1147,40 +1145,31 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
1147 } else if behind {1145 } else if behind {
1148 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already contains %s", domain.ErrPreconditionFailed, mr.Target, head)1146 return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already contains %s", domain.ErrPreconditionFailed, mr.Target, head)
1149 }1147 }
1150- if opts.auto && opts.activity != nil && !opts.activity.Settled(ctx, name, number, mr.Source, head) {1148+ if opts.auto != nil && opts.auto.activity != nil && !opts.auto.activity.Settled(ctx, name, number) {
1151 return domain.MergeRequest{}, errAutoMergeWait1149 return domain.MergeRequest{}, errAutoMergeWait
1152 }1150 }
1153 tree, err := m.checkMode(ctx, mr, mode, tip, head)1151 tree, err := m.checkMode(ctx, mr, mode, tip, head)
1154 if err != nil {1152 if err != nil {
1155- if opts.auto && errors.Is(err, domain.ErrPreconditionFailed) {1153+ if opts.auto != nil && errors.Is(err, domain.ErrPreconditionFailed) {
1156 return domain.MergeRequest{}, errAutoMergeWait1154 return domain.MergeRequest{}, errAutoMergeWait
1157 }1155 }
1158 return domain.MergeRequest{}, err1156 return domain.MergeRequest{}, err
1159 }1157 }
1160- if opts.auto && !settings.AllowAutoMerge {1158+ if opts.auto != nil && !settings.AllowAutoMerge {
1161 return domain.MergeRequest{}, fmt.Errorf("%w: auto-merge is off in %s", domain.ErrPreconditionFailed, name)1159 return domain.MergeRequest{}, fmt.Errorf("%w: auto-merge is off in %s", domain.ErrPreconditionFailed, name)
1162 }1160 }
1163- if settings.FailingChecksBlock || opts.auto {1161+ if settings.FailingChecksBlock || opts.auto != nil {
1164- if err := m.checksAllow(ctx, name, head); err != nil {1162+ checks, err := m.readMergeChecks(ctx, name, head)
1165- if opts.auto && errors.Is(err, domain.ErrPreconditionFailed) {1163+ if err == nil {
1166- return domain.MergeRequest{}, errAutoMergeWait1164+ err = checksPolicy(checks)
1167- }
1168- return domain.MergeRequest{}, err
1169 }1165 }
1170- }
1171- if opts.auto {
1172- checks, err := m.checks.CommitChecks(ctx, name, head.String())
1173 if err != nil {1166 if err != nil {
1174- return domain.MergeRequest{}, err1167+ if opts.auto != nil && errors.Is(err, domain.ErrPreconditionFailed) {
1175- }1168+ return domain.MergeRequest{}, errAutoMergeWait
1176- passed := false
1177- for _, check := range checks {
1178- if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
1179- passed = true
1180- break
1181 }1169 }
1170+ return domain.MergeRequest{}, err
1182 }1171 }
1183- if !passed {1172+ if opts.auto != nil && !hasSuccessfulCheck(checks) {
1184 return domain.MergeRequest{}, errAutoMergeWait1173 return domain.MergeRequest{}, errAutoMergeWait
1185 }1174 }
1186 }1175 }
@@ -1195,7 +1184,7 @@ func (m *MergeRequests) mergeAs(ctx context.Context, p domain.Principal, name do
1195 return domain.MergeRequest{}, err1184 return domain.MergeRequest{}, err
1196 }1185 }
1197 m.recordPush(ctx, name) // the target moved, whatever happens next1186 m.recordPush(ctx, name) // the target moved, whatever happens next
1198- mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged, opts.auto)1187+ mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged, opts.auto != nil)
1199 if err != nil {1188 if err != nil {
1200 return domain.MergeRequest{}, err1189 return domain.MergeRequest{}, err
1201 }1190 }
@@ -1307,21 +1296,34 @@ func (m *MergeRequests) WhileOpen(ctx context.Context, name domain.RepoName, num
1307 return fn()1296 return fn()
1308 }1297 }
1309 1298
1310-// checksAllow is the merge rule of FailingChecksBlock: every check of the1299+// readMergeChecks reads one snapshot, classifying an incomplete result as a
1311-// head must have ended as success, skipped or neutral. A check still queued or1300+// failed precondition rather than treating it as a passing set of checks.
1312-// running blocks too (the merge could land code that then fails), and a head1301+func (m *MergeRequests) readMergeChecks(ctx context.Context, name domain.RepoName, head domain.ObjectID) ([]domain.CICheck, error) {
1313-// without any check merges.
1314-func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, head domain.ObjectID) error {
1315 if m.checks == nil {1302 if m.checks == nil {
1316- return nil1303+ return nil, nil
1317 }1304 }
1318 checks, err := m.checks.CommitChecks(ctx, name, head.String())1305 checks, err := m.checks.CommitChecks(ctx, name, head.String())
1319 if errors.Is(err, domain.ErrResourceExhausted) {1306 if errors.Is(err, domain.ErrResourceExhausted) {
1320- return fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)1307+ return nil, fmt.Errorf("%w: too many checks to tell whether they passed: %v", domain.ErrPreconditionFailed, err)
1321 }1308 }
1322 if err != nil {1309 if err != nil {
1323- return fmt.Errorf("read the checks of %s: %w", head, err)1310+ return nil, fmt.Errorf("read the checks of %s: %w", head, err)
1324 }1311 }
1312+ return checks, nil
1313+}
1314+
1315+func hasSuccessfulCheck(checks []domain.CICheck) bool {
1316+ for _, check := range checks {
1317+ if check.Status == domain.CICompleted && check.Conclusion == domain.CISuccess {
1318+ return true
1319+ }
1320+ }
1321+ return false
1322+}
1323+
1324+// checksPolicy requires every check to have ended as success, skipped or
1325+// neutral. Queued and running checks block too; an empty set is allowed.
1326+func checksPolicy(checks []domain.CICheck) error {
1325 var failing, pending []string1327 var failing, pending []string
1326 for _, c := range checks {1328 for _, c := range checks {
1327 switch {1329 switch {
@@ -1335,7 +1337,7 @@ func (m *MergeRequests) checksAllow(ctx context.Context, name domain.RepoName, h
1335 case len(failing) > 0:1337 case len(failing) > 0:
1336 return fmt.Errorf("%w: failing checks block the merge: %s (turn off failing_checks_block to merge anyway)", domain.ErrPreconditionFailed, strings.Join(failing, ", "))1338 return fmt.Errorf("%w: failing checks block the merge: %s (turn off failing_checks_block to merge anyway)", domain.ErrPreconditionFailed, strings.Join(failing, ", "))
1337 case len(pending) > 0:1339 case len(pending) > 0:
1338- return fmt.Errorf("%w: %w: checks have not finished: %s", domain.ErrPreconditionFailed, errAutoMergeWait, strings.Join(pending, ", "))1340+ return fmt.Errorf("%w: checks have not finished: %s", domain.ErrPreconditionFailed, strings.Join(pending, ", "))
1339 }1341 }
1340 return nil1342 return nil
1341 }1343 }
M internal/usecase/mergerequests_auto.go +1 -1
@@ -91,7 +91,7 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
9191 recordActivity(ctx, m.meta, name, mr.UpdatedAt)
9292 m.bus.Publish(ctx, domain.MergeRequestAutoMergeEnabled{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
9393 if m.autoMerge != nil {
94- m.autoMerge.OnRepoChanged(ctx, name)
94+ m.autoMerge.OnAutoMergeEnabled(ctx, name)
9595 }
9696 return m.withHead(ctx, mr)
9797 }
@@ -91,7 +91,7 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
91 recordActivity(ctx, m.meta, name, mr.UpdatedAt)91 recordActivity(ctx, m.meta, name, mr.UpdatedAt)
92 m.bus.Publish(ctx, domain.MergeRequestAutoMergeEnabled{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})92 m.bus.Publish(ctx, domain.MergeRequestAutoMergeEnabled{MergeRequest: mr, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
93 if m.autoMerge != nil {93 if m.autoMerge != nil {
94- m.autoMerge.OnRepoChanged(ctx, name)94+ m.autoMerge.OnAutoMergeEnabled(ctx, name)
95 }95 }
96 return m.withHead(ctx, mr)96 return m.withHead(ctx, mr)
97 }97 }

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 44

↑↓ moveenter openesc close