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

commit: bf27c542755317d1998b1a9636519f65cd7ffa63
author: brice · 2d ago · 2026-10-19 13:30 UTC
committer: brice · 2d ago · 2026-10-19 18:30 UTC
parent: b707720
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

10 files · +477 -87

M internal/app/cli_mr.go +6 -9


@@ -976,10 +976,11 @@ func newMRMergeCmd(env cliEnv) cobra.Command {
976	976		"required: give the full id of the head that `ermite mr show <repo> <number>` printed when you\n" +
977	977		"reviewed the change. If the branch moved since, the merge is refused: review again. Needs the\n" +
978	978		"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).",
983	984		Args: cobra.ExactArgs(2),
984	985		ValidArgsFunction: completeMRRepoThenNumber(env),
985	986		}

@@ -1014,17 +1015,13 @@ func newMRMergeCmd(env cliEnv) cobra.Command {
1014	1015		}
1015	1016		m := res.Msg.GetMergeRequest()
1016	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	1018		if *asJSON {
1022	1019		return writeJSON(cmd.OutOrStdout(), jsonMerged{jsonMergeRequest: toJSONMergeRequest(m, c.URL()), SourceBranchDeleted: deleted})
1023	1020		}
1024	1021		_, err = fmt.Fprintf(cmd.OutOrStdout(), "merged %s!%d into %s (%s) at %s\n", clean(m.GetRepo()), m.GetNumber(),
1025	1022		clean(m.GetTargetBranch()), modeName(m.GetMergeMode()), shortID(m.GetMergedCommitId()))
1026	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	1026		return err
1030	1027		}

 			"required: give the full id of the head that `ermite mr show <repo> <number>` printed when you\n" +
 			"reviewed the change. If the branch moved since, the merge is refused: review again. Needs the\n" +
 			"mr:merge scope.\n\n" +
-			"--delete-branch deletes the source branch after the merge. The merge is refused when the branch\n" +
+			"--delete-branch asks the server to delete the source branch right after the merge, in the\n" +
-			"is the default branch or used by another open merge request. If the deletion fails after the\n" +
+			"background. The merge is refused when the branch is the default branch or used by another open\n" +
-			"merge, the merge stands, the branch is kept and a warning goes to standard error." + jsonMRHelp +
+			"merge request. If the deletion later fails (the branch moved, say), the merge stands and the\n" +
-			"\n--json adds source_branch_deleted (true when the branch was deleted).",
+			"branch is kept." + jsonMRHelp +
+			"\n--json adds source_branch_deleted (true when the deletion was requested: the server deletes the branch right after the merge).",
 		Args:              cobra.ExactArgs(2),
 		ValidArgsFunction: completeMRRepoThenNumber(env),
 	}
 		}
 		m := res.Msg.GetMergeRequest()
 		deleted := res.Msg.GetSourceBranchDeleted()
-		if deleteBranch && !deleted {
-			_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "ermite: warning: the branch %s was kept (it moved after the merge, or the server cannot delete it)\n",
-				clean(m.GetSourceBranch()))
-		}
 		if *asJSON {
 			return writeJSON(cmd.OutOrStdout(), jsonMerged{jsonMergeRequest: toJSONMergeRequest(m, c.URL()), SourceBranchDeleted: deleted})
 		}
 		_, err = fmt.Fprintf(cmd.OutOrStdout(), "merged %s!%d into %s (%s) at %s\n", clean(m.GetRepo()), m.GetNumber(),
 			clean(m.GetTargetBranch()), modeName(m.GetMergeMode()), shortID(m.GetMergedCommitId()))
 		if err == nil && deleted {
-			_, err = fmt.Fprintf(cmd.OutOrStdout(), "deleted branch %s\n", clean(m.GetSourceBranch()))
+			_, err = fmt.Fprintf(cmd.OutOrStdout(), "deleting branch %s (done by the server after the merge)\n", clean(m.GetSourceBranch()))
 		}
 		return err
 	}

M internal/app/cli_mr_test.go +3 -3


@@ -326,7 +326,7 @@ func TestMRMergeDeleteBranch(t *testing.T) {
326	326		url := f.serve(t)
327	327
328	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	330		assert.Empty(t, res.stderr)
331	331		_, reqs := f.snapshot()
332	332		assert.True(t, reqs[0].(*ermite.MergeMergeRequestRequest).GetDeleteSourceBranch())

@@ -334,11 +334,11 @@ func TestMRMergeDeleteBranch(t *testing.T) {
334	334		res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch", "--json").ok(t)
335	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	339		deleted = false
339	340		res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
340	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	343		// Without the flag nothing is asked and nothing is said.
344	344		res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA).ok(t)

 	url := f.serve(t)
 	res := runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
-	assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\ndeleted branch feat/x\n", res.stdout)
+	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)
 	assert.Empty(t, res.stderr)
 	_, reqs := f.snapshot()
 	assert.True(t, reqs[0].(*ermite.MergeMergeRequestRequest).GetDeleteSourceBranch())
 	res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch", "--json").ok(t)
 	assert.Contains(t, res.stdout, `"source_branch_deleted": true`)
-	// The merge stands when the branch is kept: a warning, no failure.
+	// Deletion is asynchronous: the CLI makes no claim when the server did not
+	// request it.
 	deleted = false
 	res = runCLI(t, envFor(url), "", "mr", "merge", "alpha", "1", "--head", headA, "--delete-branch").ok(t)
 	assert.Equal(t, "merged alpha!1 into main (fast-forward) at "+headA[:10]+"\n", res.stdout)
-	assert.Contains(t, res.stderr, "ermite: warning: the branch feat/x was kept")
 	// Without the flag nothing is asked and nothing is said.
 	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) {
519	519		if f.hooks != nil {
520	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	525		if f.CI != nil {
523	526		f.CI.Close(ctx)
524	527		}


@@ -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) {
375	375		Repo: "proj", Number: 1, HeadId: r.head, DeleteSourceBranch: true,
376	376		}))
377	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	380		assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat"))
380	381
381	382		// 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


@@ -394,6 +394,7 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) {
394	394		assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")
395	395		res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})
396	396		require.Equal(t, http.StatusSeeOther, res.status, res.body)
	397	+	r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
397	398		assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")
398	399		merged := r.headOf("main")
399	400		assert.NotEqual(t, head, merged, "squash makes a new commit")

@@ -513,6 +514,7 @@ func TestWebMergeDeleteBranchBox(t *testing.T) {
513	514		one := b.get("/proj/merge-requests/1", as(token))
514	515		res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})
515	516		require.Equal(t, http.StatusSeeOther, res.status, res.body)
	517	+	r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
516	518		assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))
517	519		done := b.get("/proj/merge-requests/1", nil)
518	520		assert.Equal(t, 200, done.status, "the merged page reads without its branch")


@@ -394,6 +394,7 @@ func TestWebMergeRequestSquashCloseAndList(t *testing.T) {
394	assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")	394		assert.Contains(t, b.get("/proj/merge-requests/1", as(token)).body, `name="delete_branch" value="1"`, "the box offers to delete the branch")
395	res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})	395		res = b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"squash"}, "head_id": {head}, "delete_branch": {"1"}})
396	require.Equal(t, http.StatusSeeOther, res.status, res.body)	396		require.Equal(t, http.StatusSeeOther, res.status, res.body)
		397	+	r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
397	assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")	398		assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"), "the source branch is deleted")
398	merged := r.headOf("main")	399		merged := r.headOf("main")
399	assert.NotEqual(t, head, merged, "squash makes a new commit")	400		assert.NotEqual(t, head, merged, "squash makes a new commit")

@@ -513,6 +514,7 @@ func TestWebMergeDeleteBranchBox(t *testing.T) {
513	one := b.get("/proj/merge-requests/1", as(token))	514		one := b.get("/proj/merge-requests/1", as(token))
514	res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})	515		res := b.form("/proj/merge-requests/1/merge", token, url.Values{"mode": {"fast-forward"}, "head_id": {formHead(t, one.body)}, "delete_branch": {"1"}})
515	require.Equal(t, http.StatusSeeOther, res.status, res.body)	516		require.Equal(t, http.StatusSeeOther, res.status, res.body)
		517	+	r.forge.MergeRequests.Wait(r.ctx) // the deletion follows the redirect
516	assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))	518		assert.Empty(t, r.bareGit("proj", "for-each-ref", "refs/heads/feat/search"))
517	done := b.get("/proj/merge-requests/1", nil)	519		done := b.get("/proj/merge-requests/1", nil)
518	assert.Equal(t, 200, done.status, "the merged page reads without its branch")	520		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 {
324	324		// MergeResult is a merged request and what happened to its source branch.
325	325		type MergeResult struct {
326	326		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).
329	330		SourceDeleted bool
330	331		}
331	332

 // MergeResult is a merged request and what happened to its source branch.
 type MergeResult struct {
 	domain.MergeRequest
-	// SourceDeleted is true when DeleteSource was asked and the branch is
+	// SourceDeleted is true when DeleteSource was asked: the deletion is
-	// gone.
+	// requested, and done asynchronously after the merge returned (it is
+	// skipped if the branch moved meanwhile).
 	SourceDeleted bool
 }

M internal/usecase/mergerequests.go +151 -65


@@ -60,6 +60,9 @@ type MergeRequests struct {
60	60
61	61		mu sync.Mutex
62	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	68		var (

@@ -117,6 +120,21 @@ func (m MergeRequests) WithVerifier(v ports.CommitVerifier) MergeRequests {
117	120		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	138		// lockRepo takes the merge lock of a repository, waiting until ctx ends. The
121	139		// merges of one repository, and the creation of its merge requests, run one at
122	140		// a time. The returned function releases the lock.

@@ -885,30 +903,76 @@ func (m *MergeRequests) MarkRead(ctx context.Context, p domain.Principal, name d
885	903		// domain.ErrPreconditionFailed when the request is not open, the mode is not
886	904		// allowed by the settings of the repository, the source branch no longer
887	905		// points to the head ("branch moved, read again") or is gone, the target is
888		-	// gone or is not an ancestor of the head (the author rebases and pushes
889		-	// again), or the branch brings nothing. Checks come with CI (CI-1): there are
890		-	// none yet, so the setting that lets failing checks block the merge has
891		-	// nothing to act on.
	906	+	// gone, the mode's own condition does not hold (see below), or the branch
	907	+	// brings nothing. Checks come with CI (CI-1): there are none yet, so the
	908	+	// setting that lets failing checks block the merge has nothing to act on.
892	909		//
893		-	// A fast-forward moves the target to the head, with the old value as a
894		-	// precondition. A squash writes one commit with the tree of the head, the
895		-	// target as parent and the author and message of domain.NewSquashCommit,
896		-	// signed by the instance key, and moves the target to it the same way.
	910	+	// The conditions are always checked with Git, under the lock, never from the
	911	+	// stored mergeability. A fast-forward needs the target to be an ancestor of
	912	+	// the head (the author rebases and pushes again) and moves the target to the
	913	+	// head, with the old value as a precondition. A squash and a merge commit need
	914	+	// the head to merge cleanly into the target (a behind branch is fine). A squash
	915	+	// writes one commit with the merged tree, the target as parent and the author
	916	+	// and message of domain.NewSquashCommit; a merge commit has two parents (the
	917	+	// target, then the head) and the message of domain.NewMergeCommit. Both are
	918	+	// signed by the instance key and move the target the same way.
897	919		//
898	920		// With DeleteSource the merge is also refused, before anything changes, when
899	921		// the source is the default branch or the source or target of another open
900		-	// request. After the merge the source is deleted if it still points to the
901		-	// head; the PushEvent then carries the deletion too. A failed deletion is
902		-	// logged and leaves the branch: the merge stands and SourceDeleted is false.
	922	+	// request.
	923	+	//
	924	+	// Only the merge itself runs before Merge returns, under the lock. Afterwards,
	925	+	// off the request path and without the lock: the deletion of the source branch
	926	+	// (if it still points to the head; a failure is logged and leaves the branch),
	927	+	// MergeRequestMerged then a PushEvent (which carries the deletion too), and
	928	+	// the CI and mergeability hooks. Wait waits for them. SourceDeleted therefore
	929	+	// only says that the deletion was requested.
903	930		func (m *MergeRequests) Merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (ports.MergeResult, error) {
904		-	mr, deleted, err := m.merge(ctx, p, name, number, params)
905		-	return ports.MergeResult{MergeRequest: mr, SourceDeleted: deleted}, err
	931	+	mr, err := m.merge(ctx, p, name, number, params)
	932	+	return ports.MergeResult{MergeRequest: mr, SourceDeleted: err == nil && params.DeleteSource}, err
906	933		}
907	934
908		-	func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, bool, error) {
	935	+	// maxConflictPathsShown is how many conflicting paths a refusal lists.
	936	+	const maxConflictPathsShown = 5
	937	+
	938	+	// checkMode is the authoritative per-mode check of a merge, done with Git. It
	939	+	// returns the tree a squash or merge commit gets (zero for a fast-forward).
	940	+	func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID) (domain.ObjectID, error) {
	941	+	if mode == domain.MergeModeFastForward {
	942	+	ok, err := m.git.IsAncestor(ctx, mr.Repo, tip, head)
	943	+	if err != nil {
	944	+	return domain.ObjectID{}, err
	945	+	}
	946	+	if !ok {
	947	+	return domain.ObjectID{}, fmt.Errorf("%w: the target has moved: rebase and push again, or merge with squash or a merge commit", domain.ErrPreconditionFailed)
	948	+	}
	949	+	return domain.ObjectID{}, nil
	950	+	}
	951	+	res, err := m.git.MergeTree(ctx, mr.Repo, tip, head)
	952	+	if err != nil {
	953	+	return domain.ObjectID{}, err
	954	+	}
	955	+	if !res.Clean {
	956	+	what := "no common history"
	957	+	if len(res.ConflictPaths) > 0 {
	958	+	shown := res.ConflictPaths
	959	+	if len(shown) > maxConflictPathsShown {
	960	+	shown = shown[:maxConflictPathsShown]
	961	+	}
	962	+	what = strings.Join(shown, ", ")
	963	+	if len(shown) < len(res.ConflictPaths) \|\| res.Truncated {
	964	+	what += ", ..."
	965	+	}
	966	+	}
	967	+	return domain.ObjectID{}, fmt.Errorf("%w: conflicts with %s: %s: rebase and push again", domain.ErrPreconditionFailed, mr.Target, what)
	968	+	}
	969	+	return res.Tree, nil
	970	+	}
	971	+
	972	+	func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) {
909	973		repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge)
910	974		if err != nil {
911		-	return domain.MergeRequest{}, false, err
	975	+	return domain.MergeRequest{}, err
912	976		}
913	977		mode := params.Mode
914	978		if mode == "" {

@@ -922,114 +986,131 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
922	986		v.Add("head_id", "is required")
923	987		}
924	988		if err := v.Err(); err != nil {
925		-	return domain.MergeRequest{}, false, err
	989	+	return domain.MergeRequest{}, err
926	990		}
927	991
928	992		unlock, err := m.lockRepo(ctx, name)
929	993		if err != nil {
930		-	return domain.MergeRequest{}, false, err
	994	+	return domain.MergeRequest{}, err
931	995		}
932		-	defer unlock()
	996	+	var once sync.Once
	997	+	release := func() { once.Do(unlock) }
	998	+	defer release()
933	999
934	1000		mr, err := m.store.Get(ctx, name, number)
935	1001		if err != nil {
936		-	return domain.MergeRequest{}, false, err
	1002	+	return domain.MergeRequest{}, err
937	1003		}
938	1004		// A merge moves the target and may delete the source: a token limited to
939	1005		// branch prefixes may do so only within them.
940	1006		if !p.AllowsRef("refs/heads/" + mr.Target) {
941		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
	1007	+	return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
942	1008		}
943	1009		if params.DeleteSource && !p.AllowsRef("refs/heads/"+mr.Source) {
944		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
	1010	+	return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
945	1011		}
946	1012		if !mr.IsOpen() {
947		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
	1013	+	return domain.MergeRequest{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
948	1014		}
949	1015		if mr.IsDraft() {
950		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
	1016	+	return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
951	1017		}
952	1018		// The settings are read again under the lock: a change applies at once.
953	1019		repo, err = m.meta.Get(ctx, name)
954	1020		if err != nil {
955	1021		if errors.Is(err, domain.ErrNotFound) {
956		-	return domain.MergeRequest{}, false, err // deleted meanwhile
	1022	+	return domain.MergeRequest{}, err // deleted meanwhile
957	1023		}
958		-	return domain.MergeRequest{}, false, fmt.Errorf("get repository %q: %w", name, err)
	1024	+	return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err)
959	1025		}
960	1026		settings := repo.Settings
961	1027		if settings.IsZero() {
962	1028		settings = domain.DefaultRepoSettings()
963	1029		}
964	1030		if !settings.Allows(mode) {
965		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
	1031	+	return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
966	1032		}
967	1033
968	1034		head, err := m.branchOrZero(ctx, name, mr.Source)
969	1035		switch {
970	1036		case err != nil:
971		-	return domain.MergeRequest{}, false, err
	1037	+	return domain.MergeRequest{}, err
972	1038		case head.IsZero():
973		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
	1039	+	return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
974	1040		case head != params.HeadID:
975		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
	1041	+	return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
976	1042		}
977	1043		tip, err := m.branchOrZero(ctx, name, mr.Target)
978	1044		switch {
979	1045		case err != nil:
980		-	return domain.MergeRequest{}, false, err
	1046	+	return domain.MergeRequest{}, err
981	1047		case tip.IsZero():
982		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
	1048	+	return domain.MergeRequest{}, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
983	1049		case tip == head:
984		-	return domain.MergeRequest{}, false, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
	1050	+	return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
985	1051		}
986		-	switch ok, err := m.git.IsAncestor(ctx, name, tip, head); {
987		-	case err != nil:
988		-	return domain.MergeRequest{}, false, err
989		-	case !ok:
990		-	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)
	1052	+	tree, err := m.checkMode(ctx, mr, mode, tip, head)
	1053	+	if err != nil {
	1054	+	return domain.MergeRequest{}, err
991	1055		}
992	1056		if settings.FailingChecksBlock {
993	1057		if err := m.checksAllow(ctx, name, head); err != nil {
994		-	return domain.MergeRequest{}, false, err
	1058	+	return domain.MergeRequest{}, err
995	1059		}
996	1060		}
997	1061		if params.DeleteSource {
998	1062		if err := m.checkDeletable(ctx, name, mr); err != nil {
999		-	return domain.MergeRequest{}, false, err
	1063	+	return domain.MergeRequest{}, err
1000	1064		}
1001	1065		}
1002	1066
1003		-	merged, err := m.moveTarget(ctx, mr, mode, tip, head, p)
	1067	+	merged, err := m.moveTarget(ctx, mr, mode, tip, head, tree, p)
1004	1068		if err != nil {
1005		-	return domain.MergeRequest{}, false, err
	1069	+	return domain.MergeRequest{}, err
1006	1070		}
1007	1071		m.recordPush(ctx, name) // the target moved, whatever happens next
1008	1072		mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged)
1009	1073		if err != nil {
1010		-	return domain.MergeRequest{}, false, err
	1074	+	return domain.MergeRequest{}, err
1011	1075		}
1012	1076		log := zerowrap.FromCtx(ctx)
1013	1077		log.Info().Str("repo", name.String()).Int64("number", number).Str("mode", mode.String()).
1014	1078		Str("target", mr.Target).Str("commit", merged.String()).
1015	1079		Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request merged")
	1080	+
	1081	+	// The merge is done and recorded. The rest runs without the lock and after
	1082	+	// the caller got its answer.
	1083	+	release()
	1084	+	bg := context.WithoutCancel(ctx)
	1085	+	m.after.Add(1)
	1086	+	go func() {
	1087	+	defer m.after.Done()
	1088	+	m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
	1089	+	}()
	1090	+	return mr, nil
	1091	+	}
	1092	+
	1093	+	// afterMerge is what follows a merge off the request path: errors are logged,
	1094	+	// never returned. mr is the merged request, tip the old target and merged the
	1095	+	// new one.
	1096	+	func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
	1097	+	name := repo.Name
	1098	+	log := zerowrap.FromCtx(ctx)
1016	1099		updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
1017		-	deleted := false
1018		-	if params.DeleteSource {
1019		-	// The merge is done and recorded: a failure here keeps the branch. Who
1020		-	// pushed the source goes first (the branch is gone either way for a
	1100	+	if deleteSource {
	1101	+	// Who pushed the source goes first (the branch is gone either way for a
1021	1102		// schedule; a kept branch merely has no record: not trusted).
1022		-	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 {
	1103	+	del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
	1104	+	if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
1023	1105		log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
1024	1106		}
1025		-	switch err := m.git.DeleteBranch(context.WithoutCancel(ctx), name, mr.Source, head); {
	1107	+	switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
1026	1108		case err != nil:
1027		-	log.Warn().Err(err).Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
	1109	+	log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1028	1110		Msg("source branch kept after the merge")
1029	1111		default:
1030		-	deleted = true
1031		-	updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})
1032		-	log.Info().Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
	1112	+	updates = append(updates, del)
	1113	+	log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
1033	1114		Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
1034	1115		}
1035	1116		}

@@ -1039,19 +1120,18 @@ func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name doma
1039	1120		m.bus.Publish(ctx, domain.PushEvent{
1040	1121		Repo: name, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, Updates: updates,
1041	1122		})
1042		-	if m.mergeab != nil {
1043		-	m.mergeab.OnClosed(ctx, mr)
1044		-	m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
1045		-	}
1046	1123		if m.pulls != nil {
1047		-	m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
	1124	+	m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: mr.Number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
1048	1125		}
1049	1126		if m.ci != nil {
1050	1127		// GitHub runs the "push" workflows of a merge, with the owner as actor;
1051	1128		// the run is trusted with what the merging principal may read.
1052	1129		m.ci.OnMerge(ctx, name, updates, p)
1053	1130		}
1054		-	return mr, deleted, nil
	1131	+	if m.mergeab != nil {
	1132	+	m.mergeab.OnClosed(ctx, mr)
	1133	+	m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
	1134	+	}
1055	1135		}
1056	1136
1057	1137		// WhileOpen implements ports.PullGate: fn runs under the merge lock, so a

@@ -1144,10 +1224,20 @@ func (m *MergeRequests) recordPush(ctx context.Context, name domain.RepoName) {
1144	1224		}
1145	1225
1146	1226		// moveTarget moves the target branch from tip to the head (fast-forward) or to
1147		-	// a new signed squash commit, and returns the new tip.
1148		-	func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
	1227	+	// a new signed squash or merge commit with the given tree, and returns the new tip.
	1228	+	func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head, tree domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
1149	1229		newTip := head
1150		-	if mode == domain.MergeModeSquash {
	1230	+	switch mode {
	1231	+	case domain.MergeModeMerge:
	1232	+	commit := domain.NewMergeCommit(mr, tree, tip, head, m.committer, m.clock.Now())
	1233	+	armored, err := m.signer.Sign(commit.Payload())
	1234	+	if err != nil {
	1235	+	return domain.ObjectID{}, fmt.Errorf("sign the merge commit of !%d in %q: %w", mr.Number, mr.Repo, err)
	1236	+	}
	1237	+	if newTip, err = m.git.WriteCommit(ctx, mr.Repo, commit.Signed(armored)); err != nil {
	1238	+	return domain.ObjectID{}, err
	1239	+	}
	1240	+	case domain.MergeModeSquash:
1151	1241		commits, more, err := m.git.ListRange(ctx, mr.Repo, tip, head, domain.MaxSquashCommits)
1152	1242		if err != nil {
1153	1243		return domain.ObjectID{}, err

@@ -1155,10 +1245,6 @@ func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest,
1155	1245		if more {
1156	1246		return domain.ObjectID{}, fmt.Errorf("%w: the branch has more than %d commits to squash", domain.ErrPreconditionFailed, domain.MaxSquashCommits)
1157	1247		}
1158		-	tree, err := m.git.CommitTree(ctx, mr.Repo, head)
1159		-	if err != nil {
1160		-	return domain.ObjectID{}, err
1161		-	}
1162	1248		commit := domain.NewSquashCommit(mr, tree, tip, commits, m.committer, m.clock.Now())
1163	1249		armored, err := m.signer.Sign(commit.Payload())
1164	1250		if err != nil {

 	mu    sync.Mutex
 	locks map[domain.RepoName]chan struct{}
+	// after tracks the post-merge side effects running off the request path.
+	after sync.WaitGroup
 }
 var (
 	return m
 }
+// Wait waits for the post-merge side effects still running (source branch
+// deletion, events, CI and mergeability hooks), or for ctx to end. Call it
+// before closing what they use.
+func (m *MergeRequests) Wait(ctx context.Context) {
+	done := make(chan struct{})
+	go func() {
+		m.after.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-ctx.Done():
+	}
+}
 // lockRepo takes the merge lock of a repository, waiting until ctx ends. The
 // merges of one repository, and the creation of its merge requests, run one at
 // a time. The returned function releases the lock.
 // domain.ErrPreconditionFailed when the request is not open, the mode is not
 // allowed by the settings of the repository, the source branch no longer
 // points to the head ("branch moved, read again") or is gone, the target is
-// gone or is not an ancestor of the head (the author rebases and pushes
+// gone, the mode's own condition does not hold (see below), or the branch
-// again), or the branch brings nothing. Checks come with CI (CI-1): there are
+// brings nothing. Checks come with CI (CI-1): there are none yet, so the
-// none yet, so the setting that lets failing checks block the merge has
+// setting that lets failing checks block the merge has nothing to act on.
-// nothing to act on.
 //
-// A fast-forward moves the target to the head, with the old value as a
+// The conditions are always checked with Git, under the lock, never from the
-// precondition. A squash writes one commit with the tree of the head, the
+// stored mergeability. A fast-forward needs the target to be an ancestor of
-// target as parent and the author and message of domain.NewSquashCommit,
+// the head (the author rebases and pushes again) and moves the target to the
-// signed by the instance key, and moves the target to it the same way.
+// head, with the old value as a precondition. A squash and a merge commit need
+// the head to merge cleanly into the target (a behind branch is fine). A squash
+// writes one commit with the merged tree, the target as parent and the author
+// and message of domain.NewSquashCommit; a merge commit has two parents (the
+// target, then the head) and the message of domain.NewMergeCommit. Both are
+// signed by the instance key and move the target the same way.
 //
 // With DeleteSource the merge is also refused, before anything changes, when
 // the source is the default branch or the source or target of another open
-// request. After the merge the source is deleted if it still points to the
+// request.
-// head; the PushEvent then carries the deletion too. A failed deletion is
+//
-// logged and leaves the branch: the merge stands and SourceDeleted is false.
+// Only the merge itself runs before Merge returns, under the lock. Afterwards,
+// off the request path and without the lock: the deletion of the source branch
+// (if it still points to the head; a failure is logged and leaves the branch),
+// MergeRequestMerged then a PushEvent (which carries the deletion too), and
+// the CI and mergeability hooks. Wait waits for them. SourceDeleted therefore
+// only says that the deletion was requested.
 func (m *MergeRequests) Merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (ports.MergeResult, error) {
-	mr, deleted, err := m.merge(ctx, p, name, number, params)
+	mr, err := m.merge(ctx, p, name, number, params)
-	return ports.MergeResult{MergeRequest: mr, SourceDeleted: deleted}, err
+	return ports.MergeResult{MergeRequest: mr, SourceDeleted: err == nil && params.DeleteSource}, err
 }
-func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, bool, error) {
+// maxConflictPathsShown is how many conflicting paths a refusal lists.
+const maxConflictPathsShown = 5
+// checkMode is the authoritative per-mode check of a merge, done with Git. It
+// returns the tree a squash or merge commit gets (zero for a fast-forward).
+func (m *MergeRequests) checkMode(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID) (domain.ObjectID, error) {
+	if mode == domain.MergeModeFastForward {
+		ok, err := m.git.IsAncestor(ctx, mr.Repo, tip, head)
+		if err != nil {
+			return domain.ObjectID{}, err
+		}
+		if !ok {
+			return domain.ObjectID{}, fmt.Errorf("%w: the target has moved: rebase and push again, or merge with squash or a merge commit", domain.ErrPreconditionFailed)
+		}
+		return domain.ObjectID{}, nil
+	}
+	res, err := m.git.MergeTree(ctx, mr.Repo, tip, head)
+	if err != nil {
+		return domain.ObjectID{}, err
+	}
+	if !res.Clean {
+		what := "no common history"
+		if len(res.ConflictPaths) > 0 {
+			shown := res.ConflictPaths
+			if len(shown) > maxConflictPathsShown {
+				shown = shown[:maxConflictPathsShown]
+			}
+			what = strings.Join(shown, ", ")
+			if len(shown) < len(res.ConflictPaths) || res.Truncated {
+				what += ", ..."
+			}
+		}
+		return domain.ObjectID{}, fmt.Errorf("%w: conflicts with %s: %s: rebase and push again", domain.ErrPreconditionFailed, mr.Target, what)
+	}
+	return res.Tree, nil
+}
+func (m *MergeRequests) merge(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params ports.MergeParams) (domain.MergeRequest, error) {
 	repo, err := m.writable(ctx, p, name, domain.ScopeMRMerge)
 	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
 	mode := params.Mode
 	if mode == "" {
 		v.Add("head_id", "is required")
 	}
 	if err := v.Err(); err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
 	unlock, err := m.lockRepo(ctx, name)
 	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
-	defer unlock()
+	var once sync.Once
+	release := func() { once.Do(unlock) }
+	defer release()
 	mr, err := m.store.Get(ctx, name, number)
 	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
 	// A merge moves the target and may delete the source: a token limited to
 	// branch prefixes may do so only within them.
 	if !p.AllowsRef("refs/heads/" + mr.Target) {
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
+		return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Target)
 	}
 	if params.DeleteSource && !p.AllowsRef("refs/heads/"+mr.Source) {
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
+		return domain.MergeRequest{}, fmt.Errorf("%w: this token may not write %s", domain.ErrForbidden, mr.Source)
 	}
 	if !mr.IsOpen() {
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
+		return domain.MergeRequest{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
 	}
 	if mr.IsDraft() {
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
+		return domain.MergeRequest{}, fmt.Errorf("%w: !%d is a draft: mark it ready for review first", domain.ErrPreconditionFailed, number)
 	}
 	// The settings are read again under the lock: a change applies at once.
 	repo, err = m.meta.Get(ctx, name)
 	if err != nil {
 		if errors.Is(err, domain.ErrNotFound) {
-			return domain.MergeRequest{}, false, err // deleted meanwhile
+			return domain.MergeRequest{}, err // deleted meanwhile
 		}
-		return domain.MergeRequest{}, false, fmt.Errorf("get repository %q: %w", name, err)
+		return domain.MergeRequest{}, fmt.Errorf("get repository %q: %w", name, err)
 	}
 	settings := repo.Settings
 	if settings.IsZero() {
 		settings = domain.DefaultRepoSettings()
 	}
 	if !settings.Allows(mode) {
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
+		return domain.MergeRequest{}, fmt.Errorf("%w: the %s merge mode is not allowed in %s", domain.ErrPreconditionFailed, mode, name)
 	}
 	head, err := m.branchOrZero(ctx, name, mr.Source)
 	switch {
 	case err != nil:
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	case head.IsZero():
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
+		return domain.MergeRequest{}, fmt.Errorf("%w: the source branch %q no longer exists", domain.ErrPreconditionFailed, mr.Source)
 	case head != params.HeadID:
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
+		return domain.MergeRequest{}, fmt.Errorf("%w: branch moved, read again: %q is at %s, not %s", domain.ErrPreconditionFailed, mr.Source, head, params.HeadID)
 	}
 	tip, err := m.branchOrZero(ctx, name, mr.Target)
 	switch {
 	case err != nil:
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	case tip.IsZero():
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
+		return domain.MergeRequest{}, fmt.Errorf("%w: the target branch %q no longer exists", domain.ErrPreconditionFailed, mr.Target)
 	case tip == head:
-		return domain.MergeRequest{}, false, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
+		return domain.MergeRequest{}, fmt.Errorf("%w: nothing to merge, %q already is at %s", domain.ErrPreconditionFailed, mr.Target, head)
 	}
-	switch ok, err := m.git.IsAncestor(ctx, name, tip, head); {
+	tree, err := m.checkMode(ctx, mr, mode, tip, head)
-	case err != nil:
+	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
-	case !ok:
-		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)
 	}
 	if settings.FailingChecksBlock {
 		if err := m.checksAllow(ctx, name, head); err != nil {
-			return domain.MergeRequest{}, false, err
+			return domain.MergeRequest{}, err
 		}
 	}
 	if params.DeleteSource {
 		if err := m.checkDeletable(ctx, name, mr); err != nil {
-			return domain.MergeRequest{}, false, err
+			return domain.MergeRequest{}, err
 		}
 	}
-	merged, err := m.moveTarget(ctx, mr, mode, tip, head, p)
+	merged, err := m.moveTarget(ctx, mr, mode, tip, head, tree, p)
 	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
 	m.recordPush(ctx, name) // the target moved, whatever happens next
 	mr, err = m.markMerged(ctx, name, number, mode, tip, head, merged)
 	if err != nil {
-		return domain.MergeRequest{}, false, err
+		return domain.MergeRequest{}, err
 	}
 	log := zerowrap.FromCtx(ctx)
 	log.Info().Str("repo", name.String()).Int64("number", number).Str("mode", mode.String()).
 		Str("target", mr.Target).Str("commit", merged.String()).
 		Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("merge request merged")
+	// The merge is done and recorded. The rest runs without the lock and after
+	// the caller got its answer.
+	release()
+	bg := context.WithoutCancel(ctx)
+	m.after.Add(1)
+	go func() {
+		defer m.after.Done()
+		m.afterMerge(bg, p, repo, mr, head, tip, merged, params.DeleteSource)
+	}()
+	return mr, nil
+}
+// afterMerge is what follows a merge off the request path: errors are logged,
+// never returned. mr is the merged request, tip the old target and merged the
+// new one.
+func (m *MergeRequests) afterMerge(ctx context.Context, p domain.Principal, repo domain.Repo, mr domain.MergeRequest, head, tip, merged domain.ObjectID, deleteSource bool) {
+	name := repo.Name
+	log := zerowrap.FromCtx(ctx)
 	updates := []domain.RefUpdate{{Old: tip.String(), New: merged.String(), Ref: "refs/heads/" + mr.Target}}
-	deleted := false
+	if deleteSource {
-	if params.DeleteSource {
+		// Who pushed the source goes first (the branch is gone either way for a
-		// The merge is done and recorded: a failure here keeps the branch. Who
-		// pushed the source goes first (the branch is gone either way for a
 		// schedule; a kept branch merely has no record: not trusted).
-		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 {
+		del := domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source}
+		if err := recordRefTips(ctx, m.tips, name, []domain.RefUpdate{del}, "", domain.CIPusher{}, false); err != nil {
 			log.Warn().Err(err).Str("repo", name.String()).Str("branch", mr.Source).Msg("the record of the source branch was kept")
 		}
-		switch err := m.git.DeleteBranch(context.WithoutCancel(ctx), name, mr.Source, head); {
+		switch err := m.git.DeleteBranch(ctx, name, mr.Source, head); {
 		case err != nil:
-			log.Warn().Err(err).Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
+			log.Warn().Err(err).Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
 				Msg("source branch kept after the merge")
 		default:
-			deleted = true
+			updates = append(updates, del)
-			updates = append(updates, domain.RefUpdate{Old: head.String(), New: domain.ZeroOID, Ref: "refs/heads/" + mr.Source})
+			log.Info().Str("repo", name.String()).Int64("number", mr.Number).Str("branch", mr.Source).
-			log.Info().Str("repo", name.String()).Int64("number", number).Str("branch", mr.Source).
 				Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("source branch deleted after the merge")
 		}
 	}
 	m.bus.Publish(ctx, domain.PushEvent{
 		Repo: name, At: at, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt, Updates: updates,
 	})
-	if m.mergeab != nil {
-		m.mergeab.OnClosed(ctx, mr)
-		m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
-	}
 	if m.pulls != nil {
-		m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
+		m.pulls.OnPullRequest(ctx, name, ports.PullRequestEvent{Number: mr.Number, Action: domain.PullClosed, Head: mr.HeadID, Base: mr.BaseID, Merged: true}, p)
 	}
 	if m.ci != nil {
 		// GitHub runs the "push" workflows of a merge, with the owner as actor;
 		// the run is trusted with what the merging principal may read.
 		m.ci.OnMerge(ctx, name, updates, p)
 	}
-	return mr, deleted, nil
+	if m.mergeab != nil {
+		m.mergeab.OnClosed(ctx, mr)
+		m.mergeab.OnBranchesMoved(ctx, name, updates) // the other requests of the moved target
+	}
 }
 // WhileOpen implements ports.PullGate: fn runs under the merge lock, so a
 }
 // moveTarget moves the target branch from tip to the head (fast-forward) or to
-// a new signed squash commit, and returns the new tip.
+// a new signed squash or merge commit with the given tree, and returns the new tip.
-func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
+func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest, mode domain.MergeMode, tip, head, tree domain.ObjectID, p domain.Principal) (domain.ObjectID, error) {
 	newTip := head
-	if mode == domain.MergeModeSquash {
+	switch mode {
+	case domain.MergeModeMerge:
+		commit := domain.NewMergeCommit(mr, tree, tip, head, m.committer, m.clock.Now())
+		armored, err := m.signer.Sign(commit.Payload())
+		if err != nil {
+			return domain.ObjectID{}, fmt.Errorf("sign the merge commit of !%d in %q: %w", mr.Number, mr.Repo, err)
+		}
+		if newTip, err = m.git.WriteCommit(ctx, mr.Repo, commit.Signed(armored)); err != nil {
+			return domain.ObjectID{}, err
+		}
+	case domain.MergeModeSquash:
 		commits, more, err := m.git.ListRange(ctx, mr.Repo, tip, head, domain.MaxSquashCommits)
 		if err != nil {
 			return domain.ObjectID{}, err
 		if more {
 			return domain.ObjectID{}, fmt.Errorf("%w: the branch has more than %d commits to squash", domain.ErrPreconditionFailed, domain.MaxSquashCommits)
 		}
-		tree, err := m.git.CommitTree(ctx, mr.Repo, head)
-		if err != nil {
-			return domain.ObjectID{}, err
-		}
 		commit := domain.NewSquashCommit(mr, tree, tip, commits, m.committer, m.clock.Now())
 		armored, err := m.signer.Sign(commit.Payload())
 		if err != nil {

M internal/usecase/mergerequests_delete_test.go +3 -1


@@ -31,6 +31,7 @@ func TestMergeDeletesTheSourceBranch(t *testing.T) {
31	31		require.NoError(t, err)
32	32		assert.True(t, res.SourceDeleted)
33	33		assert.Equal(t, domain.MRStateMerged, res.State)
	34	+	r.mrs.Wait(r.ctx) // the deletion happens after Merge returned
34	35		_, err = r.git.BranchHead(r.ctx, r.repo, "feat")
35	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	194		res, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
194	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	197		assert.Equal(t, domain.MRStateMerged, res.State)
	198	+	mrs.Wait(r.ctx)
197	199		assert.Equal(t, head, r.branchHead("main"))
198	200		assert.Equal(t, moved, r.branchHead("feat"), "the late commit is kept")
199	201		evs := r.drain()

 			require.NoError(t, err)
 			assert.True(t, res.SourceDeleted)
 			assert.Equal(t, domain.MRStateMerged, res.State)
+			r.mrs.Wait(r.ctx) // the deletion happens after Merge returned
 			_, err = r.git.BranchHead(r.ctx, r.repo, "feat")
 			assert.ErrorIs(t, err, domain.ErrNotFound, "the branch is gone")
 	res, err := mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, DeleteSource: true})
 	require.NoError(t, err, "the merge stands")
-	assert.False(t, res.SourceDeleted)
+	assert.True(t, res.SourceDeleted, "the deletion was requested")
 	assert.Equal(t, domain.MRStateMerged, res.State)
+	mrs.Wait(r.ctx)
 	assert.Equal(t, head, r.branchHead("main"))
 	assert.Equal(t, moved, r.branchHead("feat"), "the late commit is kept")
 	evs := r.drain()

M internal/usecase/mergerequests_integration_test.go +11 -6


@@ -207,6 +207,7 @@ func (r *mrRig) open(branch string) domain.MergeRequest {
207	207		// subscriber before Publish returns: whatever a returned call published is
208	208		// already in the channel.
209	209		func (r *mrRig) drain() []domain.Event {
	210	+	r.mrs.Wait(r.ctx) // the events of a merge are published after it returned
210	211		var out []domain.Event
211	212		for {
212	213		select {

@@ -959,11 +960,10 @@ func TestMergeRefusals(t *testing.T) {
959	960		r.push("main")
960	961		tip := r.branchHead("main")
961	962
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")
967	967		assert.Equal(t, tip, r.branchHead("main"))
968	968		d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
969	969		require.NoError(t, err)

@@ -1047,8 +1047,13 @@ func TestConcurrentMergesSerialize(t *testing.T) {
1047	1047		const n = 6
1048	1048		heads := map[int64]domain.ObjectID{}
1049	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	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	1059		type outcome struct {

 // subscriber before Publish returns: whatever a returned call published is
 // already in the channel.
 func (r *mrRig) drain() []domain.Event {
+	r.mrs.Wait(r.ctx) // the events of a merge are published after it returned
 	var out []domain.Event
 	for {
 		select {
 		r.push("main")
 		tip := r.branchHead("main")
-		for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash} {
+		// Only a fast-forward needs the target to be an ancestor.
-			_, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: mode})
+		_, err := r.mrs.Merge(r.ctx, mrM, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward})
-			require.ErrorIs(t, err, domain.ErrPreconditionFailed, mode)
+		require.ErrorIs(t, err, domain.ErrPreconditionFailed)
-			assert.Contains(t, err.Error(), "rebase", mode)
+		assert.Contains(t, err.Error(), "the target has moved: rebase and push again")
-		}
 		assert.Equal(t, tip, r.branchHead("main"))
 		d, err := r.mrs.Get(r.ctx, mrM, r.repo, mr.Number, ports.GetMergeRequestOptions{})
 		require.NoError(t, err)
 	const n = 6
 	heads := map[int64]domain.ObjectID{}
 	for i := range n {
+		// Every branch writes the same file differently: once one is merged the
+		// others neither fast-forward nor merge cleanly.
 		b := fmt.Sprintf("f%d", i)
-		heads[r.openFeature(b).Number] = r.branchHead(b)
+		r.sh("checkout", "-q", "-B", b, "main")
+		r.commit(author, "same.txt", b+"\n", "add "+b)
+		r.push(b)
+		heads[r.open(b).Number] = r.branchHead(b)
 	}
 	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	+	}