From f256cd82ff02eff57acf92a40355af53b3161c5d Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Thu, 7 May 2026 10:42:51 -0500 Subject: [PATCH 1/3] fix(git): resolve stale local base to remote-tracking ref GetBranchBase returned the literal branch..base config value, so a bare name like "main" was used as a local-branch ref. When local main lagged behind origin/main (e.g., after rebasing a feature branch onto origin/main without pulling local main), merge-base(local-main, HEAD) walked back through commits already merged into the remote trunk, and the resulting range pulled in unrelated trunk commits. GetBranchBase now translates a bare name to the remote-tracking ref (local branch's @{upstream} when configured, else origin/) when local exists and is an ancestor of the remote-tracking ref. Qualified refs (containing '/') and divergent local branches are passed through unchanged so explicit user intent is preserved. --- cmd/roborev/review_test.go | 57 +++++++++++++++++++ internal/git/git.go | 52 ++++++++++++++++- internal/git/git_test.go | 114 +++++++++++++++++++++++++++++++++++++ 3 files changed, 220 insertions(+), 3 deletions(-) diff --git a/cmd/roborev/review_test.go b/cmd/roborev/review_test.go index 9b9d5d62..58176533 100644 --- a/cmd/roborev/review_test.go +++ b/cmd/roborev/review_test.go @@ -474,6 +474,38 @@ func TestReviewBranchFlag(t *testing.T) { req := <-reqCh assert.Equal(t, mainSHA+"..HEAD", req.GitRef) }) + + t.Run("branch review uses origin counterpart when local main is stale after rebase", func(t *testing.T) { + // Regression: a feature branch rebased onto origin/main with a stale + // local main pointing at an older commit must review only the new + // branch commits — not the trunk commits that already landed on + // origin/main between the local main pointer and the rebase target. + repo, mux := setupTestEnvironment(t) + reqCh := mockEnqueue(t, mux) + + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + staleMainSHA := repo.Run("rev-parse", "HEAD") + // Two commits land on the trunk after local main was last updated. + repo.CommitFile("trunk1.txt", "t1", "trunk advance 1") + repo.CommitFile("trunk2.txt", "t2", "trunk advance 2") + freshOriginSHA := repo.Run("rev-parse", "HEAD") + // Rewind local main to simulate a stale checkout. + repo.Run("update-ref", "refs/heads/main", staleMainSHA) + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", freshOriginSHA) + // Feature branch was rebased onto origin/main. + repo.Run("checkout", "-b", "feature", freshOriginSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "main") + + _, _, err := executeReviewCmd("--repo", repo.Dir, "--branch") + require.NoError(t, err) + + req := <-reqCh + assert.Equal(t, freshOriginSHA+"..HEAD", req.GitRef, + "range must start at origin/main, not stale local main") + }) } func TestReviewFastFlag(t *testing.T) { @@ -715,6 +747,31 @@ func TestTryBranchReview(t *testing.T) { assert.False(t, ok, "local main tracking upstream/main must be treated as base branch") }) + t.Run("uses origin counterpart when local base is stale ancestor", func(t *testing.T) { + // Mirrors the rebase-onto-origin/main scenario for the post-commit + // hook path (tryBranchReview): branch..base = main with a + // stale local main must resolve to origin/main, not the rewound + // pointer, so the hook enqueues only the real branch commits. + repo := newTestGitRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + staleMainSHA := repo.Run("rev-parse", "HEAD") + repo.CommitFile("trunk.txt", "t", "trunk advance") + freshOriginSHA := repo.Run("rev-parse", "HEAD") + repo.Run("update-ref", "refs/heads/main", staleMainSHA) + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", freshOriginSHA) + repo.Run("checkout", "-b", "feature", freshOriginSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "main") + writeRoborevConfig(t, repo, `post_commit_review = "branch"`) + + ref, ok := tryBranchReview(repo.Dir, "") + require.True(t, ok, "expected branch review with stale local main") + assert.Equal(t, freshOriginSHA+"..HEAD", ref, + "range must start at origin/main, not stale local main") + }) + t.Run("prefers branch upstream over default branch", func(t *testing.T) { // Reproduces the "single commit past upstream" bug: when the default // branch (e.g., origin/main) lags behind the tip the branch actually diff --git a/internal/git/git.go b/internal/git/git.go index 7e1f97aa..f11ecc0f 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -1353,14 +1353,60 @@ func readUpstreamConfig(repoPath, ref string) (upstreamConfig, bool) { }, true } -// GetBranchBase returns roborev's branch..base config value for ref. -// Passing an empty ref is equivalent to HEAD. +// GetBranchBase returns the base ref derived from ref's branch..base +// config. Bare names (e.g. "main") are resolved to the remote-tracking ref +// (e.g. "origin/main") that the local branch tracks when local exists +// and is an ancestor of the remote-tracking ref — i.e., local is just +// a stale snapshot of the trunk. Without this, merge-base(local-main, HEAD) +// walks back to the stale pointer and the resulting range pulls in commits +// already merged into the remote trunk after a rebase-onto-origin/main. +// Qualified refs (containing '/') are returned as-is so users can pin to +// e.g. "upstream/main". Passing an empty ref is equivalent to HEAD. func GetBranchBase(repoPath, ref string) string { branch := branchNameForConfig(repoPath, ref) if branch == "" { return "" } - return readGitConfig(repoPath, "branch."+branch+".base") + raw := readGitConfig(repoPath, "branch."+branch+".base") + return resolveConfiguredBase(repoPath, raw) +} + +// resolveConfiguredBase translates a bare base name to the remote-tracking +// ref it should map to, or returns configValue unchanged when no translation +// applies. Qualified refs and divergent local branches are passed through. +func resolveConfiguredBase(repoPath, configValue string) string { + if configValue == "" || strings.Contains(configValue, "/") { + return configValue + } + remoteTracking := preferredRemoteTracking(repoPath, configValue) + if remoteTracking == "" { + return configValue + } + if !refExists(repoPath, "refs/heads/"+configValue) { + return remoteTracking + } + isAncestor, err := IsAncestor(repoPath, configValue, remoteTracking) + if err != nil || !isAncestor { + return configValue + } + return remoteTracking +} + +// preferredRemoteTracking returns the remote-tracking ref short name for a +// bare branch name, preferring the branch's configured @{upstream} so fork +// workflows (local main → upstream/main) translate correctly, and falling +// back to origin/. Returns "" when no remote-tracking counterpart +// resolves locally. +func preferredRemoteTracking(repoPath, bareName string) string { + if cfg, ok := readUpstreamConfig(repoPath, bareName); ok && + strings.HasPrefix(cfg.qualified, "refs/remotes/") && + refExists(repoPath, cfg.qualified) { + return cfg.short + } + if refExists(repoPath, "refs/remotes/origin/"+bareName) { + return "origin/" + bareName + } + return "" } func branchNameForConfig(repoPath, ref string) string { diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 9d3d65f1..366e87a6 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -2087,3 +2087,117 @@ func TestLooksLikeSHA(t *testing.T) { }) } } + +func TestGetBranchBase(t *testing.T) { + t.Run("returns empty when not configured", func(t *testing.T) { + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + repo.Run("checkout", "-b", "feature") + + assert.Empty(t, GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("returns local bare name when no remote-tracking exists", func(t *testing.T) { + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("resolves bare name to origin counterpart when local is stale ancestor", func(t *testing.T) { + // Reproduces the post-rebase bug: branch..base = main with + // a stale local main pulls extra origin/main commits into the merge- + // base..HEAD range. The fix translates the bare name to origin/main + // when local main is just an ancestor of origin/main. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + staleMainSHA := repo.HeadSHA() + repo.CommitFile("trunk.txt", "trunk", "trunk advance") + freshOriginSHA := repo.HeadSHA() + // Rewind local main so it lags one commit behind origin/main. + repo.Run("update-ref", "refs/heads/main", staleMainSHA) + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", freshOriginSHA) + repo.Run("checkout", "-b", "feature", freshOriginSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "origin/main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("keeps local name when local has diverged from origin", func(t *testing.T) { + // Divergence (local main has commits not on origin/main) means the + // user's local branch is not just stale — respect their config + // rather than silently picking a different base. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + baseSHA := repo.HeadSHA() + repo.CommitFile("origin.txt", "origin", "origin-only commit") + originMainSHA := repo.HeadSHA() + // Local main diverges with its own commit not present on origin/main. + repo.Run("update-ref", "refs/heads/main", baseSHA) + repo.Run("checkout", "main") + repo.CommitFile("local.txt", "local", "local-only commit") + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", originMainSHA) + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("returns origin counterpart when local branch is missing", func(t *testing.T) { + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/scratch") + repo.CommitFile("base.txt", "base", "initial") + originSHA := repo.HeadSHA() + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", originSHA) + // Note: no refs/heads/main. Branch from origin/main directly. + repo.Run("checkout", "-b", "feature", originSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "origin/main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("returns qualified ref unchanged", func(t *testing.T) { + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "upstream/main") + + assert.Equal(t, "upstream/main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("prefers configured upstream over origin counterpart", func(t *testing.T) { + // Fork workflow: local main tracks upstream/main, and origin/main + // also exists (e.g., points at the user's fork). The base should + // resolve to upstream/main because that's what local main tracks. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + mainSHA := repo.HeadSHA() + repo.CommitFile("trunk.txt", "trunk", "upstream advance") + upstreamSHA := repo.HeadSHA() + repo.Run("update-ref", "refs/heads/main", mainSHA) + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("remote", "add", "upstream", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", mainSHA) + repo.Run("update-ref", "refs/remotes/upstream/main", upstreamSHA) + repo.Run("config", "branch.main.remote", "upstream") + repo.Run("config", "branch.main.merge", "refs/heads/main") + repo.Run("checkout", "-b", "feature", upstreamSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "upstream/main", GetBranchBase(repo.Dir, "HEAD")) + }) +} From 7ff1e6b9c16becf8b1b2feff7d48edb2503e3398 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Thu, 7 May 2026 10:51:50 -0500 Subject: [PATCH 2/3] fix(git): tighten base resolution for slash names and fork upstreams Two refinements to the stale-local-base translation in GetBranchBase, addressing review feedback on PR #702: 1. Slash-containing local branch names. The previous logic treated any '/' in the config value as a qualified remote-tracking ref and skipped translation. Branch names like "release/1.2" or "team/main" are valid local branches, so the original stale-local-base bug resurfaced for them. isQualifiedRemoteRef now checks ref namespace directly: a value is treated as already-qualified only when refs/remotes/ resolves AND no shadowing refs/heads/ exists, matching git's standard ref lookup precedence. 2. Configured upstream that does not resolve. When local is explicitly configured to track a remote-tracking ref but that ref has not been fetched, preferredRemoteTracking previously fell through to origin/. In fork workflows where local main tracks upstream/main and origin is the user's fork, this silently switched the comparison to a different remote. The fork-fallback now only triggers when no upstream is configured at all; an explicit but unresolved upstream returns "" so the literal local branch is used instead. --- internal/git/git.go | 59 ++++++++++++++++++++++-------- internal/git/git_test.go | 77 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 15 deletions(-) diff --git a/internal/git/git.go b/internal/git/git.go index f11ecc0f..a314e1a2 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -1371,11 +1371,17 @@ func GetBranchBase(repoPath, ref string) string { return resolveConfiguredBase(repoPath, raw) } -// resolveConfiguredBase translates a bare base name to the remote-tracking -// ref it should map to, or returns configValue unchanged when no translation -// applies. Qualified refs and divergent local branches are passed through. +// resolveConfiguredBase translates a configured base name to the remote- +// tracking ref it should map to, or returns configValue unchanged when no +// translation applies. Values that already name a remote-tracking ref +// unambiguously are passed through; local branch names (including slash- +// containing names like "release/1.2") are subject to stale-ancestor +// translation; divergent local branches are also passed through. func resolveConfiguredBase(repoPath, configValue string) string { - if configValue == "" || strings.Contains(configValue, "/") { + if configValue == "" { + return configValue + } + if isQualifiedRemoteRef(repoPath, configValue) { return configValue } remoteTracking := preferredRemoteTracking(repoPath, configValue) @@ -1392,19 +1398,42 @@ func resolveConfiguredBase(repoPath, configValue string) string { return remoteTracking } +// isQualifiedRemoteRef reports whether value names a remote-tracking ref +// that is not shadowed by a same-named local branch. Defers to the local +// branch when both exist so stale-ancestor translation still applies, and +// so the resolved base matches git's normal ref lookup precedence +// (refs/heads beats refs/remotes). +func isQualifiedRemoteRef(repoPath, value string) bool { + if !strings.Contains(value, "/") { + return false + } + if !refExists(repoPath, "refs/remotes/"+value) { + return false + } + if refExists(repoPath, "refs/heads/"+value) { + return false + } + return true +} + // preferredRemoteTracking returns the remote-tracking ref short name for a -// bare branch name, preferring the branch's configured @{upstream} so fork +// branch name, preferring the branch's configured @{upstream} so fork // workflows (local main → upstream/main) translate correctly, and falling -// back to origin/. Returns "" when no remote-tracking counterpart -// resolves locally. -func preferredRemoteTracking(repoPath, bareName string) string { - if cfg, ok := readUpstreamConfig(repoPath, bareName); ok && - strings.HasPrefix(cfg.qualified, "refs/remotes/") && - refExists(repoPath, cfg.qualified) { - return cfg.short - } - if refExists(repoPath, "refs/remotes/origin/"+bareName) { - return "origin/" + bareName +// back to origin/ only when no upstream is configured. Returns "" +// when no remote-tracking counterpart resolves locally; in particular, an +// explicitly configured upstream that does not resolve is NOT silently +// substituted with origin/ — that would override the user's choice +// of remote in fork workflows. +func preferredRemoteTracking(repoPath, name string) string { + if cfg, ok := readUpstreamConfig(repoPath, name); ok { + if strings.HasPrefix(cfg.qualified, "refs/remotes/") && + refExists(repoPath, cfg.qualified) { + return cfg.short + } + return "" + } + if refExists(repoPath, "refs/remotes/origin/"+name) { + return "origin/" + name } return "" } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 366e87a6..ec22bb95 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -2177,6 +2177,83 @@ func TestGetBranchBase(t *testing.T) { assert.Equal(t, "upstream/main", GetBranchBase(repo.Dir, "HEAD")) }) + t.Run("resolves slash-containing local branch when stale", func(t *testing.T) { + // Branch names may contain slashes (release/1.2, team/main). Such + // names must still get stale-ancestor translation — treating any + // '/' as a remote prefix would skip the fix for these cases. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/release/1.2") + repo.CommitFile("base.txt", "base", "initial") + staleSHA := repo.HeadSHA() + repo.CommitFile("trunk.txt", "t", "trunk advance") + freshSHA := repo.HeadSHA() + repo.Run("update-ref", "refs/heads/release/1.2", staleSHA) + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/release/1.2", freshSHA) + repo.Run("checkout", "-b", "feature", freshSHA) + repo.CommitFile("feature.txt", "f", "feature commit") + repo.Run("config", "branch.feature.base", "release/1.2") + + assert.Equal(t, "origin/release/1.2", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("passes qualified remote-tracking ref through unchanged", func(t *testing.T) { + // An unambiguous remote-tracking ref (refs/remotes/ exists, + // no shadowing local branch) is not subject to translation. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + mainSHA := repo.HeadSHA() + repo.Run("remote", "add", "upstream", "/dev/null") + repo.Run("update-ref", "refs/remotes/upstream/main", mainSHA) + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "upstream/main") + + assert.Equal(t, "upstream/main", GetBranchBase(repo.Dir, "HEAD")) + }) + + t.Run("does not fall back to origin when configured upstream is missing", func(t *testing.T) { + // Fork workflow: local main is configured to track upstream/main + // but upstream/main has not been fetched. origin/main (the user's + // fork) must NOT be silently substituted — that would compute the + // merge-base against a different remote than the user intended. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + mainSHA := repo.HeadSHA() + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", mainSHA) + // Configure local main to track upstream/main, but never set up + // the corresponding remote-tracking ref. + repo.Run("config", "branch.main.remote", "upstream") + repo.Run("config", "branch.main.merge", "refs/heads/main") + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "main", GetBranchBase(repo.Dir, "HEAD"), + "missing upstream must not be substituted with origin counterpart") + }) + + t.Run("does not fall back to origin when configured upstream is local-only", func(t *testing.T) { + // Local-branch tracking (branch..remote = ".") explicitly + // targets another local branch. origin/ is unrelated and + // must not be silently substituted. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/develop") + repo.CommitFile("base.txt", "base", "initial") + developSHA := repo.HeadSHA() + repo.Run("checkout", "-b", "main") + repo.Run("config", "branch.main.remote", ".") + repo.Run("config", "branch.main.merge", "refs/heads/develop") + repo.Run("remote", "add", "origin", "/dev/null") + repo.Run("update-ref", "refs/remotes/origin/main", developSHA) + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "main") + + assert.Equal(t, "main", GetBranchBase(repo.Dir, "HEAD"), + "local-branch tracking must not be substituted with origin counterpart") + }) + t.Run("prefers configured upstream over origin counterpart", func(t *testing.T) { // Fork workflow: local main tracks upstream/main, and origin/main // also exists (e.g., points at the user's fork). The base should From 81ed95885a022f3e3e24b31a7ef99f0b446fa5a1 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Thu, 7 May 2026 11:03:47 -0500 Subject: [PATCH 3/3] fix(git): gate origin fallback for slash-containing names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer caught a residual case in PR #702: branch.feature.base = "upstream/main" with refs/remotes/upstream/main absent and a stray refs/remotes/origin/upstream/main present collapses to "origin/upstream/ main" — silently switching the comparison to a different remote. preferredRemoteTracking now applies the origin/ fallback for slash-containing names only when refs/heads/ exists locally. A remote-qualified value whose configured remote-tracking ref isn't fetched is passed through literally so git surfaces a clear "ref does not resolve" error instead of computing the merge-base against an unrelated remote. --- internal/git/git.go | 22 +++++++++++++++------- internal/git/git_test.go | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/internal/git/git.go b/internal/git/git.go index a314e1a2..e05751ec 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -1420,10 +1420,15 @@ func isQualifiedRemoteRef(repoPath, value string) bool { // branch name, preferring the branch's configured @{upstream} so fork // workflows (local main → upstream/main) translate correctly, and falling // back to origin/ only when no upstream is configured. Returns "" -// when no remote-tracking counterpart resolves locally; in particular, an -// explicitly configured upstream that does not resolve is NOT silently -// substituted with origin/ — that would override the user's choice -// of remote in fork workflows. +// when no remote-tracking counterpart resolves locally; in particular: +// - An explicitly configured upstream that does not resolve is NOT +// silently substituted with origin/ — that would override the +// user's choice of remote in fork workflows. +// - For slash-containing names, the origin/ fallback only applies +// when a local branch by that name exists. Otherwise a remote- +// qualified value like "upstream/main" whose configured remote- +// tracking ref isn't fetched would collapse to "origin/upstream/main", +// silently switching to a different remote. func preferredRemoteTracking(repoPath, name string) string { if cfg, ok := readUpstreamConfig(repoPath, name); ok { if strings.HasPrefix(cfg.qualified, "refs/remotes/") && @@ -1432,10 +1437,13 @@ func preferredRemoteTracking(repoPath, name string) string { } return "" } - if refExists(repoPath, "refs/remotes/origin/"+name) { - return "origin/" + name + if !refExists(repoPath, "refs/remotes/origin/"+name) { + return "" } - return "" + if strings.Contains(name, "/") && !refExists(repoPath, "refs/heads/"+name) { + return "" + } + return "origin/" + name } func branchNameForConfig(repoPath, ref string) string { diff --git a/internal/git/git_test.go b/internal/git/git_test.go index ec22bb95..912b5b44 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -2234,6 +2234,28 @@ func TestGetBranchBase(t *testing.T) { "missing upstream must not be substituted with origin counterpart") }) + t.Run("does not collapse remote-qualified value to origin counterpart", func(t *testing.T) { + // Regression: branch.feature.base = "upstream/main" with the + // upstream remote-tracking ref absent must not be rewritten to + // "origin/upstream/main" just because a stray refs/remotes/origin/ + // upstream/main happens to exist. The user explicitly remote- + // qualified the value; either it resolves or git surfaces an error. + repo := NewTestRepo(t) + repo.Run("symbolic-ref", "HEAD", "refs/heads/main") + repo.CommitFile("base.txt", "base", "initial") + mainSHA := repo.HeadSHA() + repo.Run("remote", "add", "origin", "/dev/null") + // origin holds a stray ref shaped like a remote-qualified path — + // e.g., a past push of a local branch literally named + // "upstream/main". refs/remotes/upstream/main remains absent. + repo.Run("update-ref", "refs/remotes/origin/upstream/main", mainSHA) + repo.Run("checkout", "-b", "feature") + repo.Run("config", "branch.feature.base", "upstream/main") + + assert.Equal(t, "upstream/main", GetBranchBase(repo.Dir, "HEAD"), + "explicit remote-qualified value must not collapse to origin/") + }) + t.Run("does not fall back to origin when configured upstream is local-only", func(t *testing.T) { // Local-branch tracking (branch..remote = ".") explicitly // targets another local branch. origin/ is unrelated and