detached f44d044

feat(mr): per-mode merge checks, signed merge commits and side effects after the response

commit
f44d044bbcc4f285d342bfad46ff560c5ea3a985
author
brice · · 2026-10-19 13:30 UTC
committer
brice · · 2026-10-19 14:36 UTC
parent
b292b48
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +477 -87
Split
UnifiedSplit
M internal/app/cli_mr.go +6 -9
@@ -976,10 +976,11 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
976976 "required: give the full id of the head that `ermite mr show <repo> <number>` printed when you\n" +
977977 "reviewed the change. If the branch moved since, the merge is refused: review again. Needs the\n" +
978978 "mr:merge scope.\n\n" +
979- "--delete-branch deletes the source branch after the merge. The merge is refused when the branch\n" +
980- "is the default branch or used by another open merge request. If the deletion fails after the\n" +
981- "merge, the merge stands, the branch is kept and a warning goes to standard error." + jsonMRHelp +
982- "\n--json adds source_branch_deleted (true when the branch was deleted).",
979+ "--delete-branch asks the server to delete the source branch right after the merge, in the\n" +
980+ "background. The merge is refused when the branch is the default branch or used by another open\n" +
981+ "merge request. If the deletion later fails (the branch moved, say), the merge stands and the\n" +
982+ "branch is kept." + jsonMRHelp +
983+ "\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",
983984 Args: cobra.ExactArgs(2),
984985 ValidArgsFunction: completeMRRepoThenNumber(env),
985986 }
@@ -1014,17 +1015,13 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
10141015 }
10151016 m := res.Msg.GetMergeRequest()
10161017 deleted := res.Msg.GetSourceBranchDeleted()
1017- if deleteBranch && !deleted {
1018- _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "ermite: warning: the branch %s was kept (it moved after the merge, or the server cannot delete it)\n",
1019- clean(m.GetSourceBranch()))
1020- }
10211018 if *asJSON {
10221019 return writeJSON(cmd.OutOrStdout(), jsonMerged{jsonMergeRequest: toJSONMergeRequest(m, c.URL()), SourceBranchDeleted: deleted})
10231020 }
10241021 _, err = fmt.Fprintf(cmd.OutOrStdout(), "merged %s!%d into %s (%s) at %s\n", clean(m.GetRepo()), m.GetNumber(),
10251022 clean(m.GetTargetBranch()), modeName(m.GetMergeMode()), shortID(m.GetMergedCommitId()))
10261023 if err == nil && deleted {
1027- _, err = fmt.Fprintf(cmd.OutOrStdout(), "deleted branch %s\n", clean(m.GetSourceBranch()))
1024+ _, err = fmt.Fprintf(cmd.OutOrStdout(), "deleting branch %s (done by the server after the merge)\n", clean(m.GetSourceBranch()))
10281025 }
10291026 return err
10301027 }
@@ -976,10 +976,11 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
976 "required: give the full id of the head that `ermite mr show <repo> <number>` printed when you\n" +976 "required: give the full id of the head that `ermite mr show <repo> <number>` printed when you\n" +
977 "reviewed the change. If the branch moved since, the merge is refused: review again. Needs the\n" +977 "reviewed the change. If the branch moved since, the merge is refused: review again. Needs the\n" +
978 "mr:merge scope.\n\n" +978 "mr:merge scope.\n\n" +
979- "--delete-branch deletes the source branch after the merge. The merge is refused when the branch\n" +979+ "--delete-branch asks the server to delete the source branch right after the merge, in the\n" +
980- "is the default branch or used by another open merge request. If the deletion fails after the\n" +980+ "background. The merge is refused when the branch is the default branch or used by another open\n" +
981- "merge, the merge stands, the branch is kept and a warning goes to standard error." + jsonMRHelp +981+ "merge request. If the deletion later fails (the branch moved, say), the merge stands and the\n" +
982- "\n--json adds source_branch_deleted (true when the branch was deleted).",982+ "branch is kept." + jsonMRHelp +
983+ "\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",
983 Args: cobra.ExactArgs(2),984 Args: cobra.ExactArgs(2),
984 ValidArgsFunction: completeMRRepoThenNumber(env),985 ValidArgsFunction: completeMRRepoThenNumber(env),
985 }986 }
@@ -1014,17 +1015,13 @@ func newMRMergeCmd(env *cliEnv) *cobra.Command {
1014 }1015 }
1015 m := res.Msg.GetMergeRequest()1016 m := res.Msg.GetMergeRequest()
1016 deleted := res.Msg.GetSourceBranchDeleted()1017 deleted := res.Msg.GetSourceBranchDeleted()
1017- if deleteBranch && !deleted {
1018- _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "ermite: warning: the branch %s was kept (it moved after the merge, or the server cannot delete it)\n",
1019- clean(m.GetSourceBranch()))
1020- }
1021 if *asJSON {1018 if *asJSON {
1022 return writeJSON(cmd.OutOrStdout(), jsonMerged{jsonMergeRequest: toJSONMergeRequest(m, c.URL()), SourceBranchDeleted: deleted})1019 return writeJSON(cmd.OutOrStdout(), jsonMerged{jsonMergeRequest: toJSONMergeRequest(m, c.URL()), SourceBranchDeleted: deleted})
1023 }1020 }
1024 _, err = fmt.Fprintf(cmd.OutOrStdout(), "merged %s!%d into %s (%s) at %s\n", clean(m.GetRepo()), m.GetNumber(),1021 _, err = fmt.Fprintf(cmd.OutOrStdout(), "merged %s!%d into %s (%s) at %s\n", clean(m.GetRepo()), m.GetNumber(),
1025 clean(m.GetTargetBranch()), modeName(m.GetMergeMode()), shortID(m.GetMergedCommitId()))1022 clean(m.GetTargetBranch()), modeName(m.GetMergeMode()), shortID(m.GetMergedCommitId()))
1026 if err == nil && deleted {1023 if err == nil && deleted {
1027- _, err = fmt.Fprintf(cmd.OutOrStdout(), "deleted branch %s\n", clean(m.GetSourceBranch()))1024+ _, err = fmt.Fprintf(cmd.OutOrStdout(), "deleting branch %s (done by the server after the merge)\n", clean(m.GetSourceBranch()))
1028 }1025 }
1029 return err1026 return err
1030 }1027 }
M internal/app/cli_mr_test.go +3 -3
@@ -326,7 +326,7 @@ func TestMRMergeDeleteBranch(t *testing.T) {
326326 url := f.serve(t)
327327
328328 res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
329- assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\ndeleted branch feat/x\n", res.stdout)
329+ assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\ndeleting branch feat/x (done by the server after the merge)\n", res.stdout)
330330 assert.Empty(t, res.stderr)
331331 _, reqs := f.snapshot()
332332 assert.True(t, reqs[0].(*ermite.MergeMergeRequestRequest).GetDeleteSourceBranch())
@@ -334,11 +334,11 @@ func TestMRMergeDeleteBranch(t *testing.T) {
334334 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch", "--json").ok(t)
335335 assert.Contains(t, res.stdout, `"source_branch_deleted": true`)
336336
337- // The merge stands when the branch is kept: a warning, no failure.
337+ // Deletion is asynchronous: the CLI makes no claim when the server did not
338+ // request it.
338339 deleted = false
339340 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
340341 assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\n", res.stdout)
341- assert.Contains(t, res.stderr, "ermite: warning: the branch feat/x was kept")
342342
343343 // Without the flag nothing is asked and nothing is said.
344344 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA).ok(t)
@@ -326,7 +326,7 @@ func TestMRMergeDeleteBranch(t *testing.T) {
326 url := f.serve(t)326 url := f.serve(t)
327 327
328 res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)328 res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
329- assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\ndeleted branch feat/x\n", res.stdout)329+ assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\ndeleting branch feat/x (done by the server after the merge)\n", res.stdout)
330 assert.Empty(t, res.stderr)330 assert.Empty(t, res.stderr)
331 _, reqs := f.snapshot()331 _, reqs := f.snapshot()
332 assert.True(t, reqs[0].(*ermite.MergeMergeRequestRequest).GetDeleteSourceBranch())332 assert.True(t, reqs[0].(*ermite.MergeMergeRequestRequest).GetDeleteSourceBranch())
@@ -334,11 +334,11 @@ func TestMRMergeDeleteBranch(t *testing.T) {
334 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch", "--json").ok(t)334 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch", "--json").ok(t)
335 assert.Contains(t, res.stdout, `"source_branch_deleted": true`)335 assert.Contains(t, res.stdout, `"source_branch_deleted": true`)
336 336
337- // The merge stands when the branch is kept: a warning, no failure.337+ // Deletion is asynchronous: the CLI makes no claim when the server did not
338+ // request it.
338 deleted = false339 deleted = false
339 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)340 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
340 assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\n", res.stdout)341 assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\n", res.stdout)
341- assert.Contains(t, res.stderr, "ermite: warning: the branch feat/x was kept")
342 342
343 // Without the flag nothing is asked and nothing is said.343 // Without the flag nothing is asked and nothing is said.
344 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA).ok(t)344 res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA).ok(t)
M internal/app/forge.go +3 -0
@@ -519,6 +519,9 @@ func (f *forge) Close(ctx context.Context) {
519519 if f.hooks != nil {
520520 f.hooks.Close(ctx)
521521 }
522+ if f.MergeRequests != nil {
523+ f.MergeRequests.Wait(ctx) // its side effects use CI and Mergeability
524+ }
522525 if f.CI != nil {
523526 f.CI.Close(ctx)
524527 }
@@ -519,6 +519,9 @@ func (f *forge) Close(ctx context.Context) {
519 if f.hooks != nil {519 if f.hooks != nil {
520 f.hooks.Close(ctx)520 f.hooks.Close(ctx)
521 }521 }
522+ if f.MergeRequests != nil {
523+ f.MergeRequests.Wait(ctx) // its side effects use CI and Mergeability
524+ }
522 if f.CI != nil {525 if f.CI != nil {
523 f.CI.Close(ctx)526 f.CI.Close(ctx)
524 }527 }
M internal/app/mr_api_test.go +2 -1
@@ -375,7 +375,8 @@ func TestMergeDeletesTheSourceOverTheAPI(t *testing.T) {
375375 Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true,
376376 }))
377377 require.NoError(t, err)
378- assert.True(t, res.Msg.GetSourceBranchDeleted())
378+ assert.True(t, res.Msg.GetSourceBranchDeleted(), "requested")
379+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the response
379380 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat"))
380381
381382 // The default branch as a source: refused before anything changes.
@@ -375,7 +375,8 @@ func TestMergeDeletesTheSourceOverTheAPI(t *testing.T) {
375 Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true,375 Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true,
376 }))376 }))
377 require.NoError(t, err)377 require.NoError(t, err)
378- assert.True(t, res.Msg.GetSourceBranchDeleted())378+ assert.True(t, res.Msg.GetSourceBranchDeleted(), "requested")
379+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the response
379 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat"))380 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat"))
380 381
381 // The default branch as a source: refused before anything changes.382 // The default branch as a source: refused before anything changes.
M internal/app/web_mr_test.go +2 -0
@@ -392,6 +392,7 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) {
392392 assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")
393393 res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})
394394 require.Equal(t, http.StatusSeeOther, res.status, res.body)
395+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
395396 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")
396397 merged := r.headOf("main")
397398 assert.NotEqual(t, head, merged, "squash makes a new commit")
@@ -511,6 +512,7 @@ func TestWebMergeDeleteBranchBox(t *testing.T) {
511512 one := b.get("/proj/merge-requests/1", as(token))
512513 res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})
513514 require.Equal(t, http.StatusSeeOther, res.status, res.body)
515+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
514516 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))
515517 done := b.get("/proj/merge-requests/1", nil)
516518 assert.Equal(t, 200, done.status, "the merged page reads without its branch")
@@ -392,6 +392,7 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) {
392 assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")392 assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")
393 res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})393 res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})
394 require.Equal(t, http.StatusSeeOther, res.status, res.body)394 require.Equal(t, http.StatusSeeOther, res.status, res.body)
395+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
395 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")396 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")
396 merged := r.headOf("main")397 merged := r.headOf("main")
397 assert.NotEqual(t, head, merged, "squash makes a new commit")398 assert.NotEqual(t, head, merged, "squash makes a new commit")
@@ -511,6 +512,7 @@ func TestWebMergeDeleteBranchBox(t *testing.T) {
511 one := b.get("/proj/merge-requests/1", as(token))512 one := b.get("/proj/merge-requests/1", as(token))
512 res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})513 res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})
513 require.Equal(t, http.StatusSeeOther, res.status, res.body)514 require.Equal(t, http.StatusSeeOther, res.status, res.body)
515+ r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
514 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))516 assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))
515 done := b.get("/proj/merge-requests/1", nil)517 done := b.get("/proj/merge-requests/1", nil)
516 assert.Equal(t, 200, done.status, "the merged page reads without its branch")518 assert.Equal(t, 200, done.status, "the merged page reads without its branch")
M internal/ports/dto.go +3 -2
@@ -324,8 +324,9 @@ type MergeParams struct {
324324 // MergeResult is a merged request and what happened to its source branch.
325325 type MergeResult struct {
326326 domain.MergeRequest
327- // SourceDeleted is true when DeleteSource was asked and the branch is
328- // gone.
327+ // SourceDeleted is true when DeleteSource was asked: the deletion is
328+ // requested, and done asynchronously after the merge returned (it is
329+ // skipped if the branch moved meanwhile).
329330 SourceDeleted bool
330331 }
331332
@@ -324,8 +324,9 @@ type MergeParams struct {
324 // MergeResult is a merged request and what happened to its source branch.324 // MergeResult is a merged request and what happened to its source branch.
325 type MergeResult struct {325 type MergeResult struct {
326 domain.MergeRequest326 domain.MergeRequest
327- // SourceDeleted is true when DeleteSource was asked and the branch is327+ // SourceDeleted is true when DeleteSource was asked: the deletion is
328- // gone.328+ // requested, and done asynchronously after the merge returned (it is
329+ // skipped if the branch moved meanwhile).
329 SourceDeleted bool330 SourceDeleted bool
330 }331 }
331 332
M internal/usecase/mergerequests.go +151 -65
@@ -60,6 +60,9 @@ type MergeRequests struct {
6060
6161 mu sync.Mutex
6262 locks map[domain.RepoName]chan struct{}
63+
64+ // after tracks the post-merge side effects running off the request path.
65+ after sync.WaitGroup
6366 }
6467
6568 var (
@@ -117,6 +120,21 @@ func (m *MergeRequests) WithVerifier(v ports.CommitVerifier) *MergeRequests {
117120 return m
118121 }
119122
123+// Wait waits for the post-merge side effects still running (source branch
124+// deletion, events, CI and mergeability hooks), or for ctx to end. Call it
125+// before closing what they use.
126+func (m *MergeRequests) Wait(ctx context.Context) {
127+ done := make(chan struct{})
128+ go func() {
129+ m.after.Wait()
130+ close(done)
131+ }()
132+ select {
133+ case <-done:
134+ case <-ctx.Done():
135+ }
136+}
137+
120138 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The
121139 // merges of one repository, and the creation of its merge requests, run one at
122140 // a time. The returned function releases the lock.
@@ -881,30 +899,76 @@ func (m *MergeRequests) MarkRead(ctx context.Context, p domain.Principal, name d
881899 // domain.ErrPreconditionFailed when the request is not open, the mode is not
882900 // allowed by the settings of the repository, the source branch no longer
883901 // points to the head ("branch moved, read again") or is gone, the target is
884-// gone or is not an ancestor of the head (the author rebases and pushes
885-// again), or the branch brings nothing. Checks come with CI (CI-1): there are
886-// none yet, so the setting that lets failing checks block the merge has
887-// nothing to act on.
902+// gone, the mode's own condition does not hold (see below), or the branch
903+// brings nothing. Checks come with CI (CI-1): there are none yet, so the
904+// setting that lets failing checks block the merge has nothing to act on.
888905 //
889-// A fast-forward moves the target to the head, with the old value as a
890-// precondition. A squash writes one commit with the tree of the head, the
891-// target as parent and the author and message of domain.NewSquashCommit,
892-// signed by the instance key, and moves the target to it the same way.
906+// The conditions are always checked with Git, under the lock, never from the
907+// stored mergeability. A fast-forward needs the target to be an ancestor of
908+// the head (the author rebases and pushes again) and moves the target to the
909+// head, with the old value as a precondition. A squash and a merge commit need
910+// the head to merge cleanly into the target (a behind branch is fine). A squash
911+// writes one commit with the merged tree, the target as parent and the author
912+// and message of domain.NewSquashCommit; a merge commit has two parents (the
913+// target, then the head) and the message of domain.NewMergeCommit. Both are
914+// signed by the instance key and move the target the same way.
893915 //
894916 // With DeleteSource the merge is also refused, before anything changes, when
895917 // the source is the default branch or the source or target of another open
896-// request. After the merge the source is deleted if it still points to the
897-// head; the PushEvent then carries the deletion too. A failed deletion is
898-// logged and leaves the branch: the merge stands and SourceDeleted is false.
918+// request.
919+//
920+// Only the merge itself runs before Merge returns, under the lock. Afterwards,
921+// off the request path and without the lock: the deletion of the source branch
922+// (if it still points to the head; a failure is logged and leaves the branch),
923+// MergeRequestMerged then a PushEvent (which carries the deletion too), and
924+// the CI and mergeability hooks. Wait waits for them. SourceDeleted therefore
925+// only says that the deletion was requested.
899926 func (m *MergeRequests) Merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (ports.MergeResult, error) {
900- mr, deleted, err := m.merge(ctx, p, name, number, params)
901- return ports.MergeResult{MergeRequest: mr, SourceDeleted: deleted}, err
927+ mr, err := m.merge(ctx, p, name, number, params)
928+ return ports.MergeResult{MergeRequest: mr, SourceDeleted: err == nil && params.DeleteSource}, err
902929 }
903930
904-func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, bool, error) {
931+// maxConflictPathsShown is how many conflicting paths a refusal lists.
932+const maxConflictPathsShown = 5
933+
934+// checkMode is the authoritative per-mode check of a merge, done with Git. It
935+// returns the tree a squash or merge commit gets (zero for a fast-forward).
936+func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID) (domain.ObjectID, error) {
937+ if mode == domain.MergeModeFastForward {
938+ ok, err := m.git.IsAncestor(ctx, mr.Repo, tip, head)
939+ if err != nil {
940+ return domain.ObjectID{}, err
941+ }
942+ if !ok {
943+ return domain.ObjectID{}, fmt.Errorf("%w: the target has moved: rebase and push again, or merge with squash or a merge commit", domain.ErrPreconditionFailed)
944+ }
945+ return domain.ObjectID{}, nil
946+ }
947+ res, err := m.git.MergeTree(ctx, mr.Repo, tip, head)
948+ if err != nil {
949+ return domain.ObjectID{}, err
950+ }
951+ if !res.Clean {
952+ what := "no common history"
953+ if len(res.ConflictPaths) > 0 {
954+ shown := res.ConflictPaths
955+ if len(shown) > maxConflictPathsShown {
956+ shown = shown[:maxConflictPathsShown]
957+ }
958+ what = strings.Join(shown, ", ")
959+ if len(shown) < len(res.ConflictPaths) || res.Truncated {
960+ what += ", ..."
961+ }
962+ }
963+ return domain.ObjectID{}, fmt.Errorf("%w: conflicts with %s: %s: rebase and push again", domain.ErrPreconditionFailed, mr.Target, what)
964+ }
965+ return res.Tree, nil
966+}
967+
968+func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) {
905969 repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge)
906970 if err != nil {
907- return domain.MergeRequest{}, false, err
971+ return domain.MergeRequest{}, err
908972 }
909973 mode := params.Mode
910974 if mode == "" {
@@ -918,114 +982,131 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
918982 v.Add("head_id", "is required")
919983 }
920984 if err := v.Err(); err != nil {
921- return domain.MergeRequest{}, false, err
985+ return domain.MergeRequest{}, err
922986 }
923987
924988 unlock, err := m.lockRepo(ctx, name)
925989 if err != nil {
926- return domain.MergeRequest{}, false, err
990+ return domain.MergeRequest{}, err
927991 }
928- defer unlock()
992+ var once sync.Once
993+ release := func() { once.Do(unlock) }
994+ defer release()
929995
930996 mr, err := m.store.Get(ctx, name, number)
931997 if err != nil {
932- return domain.MergeRequest{}, false, err
998+ return domain.MergeRequest{}, err
933999 }
9341000 // A merge moves the target and may delete the source: a token limited to
9351001 // branch prefixes may do so only within them.
9361002 if !p.AllowsRef("refs/heads/" + mr.Target) {
937- return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
1003+ return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
9381004 }
9391005 if params.DeleteSource && !p.AllowsRef("refs/heads/"+mr.Source) {
940- return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
1006+ return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
9411007 }
9421008 if !mr.IsOpen() {
943- return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
1009+ return domain.MergeRequest{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
9441010 }
9451011 if mr.IsDraft() {
946- return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
1012+ return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
9471013 }
9481014 // The settings are read again under the lock: a change applies at once.
9491015 repo, err = m.meta.Get(ctx, name)
9501016 if err != nil {
9511017 if errors.Is(err, domain.ErrNotFound) {
952- return domain.MergeRequest{}, false, err // deleted meanwhile
1018+ return domain.MergeRequest{}, err // deleted meanwhile
9531019 }
954- return domain.MergeRequest{}, false, fmt.Errorf("get repository %q: %w", name, err)
1020+ return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err)
9551021 }
9561022 settings := repo.Settings
9571023 if settings.IsZero() {
9581024 settings = domain.DefaultRepoSettings()
9591025 }
9601026 if !settings.Allows(mode) {
961- return domain.MergeRequest{}, false, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
1027+ return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
9621028 }
9631029
9641030 head, err := m.branchOrZero(ctx, name, mr.Source)
9651031 switch {
9661032 case err != nil:
967- return domain.MergeRequest{}, false, err
1033+ return domain.MergeRequest{}, err
9681034 case head.IsZero():
969- return domain.MergeRequest{}, false, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
1035+ return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
9701036 case head != params.HeadID:
971- return domain.MergeRequest{}, false, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
1037+ return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
9721038 }
9731039 tip, err := m.branchOrZero(ctx, name, mr.Target)
9741040 switch {
9751041 case err != nil:
976- return domain.MergeRequest{}, false, err
1042+ return domain.MergeRequest{}, err
9771043 case tip.IsZero():
978- return domain.MergeRequest{}, false, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
1044+ return domain.MergeRequest{}, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
9791045 case tip == head:
980- return domain.MergeRequest{}, false, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
1046+ return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
9811047 }
982- switch ok, err := m.git.IsAncestor(ctx, name, tip, head); {
983- case err != nil:
984- return domain.MergeRequest{}, false, err
985- case !ok:
986- return domain.MergeRequest{}, false, fmt.Errorf("%w: %q is not an ancestor of %q: rebase it on %q and push again", domain.ErrPreconditionFailed, mr.Target, mr.Source, mr.Target)
1048+ tree, err := m.checkMode(ctx, mr, mode, tip, head)
1049+ if err != nil {
1050+ return domain.MergeRequest{}, err
9871051 }
9881052 if settings.FailingChecksBlock {
9891053 if err := m.checksAllow(ctx, name, head); err != nil {
990- return domain.MergeRequest{}, false, err
1054+ return domain.MergeRequest{}, err
9911055 }
9921056 }
9931057 if params.DeleteSource {
9941058 if err := m.checkDeletable(ctx, name, mr); err != nil {
995- return domain.MergeRequest{}, false, err
1059+ return domain.MergeRequest{}, err
9961060 }
9971061 }
9981062
999- merged, err := m.moveTarget(ctx, mr, mode, tip, head, p)
1063+ merged, err := m.moveTarget(ctx, mr, mode, tip, head, tree, p)
10001064 if err != nil {
1001- return domain.MergeRequest{}, false, err
1065+ return domain.MergeRequest{}, err
10021066 }
10031067 m.recordPush(ctx, name) // the target moved, whatever happens next
10041068 mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged)
10051069 if err != nil {
1006- return domain.MergeRequest{}, false, err
1070+ return domain.MergeRequest{}, err
10071071 }
10081072 log := zerowrap.FromCtx(ctx)
10091073 log.Info().Str("repo", name.String()).Int64("number", number).Str("mode", mode.String()).
10101074 Str("target", mr.Target).Str("commit", merged.String()).
10111075 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request merged")
1076+
1077+ // The merge is done and recorded. The rest runs without the lock and after
1078+ // the caller got its answer.
1079+ release()
1080+ bg := context.WithoutCancel(ctx)
1081+ m.after.Add(1)
1082+ go func() {
1083+ defer m.after.Done()
1084+ m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
1085+ }()
1086+ return mr, nil
1087+}
1088+
1089+// afterMerge is what follows a merge off the request path: errors are logged,
1090+// never returned. mr is the merged request, tip the old target and merged the
1091+// new one.
1092+func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
1093+ name := repo.Name
1094+ log := zerowrap.FromCtx(ctx)
10121095 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
1013- deleted := false
1014- if params.DeleteSource {
1015- // The merge is done and recorded: a failure here keeps the branch. Who
1016- // pushed the source goes first (the branch is gone either way for a
1096+ if deleteSource {
1097+ // Who pushed the source goes first (the branch is gone either way for a
10171098 // schedule; a kept branch merely has no record: not trusted).
1018- if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}}, "", domain.CIPusher{}, false); err != nil {
1099+ del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
1100+ if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
10191101 log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
10201102 }
1021- switch err := m.git.DeleteBranch(context.WithoutCancel(ctx), name, mr.Source, head); {
1103+ switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
10221104 case err != nil:
1023- log.Warn().Err(err).Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
1105+ log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
10241106 Msg("source branch kept after the merge")
10251107 default:
1026- deleted = true
1027- updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})
1028- log.Info().Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
1108+ updates = append(updates, del)
1109+ log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
10291110 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
10301111 }
10311112 }
@@ -1034,19 +1115,18 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
10341115 m.bus.Publish(ctx, domain.PushEvent{
10351116 Repo: name, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, Updates: updates,
10361117 })
1037- if m.mergeab != nil {
1038- m.mergeab.OnClosed(ctx, mr)
1039- m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
1040- }
10411118 if m.pulls != nil {
1042- m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
1119+ m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: mr.Number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
10431120 }
10441121 if m.ci != nil {
10451122 // GitHub runs the "push" workflows of a merge, with the owner as actor;
10461123 // the run is trusted with what the merging principal may read.
10471124 m.ci.OnMerge(ctx, name, updates, p)
10481125 }
1049- return mr, deleted, nil
1126+ if m.mergeab != nil {
1127+ m.mergeab.OnClosed(ctx, mr)
1128+ m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
1129+ }
10501130 }
10511131
10521132 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a
@@ -1139,10 +1219,20 @@ func (m *MergeRequests) recordPush(ctx context.Context, name domain.RepoName) {
11391219 }
11401220
11411221 // moveTarget moves the target branch from tip to the head (fast-forward) or to
1142-// a new signed squash commit, and returns the new tip.
1143-func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
1222+// a new signed squash or merge commit with the given tree, and returns the new tip.
1223+func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head, tree domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
11441224 newTip := head
1145- if mode == domain.MergeModeSquash {
1225+ switch mode {
1226+ case domain.MergeModeMerge:
1227+ commit := domain.NewMergeCommit(mr, tree, tip, head, m.committer, m.clock.Now())
1228+ armored, err := m.signer.Sign(commit.Payload())
1229+ if err != nil {
1230+ return domain.ObjectID{}, fmt.Errorf("sign the merge commit of !%d in %q: %w", mr.Number, mr.Repo, err)
1231+ }
1232+ if newTip, err = m.git.WriteCommit(ctx, mr.Repo, commit.Signed(armored)); err != nil {
1233+ return domain.ObjectID{}, err
1234+ }
1235+ case domain.MergeModeSquash:
11461236 commits, more, err := m.git.ListRange(ctx, mr.Repo, tip, head, domain.MaxSquashCommits)
11471237 if err != nil {
11481238 return domain.ObjectID{}, err
@@ -1150,10 +1240,6 @@ func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest,
11501240 if more {
11511241 return domain.ObjectID{}, fmt.Errorf("%w: the branch has more than %d commits to squash", domain.ErrPreconditionFailed, domain.MaxSquashCommits)
11521242 }
1153- tree, err := m.git.CommitTree(ctx, mr.Repo, head)
1154- if err != nil {
1155- return domain.ObjectID{}, err
1156- }
11571243 commit := domain.NewSquashCommit(mr, tree, tip, commits, m.committer, m.clock.Now())
11581244 armored, err := m.signer.Sign(commit.Payload())
11591245 if err != nil {
@@ -60,6 +60,9 @@ type MergeRequests struct {
60 60
61 mu sync.Mutex61 mu sync.Mutex
62 locks map[domain.RepoName]chan struct{}62 locks map[domain.RepoName]chan struct{}
63+
64+ // after tracks the post-merge side effects running off the request path.
65+ after sync.WaitGroup
63 }66 }
64 67
65 var (68 var (
@@ -117,6 +120,21 @@ func (m *MergeRequests) WithVerifier(v ports.CommitVerifier) *MergeRequests {
117 return m120 return m
118 }121 }
119 122
123+// Wait waits for the post-merge side effects still running (source branch
124+// deletion, events, CI and mergeability hooks), or for ctx to end. Call it
125+// before closing what they use.
126+func (m *MergeRequests) Wait(ctx context.Context) {
127+ done := make(chan struct{})
128+ go func() {
129+ m.after.Wait()
130+ close(done)
131+ }()
132+ select {
133+ case <-done:
134+ case <-ctx.Done():
135+ }
136+}
137+
120 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The138 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The
121 // merges of one repository, and the creation of its merge requests, run one at139 // merges of one repository, and the creation of its merge requests, run one at
122 // a time. The returned function releases the lock.140 // a time. The returned function releases the lock.
@@ -881,30 +899,76 @@ func (m *MergeRequests) MarkRead(ctx context.Context, p domain.Principal, name d
881 // domain.ErrPreconditionFailed when the request is not open, the mode is not899 // domain.ErrPreconditionFailed when the request is not open, the mode is not
882 // allowed by the settings of the repository, the source branch no longer900 // allowed by the settings of the repository, the source branch no longer
883 // points to the head ("branch moved, read again") or is gone, the target is901 // points to the head ("branch moved, read again") or is gone, the target is
884-// gone or is not an ancestor of the head (the author rebases and pushes902+// gone, the mode's own condition does not hold (see below), or the branch
885-// again), or the branch brings nothing. Checks come with CI (CI-1): there are903+// brings nothing. Checks come with CI (CI-1): there are none yet, so the
886-// none yet, so the setting that lets failing checks block the merge has904+// setting that lets failing checks block the merge has nothing to act on.
887-// nothing to act on.
888 //905 //
889-// A fast-forward moves the target to the head, with the old value as a906+// The conditions are always checked with Git, under the lock, never from the
890-// precondition. A squash writes one commit with the tree of the head, the907+// stored mergeability. A fast-forward needs the target to be an ancestor of
891-// target as parent and the author and message of domain.NewSquashCommit,908+// the head (the author rebases and pushes again) and moves the target to the
892-// signed by the instance key, and moves the target to it the same way.909+// head, with the old value as a precondition. A squash and a merge commit need
910+// the head to merge cleanly into the target (a behind branch is fine). A squash
911+// writes one commit with the merged tree, the target as parent and the author
912+// and message of domain.NewSquashCommit; a merge commit has two parents (the
913+// target, then the head) and the message of domain.NewMergeCommit. Both are
914+// signed by the instance key and move the target the same way.
893 //915 //
894 // With DeleteSource the merge is also refused, before anything changes, when916 // With DeleteSource the merge is also refused, before anything changes, when
895 // the source is the default branch or the source or target of another open917 // the source is the default branch or the source or target of another open
896-// request. After the merge the source is deleted if it still points to the918+// request.
897-// head; the PushEvent then carries the deletion too. A failed deletion is919+//
898-// logged and leaves the branch: the merge stands and SourceDeleted is false.920+// Only the merge itself runs before Merge returns, under the lock. Afterwards,
921+// off the request path and without the lock: the deletion of the source branch
922+// (if it still points to the head; a failure is logged and leaves the branch),
923+// MergeRequestMerged then a PushEvent (which carries the deletion too), and
924+// the CI and mergeability hooks. Wait waits for them. SourceDeleted therefore
925+// only says that the deletion was requested.
899 func (m *MergeRequests) Merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (ports.MergeResult, error) {926 func (m *MergeRequests) Merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (ports.MergeResult, error) {
900- mr, deleted, err := m.merge(ctx, p, name, number, params)927+ mr, err := m.merge(ctx, p, name, number, params)
901- return ports.MergeResult{MergeRequest: mr, SourceDeleted: deleted}, err928+ return ports.MergeResult{MergeRequest: mr, SourceDeleted: err == nil && params.DeleteSource}, err
902 }929 }
903 930
904-func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, bool, error) {931+// maxConflictPathsShown is how many conflicting paths a refusal lists.
932+const maxConflictPathsShown = 5
933+
934+// checkMode is the authoritative per-mode check of a merge, done with Git. It
935+// returns the tree a squash or merge commit gets (zero for a fast-forward).
936+func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID) (domain.ObjectID, error) {
937+ if mode == domain.MergeModeFastForward {
938+ ok, err := m.git.IsAncestor(ctx, mr.Repo, tip, head)
939+ if err != nil {
940+ return domain.ObjectID{}, err
941+ }
942+ if !ok {
943+ return domain.ObjectID{}, fmt.Errorf("%w: the target has moved: rebase and push again, or merge with squash or a merge commit", domain.ErrPreconditionFailed)
944+ }
945+ return domain.ObjectID{}, nil
946+ }
947+ res, err := m.git.MergeTree(ctx, mr.Repo, tip, head)
948+ if err != nil {
949+ return domain.ObjectID{}, err
950+ }
951+ if !res.Clean {
952+ what := "no common history"
953+ if len(res.ConflictPaths) > 0 {
954+ shown := res.ConflictPaths
955+ if len(shown) > maxConflictPathsShown {
956+ shown = shown[:maxConflictPathsShown]
957+ }
958+ what = strings.Join(shown, ", ")
959+ if len(shown) < len(res.ConflictPaths) || res.Truncated {
960+ what += ", ..."
961+ }
962+ }
963+ return domain.ObjectID{}, fmt.Errorf("%w: conflicts with %s: %s: rebase and push again", domain.ErrPreconditionFailed, mr.Target, what)
964+ }
965+ return res.Tree, nil
966+}
967+
968+func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) {
905 repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge)969 repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge)
906 if err != nil {970 if err != nil {
907- return domain.MergeRequest{}, false, err971+ return domain.MergeRequest{}, err
908 }972 }
909 mode := params.Mode973 mode := params.Mode
910 if mode == "" {974 if mode == "" {
@@ -918,114 +982,131 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
918 v.Add("head_id", "is required")982 v.Add("head_id", "is required")
919 }983 }
920 if err := v.Err(); err != nil {984 if err := v.Err(); err != nil {
921- return domain.MergeRequest{}, false, err985+ return domain.MergeRequest{}, err
922 }986 }
923 987
924 unlock, err := m.lockRepo(ctx, name)988 unlock, err := m.lockRepo(ctx, name)
925 if err != nil {989 if err != nil {
926- return domain.MergeRequest{}, false, err990+ return domain.MergeRequest{}, err
927 }991 }
928- defer unlock()992+ var once sync.Once
993+ release := func() { once.Do(unlock) }
994+ defer release()
929 995
930 mr, err := m.store.Get(ctx, name, number)996 mr, err := m.store.Get(ctx, name, number)
931 if err != nil {997 if err != nil {
932- return domain.MergeRequest{}, false, err998+ return domain.MergeRequest{}, err
933 }999 }
934 // A merge moves the target and may delete the source: a token limited to1000 // A merge moves the target and may delete the source: a token limited to
935 // branch prefixes may do so only within them.1001 // branch prefixes may do so only within them.
936 if !p.AllowsRef("refs/heads/" + mr.Target) {1002 if !p.AllowsRef("refs/heads/" + mr.Target) {
937- return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)1003+ return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
938 }1004 }
939 if params.DeleteSource && !p.AllowsRef("refs/heads/"+mr.Source) {1005 if params.DeleteSource && !p.AllowsRef("refs/heads/"+mr.Source) {
940- return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)1006+ return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
941 }1007 }
942 if !mr.IsOpen() {1008 if !mr.IsOpen() {
943- return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)1009+ return domain.MergeRequest{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
944 }1010 }
945 if mr.IsDraft() {1011 if mr.IsDraft() {
946- return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)1012+ return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
947 }1013 }
948 // The settings are read again under the lock: a change applies at once.1014 // The settings are read again under the lock: a change applies at once.
949 repo, err = m.meta.Get(ctx, name)1015 repo, err = m.meta.Get(ctx, name)
950 if err != nil {1016 if err != nil {
951 if errors.Is(err, domain.ErrNotFound) {1017 if errors.Is(err, domain.ErrNotFound) {
952- return domain.MergeRequest{}, false, err // deleted meanwhile1018+ return domain.MergeRequest{}, err // deleted meanwhile
953 }1019 }
954- return domain.MergeRequest{}, false, fmt.Errorf("get repository %q: %w", name, err)1020+ return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err)
955 }1021 }
956 settings := repo.Settings1022 settings := repo.Settings
957 if settings.IsZero() {1023 if settings.IsZero() {
958 settings = domain.DefaultRepoSettings()1024 settings = domain.DefaultRepoSettings()
959 }1025 }
960 if !settings.Allows(mode) {1026 if !settings.Allows(mode) {
961- return domain.MergeRequest{}, false, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)1027+ return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
962 }1028 }
963 1029
964 head, err := m.branchOrZero(ctx, name, mr.Source)1030 head, err := m.branchOrZero(ctx, name, mr.Source)
965 switch {1031 switch {
966 case err != nil:1032 case err != nil:
967- return domain.MergeRequest{}, false, err1033+ return domain.MergeRequest{}, err
968 case head.IsZero():1034 case head.IsZero():
969- return domain.MergeRequest{}, false, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)1035+ return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
970 case head != params.HeadID:1036 case head != params.HeadID:
971- return domain.MergeRequest{}, false, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)1037+ return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
972 }1038 }
973 tip, err := m.branchOrZero(ctx, name, mr.Target)1039 tip, err := m.branchOrZero(ctx, name, mr.Target)
974 switch {1040 switch {
975 case err != nil:1041 case err != nil:
976- return domain.MergeRequest{}, false, err1042+ return domain.MergeRequest{}, err
977 case tip.IsZero():1043 case tip.IsZero():
978- return domain.MergeRequest{}, false, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)1044+ return domain.MergeRequest{}, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
979 case tip == head:1045 case tip == head:
980- return domain.MergeRequest{}, false, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)1046+ return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
981 }1047 }
982- switch ok, err := m.git.IsAncestor(ctx, name, tip, head); {1048+ tree, err := m.checkMode(ctx, mr, mode, tip, head)
983- case err != nil:1049+ if err != nil {
984- return domain.MergeRequest{}, false, err1050+ return domain.MergeRequest{}, err
985- case !ok:
986- return domain.MergeRequest{}, false, fmt.Errorf("%w: %q is not an ancestor of %q: rebase it on %q and push again", domain.ErrPreconditionFailed, mr.Target, mr.Source, mr.Target)
987 }1051 }
988 if settings.FailingChecksBlock {1052 if settings.FailingChecksBlock {
989 if err := m.checksAllow(ctx, name, head); err != nil {1053 if err := m.checksAllow(ctx, name, head); err != nil {
990- return domain.MergeRequest{}, false, err1054+ return domain.MergeRequest{}, err
991 }1055 }
992 }1056 }
993 if params.DeleteSource {1057 if params.DeleteSource {
994 if err := m.checkDeletable(ctx, name, mr); err != nil {1058 if err := m.checkDeletable(ctx, name, mr); err != nil {
995- return domain.MergeRequest{}, false, err1059+ return domain.MergeRequest{}, err
996 }1060 }
997 }1061 }
998 1062
999- merged, err := m.moveTarget(ctx, mr, mode, tip, head, p)1063+ merged, err := m.moveTarget(ctx, mr, mode, tip, head, tree, p)
1000 if err != nil {1064 if err != nil {
1001- return domain.MergeRequest{}, false, err1065+ return domain.MergeRequest{}, err
1002 }1066 }
1003 m.recordPush(ctx, name) // the target moved, whatever happens next1067 m.recordPush(ctx, name) // the target moved, whatever happens next
1004 mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged)1068 mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged)
1005 if err != nil {1069 if err != nil {
1006- return domain.MergeRequest{}, false, err1070+ return domain.MergeRequest{}, err
1007 }1071 }
1008 log := zerowrap.FromCtx(ctx)1072 log := zerowrap.FromCtx(ctx)
1009 log.Info().Str("repo", name.String()).Int64("number", number).Str("mode", mode.String()).1073 log.Info().Str("repo", name.String()).Int64("number", number).Str("mode", mode.String()).
1010 Str("target", mr.Target).Str("commit", merged.String()).1074 Str("target", mr.Target).Str("commit", merged.String()).
1011 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request merged")1075 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request merged")
1076+
1077+ // The merge is done and recorded. The rest runs without the lock and after
1078+ // the caller got its answer.
1079+ release()
1080+ bg := context.WithoutCancel(ctx)
1081+ m.after.Add(1)
1082+ go func() {
1083+ defer m.after.Done()
1084+ m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
1085+ }()
1086+ return mr, nil
1087+}
1088+
1089+// afterMerge is what follows a merge off the request path: errors are logged,
1090+// never returned. mr is the merged request, tip the old target and merged the
1091+// new one.
1092+func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
1093+ name := repo.Name
1094+ log := zerowrap.FromCtx(ctx)
1012 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}1095 updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
1013- deleted := false1096+ if deleteSource {
1014- if params.DeleteSource {1097+ // Who pushed the source goes first (the branch is gone either way for a
1015- // The merge is done and recorded: a failure here keeps the branch. Who
1016- // pushed the source goes first (the branch is gone either way for a
1017 // schedule; a kept branch merely has no record: not trusted).1098 // schedule; a kept branch merely has no record: not trusted).
1018- if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}}, "", domain.CIPusher{}, false); err != nil {1099+ del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
1100+ if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
1019 log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")1101 log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
1020 }1102 }
1021- switch err := m.git.DeleteBranch(context.WithoutCancel(ctx), name, mr.Source, head); {1103+ switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
1022 case err != nil:1104 case err != nil:
1023- log.Warn().Err(err).Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).1105+ log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1024 Msg("source branch kept after the merge")1106 Msg("source branch kept after the merge")
1025 default:1107 default:
1026- deleted = true1108+ updates = append(updates, del)
1027- updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})1109+ log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1028- log.Info().Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
1029 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")1110 Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1030 }1111 }
1031 }1112 }
@@ -1034,19 +1115,18 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
1034 m.bus.Publish(ctx, domain.PushEvent{1115 m.bus.Publish(ctx, domain.PushEvent{
1035 Repo: name, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, Updates: updates,1116 Repo: name, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, Updates: updates,
1036 })1117 })
1037- if m.mergeab != nil {
1038- m.mergeab.OnClosed(ctx, mr)
1039- m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
1040- }
1041 if m.pulls != nil {1118 if m.pulls != nil {
1042- m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)1119+ m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: mr.Number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
1043 }1120 }
1044 if m.ci != nil {1121 if m.ci != nil {
1045 // GitHub runs the "push" workflows of a merge, with the owner as actor;1122 // GitHub runs the "push" workflows of a merge, with the owner as actor;
1046 // the run is trusted with what the merging principal may read.1123 // the run is trusted with what the merging principal may read.
1047 m.ci.OnMerge(ctx, name, updates, p)1124 m.ci.OnMerge(ctx, name, updates, p)
1048 }1125 }
1049- return mr, deleted, nil1126+ if m.mergeab != nil {
1127+ m.mergeab.OnClosed(ctx, mr)
1128+ m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
1129+ }
1050 }1130 }
1051 1131
1052 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a1132 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a
@@ -1139,10 +1219,20 @@ func (m *MergeRequests) recordPush(ctx context.Context, name domain.RepoName) {
1139 }1219 }
1140 1220
1141 // moveTarget moves the target branch from tip to the head (fast-forward) or to1221 // moveTarget moves the target branch from tip to the head (fast-forward) or to
1142-// a new signed squash commit, and returns the new tip.1222+// a new signed squash or merge commit with the given tree, and returns the new tip.
1143-func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {1223+func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head, tree domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
1144 newTip := head1224 newTip := head
1145- if mode == domain.MergeModeSquash {1225+ switch mode {
1226+ case domain.MergeModeMerge:
1227+ commit := domain.NewMergeCommit(mr, tree, tip, head, m.committer, m.clock.Now())
1228+ armored, err := m.signer.Sign(commit.Payload())
1229+ if err != nil {
1230+ return domain.ObjectID{}, fmt.Errorf("sign the merge commit of !%d in %q: %w", mr.Number, mr.Repo, err)
1231+ }
1232+ if newTip, err = m.git.WriteCommit(ctx, mr.Repo, commit.Signed(armored)); err != nil {
1233+ return domain.ObjectID{}, err
1234+ }
1235+ case domain.MergeModeSquash:
1146 commits, more, err := m.git.ListRange(ctx, mr.Repo, tip, head, domain.MaxSquashCommits)1236 commits, more, err := m.git.ListRange(ctx, mr.Repo, tip, head, domain.MaxSquashCommits)
1147 if err != nil {1237 if err != nil {
1148 return domain.ObjectID{}, err1238 return domain.ObjectID{}, err
@@ -1150,10 +1240,6 @@ func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest,
1150 if more {1240 if more {
1151 return domain.ObjectID{}, fmt.Errorf("%w: the branch has more than %d commits to squash", domain.ErrPreconditionFailed, domain.MaxSquashCommits)1241 return domain.ObjectID{}, fmt.Errorf("%w: the branch has more than %d commits to squash", domain.ErrPreconditionFailed, domain.MaxSquashCommits)
1152 }1242 }
1153- tree, err := m.git.CommitTree(ctx, mr.Repo, head)
1154- if err != nil {
1155- return domain.ObjectID{}, err
1156- }
1157 commit := domain.NewSquashCommit(mr, tree, tip, commits, m.committer, m.clock.Now())1243 commit := domain.NewSquashCommit(mr, tree, tip, commits, m.committer, m.clock.Now())
1158 armored, err := m.signer.Sign(commit.Payload())1244 armored, err := m.signer.Sign(commit.Payload())
1159 if err != nil {1245 if err != nil {
M internal/usecase/mergerequests_delete_test.go +3 -1
@@ -31,6 +31,7 @@ func TestMergeDeletesTheSourceBranch(t *testing.T) {
3131 require.NoError(t, err)
3232 assert.True(t, res.SourceDeleted)
3333 assert.Equal(t, domain.MRStateMerged, res.State)
34+ r.mrs.Wait(r.ctx) // the deletion happens after Merge returned
3435 _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
3536 assert.ErrorIs(t, err, domain.ErrNotFound, "the branch is gone")
3637
@@ -192,8 +193,9 @@ func TestMergeKeepsASourceThatMovedAfterTheMerge(t *testing.T) {
192193
193194 res, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
194195 require.NoError(t, err, "the merge stands")
195- assert.False(t, res.SourceDeleted)
196+ assert.True(t, res.SourceDeleted, "the deletion was requested")
196197 assert.Equal(t, domain.MRStateMerged, res.State)
198+ mrs.Wait(r.ctx)
197199 assert.Equal(t, head, r.branchHead("main"))
198200 assert.Equal(t, moved, r.branchHead("feat"), "the late commit is kept")
199201 evs := r.drain()
@@ -31,6 +31,7 @@ func TestMergeDeletesTheSourceBranch(t *testing.T) {
31 require.NoError(t, err)31 require.NoError(t, err)
32 assert.True(t, res.SourceDeleted)32 assert.True(t, res.SourceDeleted)
33 assert.Equal(t, domain.MRStateMerged, res.State)33 assert.Equal(t, domain.MRStateMerged, res.State)
34+ r.mrs.Wait(r.ctx) // the deletion happens after Merge returned
34 _, err = r.git.BranchHead(r.ctx, r.repo, "feat")35 _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
35 assert.ErrorIs(t, err, domain.ErrNotFound, "the branch is gone")36 assert.ErrorIs(t, err, domain.ErrNotFound, "the branch is gone")
36 37
@@ -192,8 +193,9 @@ func TestMergeKeepsASourceThatMovedAfterTheMerge(t *testing.T) {
192 193
193 res, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})194 res, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
194 require.NoError(t, err, "the merge stands")195 require.NoError(t, err, "the merge stands")
195- assert.False(t, res.SourceDeleted)196+ assert.True(t, res.SourceDeleted, "the deletion was requested")
196 assert.Equal(t, domain.MRStateMerged, res.State)197 assert.Equal(t, domain.MRStateMerged, res.State)
198+ mrs.Wait(r.ctx)
197 assert.Equal(t, head, r.branchHead("main"))199 assert.Equal(t, head, r.branchHead("main"))
198 assert.Equal(t, moved, r.branchHead("feat"), "the late commit is kept")200 assert.Equal(t, moved, r.branchHead("feat"), "the late commit is kept")
199 evs := r.drain()201 evs := r.drain()
M internal/usecase/mergerequests_integration_test.go +11 -6
@@ -207,6 +207,7 @@ func (r *mrRig) open(branch string) domain.MergeRequest {
207207 // subscriber before Publish returns: whatever a returned call published is
208208 // already in the channel.
209209 func (r *mrRig) drain() []domain.Event {
210+ r.mrs.Wait(r.ctx) // the events of a merge are published after it returned
210211 var out []domain.Event
211212 for {
212213 select {
@@ -959,11 +960,10 @@ func TestMergeRefusals(t *testing.T) {
959960 r.push("main")
960961 tip := r.branchHead("main")
961962
962- for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {
963- _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
964- require.ErrorIs(t, err, domain.ErrPreconditionFailed, mode)
965- assert.Contains(t, err.Error(), "rebase", mode)
966- }
963+ // Only a fast-forward needs the target to be an ancestor.
964+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward})
965+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
966+ assert.Contains(t, err.Error(), "the target has moved: rebase and push again")
967967 assert.Equal(t, tip, r.branchHead("main"))
968968 d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
969969 require.NoError(t, err)
@@ -1047,8 +1047,13 @@ func TestConcurrentMergesSerialize(t *testing.T) {
10471047 const n = 6
10481048 heads := map[int64]domain.ObjectID{}
10491049 for i := range n {
1050+ // Every branch writes the same file differently: once one is merged the
1051+ // others neither fast-forward nor merge cleanly.
10501052 b := fmt.Sprintf("f%d", i)
1051- heads[r.openFeature(b).Number] = r.branchHead(b)
1053+ r.sh("checkout", "-q", "-B", b, "main")
1054+ r.commit(author, "same.txt", b+"\n", "add "+b)
1055+ r.push(b)
1056+ heads[r.open(b).Number] = r.branchHead(b)
10521057 }
10531058
10541059 type outcome struct {
@@ -207,6 +207,7 @@ func (r *mrRig) open(branch string) domain.MergeRequest {
207 // subscriber before Publish returns: whatever a returned call published is207 // subscriber before Publish returns: whatever a returned call published is
208 // already in the channel.208 // already in the channel.
209 func (r *mrRig) drain() []domain.Event {209 func (r *mrRig) drain() []domain.Event {
210+ r.mrs.Wait(r.ctx) // the events of a merge are published after it returned
210 var out []domain.Event211 var out []domain.Event
211 for {212 for {
212 select {213 select {
@@ -959,11 +960,10 @@ func TestMergeRefusals(t *testing.T) {
959 r.push("main")960 r.push("main")
960 tip := r.branchHead("main")961 tip := r.branchHead("main")
961 962
962- for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {963+ // Only a fast-forward needs the target to be an ancestor.
963- _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})964+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward})
964- require.ErrorIs(t, err, domain.ErrPreconditionFailed, mode)965+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
965- assert.Contains(t, err.Error(), "rebase", mode)966+ assert.Contains(t, err.Error(), "the target has moved: rebase and push again")
966- }
967 assert.Equal(t, tip, r.branchHead("main"))967 assert.Equal(t, tip, r.branchHead("main"))
968 d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})968 d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
969 require.NoError(t, err)969 require.NoError(t, err)
@@ -1047,8 +1047,13 @@ func TestConcurrentMergesSerialize(t *testing.T) {
1047 const n = 61047 const n = 6
1048 heads := map[int64]domain.ObjectID{}1048 heads := map[int64]domain.ObjectID{}
1049 for i := range n {1049 for i := range n {
1050+ // Every branch writes the same file differently: once one is merged the
1051+ // others neither fast-forward nor merge cleanly.
1050 b := fmt.Sprintf("f%d", i)1052 b := fmt.Sprintf("f%d", i)
1051- heads[r.openFeature(b).Number] = r.branchHead(b)1053+ r.sh("checkout", "-q", "-B", b, "main")
1054+ r.commit(author, "same.txt", b+"\n", "add "+b)
1055+ r.push(b)
1056+ heads[r.open(b).Number] = r.branchHead(b)
1052 }1057 }
1053 1058
1054 type outcome struct {1059 type outcome struct {
A internal/usecase/mergerequests_merge_test.go +293 -0
@@ -0,0 +1,293 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase_test
4+
5+import (
6+ "bytes"
7+ "context"
8+ "crypto/ed25519"
9+ "strings"
10+ "sync"
11+ "testing"
12+ "time"
13+
14+ "github.com/stretchr/testify/assert"
15+ "github.com/stretchr/testify/require"
16+
17+ "bnema.dev/ermite/internal/adapters/sshsig"
18+ "bnema.dev/ermite/internal/domain"
19+ "bnema.dev/ermite/internal/ports"
20+ "bnema.dev/ermite/internal/usecase"
21+)
22+
23+// gateHooks is a CI and a pull_request trigger that block on a channel, to
24+// tell what runs before Merge returns.
25+type gateHooks struct {
26+ release chan struct{}
27+ mu sync.Mutex
28+ calls []string
29+}
30+
31+func newGateHooks() *gateHooks { return &gateHooks{release: make(chan struct{})} }
32+
33+func (g *gateHooks) note(s string) {
34+ <-g.release
35+ g.mu.Lock()
36+ defer g.mu.Unlock()
37+ g.calls = append(g.calls, s)
38+}
39+
40+func (g *gateHooks) ran() []string {
41+ g.mu.Lock()
42+ defer g.mu.Unlock()
43+ return append([]string(nil), g.calls...)
44+}
45+
46+func (g *gateHooks) OnPush(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {}
47+
48+func (g *gateHooks) OnMerge(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {
49+ g.note("ci")
50+}
51+
52+// OnPullRequest blocks for the closing of a request only: Create reports the
53+// opening on the request path.
54+func (g *gateHooks) OnPullRequest(_ context.Context, _ domain.RepoName, ev ports.PullRequestEvent, _ domain.Principal) {
55+ if ev.Action == domain.PullClosed {
56+ g.note("pulls")
57+ }
58+}
59+
60+// gated builds merge request use cases over the rig whose CI and pull hooks
61+// block until g.release is closed.
62+func (r *mrRig) gated(g *gateHooks) *usecase.MergeRequests {
63+ signer, err := sshsig.New(bytes.Repeat([]byte{9}, ed25519.SeedSize))
64+ require.NoError(r.t, err)
65+ return usecase.NewMergeRequests(usecase.MergeRequestsConfig{
66+ Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Signer: signer,
67+ Committer: domain.Identity{Name: "ermite", Email: "ermite@git.example.org"},
68+ Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, CI: g, PullRequests: g,
69+ })
70+}
71+
72+// behind moves main on with a commit of its own (a file the feature does not
73+// touch), and returns the new tip.
74+func (r *mrRig) behind() domain.ObjectID {
75+ r.sh("checkout", "-q", "main")
76+ r.commit(author, "news.txt", "n\n", "main moved")
77+ r.push("main")
78+ return r.branchHead("main")
79+}
80+
81+func TestMergeModesWhenBehind(t *testing.T) {
82+ t.Parallel()
83+ t.Run("fast-forward is refused", func(t *testing.T) {
84+ t.Parallel()
85+ r := newMRRig(t)
86+ r.seed()
87+ head := r.feature("feat")
88+ mr := r.open("feat")
89+ tip := r.behind()
90+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
91+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
92+ assert.Contains(t, err.Error(), "the target has moved: rebase and push again, or merge with squash or a merge commit")
93+ assert.Equal(t, tip, r.branchHead("main"))
94+ })
95+
96+ t.Run("squash merges the merged tree", func(t *testing.T) {
97+ t.Parallel()
98+ r := newMRRig(t)
99+ r.seed()
100+ head := r.feature("feat")
101+ mr := r.open("feat")
102+ tip := r.behind()
103+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeSquash})
104+ require.NoError(t, err)
105+ sq := res.MergedCommitID.String()
106+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
107+ assert.Equal(t, tip.String(), g("rev-parse", sq+"^"), "the current tip is the parent")
108+ files := g("ls-tree", "-r", "--name-only", sq)
109+ assert.Contains(t, files, "news.txt", "the target's own change is kept")
110+ assert.Contains(t, files, "c.txt", "the branch's change is there")
111+ r.verifyCommit(sq)
112+ })
113+
114+ t.Run("merge commit has two parents, the merge tree and a good signature", func(t *testing.T) {
115+ t.Parallel()
116+ r := newMRRig(t)
117+ r.seed()
118+ head := r.feature("feat")
119+ mr := r.open("feat")
120+ tip := r.behind()
121+ want := r.run(r.bare(), r.env(author), "merge-tree", "--write-tree", tip.String(), head.String())
122+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeMerge})
123+ require.NoError(t, err)
124+ assert.Equal(t, domain.MergeModeMerge, res.MergeMode)
125+ assert.Equal(t, tip, res.BaseID)
126+ id := res.MergedCommitID.String()
127+ assert.Equal(t, id, r.branchHead("main").String())
128+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
129+ assert.Equal(t, tip.String()+" "+head.String(), g("log", "-1", "--format=%P", id), "parents: target, head")
130+ assert.Equal(t, strings.TrimSpace(want), g("rev-parse", id+"^{tree}"))
131+ assert.Equal(t, "Merge branch 'feat' into main (!1)\n\nAdd feat", g("log", "-1", "--format=%B", id))
132+ assert.Equal(t, "ermite <ermite@git.example.org>", g("log", "-1", "--format=%an <%ae>", id))
133+ assert.Equal(t, "ermite <ermite@git.example.org>", g("log", "-1", "--format=%cn <%ce>", id))
134+ r.verifyCommit(id)
135+ })
136+
137+ t.Run("merge commit when up to date", func(t *testing.T) {
138+ t.Parallel()
139+ r := newMRRig(t)
140+ main := r.seed()
141+ head := r.feature("feat")
142+ mr := r.open("feat")
143+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeMerge})
144+ require.NoError(t, err)
145+ id := res.MergedCommitID.String()
146+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
147+ assert.Equal(t, main.String()+" "+head.String(), g("log", "-1", "--format=%P", id))
148+ assert.Equal(t, g("rev-parse", head.String()+"^{tree}"), g("rev-parse", id+"^{tree}"))
149+ })
150+}
151+
152+func TestMergeRefusedOnConflictAndNoCommonHistory(t *testing.T) {
153+ t.Parallel()
154+ conflict := func(t *testing.T) (*mrRig, domain.MergeRequest, domain.ObjectID, domain.ObjectID) {
155+ r := newMRRig(t)
156+ r.seed()
157+ head := r.feature("feat") // changes line 2 of a.txt
158+ mr := r.open("feat")
159+ r.sh("checkout", "-q", "main")
160+ r.commit(author, "a.txt", "one\nTWO\nthree\n", "main changes a too")
161+ r.push("main")
162+ return r, mr, head, r.branchHead("main")
163+ }
164+ for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash, domain.MergeModeMerge} {
165+ t.Run("conflict "+string(mode), func(t *testing.T) {
166+ t.Parallel()
167+ r, mr, head, tip := conflict(t)
168+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
169+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
170+ if mode != domain.MergeModeFastForward {
171+ assert.Contains(t, err.Error(), "conflicts with main: a.txt: rebase and push again")
172+ }
173+ assert.Equal(t, tip, r.branchHead("main"))
174+ d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
175+ require.NoError(t, err)
176+ assert.Equal(t, domain.MRStateOpen, d.MergeRequest.State)
177+ })
178+ }
179+
180+ for _, mode := range []domain.MergeMode{domain.MergeModeSquash, domain.MergeModeMerge} {
181+ t.Run("no common history "+string(mode), func(t *testing.T) {
182+ t.Parallel()
183+ r := newMRRig(t)
184+ r.seed()
185+ r.sh("checkout", "-q", "--orphan", "island")
186+ r.commit(author, "island.txt", "i\n", "an unrelated root")
187+ r.push("island")
188+ head := r.branchHead("island")
189+ mr, err := r.mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "island", Source: "island"})
190+ if err != nil {
191+ t.Skipf("a request without common history cannot be opened: %v", err)
192+ }
193+ tip := r.branchHead("main")
194+ _, err = r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
195+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
196+ assert.Contains(t, err.Error(), "no common history")
197+ assert.Equal(t, tip, r.branchHead("main"))
198+ })
199+ }
200+}
201+
202+func TestMergeReturnsBeforeTheSideEffects(t *testing.T) {
203+ t.Parallel()
204+ r := newMRRig(t)
205+ r.seed()
206+ head := r.feature("feat")
207+ mr := r.open("feat")
208+ r.drain()
209+ g := newGateHooks()
210+ mrs := r.gated(g)
211+
212+ done := make(chan error, 1)
213+ go func() {
214+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
215+ done <- err
216+ }()
217+ select {
218+ case err := <-done:
219+ require.NoError(t, err, "Merge does not wait for the blocked hooks")
220+ case <-time.After(10 * time.Second):
221+ t.Fatal("Merge waited for the side effects")
222+ }
223+ // The merge is complete and recorded though the hooks have not run.
224+ d, err := mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
225+ require.NoError(t, err)
226+ assert.Equal(t, domain.MRStateMerged, d.MergeRequest.State)
227+ assert.Equal(t, d.MergeRequest.MergedCommitID, r.branchHead("main"))
228+ assert.Empty(t, g.ran())
229+
230+ // Wait waits for them: it times out while they are blocked, then returns.
231+ ctx, cancel := context.WithTimeout(r.ctx, 50*time.Millisecond)
232+ start := time.Now()
233+ mrs.Wait(ctx)
234+ cancel()
235+ assert.GreaterOrEqual(t, time.Since(start), 40*time.Millisecond, "Wait waits for the pending side effects")
236+
237+ waited := make(chan struct{})
238+ go func() { mrs.Wait(context.Background()); close(waited) }()
239+ select {
240+ case <-waited:
241+ t.Fatal("Wait returned with side effects pending")
242+ case <-time.After(50 * time.Millisecond):
243+ }
244+ close(g.release)
245+ select {
246+ case <-waited:
247+ case <-time.After(10 * time.Second):
248+ t.Fatal("Wait did not return after the side effects ended")
249+ }
250+ assert.ElementsMatch(t, []string{"ci", "pulls"}, g.ran())
251+ _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
252+ assert.ErrorIs(t, err, domain.ErrNotFound, "the source was deleted")
253+
254+ // Merged, then pushed, with the deletion in the push.
255+ evs := r.drain()
256+ require.Len(t, evs, 2)
257+ assert.IsType(t, domain.MergeRequestMerged{}, evs[0])
258+ push, ok := evs[1].(domain.PushEvent)
259+ require.True(t, ok)
260+ assert.Len(t, push.Updates, 2)
261+}
262+
263+func TestMergeReleasesTheLockBeforeTheSideEffects(t *testing.T) {
264+ t.Parallel()
265+ r := newMRRig(t)
266+ r.seed()
267+ head := r.feature("feat")
268+ mr := r.open("feat")
269+ r.sh("checkout", "-q", "main")
270+ other := r.openFeature("other")
271+ g := newGateHooks()
272+ defer func() {
273+ close(g.release)
274+ }()
275+ mrs := r.gated(g)
276+
277+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
278+ require.NoError(t, err)
279+ t.Cleanup(func() { mrs.Wait(context.Background()) })
280+ require.Empty(t, g.ran(), "the side effects are still blocked")
281+
282+ // Create and another Merge need the repository lock: they must finish.
283+ ctx, cancel := context.WithTimeout(r.ctx, 10*time.Second)
284+ defer cancel()
285+ r.sh("checkout", "-q", "-B", "third", "main")
286+ r.commit(author, "third.txt", "t\n", "third")
287+ r.push("third")
288+ _, err = mrs.Create(ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "third", Source: "third"})
289+ require.NoError(t, err, "Create is not blocked by the side effects of a merge")
290+ _, err = mrs.Merge(ctx, mrM, r.repo, other.Number, ports.MergeParams{HeadID: r.branchHead("other"), Mode: domain.MergeModeMerge})
291+ require.NoError(t, err, "a second merge is not blocked either")
292+ assert.Empty(t, g.ran())
293+}
@@ -0,0 +1,293 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package usecase_test
4+
5+import (
6+ "bytes"
7+ "context"
8+ "crypto/ed25519"
9+ "strings"
10+ "sync"
11+ "testing"
12+ "time"
13+
14+ "github.com/stretchr/testify/assert"
15+ "github.com/stretchr/testify/require"
16+
17+ "bnema.dev/ermite/internal/adapters/sshsig"
18+ "bnema.dev/ermite/internal/domain"
19+ "bnema.dev/ermite/internal/ports"
20+ "bnema.dev/ermite/internal/usecase"
21+)
22+
23+// gateHooks is a CI and a pull_request trigger that block on a channel, to
24+// tell what runs before Merge returns.
25+type gateHooks struct {
26+ release chan struct{}
27+ mu sync.Mutex
28+ calls []string
29+}
30+
31+func newGateHooks() *gateHooks { return &gateHooks{release: make(chan struct{})} }
32+
33+func (g *gateHooks) note(s string) {
34+ <-g.release
35+ g.mu.Lock()
36+ defer g.mu.Unlock()
37+ g.calls = append(g.calls, s)
38+}
39+
40+func (g *gateHooks) ran() []string {
41+ g.mu.Lock()
42+ defer g.mu.Unlock()
43+ return append([]string(nil), g.calls...)
44+}
45+
46+func (g *gateHooks) OnPush(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {}
47+
48+func (g *gateHooks) OnMerge(context.Context, domain.RepoName, []domain.RefUpdate, domain.Principal) {
49+ g.note("ci")
50+}
51+
52+// OnPullRequest blocks for the closing of a request only: Create reports the
53+// opening on the request path.
54+func (g *gateHooks) OnPullRequest(_ context.Context, _ domain.RepoName, ev ports.PullRequestEvent, _ domain.Principal) {
55+ if ev.Action == domain.PullClosed {
56+ g.note("pulls")
57+ }
58+}
59+
60+// gated builds merge request use cases over the rig whose CI and pull hooks
61+// block until g.release is closed.
62+func (r *mrRig) gated(g *gateHooks) *usecase.MergeRequests {
63+ signer, err := sshsig.New(bytes.Repeat([]byte{9}, ed25519.SeedSize))
64+ require.NoError(r.t, err)
65+ return usecase.NewMergeRequests(usecase.MergeRequestsConfig{
66+ Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Signer: signer,
67+ Committer: domain.Identity{Name: "ermite", Email: "ermite@git.example.org"},
68+ Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, CI: g, PullRequests: g,
69+ })
70+}
71+
72+// behind moves main on with a commit of its own (a file the feature does not
73+// touch), and returns the new tip.
74+func (r *mrRig) behind() domain.ObjectID {
75+ r.sh("checkout", "-q", "main")
76+ r.commit(author, "news.txt", "n\n", "main moved")
77+ r.push("main")
78+ return r.branchHead("main")
79+}
80+
81+func TestMergeModesWhenBehind(t *testing.T) {
82+ t.Parallel()
83+ t.Run("fast-forward is refused", func(t *testing.T) {
84+ t.Parallel()
85+ r := newMRRig(t)
86+ r.seed()
87+ head := r.feature("feat")
88+ mr := r.open("feat")
89+ tip := r.behind()
90+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
91+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
92+ assert.Contains(t, err.Error(), "the target has moved: rebase and push again, or merge with squash or a merge commit")
93+ assert.Equal(t, tip, r.branchHead("main"))
94+ })
95+
96+ t.Run("squash merges the merged tree", func(t *testing.T) {
97+ t.Parallel()
98+ r := newMRRig(t)
99+ r.seed()
100+ head := r.feature("feat")
101+ mr := r.open("feat")
102+ tip := r.behind()
103+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeSquash})
104+ require.NoError(t, err)
105+ sq := res.MergedCommitID.String()
106+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
107+ assert.Equal(t, tip.String(), g("rev-parse", sq+"^"), "the current tip is the parent")
108+ files := g("ls-tree", "-r", "--name-only", sq)
109+ assert.Contains(t, files, "news.txt", "the target's own change is kept")
110+ assert.Contains(t, files, "c.txt", "the branch's change is there")
111+ r.verifyCommit(sq)
112+ })
113+
114+ t.Run("merge commit has two parents, the merge tree and a good signature", func(t *testing.T) {
115+ t.Parallel()
116+ r := newMRRig(t)
117+ r.seed()
118+ head := r.feature("feat")
119+ mr := r.open("feat")
120+ tip := r.behind()
121+ want := r.run(r.bare(), r.env(author), "merge-tree", "--write-tree", tip.String(), head.String())
122+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeMerge})
123+ require.NoError(t, err)
124+ assert.Equal(t, domain.MergeModeMerge, res.MergeMode)
125+ assert.Equal(t, tip, res.BaseID)
126+ id := res.MergedCommitID.String()
127+ assert.Equal(t, id, r.branchHead("main").String())
128+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
129+ assert.Equal(t, tip.String()+" "+head.String(), g("log", "-1", "--format=%P", id), "parents: target, head")
130+ assert.Equal(t, strings.TrimSpace(want), g("rev-parse", id+"^{tree}"))
131+ assert.Equal(t, "Merge branch 'feat' into main (!1)\n\nAdd feat", g("log", "-1", "--format=%B", id))
132+ assert.Equal(t, "ermite <ermite@git.example.org>", g("log", "-1", "--format=%an <%ae>", id))
133+ assert.Equal(t, "ermite <ermite@git.example.org>", g("log", "-1", "--format=%cn <%ce>", id))
134+ r.verifyCommit(id)
135+ })
136+
137+ t.Run("merge commit when up to date", func(t *testing.T) {
138+ t.Parallel()
139+ r := newMRRig(t)
140+ main := r.seed()
141+ head := r.feature("feat")
142+ mr := r.open("feat")
143+ res, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeMerge})
144+ require.NoError(t, err)
145+ id := res.MergedCommitID.String()
146+ g := func(args ...string) string { return r.run(r.bare(), r.env(author), args...) }
147+ assert.Equal(t, main.String()+" "+head.String(), g("log", "-1", "--format=%P", id))
148+ assert.Equal(t, g("rev-parse", head.String()+"^{tree}"), g("rev-parse", id+"^{tree}"))
149+ })
150+}
151+
152+func TestMergeRefusedOnConflictAndNoCommonHistory(t *testing.T) {
153+ t.Parallel()
154+ conflict := func(t *testing.T) (*mrRig, domain.MergeRequest, domain.ObjectID, domain.ObjectID) {
155+ r := newMRRig(t)
156+ r.seed()
157+ head := r.feature("feat") // changes line 2 of a.txt
158+ mr := r.open("feat")
159+ r.sh("checkout", "-q", "main")
160+ r.commit(author, "a.txt", "one\nTWO\nthree\n", "main changes a too")
161+ r.push("main")
162+ return r, mr, head, r.branchHead("main")
163+ }
164+ for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash, domain.MergeModeMerge} {
165+ t.Run("conflict "+string(mode), func(t *testing.T) {
166+ t.Parallel()
167+ r, mr, head, tip := conflict(t)
168+ _, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
169+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
170+ if mode != domain.MergeModeFastForward {
171+ assert.Contains(t, err.Error(), "conflicts with main: a.txt: rebase and push again")
172+ }
173+ assert.Equal(t, tip, r.branchHead("main"))
174+ d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
175+ require.NoError(t, err)
176+ assert.Equal(t, domain.MRStateOpen, d.MergeRequest.State)
177+ })
178+ }
179+
180+ for _, mode := range []domain.MergeMode{domain.MergeModeSquash, domain.MergeModeMerge} {
181+ t.Run("no common history "+string(mode), func(t *testing.T) {
182+ t.Parallel()
183+ r := newMRRig(t)
184+ r.seed()
185+ r.sh("checkout", "-q", "--orphan", "island")
186+ r.commit(author, "island.txt", "i\n", "an unrelated root")
187+ r.push("island")
188+ head := r.branchHead("island")
189+ mr, err := r.mrs.Create(r.ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "island", Source: "island"})
190+ if err != nil {
191+ t.Skipf("a request without common history cannot be opened: %v", err)
192+ }
193+ tip := r.branchHead("main")
194+ _, err = r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
195+ require.ErrorIs(t, err, domain.ErrPreconditionFailed)
196+ assert.Contains(t, err.Error(), "no common history")
197+ assert.Equal(t, tip, r.branchHead("main"))
198+ })
199+ }
200+}
201+
202+func TestMergeReturnsBeforeTheSideEffects(t *testing.T) {
203+ t.Parallel()
204+ r := newMRRig(t)
205+ r.seed()
206+ head := r.feature("feat")
207+ mr := r.open("feat")
208+ r.drain()
209+ g := newGateHooks()
210+ mrs := r.gated(g)
211+
212+ done := make(chan error, 1)
213+ go func() {
214+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
215+ done <- err
216+ }()
217+ select {
218+ case err := <-done:
219+ require.NoError(t, err, "Merge does not wait for the blocked hooks")
220+ case <-time.After(10 * time.Second):
221+ t.Fatal("Merge waited for the side effects")
222+ }
223+ // The merge is complete and recorded though the hooks have not run.
224+ d, err := mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
225+ require.NoError(t, err)
226+ assert.Equal(t, domain.MRStateMerged, d.MergeRequest.State)
227+ assert.Equal(t, d.MergeRequest.MergedCommitID, r.branchHead("main"))
228+ assert.Empty(t, g.ran())
229+
230+ // Wait waits for them: it times out while they are blocked, then returns.
231+ ctx, cancel := context.WithTimeout(r.ctx, 50*time.Millisecond)
232+ start := time.Now()
233+ mrs.Wait(ctx)
234+ cancel()
235+ assert.GreaterOrEqual(t, time.Since(start), 40*time.Millisecond, "Wait waits for the pending side effects")
236+
237+ waited := make(chan struct{})
238+ go func() { mrs.Wait(context.Background()); close(waited) }()
239+ select {
240+ case <-waited:
241+ t.Fatal("Wait returned with side effects pending")
242+ case <-time.After(50 * time.Millisecond):
243+ }
244+ close(g.release)
245+ select {
246+ case <-waited:
247+ case <-time.After(10 * time.Second):
248+ t.Fatal("Wait did not return after the side effects ended")
249+ }
250+ assert.ElementsMatch(t, []string{"ci", "pulls"}, g.ran())
251+ _, err = r.git.BranchHead(r.ctx, r.repo, "feat")
252+ assert.ErrorIs(t, err, domain.ErrNotFound, "the source was deleted")
253+
254+ // Merged, then pushed, with the deletion in the push.
255+ evs := r.drain()
256+ require.Len(t, evs, 2)
257+ assert.IsType(t, domain.MergeRequestMerged{}, evs[0])
258+ push, ok := evs[1].(domain.PushEvent)
259+ require.True(t, ok)
260+ assert.Len(t, push.Updates, 2)
261+}
262+
263+func TestMergeReleasesTheLockBeforeTheSideEffects(t *testing.T) {
264+ t.Parallel()
265+ r := newMRRig(t)
266+ r.seed()
267+ head := r.feature("feat")
268+ mr := r.open("feat")
269+ r.sh("checkout", "-q", "main")
270+ other := r.openFeature("other")
271+ g := newGateHooks()
272+ defer func() {
273+ close(g.release)
274+ }()
275+ mrs := r.gated(g)
276+
277+ _, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head})
278+ require.NoError(t, err)
279+ t.Cleanup(func() { mrs.Wait(context.Background()) })
280+ require.Empty(t, g.ran(), "the side effects are still blocked")
281+
282+ // Create and another Merge need the repository lock: they must finish.
283+ ctx, cancel := context.WithTimeout(r.ctx, 10*time.Second)
284+ defer cancel()
285+ r.sh("checkout", "-q", "-B", "third", "main")
286+ r.commit(author, "third.txt", "t\n", "third")
287+ r.push("third")
288+ _, err = mrs.Create(ctx, root, r.repo, ports.CreateMergeRequestParams{Title: "third", Source: "third"})
289+ require.NoError(t, err, "Create is not blocked by the side effects of a merge")
290+ _, err = mrs.Merge(ctx, mrM, r.repo, other.Number, ports.MergeParams{HeadID: r.branchHead("other"), Mode: domain.MergeModeMerge})
291+ require.NoError(t, err, "a second merge is not blocked either")
292+ assert.Empty(t, g.ran())
293+}

Keyboard shortcuts

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

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

Branches 4

Tags 45

↑↓ moveenter openesc close