detached 63e55ec

feat(mr): require the head the caller saw to enable auto-merge

commit
63e55ec86b44a35b7df8147dcff3e73a64dd60ca
author
brice · · 2026-10-20 16:24 UTC
parent
b5db7e3
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +78 -33
M internal/adapters/rpc/mergerequest.go +5 -1
@@ -28,7 +28,11 @@ func (s *mergeRequestService) EnableAutoMerge(ctx context.Context, req *connect.
2828 if err != nil {
2929 return nil, connectError(ctx, err)
3030 }
31- mr, err := s.mrs.EnableAutoMerge(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.EnableAutoMergeParams{Mode: mode})
31+ head, err := headID(req.Msg.GetHeadId())
32+ if err != nil {
33+ return nil, connectError(ctx, err)
34+ }
35+ mr, err := s.mrs.EnableAutoMerge(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.EnableAutoMergeParams{Mode: mode, HeadID: head})
3236 if err != nil {
3337 return nil, connectError(ctx, err)
3438 }
@@ -28,7 +28,11 @@ func (s *mergeRequestService) EnableAutoMerge(ctx context.Context, req *connect.
28 if err != nil {28 if err != nil {
29 return nil, connectError(ctx, err)29 return nil, connectError(ctx, err)
30 }30 }
31- mr, err := s.mrs.EnableAutoMerge(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.EnableAutoMergeParams{Mode: mode})31+ head, err := headID(req.Msg.GetHeadId())
32+ if err != nil {
33+ return nil, connectError(ctx, err)
34+ }
35+ mr, err := s.mrs.EnableAutoMerge(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.EnableAutoMergeParams{Mode: mode, HeadID: head})
32 if err != nil {36 if err != nil {
33 return nil, connectError(ctx, err)37 return nil, connectError(ctx, err)
34 }38 }
M internal/adapters/rpc/mr_api_test.go +6 -4
@@ -149,8 +149,8 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
149149
150150 // Success: the mode and the request reach the service, the fields come back.
151151 m := mocks.NewMergeRequestService(t)
152- m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash}).Return(armed, nil).Once()
153- res, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode_MERGE_MODE_SQUASH}))
152+ m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: mustObjectID(t, strings.Repeat("a", domain.ObjectIDLen))}).Return(armed, nil).Once()
153+ res, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode_MERGE_MODE_SQUASH, HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
154154 require.NoError(t, err)
155155 assert.Equal(t, pbArmed, res.Msg.GetMergeRequest())
156156 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(idle, nil).Once()
@@ -159,7 +159,9 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
159159 assert.Nil(t, cres.Msg.GetMergeRequest().GetAutoMerge())
160160
161161 // Bad input never reaches the service.
162- _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode(99)}))
162+ _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode(99), HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
163+ requireCode(t, connect.CodeInvalidArgument, err)
164+ _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, HeadId: "abc"}))
163165 requireCode(t, connect.CodeInvalidArgument, err)
164166 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "", Number: 1}))
165167 requireCode(t, connect.CodeInvalidArgument, err)
@@ -175,7 +177,7 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
175177 } {
176178 m := mocks.NewMergeRequestService(t)
177179 m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), mock.Anything).Return(domain.MergeRequest{}, tc.err).Once()
178- _, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1}))
180+ _, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
179181 requireCode(t, tc.code, err)
180182 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(domain.MergeRequest{}, tc.err).Once()
181183 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "demo", Number: 1}))
@@ -149,8 +149,8 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
149 149
150 // Success: the mode and the request reach the service, the fields come back.150 // Success: the mode and the request reach the service, the fields come back.
151 m := mocks.NewMergeRequestService(t)151 m := mocks.NewMergeRequestService(t)
152- m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash}).Return(armed, nil).Once()152+ m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: mustObjectID(t, strings.Repeat("a", domain.ObjectIDLen))}).Return(armed, nil).Once()
153- res, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode_MERGE_MODE_SQUASH}))153+ res, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode_MERGE_MODE_SQUASH, HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
154 require.NoError(t, err)154 require.NoError(t, err)
155 assert.Equal(t, pbArmed, res.Msg.GetMergeRequest())155 assert.Equal(t, pbArmed, res.Msg.GetMergeRequest())
156 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(idle, nil).Once()156 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(idle, nil).Once()
@@ -159,7 +159,9 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
159 assert.Nil(t, cres.Msg.GetMergeRequest().GetAutoMerge())159 assert.Nil(t, cres.Msg.GetMergeRequest().GetAutoMerge())
160 160
161 // Bad input never reaches the service.161 // Bad input never reaches the service.
162- _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode(99)}))162+ _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, Mode: pb.MergeMode(99), HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
163+ requireCode(t, connect.CodeInvalidArgument, err)
164+ _, err = (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, HeadId: "abc"}))
163 requireCode(t, connect.CodeInvalidArgument, err)165 requireCode(t, connect.CodeInvalidArgument, err)
164 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "", Number: 1}))166 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "", Number: 1}))
165 requireCode(t, connect.CodeInvalidArgument, err)167 requireCode(t, connect.CodeInvalidArgument, err)
@@ -175,7 +177,7 @@ func TestAutoMergeRPCCodesAndFields(t *testing.T) {
175 } {177 } {
176 m := mocks.NewMergeRequestService(t)178 m := mocks.NewMergeRequestService(t)
177 m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), mock.Anything).Return(domain.MergeRequest{}, tc.err).Once()179 m.EXPECT().EnableAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), mock.Anything).Return(domain.MergeRequest{}, tc.err).Once()
178- _, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1}))180+ _, err := (&mergeRequestService{m}).EnableAutoMerge(ownerCtx(), connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: "demo", Number: 1, HeadId: strings.Repeat("a", domain.ObjectIDLen)}))
179 requireCode(t, tc.code, err)181 requireCode(t, tc.code, err)
180 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(domain.MergeRequest{}, tc.err).Once()182 m.EXPECT().CancelAutoMerge(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1)).Return(domain.MergeRequest{}, tc.err).Once()
181 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "demo", Number: 1}))183 _, err = (&mergeRequestService{m}).CancelAutoMerge(ownerCtx(), connect.NewRequest(&pb.CancelAutoMergeRequest{Repo: "demo", Number: 1}))
M internal/adapters/web/mr_post.go +4 -4
@@ -393,8 +393,7 @@ func (s *Server) mrMerge(ctx context.Context, r *http.Request) (string, string,
393393
394394 // mrAutoMerge is POST /<repo>/merge-requests/<n>/auto-merge: the form of the
395395 // merge (mode, head_id) asks to merge once the checks pass. The server records
396-// the head as it is when it enables (the params carry no head); head_id is only
397-// validated.
396+// refuses to enable when the source branch is no longer at head_id.
398397 func (s *Server) mrAutoMerge(ctx context.Context, r *http.Request) (string, string, error) {
399398 repo := r.PathValue("repo")
400399 n, err := number(r)
@@ -418,10 +417,11 @@ func (s *Server) mrAutoMerge(ctx context.Context, r *http.Request) (string, stri
418417 if err != nil {
419418 return "", back, err
420419 }
421- if _, err := headID(r.PostFormValue("head_id")); err != nil {
420+ head, err := headID(r.PostFormValue("head_id"))
421+ if err != nil {
422422 return "", back, err
423423 }
424- _, err = s.cfg.MergeRequests.EnableAutoMerge(ctx, principal(ctx), name, n, ports.EnableAutoMergeParams{Mode: mode})
424+ _, err = s.cfg.MergeRequests.EnableAutoMerge(ctx, principal(ctx), name, n, ports.EnableAutoMergeParams{Mode: mode, HeadID: head})
425425 switch {
426426 case errors.Is(err, domain.ErrPreconditionFailed), errors.Is(err, domain.ErrConflict):
427427 return "", back, &rerender{status: http.StatusConflict, msg: rule(err.Error(), "the merge request is not in a state that allows this."), err: err}
@@ -393,8 +393,7 @@ func (s *Server) mrMerge(ctx context.Context, r *http.Request) (string, string,
393 393
394 // mrAutoMerge is POST /<repo>/merge-requests/<n>/auto-merge: the form of the394 // mrAutoMerge is POST /<repo>/merge-requests/<n>/auto-merge: the form of the
395 // merge (mode, head_id) asks to merge once the checks pass. The server records395 // merge (mode, head_id) asks to merge once the checks pass. The server records
396-// the head as it is when it enables (the params carry no head); head_id is only396+// refuses to enable when the source branch is no longer at head_id.
397-// validated.
398 func (s *Server) mrAutoMerge(ctx context.Context, r *http.Request) (string, string, error) {397 func (s *Server) mrAutoMerge(ctx context.Context, r *http.Request) (string, string, error) {
399 repo := r.PathValue("repo")398 repo := r.PathValue("repo")
400 n, err := number(r)399 n, err := number(r)
@@ -418,10 +417,11 @@ func (s *Server) mrAutoMerge(ctx context.Context, r *http.Request) (string, stri
418 if err != nil {417 if err != nil {
419 return "", back, err418 return "", back, err
420 }419 }
421- if _, err := headID(r.PostFormValue("head_id")); err != nil {420+ head, err := headID(r.PostFormValue("head_id"))
421+ if err != nil {
422 return "", back, err422 return "", back, err
423 }423 }
424- _, err = s.cfg.MergeRequests.EnableAutoMerge(ctx, principal(ctx), name, n, ports.EnableAutoMergeParams{Mode: mode})424+ _, err = s.cfg.MergeRequests.EnableAutoMerge(ctx, principal(ctx), name, n, ports.EnableAutoMergeParams{Mode: mode, HeadID: head})
425 switch {425 switch {
426 case errors.Is(err, domain.ErrPreconditionFailed), errors.Is(err, domain.ErrConflict):426 case errors.Is(err, domain.ErrPreconditionFailed), errors.Is(err, domain.ErrConflict):
427 return "", back, &rerender{status: http.StatusConflict, msg: rule(err.Error(), "the merge request is not in a state that allows this."), err: err}427 return "", back, &rerender{status: http.StatusConflict, msg: rule(err.Error(), "the merge request is not in a state that allows this."), err: err}
M internal/adapters/web/mr_test.go +5 -3
@@ -241,12 +241,14 @@ func TestAutoMergeHandlers(t *testing.T) {
241241 require.NoError(t, err)
242242 ctx := context.Background()
243243
244+ head, err := domain.ParseObjectID(strings.Repeat("a", domain.ObjectIDLen))
245+ require.NoError(t, err)
244246 svc := mocks.NewMergeRequestService(t)
245- svc.EXPECT().EnableAutoMerge(mock.Anything, mock.Anything, name, int64(3), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash}).
247+ svc.EXPECT().EnableAutoMerge(mock.Anything, mock.Anything, name, int64(3), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: head}).
246248 Return(domain.MergeRequest{}, nil).Once()
247249 svc.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).Return(domain.MergeRequest{}, nil).Once()
248250 s := &Server{cfg: Config{MergeRequests: svc}}
249- to, _, err := s.mrAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge", "mode=squash"))
251+ to, _, err := s.mrAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge", "mode=squash&head_id="+head.String()))
250252 require.NoError(t, err)
251253 assert.Equal(t, mrHref("demo", 3), to)
252254 to, _, err = s.mrCancelAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge/cancel", ""))
@@ -266,7 +268,7 @@ func TestAutoMergeHandlers(t *testing.T) {
266268 svc2.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).
267269 Return(domain.MergeRequest{}, domain.ErrNotFound).Once()
268270 s2 := &Server{cfg: Config{MergeRequests: svc2}}
269- _, _, err = s2.mrAutoMerge(ctx, autoMergeRequest("/x", "mode=squash"))
271+ _, _, err = s2.mrAutoMerge(ctx, autoMergeRequest("/x", "mode=squash&head_id="+head.String()))
270272 require.ErrorAs(t, err, &re)
271273 assert.Equal(t, http.StatusConflict, re.status)
272274 _, _, err = s2.mrCancelAutoMerge(ctx, autoMergeRequest("/x", ""))
@@ -241,12 +241,14 @@ func TestAutoMergeHandlers(t *testing.T) {
241 require.NoError(t, err)241 require.NoError(t, err)
242 ctx := context.Background()242 ctx := context.Background()
243 243
244+ head, err := domain.ParseObjectID(strings.Repeat("a", domain.ObjectIDLen))
245+ require.NoError(t, err)
244 svc := mocks.NewMergeRequestService(t)246 svc := mocks.NewMergeRequestService(t)
245- svc.EXPECT().EnableAutoMerge(mock.Anything, mock.Anything, name, int64(3), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash}).247+ svc.EXPECT().EnableAutoMerge(mock.Anything, mock.Anything, name, int64(3), ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: head}).
246 Return(domain.MergeRequest{}, nil).Once()248 Return(domain.MergeRequest{}, nil).Once()
247 svc.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).Return(domain.MergeRequest{}, nil).Once()249 svc.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).Return(domain.MergeRequest{}, nil).Once()
248 s := &Server{cfg: Config{MergeRequests: svc}}250 s := &Server{cfg: Config{MergeRequests: svc}}
249- to, _, err := s.mrAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge", "mode=squash"))251+ to, _, err := s.mrAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge", "mode=squash&head_id="+head.String()))
250 require.NoError(t, err)252 require.NoError(t, err)
251 assert.Equal(t, mrHref("demo", 3), to)253 assert.Equal(t, mrHref("demo", 3), to)
252 to, _, err = s.mrCancelAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge/cancel", ""))254 to, _, err = s.mrCancelAutoMerge(ctx, autoMergeRequest("/demo/merge-requests/3/auto-merge/cancel", ""))
@@ -266,7 +268,7 @@ func TestAutoMergeHandlers(t *testing.T) {
266 svc2.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).268 svc2.EXPECT().CancelAutoMerge(mock.Anything, mock.Anything, name, int64(3)).
267 Return(domain.MergeRequest{}, domain.ErrNotFound).Once()269 Return(domain.MergeRequest{}, domain.ErrNotFound).Once()
268 s2 := &Server{cfg: Config{MergeRequests: svc2}}270 s2 := &Server{cfg: Config{MergeRequests: svc2}}
269- _, _, err = s2.mrAutoMerge(ctx, autoMergeRequest("/x", "mode=squash"))271+ _, _, err = s2.mrAutoMerge(ctx, autoMergeRequest("/x", "mode=squash&head_id="+head.String()))
270 require.ErrorAs(t, err, &re)272 require.ErrorAs(t, err, &re)
271 assert.Equal(t, http.StatusConflict, re.status)273 assert.Equal(t, http.StatusConflict, re.status)
272 _, _, err = s2.mrCancelAutoMerge(ctx, autoMergeRequest("/x", ""))274 _, _, err = s2.mrCancelAutoMerge(ctx, autoMergeRequest("/x", ""))
M internal/app/automerge_e2e_test.go +6 -4
@@ -38,8 +38,10 @@ jobs:
3838 allow := true
3939 _, err := c.forge.Repos.Update(c.ctx, domain.Root(), repo, ports.UpdateRepoParams{AllowAutoMerge: &allow})
4040 require.NoError(t, err)
41+ headID, err := domain.ParseObjectID(head)
42+ require.NoError(t, err)
4143 events := c.forge.Bus.Subscribe(c.ctx)
42- enabled, err := c.forge.MergeRequests.EnableAutoMerge(c.ctx, domain.Owner(), repo, number, ports.EnableAutoMergeParams{})
44+ enabled, err := c.forge.MergeRequests.EnableAutoMerge(c.ctx, domain.Owner(), repo, number, ports.EnableAutoMergeParams{HeadID: headID})
4345 require.NoError(t, err)
4446 require.NotNil(t, enabled.AutoMerge)
4547 assert.Equal(t, head, enabled.AutoMerge.HeadID.String())
@@ -87,11 +89,11 @@ jobs:
8789 steps:
8890 - run: echo hello
8991 `}, "")
90- c.push("feat", map[string]string{"change.txt": "feature\n"}, "")
92+ head := c.push("feat", map[string]string{"change.txt": "feature\n"}, "")
9193 number := c.openMR(c.root, "feat")
9294
9395 // Off by default: the server refuses.
94- _, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: c.repo, Number: number}))
96+ _, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: c.repo, Number: number, HeadId: head}))
9597 require.Error(t, err)
9698 assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(err))
9799
@@ -109,7 +111,7 @@ jobs:
109111
110112 w := c.watch(c.root, "", "home")
111113 res, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{
112- Repo: c.repo, Number: number, Mode: pb.MergeMode_MERGE_MODE_SQUASH,
114+ Repo: c.repo, Number: number, Mode: pb.MergeMode_MERGE_MODE_SQUASH, HeadId: head,
113115 }))
114116 require.NoError(t, err)
115117 am := res.Msg.GetMergeRequest().GetAutoMerge()
@@ -38,8 +38,10 @@ jobs:
38 allow := true38 allow := true
39 _, err := c.forge.Repos.Update(c.ctx, domain.Root(), repo, ports.UpdateRepoParams{AllowAutoMerge: &allow})39 _, err := c.forge.Repos.Update(c.ctx, domain.Root(), repo, ports.UpdateRepoParams{AllowAutoMerge: &allow})
40 require.NoError(t, err)40 require.NoError(t, err)
41+ headID, err := domain.ParseObjectID(head)
42+ require.NoError(t, err)
41 events := c.forge.Bus.Subscribe(c.ctx)43 events := c.forge.Bus.Subscribe(c.ctx)
42- enabled, err := c.forge.MergeRequests.EnableAutoMerge(c.ctx, domain.Owner(), repo, number, ports.EnableAutoMergeParams{})44+ enabled, err := c.forge.MergeRequests.EnableAutoMerge(c.ctx, domain.Owner(), repo, number, ports.EnableAutoMergeParams{HeadID: headID})
43 require.NoError(t, err)45 require.NoError(t, err)
44 require.NotNil(t, enabled.AutoMerge)46 require.NotNil(t, enabled.AutoMerge)
45 assert.Equal(t, head, enabled.AutoMerge.HeadID.String())47 assert.Equal(t, head, enabled.AutoMerge.HeadID.String())
@@ -87,11 +89,11 @@ jobs:
87 steps:89 steps:
88 - run: echo hello90 - run: echo hello
89 `}, "")91 `}, "")
90- c.push("feat", map[string]string{"change.txt": "feature\n"}, "")92+ head := c.push("feat", map[string]string{"change.txt": "feature\n"}, "")
91 number := c.openMR(c.root, "feat")93 number := c.openMR(c.root, "feat")
92 94
93 // Off by default: the server refuses.95 // Off by default: the server refuses.
94- _, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: c.repo, Number: number}))96+ _, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{Repo: c.repo, Number: number, HeadId: head}))
95 require.Error(t, err)97 require.Error(t, err)
96 assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(err))98 assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(err))
97 99
@@ -109,7 +111,7 @@ jobs:
109 111
110 w := c.watch(c.root, "", "home")112 w := c.watch(c.root, "", "home")
111 res, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{113 res, err := c.root.mr.EnableAutoMerge(c.ctx, connect.NewRequest(&pb.EnableAutoMergeRequest{
112- Repo: c.repo, Number: number, Mode: pb.MergeMode_MERGE_MODE_SQUASH,114+ Repo: c.repo, Number: number, Mode: pb.MergeMode_MERGE_MODE_SQUASH, HeadId: head,
113 }))115 }))
114 require.NoError(t, err)116 require.NoError(t, err)
115 am := res.Msg.GetMergeRequest().GetAutoMerge()117 am := res.Msg.GetMergeRequest().GetAutoMerge()
M internal/app/cli_mr.go +7 -8
@@ -1112,8 +1112,8 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
11121112 "--auto does not merge now: it asks the server to merge by itself, with --mode, once CI has settled\n" +
11131113 "with at least one successful check on the head and every check allows it. Needs the mr:merge scope,\n" +
11141114 "a server with CI, and `ermite repo edit --allow-auto-merge` on the repository. A new push to the\n" +
1115- "source branch, a change of the settings or of your permissions cancels it. --auto takes no --head:\n" +
1116- "the server merges the head current when you enable it. --cancel-auto cancels a pending auto-merge." + jsonMRHelp +
1115+ "source branch, a change of the settings or of your permissions cancels it. --auto takes --head\n" +
1116+ "like a plain merge: it is enabled only if the source branch still points to it. --cancel-auto cancels a pending auto-merge." + jsonMRHelp +
11171117 "\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",
11181118 Args: cobra.ExactArgs(2),
11191119 ValidArgsFunction: completeMRRepoThenNumber(env),
@@ -1127,7 +1127,6 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
11271127 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "delete-branch")
11281128 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "head")
11291129 cmd.MarkFlagsMutuallyExclusive("auto", "delete-branch")
1130- cmd.MarkFlagsMutuallyExclusive("auto", "head")
11311130 _ = cmd.RegisterFlagCompletionFunc("mode", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {
11321131 return []string{modeFastForward, modeSquash, modeMerge}, cobra.ShellCompDirectiveNoFileComp
11331132 })
@@ -1144,14 +1143,14 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
11441143 if err != nil {
11451144 return err
11461145 }
1147- if auto {
1148- return runMREnableAuto(env, cmd, args[0], n, pbMode, *asJSON)
1149- }
11501146 head = strings.TrimSpace(head)
11511147 if head == "" {
11521148 return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+
11531149 "so that only the commit you reviewed is merged", args[0], n)
11541150 }
1151+ if auto {
1152+ return runMREnableAuto(env, cmd, args[0], n, pbMode, head, *asJSON)
1153+ }
11551154 c, err := env.client(cmd)
11561155 if err != nil {
11571156 return err
@@ -1188,7 +1187,7 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
11881187 }
11891188
11901189 // runMREnableAuto is `mr merge --auto`: it asks the server to merge once ready.
1191-func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode ermite.MergeMode, asJSON bool) error {
1190+func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode ermite.MergeMode, head string, asJSON bool) error {
11921191 c, err := env.client(cmd)
11931192 if err != nil {
11941193 return err
@@ -1197,7 +1196,7 @@ func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode
11971196 if err := requireFeature(ctx, c, client.FeatureAutoMerge, "cannot merge by itself (--auto)"); err != nil {
11981197 return err
11991198 }
1200- res, err := c.MergeRequests().EnableAutoMerge(ctx, connect.NewRequest(&ermite.EnableAutoMergeRequest{Repo: repo, Number: n, Mode: mode}))
1199+ res, err := c.MergeRequests().EnableAutoMerge(ctx, connect.NewRequest(&ermite.EnableAutoMergeRequest{Repo: repo, Number: n, Mode: mode, HeadId: head}))
12011200 if err != nil {
12021201 return c.Describe(err)
12031202 }
@@ -1112,8 +1112,8 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
1112 "--auto does not merge now: it asks the server to merge by itself, with --mode, once CI has settled\n" +1112 "--auto does not merge now: it asks the server to merge by itself, with --mode, once CI has settled\n" +
1113 "with at least one successful check on the head and every check allows it. Needs the mr:merge scope,\n" +1113 "with at least one successful check on the head and every check allows it. Needs the mr:merge scope,\n" +
1114 "a server with CI, and `ermite repo edit --allow-auto-merge` on the repository. A new push to the\n" +1114 "a server with CI, and `ermite repo edit --allow-auto-merge` on the repository. A new push to the\n" +
1115- "source branch, a change of the settings or of your permissions cancels it. --auto takes no --head:\n" +1115+ "source branch, a change of the settings or of your permissions cancels it. --auto takes --head\n" +
1116- "the server merges the head current when you enable it. --cancel-auto cancels a pending auto-merge." + jsonMRHelp +1116+ "like a plain merge: it is enabled only if the source branch still points to it. --cancel-auto cancels a pending auto-merge." + jsonMRHelp +
1117 "\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",1117 "\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",
1118 Args: cobra.ExactArgs(2),1118 Args: cobra.ExactArgs(2),
1119 ValidArgsFunction: completeMRRepoThenNumber(env),1119 ValidArgsFunction: completeMRRepoThenNumber(env),
@@ -1127,7 +1127,6 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
1127 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "delete-branch")1127 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "delete-branch")
1128 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "head")1128 cmd.MarkFlagsMutuallyExclusive("cancel-auto", "head")
1129 cmd.MarkFlagsMutuallyExclusive("auto", "delete-branch")1129 cmd.MarkFlagsMutuallyExclusive("auto", "delete-branch")
1130- cmd.MarkFlagsMutuallyExclusive("auto", "head")
1131 _ = cmd.RegisterFlagCompletionFunc("mode", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {1130 _ = cmd.RegisterFlagCompletionFunc("mode", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {
1132 return []string{modeFastForward, modeSquash, modeMerge}, cobra.ShellCompDirectiveNoFileComp1131 return []string{modeFastForward, modeSquash, modeMerge}, cobra.ShellCompDirectiveNoFileComp
1133 })1132 })
@@ -1144,14 +1143,14 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
1144 if err != nil {1143 if err != nil {
1145 return err1144 return err
1146 }1145 }
1147- if auto {
1148- return runMREnableAuto(env, cmd, args[0], n, pbMode, *asJSON)
1149- }
1150 head = strings.TrimSpace(head)1146 head = strings.TrimSpace(head)
1151 if head == "" {1147 if head == "" {
1152 return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+1148 return fmt.Errorf("--head is required: run `ermite mr show %s %d` and give the head id it prints, "+
1153 "so that only the commit you reviewed is merged", args[0], n)1149 "so that only the commit you reviewed is merged", args[0], n)
1154 }1150 }
1151+ if auto {
1152+ return runMREnableAuto(env, cmd, args[0], n, pbMode, head, *asJSON)
1153+ }
1155 c, err := env.client(cmd)1154 c, err := env.client(cmd)
1156 if err != nil {1155 if err != nil {
1157 return err1156 return err
@@ -1188,7 +1187,7 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
1188 }1187 }
1189 1188
1190 // runMREnableAuto is `mr merge --auto`: it asks the server to merge once ready.1189 // runMREnableAuto is `mr merge --auto`: it asks the server to merge once ready.
1191-func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode ermite.MergeMode, asJSON bool) error {1190+func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode ermite.MergeMode, head string, asJSON bool) error {
1192 c, err := env.client(cmd)1191 c, err := env.client(cmd)
1193 if err != nil {1192 if err != nil {
1194 return err1193 return err
@@ -1197,7 +1196,7 @@ func runMREnableAuto(env *cliEnv, cmd *cobra.Command, repo string, n int64, mode
1197 if err := requireFeature(ctx, c, client.FeatureAutoMerge, "cannot merge by itself (--auto)"); err != nil {1196 if err := requireFeature(ctx, c, client.FeatureAutoMerge, "cannot merge by itself (--auto)"); err != nil {
1198 return err1197 return err
1199 }1198 }
1200- res, err := c.MergeRequests().EnableAutoMerge(ctx, connect.NewRequest(&ermite.EnableAutoMergeRequest{Repo: repo, Number: n, Mode: mode}))1199+ res, err := c.MergeRequests().EnableAutoMerge(ctx, connect.NewRequest(&ermite.EnableAutoMergeRequest{Repo: repo, Number: n, Mode: mode, HeadId: head}))
1201 if err != nil {1200 if err != nil {
1202 return c.Describe(err)1201 return c.Describe(err)
1203 }1202 }
M internal/app/cli_mr_test.go +5 -4
@@ -599,7 +599,7 @@ func autoMergeFake(t *testing.T, features ...string) (*fakeAPI, string) {
599599
600600 func TestMRMergeAuto(t *testing.T) {
601601 f, url := autoMergeFake(t, "merge_requests.auto_merge")
602- res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--mode", "squash").ok(t)
602+ res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--mode", "squash", "--head", headA).ok(t)
603603 assert.Equal(t, "auto-merge enabled on alpha!1: squash into main when ready (enabled by owner)\n", res.stdout)
604604 calls, reqs := f.snapshot()
605605 assert.Equal(t, []string{"GetVersion", "EnableAutoMerge"}, calls)
@@ -607,8 +607,9 @@ func TestMRMergeAuto(t *testing.T) {
607607 assert.Equal(t, "alpha", got.GetRepo())
608608 assert.EqualValues(t, 1, got.GetNumber())
609609 assert.Equal(t, ermite.MergeMode_MERGE_MODE_SQUASH, got.GetMode())
610+ assert.Equal(t, headA, got.GetHeadId())
610611
611- res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--json").ok(t)
612+ res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--head", headA, "--json").ok(t)
612613 var out struct {
613614 AutoMerge struct {
614615 Mode string `json:"mode"`
@@ -626,7 +627,7 @@ func TestMRMergeAuto(t *testing.T) {
626627 assert.Equal(t, ermite.MergeMode_MERGE_MODE_FAST_FORWARD, reqs[len(reqs)-1].(*ermite.EnableAutoMergeRequest).GetMode(), "fast-forward by default")
627628
628629 for _, args := range [][]string{
629- {"--auto", "--cancel-auto"}, {"--auto", "--delete-branch"}, {"--auto", "--head", headA},
630+ {"--auto", "--head", headA, "--cancel-auto"}, {"--auto", "--head", headA, "--delete-branch"}, {"--cancel-auto", "--head", headA}, {"--auto"},
630631 } {
631632 require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "merge", "alpha", "1"}, args...)...).err, "%v", args)
632633 }
@@ -634,7 +635,7 @@ func TestMRMergeAuto(t *testing.T) {
634635
635636 func TestMRMergeAutoNeedsTheFeature(t *testing.T) {
636637 f, url := autoMergeFake(t, "merge_requests")
637- res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto")
638+ res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--head", headA)
638639 require.Error(t, res.err)
639640 assert.Contains(t, res.err.Error(), "merge_requests.auto_merge")
640641 calls, _ := f.snapshot()
@@ -599,7 +599,7 @@ func autoMergeFake(t *testing.T, features ...string) (*fakeAPI, string) {
599 599
600 func TestMRMergeAuto(t *testing.T) {600 func TestMRMergeAuto(t *testing.T) {
601 f, url := autoMergeFake(t, "merge_requests.auto_merge")601 f, url := autoMergeFake(t, "merge_requests.auto_merge")
602- res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--mode", "squash").ok(t)602+ res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--mode", "squash", "--head", headA).ok(t)
603 assert.Equal(t, "auto-merge enabled on alpha!1: squash into main when ready (enabled by owner)\n", res.stdout)603 assert.Equal(t, "auto-merge enabled on alpha!1: squash into main when ready (enabled by owner)\n", res.stdout)
604 calls, reqs := f.snapshot()604 calls, reqs := f.snapshot()
605 assert.Equal(t, []string{"GetVersion", "EnableAutoMerge"}, calls)605 assert.Equal(t, []string{"GetVersion", "EnableAutoMerge"}, calls)
@@ -607,8 +607,9 @@ func TestMRMergeAuto(t *testing.T) {
607 assert.Equal(t, "alpha", got.GetRepo())607 assert.Equal(t, "alpha", got.GetRepo())
608 assert.EqualValues(t, 1, got.GetNumber())608 assert.EqualValues(t, 1, got.GetNumber())
609 assert.Equal(t, ermite.MergeMode_MERGE_MODE_SQUASH, got.GetMode())609 assert.Equal(t, ermite.MergeMode_MERGE_MODE_SQUASH, got.GetMode())
610+ assert.Equal(t, headA, got.GetHeadId())
610 611
611- res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--json").ok(t)612+ res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--head", headA, "--json").ok(t)
612 var out struct {613 var out struct {
613 AutoMerge struct {614 AutoMerge struct {
614 Mode string `json:"mode"`615 Mode string `json:"mode"`
@@ -626,7 +627,7 @@ func TestMRMergeAuto(t *testing.T) {
626 assert.Equal(t, ermite.MergeMode_MERGE_MODE_FAST_FORWARD, reqs[len(reqs)-1].(*ermite.EnableAutoMergeRequest).GetMode(), "fast-forward by default")627 assert.Equal(t, ermite.MergeMode_MERGE_MODE_FAST_FORWARD, reqs[len(reqs)-1].(*ermite.EnableAutoMergeRequest).GetMode(), "fast-forward by default")
627 628
628 for _, args := range [][]string{629 for _, args := range [][]string{
629- {"--auto", "--cancel-auto"}, {"--auto", "--delete-branch"}, {"--auto", "--head", headA},630+ {"--auto", "--head", headA, "--cancel-auto"}, {"--auto", "--head", headA, "--delete-branch"}, {"--cancel-auto", "--head", headA}, {"--auto"},
630 } {631 } {
631 require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "merge", "alpha", "1"}, args...)...).err, "%v", args)632 require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "merge", "alpha", "1"}, args...)...).err, "%v", args)
632 }633 }
@@ -634,7 +635,7 @@ func TestMRMergeAuto(t *testing.T) {
634 635
635 func TestMRMergeAutoNeedsTheFeature(t *testing.T) {636 func TestMRMergeAutoNeedsTheFeature(t *testing.T) {
636 f, url := autoMergeFake(t, "merge_requests")637 f, url := autoMergeFake(t, "merge_requests")
637- res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto")638+ res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--auto", "--head", headA)
638 require.Error(t, res.err)639 require.Error(t, res.err)
639 assert.Contains(t, res.err.Error(), "merge_requests.auto_merge")640 assert.Contains(t, res.err.Error(), "merge_requests.auto_merge")
640 calls, _ := f.snapshot()641 calls, _ := f.snapshot()
M internal/ports/dto.go +3 -0
@@ -59,6 +59,9 @@ type UpdateRepoParams struct {
5959 type EnableAutoMergeParams struct {
6060 // Mode is the merge mode to use; empty means fast-forward.
6161 Mode domain.MergeMode
62+ // HeadID is the head the caller saw; required. Auto-merge is enabled only
63+ // if the source branch still points to it.
64+ HeadID domain.ObjectID
6265 }
6366
6467 // TokenPage is a page of stored tokens, newest first.
@@ -59,6 +59,9 @@ type UpdateRepoParams struct {
59 type EnableAutoMergeParams struct {59 type EnableAutoMergeParams struct {
60 // Mode is the merge mode to use; empty means fast-forward.60 // Mode is the merge mode to use; empty means fast-forward.
61 Mode domain.MergeMode61 Mode domain.MergeMode
62+ // HeadID is the head the caller saw; required. Auto-merge is enabled only
63+ // if the source branch still points to it.
64+ HeadID domain.ObjectID
62 }65 }
63 66
64 // TokenPage is a page of stored tokens, newest first.67 // TokenPage is a page of stored tokens, newest first.
M internal/usecase/automerge_test.go +25 -4
@@ -37,7 +37,7 @@ func autoEvaluator(r *mrRig, checks ports.CIChecksReader, activity ports.CIActiv
3737
3838 func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest {
3939 t.Helper()
40- enabled, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{})
40+ enabled, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: r.head()})
4141 require.NoError(t, err)
4242 require.NotNil(t, enabled.AutoMerge)
4343 return enabled
@@ -78,7 +78,7 @@ func TestEnableAutoMergeNeedsSettingScopeAndRef(t *testing.T) {
7878 _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{AllowedMergeModes: ptr([]domain.MergeMode{domain.MergeModeSquash})})
7979 require.NoError(t, err)
8080 }
81- _, err := r.mrs.EnableAutoMerge(r.ctx, p, r.repo, mr.Number, ports.EnableAutoMergeParams{})
81+ _, err := r.mrs.EnableAutoMerge(r.ctx, p, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: r.head()})
8282 require.ErrorIs(t, err, want)
8383 })
8484 }
@@ -90,7 +90,7 @@ func TestEnableAutoMergeRecordsEnablerModeHeadAndPublishes(t *testing.T) {
9090 trigger.EXPECT().OnRepoChanged(mock.Anything, r.repo).Return().Twice()
9191 r.mrs.WithAutoMerge(trigger)
9292 r.drain()
93- got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash})
93+ got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: r.head()})
9494 require.NoError(t, err)
9595 assert.Equal(t, &domain.AutoMerge{Mode: domain.MergeModeSquash, HeadID: r.head(), EnabledBy: domain.PusherOf(domain.Owner()), EnablerLabel: domain.AuthorName(domain.Owner()), EnabledAt: now}, got.AutoMerge)
9696 events := r.drain()
@@ -226,7 +226,7 @@ func TestAutoMergeCancelsOnPermanentMergeFailure(t *testing.T) {
226226 for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {
227227 t.Run(mode.String(), func(t *testing.T) {
228228 r, mr, checks := autoRig(t)
229- _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: mode})
229+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: mode, HeadID: r.head()})
230230 require.NoError(t, err)
231231 head := r.head()
232232 r.sh("checkout", "main")
@@ -351,3 +351,24 @@ func TestAutoMergeRequiresSuccessfulCheck(t *testing.T) {
351351 assert.NotNil(t, storedAuto(t, r, mr).AutoMerge)
352352 }
353353 }
354+
355+func TestEnableAutoMergeNeedsTheHeadTheCallerSaw(t *testing.T) {
356+ t.Run("stale head", func(t *testing.T) {
357+ r, mr, _ := autoRig(t)
358+ stale := r.head()
359+ r.commit(author, "later.txt", "later\n", "push after review")
360+ r.push("feat")
361+ r.drain()
362+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: stale})
363+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
364+ assert.Contains(t, err.Error(), "branch moved, read again")
365+ assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
366+ assert.Empty(t, r.drain(), "nothing is published")
367+ })
368+ t.Run("missing head", func(t *testing.T) {
369+ r, mr, _ := autoRig(t)
370+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{})
371+ require.ErrorIs(t, err, domain.ErrInvalid)
372+ assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
373+ })
374+}
@@ -37,7 +37,7 @@ func autoEvaluator(r *mrRig, checks ports.CIChecksReader, activity ports.CIActiv
37 37
38 func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest {38 func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest {
39 t.Helper()39 t.Helper()
40- enabled, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{})40+ enabled, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: r.head()})
41 require.NoError(t, err)41 require.NoError(t, err)
42 require.NotNil(t, enabled.AutoMerge)42 require.NotNil(t, enabled.AutoMerge)
43 return enabled43 return enabled
@@ -78,7 +78,7 @@ func TestEnableAutoMergeNeedsSettingScopeAndRef(t *testing.T) {
78 _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{AllowedMergeModes: ptr([]domain.MergeMode{domain.MergeModeSquash})})78 _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{AllowedMergeModes: ptr([]domain.MergeMode{domain.MergeModeSquash})})
79 require.NoError(t, err)79 require.NoError(t, err)
80 }80 }
81- _, err := r.mrs.EnableAutoMerge(r.ctx, p, r.repo, mr.Number, ports.EnableAutoMergeParams{})81+ _, err := r.mrs.EnableAutoMerge(r.ctx, p, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: r.head()})
82 require.ErrorIs(t, err, want)82 require.ErrorIs(t, err, want)
83 })83 })
84 }84 }
@@ -90,7 +90,7 @@ func TestEnableAutoMergeRecordsEnablerModeHeadAndPublishes(t *testing.T) {
90 trigger.EXPECT().OnRepoChanged(mock.Anything, r.repo).Return().Twice()90 trigger.EXPECT().OnRepoChanged(mock.Anything, r.repo).Return().Twice()
91 r.mrs.WithAutoMerge(trigger)91 r.mrs.WithAutoMerge(trigger)
92 r.drain()92 r.drain()
93- got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash})93+ got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: r.head()})
94 require.NoError(t, err)94 require.NoError(t, err)
95 assert.Equal(t, &domain.AutoMerge{Mode: domain.MergeModeSquash, HeadID: r.head(), EnabledBy: domain.PusherOf(domain.Owner()), EnablerLabel: domain.AuthorName(domain.Owner()), EnabledAt: now}, got.AutoMerge)95 assert.Equal(t, &domain.AutoMerge{Mode: domain.MergeModeSquash, HeadID: r.head(), EnabledBy: domain.PusherOf(domain.Owner()), EnablerLabel: domain.AuthorName(domain.Owner()), EnabledAt: now}, got.AutoMerge)
96 events := r.drain()96 events := r.drain()
@@ -226,7 +226,7 @@ func TestAutoMergeCancelsOnPermanentMergeFailure(t *testing.T) {
226 for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {226 for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {
227 t.Run(mode.String(), func(t *testing.T) {227 t.Run(mode.String(), func(t *testing.T) {
228 r, mr, checks := autoRig(t)228 r, mr, checks := autoRig(t)
229- _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: mode})229+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: mode, HeadID: r.head()})
230 require.NoError(t, err)230 require.NoError(t, err)
231 head := r.head()231 head := r.head()
232 r.sh("checkout", "main")232 r.sh("checkout", "main")
@@ -351,3 +351,24 @@ func TestAutoMergeRequiresSuccessfulCheck(t *testing.T) {
351 assert.NotNil(t, storedAuto(t, r, mr).AutoMerge)351 assert.NotNil(t, storedAuto(t, r, mr).AutoMerge)
352 }352 }
353 }353 }
354+
355+func TestEnableAutoMergeNeedsTheHeadTheCallerSaw(t *testing.T) {
356+ t.Run("stale head", func(t *testing.T) {
357+ r, mr, _ := autoRig(t)
358+ stale := r.head()
359+ r.commit(author, "later.txt", "later\n", "push after review")
360+ r.push("feat")
361+ r.drain()
362+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{HeadID: stale})
363+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
364+ assert.Contains(t, err.Error(), "branch moved, read again")
365+ assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
366+ assert.Empty(t, r.drain(), "nothing is published")
367+ })
368+ t.Run("missing head", func(t *testing.T) {
369+ r, mr, _ := autoRig(t)
370+ _, err := r.mrs.EnableAutoMerge(r.ctx, root, r.repo, mr.Number, ports.EnableAutoMergeParams{})
371+ require.ErrorIs(t, err, domain.ErrInvalid)
372+ assert.Nil(t, storedAuto(t, r, mr).AutoMerge)
373+ })
374+}
M internal/usecase/mergerequests_auto.go +12 -1
@@ -17,7 +17,8 @@ func (m *MergeRequests) WithAutoMerge(t ports.AutoMergeTrigger) *MergeRequests {
1717 return m
1818 }
1919
20-// EnableAutoMerge records the actor and current head; re-enabling replaces the
20+// EnableAutoMerge records the actor and the head the caller saw, which must be
21+// the current head of the source branch; re-enabling replaces the
2122 // previous intent rather than preserving an older actor's authority.
2223 func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.EnableAutoMergeParams) (domain.MergeRequest, error) {
2324 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {
@@ -30,7 +31,14 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
3031 if mode == "" {
3132 mode = domain.MergeModeFastForward
3233 }
34+ var v domain.Violations
3335 if _, err := domain.ParseMergeMode(mode.String()); err != nil {
36+ v.Add("mode", "is not a merge mode")
37+ }
38+ if params.HeadID.IsZero() {
39+ v.Add("head_id", "is required")
40+ }
41+ if err := v.Err(); err != nil {
3442 return domain.MergeRequest{}, err
3543 }
3644 unlock, err := m.lockRepo(ctx, name)
@@ -70,6 +78,9 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
7078 if head.IsZero() {
7179 return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
7280 }
81+ if head != params.HeadID {
82+ return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
83+ }
7384 mr.UpdatedAt = m.clock.Now().UTC()
7485 mr.AutoMerge = &domain.AutoMerge{Mode: mode, HeadID: head, EnabledBy: domain.PusherOf(p), EnablerLabel: domain.AuthorName(p), EnabledAt: mr.UpdatedAt}
7586 if err := m.store.Update(ctx, mr); errors.Is(err, domain.ErrConflict) {
@@ -17,7 +17,8 @@ func (m *MergeRequests) WithAutoMerge(t ports.AutoMergeTrigger) *MergeRequests {
17 return m17 return m
18 }18 }
19 19
20-// EnableAutoMerge records the actor and current head; re-enabling replaces the20+// EnableAutoMerge records the actor and the head the caller saw, which must be
21+// the current head of the source branch; re-enabling replaces the
21 // previous intent rather than preserving an older actor's authority.22 // previous intent rather than preserving an older actor's authority.
22 func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.EnableAutoMergeParams) (domain.MergeRequest, error) {23 func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.EnableAutoMergeParams) (domain.MergeRequest, error) {
23 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {24 if _, err := m.writable(ctx, p, name, domain.ScopeMRMerge); err != nil {
@@ -30,7 +31,14 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
30 if mode == "" {31 if mode == "" {
31 mode = domain.MergeModeFastForward32 mode = domain.MergeModeFastForward
32 }33 }
34+ var v domain.Violations
33 if _, err := domain.ParseMergeMode(mode.String()); err != nil {35 if _, err := domain.ParseMergeMode(mode.String()); err != nil {
36+ v.Add("mode", "is not a merge mode")
37+ }
38+ if params.HeadID.IsZero() {
39+ v.Add("head_id", "is required")
40+ }
41+ if err := v.Err(); err != nil {
34 return domain.MergeRequest{}, err42 return domain.MergeRequest{}, err
35 }43 }
36 unlock, err := m.lockRepo(ctx, name)44 unlock, err := m.lockRepo(ctx, name)
@@ -70,6 +78,9 @@ func (m *MergeRequests) EnableAutoMerge(ctx context.Context, p domain.Principal,
70 if head.IsZero() {78 if head.IsZero() {
71 return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)79 return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
72 }80 }
81+ if head != params.HeadID {
82+ return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
83+ }
73 mr.UpdatedAt = m.clock.Now().UTC()84 mr.UpdatedAt = m.clock.Now().UTC()
74 mr.AutoMerge = &domain.AutoMerge{Mode: mode, HeadID: head, EnabledBy: domain.PusherOf(p), EnablerLabel: domain.AuthorName(p), EnabledAt: mr.UpdatedAt}85 mr.AutoMerge = &domain.AutoMerge{Mode: mode, HeadID: head, EnabledBy: domain.PusherOf(p), EnablerLabel: domain.AuthorName(p), EnabledAt: mr.UpdatedAt}
75 if err := m.store.Update(ctx, mr); errors.Is(err, domain.ErrConflict) {86 if err := m.store.Update(ctx, mr); errors.Is(err, domain.ErrConflict) {

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