diff --git a/pkg/commands/git_commands/branch_loader.go b/pkg/commands/git_commands/branch_loader.go index b41b0564f..321141c69 100644 --- a/pkg/commands/git_commands/branch_loader.go +++ b/pkg/commands/git_commands/branch_loader.go @@ -343,23 +343,69 @@ func (self *BranchLoader) GetBaseBranch(branch *models.Branch, mainBranches *Mai return "", nil } + mainBranchRefs := mainBranches.Get() output, err := self.cmd.New( NewGitCmd("for-each-ref"). Arg("--contains"). Arg(mergeBase). Arg("--format=%(refname)"). - Arg(mainBranches.Get()...). + Arg(mainBranchRefs...). ToArgv(), ).DontLog().RunWithOutput() if err != nil { return "", err } trimmedOutput := strings.TrimSpace(output) - split := strings.Split(trimmedOutput, "\n") - if len(split) == 0 || split[0] == "" { + if trimmedOutput == "" { return "", nil } - return split[0], nil + contained := strings.Split(trimmedOutput, "\n") + + // for-each-ref sorts its output alphabetically by refname regardless of + // the order we passed the refs in. Restore the user's configured order so + // it can serve as the natural tiebreaker. + candidates := lo.Filter(mainBranchRefs, func(ref string, _ int) bool { + return lo.Contains(contained, ref) + }) + if len(candidates) == 0 { + return "", nil + } + if len(candidates) == 1 { + return candidates[0], nil + } + + // Multiple main branches contain the merge-base. Pick the "closest" by + // the same definition the fast path uses (smallest ahead value, i.e. + // fewest branch commits not in the base). Ties fall back to config + // order, which `candidates` already preserves. + bestIdx := 0 + bestAhead := -1 + for i, ref := range candidates { + revListOutput, err := self.cmd.New( + NewGitCmd("rev-list"). + Arg("--left-right"). + Arg("--count"). + Arg(fmt.Sprintf("%s...%s", branch.FullRefName(), ref)). + ToArgv(), + ).DontLog().RunWithOutput() + if err != nil { + return "", err + } + parts := strings.Fields(strings.TrimSpace(revListOutput)) + if len(parts) != 2 { + continue + } + ahead, err := strconv.Atoi(parts[0]) + if err != nil { + continue + } + if bestAhead < 0 || ahead < bestAhead { + bestAhead = ahead + bestIdx = i + } + } + + return candidates[bestIdx], nil } func (self *BranchLoader) obtainBranches() []*models.Branch { diff --git a/pkg/commands/git_commands/branch_loader_test.go b/pkg/commands/git_commands/branch_loader_test.go index 339bfe59f..2a0da414e 100644 --- a/pkg/commands/git_commands/branch_loader_test.go +++ b/pkg/commands/git_commands/branch_loader_test.go @@ -492,13 +492,11 @@ func TestGetBehindBaseBranchValuesForAllBranches_LegacyPath(t *testing.T) { runner.CheckForMissingCalls() } -// When the merge-base is contained in more than one configured main branch, -// git for-each-ref returns those refs sorted alphabetically by refname, -// regardless of the order we pass them in. The chosen base should respect -// the user's configured order ("main" first), not the alphabetical accident. -// -// Demonstrates the bug; the expected behavior is asserted in the next commit. -func TestGetBaseBranch_AmbiguousPicksAlphabeticalNotConfigOrder(t *testing.T) { +// When the branch's merge-base is contained in more than one configured main +// branch and the ahead counts are equal, the chosen base must respect the +// user's configured order rather than the alphabetical order of +// for-each-ref's output. +func TestGetBaseBranch_AmbiguousFallsBackToConfigOrder(t *testing.T) { mainBranchRefs := []string{"refs/heads/main", "refs/heads/develop"} branch := &models.Branch{Name: "feat-x"} @@ -511,7 +509,13 @@ func TestGetBaseBranch_AmbiguousPicksAlphabeticalNotConfigOrder(t *testing.T) { "for-each-ref", "--contains", "abc123", "--format=%(refname)", "refs/heads/main", "refs/heads/develop", }, - "refs/heads/develop\nrefs/heads/main\n", nil) + "refs/heads/develop\nrefs/heads/main\n", nil). + ExpectGitArgs( + []string{"rev-list", "--left-right", "--count", "refs/heads/feat-x...refs/heads/main"}, + "5\t10\n", nil). + ExpectGitArgs( + []string{"rev-list", "--left-right", "--count", "refs/heads/feat-x...refs/heads/develop"}, + "5\t8\n", nil) gitCommon := buildGitCommon(commonDeps{runner: runner}) @@ -530,9 +534,54 @@ func TestGetBaseBranch_AmbiguousPicksAlphabeticalNotConfigOrder(t *testing.T) { baseBranch, err := loader.GetBaseBranch(branch, mainBranches) assert.NoError(t, err) - // Want: "refs/heads/main" (first in config order among the tied candidates). - // Have: "refs/heads/develop" (alphabetical first from for-each-ref). - assert.Equal(t, "refs/heads/develop", baseBranch) + assert.Equal(t, "refs/heads/main", baseBranch) + + runner.CheckForMissingCalls() +} + +// When a configured main branch has a strictly smaller ahead count than any +// other (e.g. the branch was forked off `main` after main's last merge into +// `develop`, so `develop` doesn't yet contain the fork point's recent main +// history), that base wins outright regardless of config order. +func TestGetBaseBranch_UnambiguousPicksSmallestAhead(t *testing.T) { + mainBranchRefs := []string{"refs/heads/develop", "refs/heads/main"} + branch := &models.Branch{Name: "feat-x"} + + runner := oscommands.NewFakeRunner(t). + ExpectGitArgs( + []string{"merge-base", "refs/heads/feat-x", "refs/heads/develop", "refs/heads/main"}, + "abc123\n", nil). + ExpectGitArgs( + []string{ + "for-each-ref", "--contains", "abc123", "--format=%(refname)", + "refs/heads/develop", "refs/heads/main", + }, + "refs/heads/develop\nrefs/heads/main\n", nil). + ExpectGitArgs( + []string{"rev-list", "--left-right", "--count", "refs/heads/feat-x...refs/heads/develop"}, + "8\t3\n", nil). + ExpectGitArgs( + []string{"rev-list", "--left-right", "--count", "refs/heads/feat-x...refs/heads/main"}, + "5\t10\n", nil) + + gitCommon := buildGitCommon(commonDeps{runner: runner}) + + loader := &BranchLoader{ + Common: gitCommon.Common, + GitCommon: gitCommon, + cmd: gitCommon.cmd, + } + + mainBranches := &MainBranches{ + c: gitCommon.Common, + cmd: gitCommon.cmd, + existingMainBranches: mainBranchRefs, + previousMainBranches: gitCommon.Common.UserConfig().Git.MainBranches, + } + + baseBranch, err := loader.GetBaseBranch(branch, mainBranches) + assert.NoError(t, err) + assert.Equal(t, "refs/heads/main", baseBranch) runner.CheckForMissingCalls() }