pr: No need to try to merge changes

We can reset current worktree and clobber it with the merged changes.
We want to emulate `git merge -s theirs` strategy while
`git merge -Xtheirs` only picks `theirs` in case of conflicts.
So, resetting the changes and reading exact is sufficient

`git read-tree -u` updates the current work tree so we do not have
unstaged changes.

common/config.go

@@ -39,6 +39,10 @@ const (
 
 	Permission_ForceMerge = "force-merge"
 	Permission_Group      = "release-engineering"
+
+	MergeModeFF      = "ff-only"
+	MergeModeReplace = "replace"
+	MergeModeDevel   = "devel"
 )
 
 type ConfigFile struct {
@@ -52,9 +56,9 @@ type ReviewGroup struct {
 }
 
 type QAConfig struct {
-	Name   string
-	Origin string
-	Label  string // requires this gitea lable to be set or skipped
+	Name              string
+	Origin            string
+	Label             string   // requires this gitea lable to be set or skipped
 	BuildDisableRepos []string // which repos to build disable in the new project
 }
 
@@ -89,7 +93,8 @@ type AutogitConfig struct {
 	Committers     []string // group in addition to Reviewers and Maintainers that can order the bot around, mostly as helper for factory-maintainers
 	Subdirs        []string // list of directories to sort submodules into. Needed b/c _manifest cannot list non-existent directories
 
-	Labels map[string]string // list of tags, if not default, to apply
+	Labels    map[string]string // list of tags, if not default, to apply
+	MergeMode string            // project merge mode
 
 	NoProjectGitPR     bool // do not automatically create project git PRs, just assign reviewers and assume somethign else creates the ProjectGit PR
 	ManualMergeOnly    bool // only merge with "Merge OK" comment by Project Maintainers and/or Package Maintainers and/or reviewers
@@ -184,6 +189,17 @@ func ReadWorkflowConfig(gitea GiteaFileContentAndRepoFetcher, git_project string
 		}
 	}
 	config.GitProjectName = config.GitProjectName + "#" + branch
+
+	// verify merge modes
+	switch config.MergeMode {
+	case MergeModeFF, MergeModeDevel, MergeModeReplace:
+		break // good results
+	case "":
+		config.MergeMode = MergeModeFF
+	default:
+		return nil, fmt.Errorf("Unsupported merge mode in %s: %s", git_project, config.MergeMode)
+	}
+
 	return config, nil
 }
 

common/config_test.go

@@ -341,3 +341,67 @@ func TestConfigPermissions(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigMergeModeParser(t *testing.T) {
+	tests := []struct {
+		name      string
+		json      string
+		mergeMode string
+		wantErr   bool
+	}{
+		{
+			name:      "empty",
+			json:      "{}",
+			mergeMode: common.MergeModeFF,
+		},
+		{
+			name:      "ff-only",
+			json:      `{"MergeMode": "ff-only"}`,
+			mergeMode: common.MergeModeFF,
+		},
+		{
+			name:      "replace",
+			json:      `{"MergeMode": "replace"}`,
+			mergeMode: common.MergeModeReplace,
+		},
+		{
+			name:      "devel",
+			json:      `{"MergeMode": "devel"}`,
+			mergeMode: common.MergeModeDevel,
+		},
+		{
+			name:    "unsupported",
+			json:    `{"MergeMode": "invalid"}`,
+			wantErr: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			repo := models.Repository{
+				DefaultBranch: "master",
+			}
+
+			ctl := gomock.NewController(t)
+			gitea := mock_common.NewMockGiteaFileContentAndRepoFetcher(ctl)
+			gitea.EXPECT().GetRepositoryFileContent("foo", "bar", "", "workflow.config").Return([]byte(test.json), "abc", nil)
+			gitea.EXPECT().GetRepository("foo", "bar").Return(&repo, nil)
+
+			config, err := common.ReadWorkflowConfig(gitea, "foo/bar")
+			if test.wantErr {
+				if err == nil {
+					t.Fatal("Expected error, got nil")
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if config.MergeMode != test.mergeMode {
+				t.Errorf("Expected MergeMode %s, got %s", test.mergeMode, config.MergeMode)
+			}
+		})
+	}
+}

common/pr.go

@@ -468,7 +468,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 				LogError("Cannot fetch gita reaviews for PR:", err)
 				return false
 			}
-			r.SetRequiredReviewers(reviewers)
+			r.RequestedReviewers = reviewers
 			prjgit.Reviews = r
 			if prjgit.Reviews.IsManualMergeOK() {
 				is_manually_reviewed_ok = true
@@ -489,7 +489,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 					LogError("Cannot fetch gita reaviews for PR:", err)
 					return false
 				}
-				r.SetRequiredReviewers(reviewers)
+				r.RequestedReviewers = reviewers
 				pr.Reviews = r
 				if !pr.Reviews.IsManualMergeOK() {
 					LogInfo("Not approved manual merge. PR:", pr.PR.URL)
@@ -530,7 +530,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 			LogError("Cannot fetch gitea reaviews for PR:", err)
 			return false
 		}
-		r.SetRequiredReviewers(reviewers)
+		r.RequestedReviewers = reviewers
 
 		is_manually_reviewed_ok = r.IsApproved()
 		LogDebug("PR to", pr.PR.Base.Repo.Name, "reviewed?", is_manually_reviewed_ok)
@@ -554,6 +554,144 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 	return is_manually_reviewed_ok
 }
 
+func (rs *PRSet) AddMergeCommit(git Git, remote string, pr int) bool {
+	prinfo := rs.PRs[pr]
+
+	LogDebug("Adding merge commit for %s", PRtoString(prinfo.PR))
+	if !prinfo.PR.AllowMaintainerEdit {
+		LogError(" PR is not editable by maintainer")
+		return false
+	}
+
+	repo := prinfo.PR.Base.Repo
+	head := prinfo.PR.Head
+	br := rs.Config.Branch
+	if len(br) == 0 {
+		br = prinfo.PR.Base.Name
+	}
+
+	msg := fmt.Sprintf("Merge branch '%s' into %s", br, head.Name)
+	if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "-X", "theirs", head.Sha); err != nil {
+		if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "--allow-unrelated-histories", "-X", "theirs", head.Sha); err != nil {
+			return false
+		}
+		LogError("WARNING: Merging unrelated histories")
+	}
+
+	// ensure only files that are in head.Sha are kept
+	git.GitExecOrPanic(repo.Name, "read-tree", "--reset", "-u", head.Sha)
+	git.GitExecOrPanic(repo.Name, "commit", "-m", msg)
+
+	if !IsDryRun {
+		git.GitExecOrPanic(repo.Name, "push", remote, "HEAD:"+head.Name)
+		prinfo.PR.Head.Sha = strings.TrimSpace(git.GitExecWithOutputOrPanic(repo.Name, "rev-list", "-1", "HEAD")) // need to update as it's pushed but pr not refetched
+	}
+
+	return true
+}
+
+func (rs *PRSet) HasMerge(git Git, pr int) bool {
+	prinfo := rs.PRs[pr]
+
+	repo := prinfo.PR.Base.Repo
+	head := prinfo.PR.Head
+	br := rs.Config.Branch
+	if len(br) == 0 {
+		br = prinfo.PR.Base.Name
+	}
+
+	parents, err := git.GitExecWithOutput(repo.Name, "show", "-s", "--format=%P", head.Sha)
+	if err == nil {
+		p := strings.Fields(strings.TrimSpace(parents))
+		if len(p) == 2 {
+			targetHead, _ := git.GitExecWithOutput(repo.Name, "rev-parse", "HEAD")
+			targetHead = strings.TrimSpace(targetHead)
+			if p[0] == targetHead || p[1] == targetHead {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func (rs *PRSet) PrepareForMerge(git Git) bool {
+	// verify that package can merge here. Checkout current target branch of each PRSet, make a temporary branch
+	// PR_#_mergetest and perform the merge based
+
+	if rs.Config.MergeMode == MergeModeDevel {
+		return true // always can merge as we set branch here, not merge anything
+	} else {
+		// make sure that all the package PRs are in mergeable state
+		for idx, prinfo := range rs.PRs {
+			if rs.IsPrjGitPR(prinfo.PR) {
+				continue
+			}
+
+			repo := prinfo.PR.Base.Repo
+			head := prinfo.PR.Head
+			br := rs.Config.Branch
+			if len(br) == 0 {
+				br = prinfo.PR.Base.Name
+			}
+
+			remote, err := git.GitClone(repo.Name, br, repo.SSHURL)
+			if err != nil {
+				return false
+			}
+			git.GitExecOrPanic(repo.Name, "fetch", remote, head.Sha)
+			switch rs.Config.MergeMode {
+			case MergeModeFF:
+				if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil {
+					return false
+				}
+			case MergeModeReplace:
+			Verify:
+				if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil {
+					if !rs.HasMerge(git, idx) {
+						forkRemote, err := git.GitClone(repo.Name, head.Name, head.Repo.SSHURL)
+						if err != nil {
+							LogError("Failed to clone head repo:", head.Name, head.Repo.SSHURL)
+							return false
+						}
+						LogDebug("Merge commit is missing and this is not FF merge possibility")
+						git.GitExecOrPanic(repo.Name, "checkout", remote+"/"+br)
+						if !rs.AddMergeCommit(git, forkRemote, idx) {
+							return false
+						}
+						if !IsDryRun {
+							goto Verify
+						}
+					}
+				}
+			}
+		}
+	}
+
+	// now we check project git if mergeable
+	prjgit_info, err := rs.GetPrjGitPR()
+	if err != nil {
+		return false
+	}
+	prjgit := prjgit_info.PR
+
+	_, _, prjgitBranch := rs.Config.GetPrjGit()
+	remote, err := git.GitClone(DefaultGitPrj, prjgitBranch, prjgit.Base.Repo.SSHURL)
+	if err != nil {
+		return false
+	}
+
+	testBranch := fmt.Sprintf("PR_%d_mergetest", prjgit.Index)
+	git.GitExecOrPanic(DefaultGitPrj, "fetch", remote, prjgit.Head.Sha)
+	if err := git.GitExec(DefaultGitPrj, "checkout", "-B", testBranch, prjgit.Base.Sha); err != nil {
+		return false
+	}
+	if err := git.GitExec(DefaultGitPrj, "merge", "--no-ff", "--no-commit", prjgit.Head.Sha); err != nil {
+		return false
+	}
+
+	return true
+}
+
 func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error {
 	prjgit_info, err := rs.GetPrjGitPR()
 	if err != nil {
@@ -718,11 +856,10 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error {
 		prinfo.RemoteName, err = git.GitClone(repo.Name, br, repo.SSHURL)
 		PanicOnError(err)
 		git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha)
-
-		if isNewRepo {
-			LogInfo("Force-pushing new repository branch", br, "to", head.Sha)
-			// we don't merge, we just set the branch to this commit
+		if rs.Config.MergeMode == MergeModeDevel || isNewRepo {
+			git.GitExecOrPanic(repo.Name, "checkout", "-B", br, head.Sha)
 		} else {
+			git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha)
 			git.GitExecOrPanic(repo.Name, "merge", "--ff", head.Sha)
 		}
 	}
@@ -748,11 +885,15 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error {
 		}
 
 		if !IsDryRun {
-			if isNewRepo {
-				git.GitExecOrPanic(repo.Name, "push", "-f", prinfo.RemoteName, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name)
-			} else {
-				git.GitExecOrPanic(repo.Name, "push", prinfo.RemoteName)
+			params := []string{"push"}
+			if rs.Config.MergeMode == MergeModeDevel || isNewRepo {
+				params = append(params, "-f")
 			}
+			params = append(params, prinfo.RemoteName)
+			if isNewRepo {
+				params = append(params, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name)
+			}
+			git.GitExecOrPanic(repo.Name, params...)
 		} else {
 			LogInfo("*** WOULD push", repo.Name, "to", prinfo.RemoteName)
 		}

common/pr_test.go

@@ -2,6 +2,7 @@ package common_test
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"path"
@@ -807,8 +808,9 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
 						},
 						Reviews: &common.PRReviews{
-							Reviews: []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "m1"}}},
-							RequestedReviewers: []*models.TimelineComment{
+							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "m1"}}},
+							RequestedReviewers: []string{"m1"},
+							FullTimeline: []*models.TimelineComment{
 								{User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "m1"}, Type: common.TimelineCommentType_ReviewRequested},
 							},
 						},
@@ -918,7 +920,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 						},
 						Reviews: &common.PRReviews{
 							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
-							RequestedReviewers: []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []string{"reviewer"},
+							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
 						},
 					},
 					{
@@ -928,7 +931,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 						},
 						Reviews: &common.PRReviews{
 							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
-							RequestedReviewers: []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []string{"reviewer"},
+							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
 						},
 					},
 				},
@@ -963,7 +967,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateApproved, User: &models.User{UserName: "pkgmaintainer"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prjmaintainer"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user2"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
@@ -981,7 +986,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 							},
@@ -1021,7 +1027,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "someother"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "pkgm2", "someother", "prj1", "prj2"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
@@ -1044,7 +1051,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj1"}},
 								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1", "prj2"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj2"}},
@@ -1083,7 +1091,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "someother"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "someother", "prj1"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgm1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
@@ -1104,7 +1113,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
 							},
-							RequestedReviewers: []*models.TimelineComment{
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1"},
+							FullTimeline: []*models.TimelineComment{
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
 								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "!bot"}, Assignee: &models.User{UserName: "user1"}},
@@ -1190,9 +1200,6 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			test.prset.HasAutoStaging = !test.noAutoStaging
 			for idx, pr := range test.prset.PRs {
-				if pr.Reviews != nil {
-					pr.Reviews.SetRequiredReviewers(test.prset.Config.Reviewers)
-				}
 				missing, extra := test.prset.FindMissingAndExtraReviewers(test.maintainers, idx)
 
 				// avoid nil dereference below, by adding empty array elements
@@ -1267,7 +1274,7 @@ func TestPRMerge(t *testing.T) {
 						Owner: &models.User{
 							UserName: "org",
 						},
-						SSHURL: "file://" + path.Join(repoDir, "prjgit"),
+						SSHURL: "ssh://git@src.opensuse.org/org/prj.git",
 					},
 				},
 				Head: &models.PRBranchInfo{
@@ -1289,7 +1296,7 @@ func TestPRMerge(t *testing.T) {
 						Owner: &models.User{
 							UserName: "org",
 						},
-						SSHURL: "file://" + path.Join(cmd.Dir, "prjgit"),
+						SSHURL: "ssh://git@src.opensuse.org/org/prj.git",
 					},
 				},
 				Head: &models.PRBranchInfo{
@@ -1399,3 +1406,344 @@ func TestPRChanges(t *testing.T) {
 		})
 	}
 }
+
+func TestPRPrepareForMerge(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest)
+		config   *common.AutogitConfig
+		expected bool
+		editable bool
+	}{
+		{
+			name: "Success Devel",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeDevel,
+			},
+			setup:    func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {},
+			expected: true,
+		},
+		{
+			name: "Success FF",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeFF,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha)
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil)
+
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil)
+			},
+			expected: true,
+		},
+		{
+			name: "Success Replace MergeCommit",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeReplace,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha)
+				// merge-base fails initially
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor"))
+				// HasMerge returns true
+				m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1 target_head", nil)
+				m.EXPECT().GitExecWithOutput("pkg", "rev-parse", "HEAD").Return("target_head", nil)
+
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil)
+			},
+			expected: true,
+		},
+		{
+			name: "Merge Conflict in PrjGit",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeFF,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha)
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil)
+
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(fmt.Errorf("conflict"))
+			},
+			expected: false,
+		},
+		{
+			name: "Not FF in PkgGit",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeFF,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha)
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor"))
+			},
+			expected: false,
+		},
+		{
+			name: "Success Replace with AddMergeCommit",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeReplace,
+			},
+			editable: true,
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha)
+				// First merge-base fails
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor"))
+				// HasMerge returns false
+				m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1", nil)
+				m.EXPECT().GitClone("pkg", pkgPR.Head.Name, pkgPR.Base.Repo.SSHURL).Return("origin_fork", nil)
+				// AddMergeCommit is called
+				m.EXPECT().GitExecOrPanic("pkg", "checkout", "origin/master")
+				m.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil)
+				m.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha)
+				m.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any())
+				m.EXPECT().GitExecOrPanic("pkg", "push", "origin_fork", "HEAD:"+pkgPR.Head.Name)
+				m.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_pkg_head_sha")
+				// Second merge-base succeeds (after goto Verify)
+				m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", "new_pkg_head_sha").Return(nil)
+
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil)
+			},
+			expected: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			prjPR := &models.PullRequest{
+				Index: 1,
+				Base: &models.PRBranchInfo{
+					Name: "master",
+					Sha:  "base_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "_ObsPrj",
+						SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git",
+					},
+				},
+				Head: &models.PRBranchInfo{
+					Sha: "head_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "_ObsPrj",
+						SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git",
+					},
+				},
+			}
+
+			pkgPR := &models.PullRequest{
+				Index: 2,
+				Base: &models.PRBranchInfo{
+					Name: "master",
+					Sha:  "pkg_base_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "pkg",
+						SSHURL: "ssh://git@src.opensuse.org/org/pkg.git",
+					},
+				},
+				Head: &models.PRBranchInfo{
+					Name: "branch_name",
+					Sha:  "pkg_head_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "pkg",
+						SSHURL: "ssh://git@src.opensuse.org/org/pkg.git",
+					},
+				},
+				AllowMaintainerEdit: test.editable,
+			}
+
+			ctl := gomock.NewController(t)
+			git := mock_common.NewMockGit(ctl)
+			test.setup(git, prjPR, pkgPR)
+
+			prset := &common.PRSet{
+				Config: test.config,
+				PRs: []*common.PRInfo{
+					{PR: prjPR},
+					{PR: pkgPR},
+				},
+			}
+
+			if res := prset.PrepareForMerge(git); res != test.expected {
+				t.Errorf("Expected %v, got %v", test.expected, res)
+			}
+		})
+	}
+}
+
+func TestPRMergeMock(t *testing.T) {
+	tests := []struct {
+		name   string
+		setup  func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest)
+		config *common.AutogitConfig
+	}{
+		{
+			name: "Success FF",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeFF,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil)
+
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin_pkg", pkgPR.Head.Sha)
+				m.EXPECT().GitExecOrPanic("pkg", "merge", "--ff", pkgPR.Head.Sha)
+				m.EXPECT().GitExecOrPanic("pkg", "push", "origin_pkg")
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin")
+			},
+		},
+		{
+			name: "Success Devel",
+			config: &common.AutogitConfig{
+				Organization:   "org",
+				GitProjectName: "org/_ObsPrj#master",
+				MergeMode:      common.MergeModeDevel,
+			},
+			setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {
+				m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil)
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha)
+				m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil)
+
+				m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil)
+				m.EXPECT().GitExecOrPanic("pkg", "checkout", "-B", "master", pkgPR.Head.Sha)
+				m.EXPECT().GitExecOrPanic("pkg", "push", "-f", "origin_pkg")
+				m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin")
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			prjPR := &models.PullRequest{
+				Index: 1,
+				Base: &models.PRBranchInfo{
+					Name: "master",
+					Sha:  "prj_base_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "_ObsPrj",
+						SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git",
+					},
+				},
+				Head: &models.PRBranchInfo{
+					Sha: "prj_head_sha",
+				},
+			}
+			pkgPR := &models.PullRequest{
+				Index: 2,
+				Base: &models.PRBranchInfo{
+					Name: "master",
+					Sha:  "pkg_base_sha",
+					Repo: &models.Repository{
+						Owner:  &models.User{UserName: "org"},
+						Name:   "pkg",
+						SSHURL: "ssh://git@src.opensuse.org/org/pkg.git",
+					},
+				},
+				Head: &models.PRBranchInfo{
+					Sha: "pkg_head_sha",
+				},
+			}
+
+			ctl := gomock.NewController(t)
+			git := mock_common.NewMockGit(ctl)
+			reviewUnrequestMock := mock_common.NewMockGiteaReviewUnrequester(ctl)
+			reviewUnrequestMock.EXPECT().UnrequestReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil)
+
+			test.setup(git, prjPR, pkgPR)
+
+			prset := &common.PRSet{
+				Config: test.config,
+				PRs: []*common.PRInfo{
+					{PR: prjPR},
+					{PR: pkgPR},
+				},
+			}
+
+			if err := prset.Merge(reviewUnrequestMock, git); err != nil {
+				t.Errorf("Unexpected error: %v", err)
+			}
+		})
+	}
+}
+
+func TestPRAddMergeCommit(t *testing.T) {
+	pkgPR := &models.PullRequest{
+		Index: 2,
+		Base: &models.PRBranchInfo{
+			Name: "master",
+			Sha:  "pkg_base_sha",
+			Repo: &models.Repository{
+				Owner:  &models.User{UserName: "org"},
+				Name:   "pkg",
+				SSHURL: "ssh://git@src.opensuse.org/org/pkg.git",
+			},
+		},
+		Head: &models.PRBranchInfo{
+			Name: "branch_name",
+			Sha:  "pkg_head_sha",
+		},
+		AllowMaintainerEdit: true,
+	}
+
+	config := &common.AutogitConfig{
+		Organization:   "org",
+		GitProjectName: "org/_ObsPrj#master",
+		MergeMode:      common.MergeModeReplace,
+	}
+
+	ctl := gomock.NewController(t)
+	git := mock_common.NewMockGit(ctl)
+
+	git.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil)
+	git.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha)
+	git.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any())
+	git.EXPECT().GitExecOrPanic("pkg", "push", "origin", "HEAD:branch_name")
+	git.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_head_sha")
+
+	prset := &common.PRSet{
+		Config: config,
+		PRs: []*common.PRInfo{
+			{PR: &models.PullRequest{}}, // prjgit at index 0
+			{PR: pkgPR},                 // pkg at index 1
+		},
+	}
+
+	if res := prset.AddMergeCommit(git, "origin", 1); !res {
+		t.Errorf("Expected true, got %v", res)
+	}
+}
+

common/reviews.go

@@ -8,28 +8,12 @@ import (
 	"src.opensuse.org/autogits/common/gitea-generated/models"
 )
 
-type ReviewInterface interface {
-	IsManualMergeOK() bool
-	IsApproved() bool
-	MisingReviews() []string
-	FindReviewRequester(reviewer string) *models.TimelineComment
-	HasPendingReviewBy(reviewer string) bool
-	IsReviewedBy(reviewer string) bool
-	IsReviewedByOneOf(reviewers ...string) bool
-
-	SetRequiredReviewers(reviewers []string)
-}
-
 type PRReviews struct {
 	Reviews            []*models.PullReview
-	RequestedReviewers []*models.TimelineComment
+	RequestedReviewers []string
 	Comments           []*models.TimelineComment
 
-	RequiredReviewers []string
-}
-
-func (r *PRReviews) SetRequiredReviewers(reviewers []string) {
-	r.RequiredReviewers = reviewers
+	FullTimeline []*models.TimelineComment
 }
 
 func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64) (*PRReviews, error) {
@@ -44,11 +28,11 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	}
 
 	reviews := make([]*models.PullReview, 0, 10)
+	needNewReviews := []string{}
 	var comments []*models.TimelineComment
 
-	var foundUsers []string
 	alreadyHaveUserReview := func(user string) bool {
-		if slices.Contains(foundUsers, user) {
+		if slices.Contains(needNewReviews, user) {
 			return true
 		}
 		for _, r := range reviews {
@@ -64,24 +48,20 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	LogDebug("Number of items in timeline:", len(timeline))
 
 	cutOffIdx := len(timeline)
-	var PendingRequestedReviews []*models.TimelineComment
 	for idx, item := range timeline {
-		if item.Type == TimelineCommentType_Review {
+		if item.Type == TimelineCommentType_Review || item.Type == TimelineCommentType_ReviewRequested {
 			for _, r := range rawReviews {
 				if r.ID == item.ReviewID && r.User != nil {
 					if !alreadyHaveUserReview(r.User.UserName) {
-						if idx < cutOffIdx {
+						if item.Type == TimelineCommentType_Review && idx > cutOffIdx {
+							needNewReviews = append(needNewReviews, r.User.UserName)
+						} else {
 							reviews = append(reviews, r)
 						}
-						foundUsers = append(foundUsers, r.User.UserName)
 					}
 					break
 				}
 			}
-		} else if item.Type == TimelineCommentType_ReviewRequested && item.Assignee != nil && !alreadyHaveUserReview(item.Assignee.UserName) {
-			PendingRequestedReviews = append(PendingRequestedReviews, item)
-		} else if item.Type == TimelineCommentType_DismissReview && item.Assignee != nil && !alreadyHaveUserReview(item.Assignee.UserName) {
-			foundUsers = append(foundUsers, item.Assignee.UserName)
 		} else if item.Type == TimelineCommentType_Comment && cutOffIdx > idx {
 			comments = append(comments, item)
 		} else if item.Type == TimelineCommentType_PushPull && cutOffIdx == len(timeline) {
@@ -94,9 +74,9 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64
 	LogDebug("num comments:", len(comments), "timeline:", len(reviews))
 
 	return &PRReviews{
-		Reviews:            reviews,
-		Comments:           comments,
-		RequestedReviewers: PendingRequestedReviews,
+		Reviews:      reviews,
+		Comments:     comments,
+		FullTimeline: timeline,
 	}, nil
 }
 
@@ -124,7 +104,7 @@ func (r *PRReviews) IsManualMergeOK() bool {
 			continue
 		}
 		LogDebug("comment:", c.User.UserName, c.Body)
-		if slices.Contains(r.RequiredReviewers, c.User.UserName) {
+		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
@@ -135,7 +115,7 @@ func (r *PRReviews) IsManualMergeOK() bool {
 		if c.Updated != c.Submitted {
 			continue
 		}
-		if slices.Contains(r.RequiredReviewers, c.User.UserName) {
+		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
@@ -151,7 +131,7 @@ func (r *PRReviews) IsApproved() bool {
 	}
 	goodReview := true
 
-	for _, reviewer := range r.RequiredReviewers {
+	for _, reviewer := range r.RequestedReviewers {
 		goodReview = false
 		for _, review := range r.Reviews {
 			if review.User.UserName == reviewer && review.State == ReviewStateApproved && !review.Stale && !review.Dismissed {
@@ -175,7 +155,7 @@ func (r *PRReviews) MissingReviews() []string {
 		return missing
 	}
 
-	for _, reviewer := range r.RequiredReviewers {
+	for _, reviewer := range r.RequestedReviewers {
 		if !r.IsReviewedBy(reviewer) {
 			missing = append(missing, reviewer)
 		}
@@ -188,11 +168,12 @@ func (r *PRReviews) FindReviewRequester(reviewer string) *models.TimelineComment
 		return nil
 	}
 
-	for _, t := range r.RequestedReviewers {
-		if t.Assignee.UserName == reviewer {
-			return t
+	for _, r := range r.FullTimeline {
+		if r.Type == TimelineCommentType_ReviewRequested && r.Assignee.UserName == reviewer {
+			return r
 		}
 	}
+
 	return nil
 }
 
@@ -212,13 +193,6 @@ func (r *PRReviews) HasPendingReviewBy(reviewer string) bool {
 		}
 	}
 
-	// at this point, we do not have actual review by user. Check if we have a pending review
-	for _, t := range r.RequestedReviewers {
-		if t.Assignee != nil && t.Assignee.UserName == reviewer {
-			return true
-		}
-	}
-
 	return false
 }
 

common/reviews_test.go

@@ -145,53 +145,6 @@ func TestReviews(t *testing.T) {
 			reviewers:  []string{"user1"},
 			isApproved: false,
 		},
-		{
-			name:    "ReviewRequested predates PushPull should be seen as pending",
-			reviews: []*models.PullReview{},
-			timeline: []*models.TimelineComment{
-				{Type: common.TimelineCommentType_PushPull},
-				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
-			},
-			reviewers:        []string{"user1"},
-			isPendingByTest1: true,
-		},
-		{
-			name:    "ReviewRequested postdates PushPull but blocked by older dismiss",
-			reviews: []*models.PullReview{},
-			timeline: []*models.TimelineComment{
-				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
-				{Type: common.TimelineCommentType_PushPull},
-				{Type: common.TimelineCommentType_ReviewDismissed, Assignee: &models.User{UserName: "user1"}},
-			},
-			reviewers:        []string{"user1"},
-			isPendingByTest1: true,
-		},
-		{
-			name: "ReviewRequested predates PushPull should be seen as pending",
-			reviews: []*models.PullReview{
-				{ID: 101, State: common.ReviewStateRequestReview, User: &models.User{UserName: "user1"}},
-			},
-			timeline: []*models.TimelineComment{
-				{Type: common.TimelineCommentType_PushPull},
-				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
-			},
-			reviewers:        []string{"user1"},
-			isPendingByTest1: true,
-		},
-		{
-			name: "Review requested, review, then push needs re-requesting",
-			reviews: []*models.PullReview{
-				{ID: 100, State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
-			},
-			timeline: []*models.TimelineComment{
-				{Type: common.TimelineCommentType_PushPull},
-				{Type: common.TimelineCommentType_Review, ReviewID: 100},
-				{Type: common.TimelineCommentType_ReviewRequested, Assignee: &models.User{UserName: "user1"}},
-			},
-			reviewers:         []string{"user1"},
-			isReviewedByTest1: false, // Should be stale
-			isPendingByTest1:  false, // Should be stale
-		},
 	}
 
 	for _, test := range tests {
@@ -213,7 +166,7 @@ func TestReviews(t *testing.T) {
 				}
 				return
 			}
-			reviews.SetRequiredReviewers(test.reviewers)
+			reviews.RequestedReviewers = test.reviewers
 
 			if r := reviews.IsApproved(); r != test.isApproved {
 				t.Fatal("Unexpected IsReviewed():", r, "vs. expected", test.isApproved)

common/timeline.go

@@ -8,7 +8,6 @@ import (
 )
 
 const (
-	TimelineCommentType_ReviewDismissed = "dismiss_review"
 	TimelineCommentType_ReviewRequested = "review_request"
 	TimelineCommentType_Review          = "review"
 	TimelineCommentType_PushPull        = "pull_push"

integration/test-plan.md

@@ -65,11 +65,11 @@ The testing will be conducted in a dedicated test environment that mimics the pr
 | **TC-REVIEW-001** | P | **Add mandatory reviewers** | 1. Create a new PackageGit PR. | 1. All mandatory reviewers are added to both the PackageGit and ProjectGit PRs. | High |
 | **TC-REVIEW-002** | - | **Add advisory reviewers** | 1. Create a new PackageGit PR with advisory reviewers defined in the configuration. | 1. Advisory reviewers are added to the PR, but their approval is not required for merging. | Medium |
 | **TC-REVIEW-003** | - | **Re-add reviewers** | 1. Push a new commit to a PackageGit PR after it has been approved. | 1. The original reviewers are re-added to the PR. | Medium |
-| **TC-REVIEW-004** | P | **Package PR created by a maintainer** | 1. Create a PackageGit PR from the account of a package maintainer. | 1. No review is requested from other package maintainers. | High |
+| **TC-REVIEW-004** | x | **Package PR created by a maintainer** | 1. Create a PackageGit PR from the account of a package maintainer. | 1. No review is requested from other package maintainers. | High |
 | **TC-REVIEW-005** | P | **Package PR created by an external user (approve)** | 1. Create a PackageGit PR from the account of a user who is not a package maintainer.<br>2. One of the package maintainers approves the PR. | 1. All package maintainers are added as reviewers.<br>2. Once one maintainer approves the PR, the other maintainers are removed as reviewers. | High |
 | **TC-REVIEW-006** | P | **Package PR created by an external user (reject)** | 1. Create a PackageGit PR from the account of a user who is not a package maintainer.<br>2. One of the package maintainers rejects the PR. | 1. All package maintainers are added as reviewers.<br>2. Once one maintainer rejects the PR, the other maintainers are removed as reviewers. | High |
 | **TC-REVIEW-007** | P | **Package PR created by a maintainer with ReviewRequired=true** | 1. Set `ReviewRequired = true` in `workflow.config`.<br>2. Create a PackageGit PR from the account of a package maintainer. | 1. A review is requested from other package maintainers if available. | High |
-| **TC-MERGE-001** | P | **Automatic Merge** | 1. Create a PackageGit PR.<br>2. Ensure all mandatory reviews are completed on both project and package PRs. | 1. The PR is automatically merged. | High |
+| **TC-MERGE-001** | x | **Automatic Merge** | 1. Create a PackageGit PR.<br>2. Ensure all mandatory reviews are completed on both project and package PRs. | 1. The PR is automatically merged. | High |
 | **TC-MERGE-002** | - | **ManualMergeOnly with Package Maintainer** | 1. Create a PackageGit PR with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on the package PR from the account of a package maintainer for that package. | 1. The PR is merged. | High |
 | **TC-MERGE-003** | - | **ManualMergeOnly with unauthorized user** | 1. Create a PackageGit PR with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on the package PR from the account of a user who is not a maintainer for that package. | 1. The PR is not merged. | High |
 | **TC-MERGE-004** | - | **ManualMergeOnly with multiple packages** | 1. Create a ProjectGit PR that references multiple PackageGit PRs with `ManualMergeOnly` set to `true`.<br>2. Ensure all mandatory reviews are completed on both project and package PRs.<br>3. Comment "merge ok" on each package PR from the account of a package maintainer. | 1. The PR is merged only after "merge ok" is commented on all associated PackageGit PRs. | High |

integration/tests/workflow_pr_merge_test.py

@@ -5,6 +5,7 @@ from pathlib import Path
 from tests.lib.common_test_utils import GiteaAPIClient
 
 @pytest.mark.t001
+@pytest.mark.xfail(reason="The bot sometimes re-request reviews despite having all the approvals")
 def test_001_automerge(automerge_env, test_user_client):
     """
     Test scenario:

integration/tests/workflow_pr_review_test.py

@@ -43,6 +43,7 @@ index 0000000..e69de29
 
 
 @pytest.mark.t004
+@pytest.mark.xfail(reason="the bot sometimes re-requests review from autogits_obs_staging_bot despite having the approval")
 def test_004_maintainer(maintainer_env, ownerA_client):
     """
     Test scenario:
@@ -151,6 +152,7 @@ index 0000000..e69de29
 
 
 @pytest.mark.t005
+# @pytest.mark.xfail(reason="TBD troubleshoot")
 def test_005_any_maintainer_approval_sufficient(maintainer_env, ownerA_client, ownerBB_client):
     """
     Test scenario:

workflow-pr/README.md

@@ -96,6 +96,19 @@ Package Deletion Requests
   If you wish to re-add a package, create a new PrjGit PR which adds again the submodule on the branch that has the "-removed" suffix. The bot will automatically remove this suffix from the project branch in the pool.
 
 
+Merge Modes
+-----------
+
+| Merge Mode | Description
+|------------|--------------------------------------------------------------------------------
+| ff-only    | Only allow --ff-only merges in the package branch. This is best suited for
+|            | devel projects and openSUSE Tumbleweed development, where history should be linear
+| replace    | Merge is done via `-X theirs` strategy and old files are removed in the merge.
+|            | This works well for downstream codestreams, like Leap, that would update their branch
+|            | using latest version.
+| devel      | No merge, just set the project branch to PR HEAD. This is suitable for downstream
+|            | projects like Leap during development cycle, where keeping maintenance history is not important
+
 Labels
 ------
 

workflow-pr/pr_processor.go

@@ -413,6 +413,12 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
 	}
 	common.LogInfo("fetched PRSet of size:", len(prset.PRs))
 
+	if !prset.PrepareForMerge(git) {
+		common.LogError("PRs are NOT mergeable.")
+	} else {
+		common.LogInfo("PRs are in mergeable state.")
+	}
+
 	prjGitPRbranch := prGitBranchNameForPR(prRepo, prNo)
 	prjGitPR, err := prset.GetPrjGitPR()
 	if err == common.PRSet_PrjGitMissing {