fix: use branch upstream (not default branch) for --branch reviews

review --branch, tryBranchReview, analyze --branch, and refine
resolved the merge-base against the repository's default branch
(typically origin/main). In fork workflows where the current branch's
actual upstream (e.g. upstream/main) is ahead of origin/main, this
pulled in commits that were already merged upstream. Each command now
prefers @{upstream} of the current branch and falls back to the
default branch only when no upstream is configured.

- Add git.GetUpstream(repoPath, ref). Returns ("", nil) when no
  @{upstream} is set, the resolved short name when the ref exists
  locally, and ("", *UpstreamMissingError) when the upstream is
  configured but the referenced ref does not resolve. The verification
  resolves the fully-qualified ref from branch.<name>.remote /
  branch.<name>.merge — refs/heads/<merge> for local-branch tracking
  (remote == "."), refs/remotes/<remote>/<merge> otherwise — so a
  lookalike tag or local branch with the same short name cannot
  shadow a missing remote-tracking ref. Callers in
  analyze/review/refine surface UpstreamMissingError to the user so
  fork workflows don't silently analyze the wrong commit range; the
  post-commit hook path (tryBranchReview) treats it as "skip" since
  hooks must never block commits.
- Add git.IsOnBaseBranch(repoPath, currentBranch, base), replacing
  the "currentBranch == LocalBranchName(base)" check. A slash-
  containing base is only treated as a remote-tracking ref when
  refs/remotes/<base> resolves, refs/heads/<base> does not, and the
  prefix matches one of the repository's configured remote names
  (longest match wins). That handles non-origin remotes
  (upstream/main), multi-slash remote names (company/fork/main),
  local-branch upstreams (git branch -u <local>), and avoids
  misclassifying local branches with slashes (feature/foo alongside
  a "feature" remote, or even a local branch literally named
  origin/foo).
- refine's validateRefineContext only resolves @{upstream} when
  --since is empty, so an unresolvable upstream doesn't block an
  otherwise-valid --since invocation.
- Add refExists, listRemotes, readGitConfig, and readUpstreamConfig
  helpers used by the above.

Tests:
- TestGetUpstream: no upstream, remote tracking branch, named ref,
  empty ref defaults to HEAD, local-branch upstream, missing-ref
  surfaces UpstreamMissingError, configured-but-never-fetched
  tracking, and a colliding local ref at the same short name as a
  missing remote-tracking ref.
- TestIsOnBaseBranch: bare local, origin/-prefixed, non-origin
  remote prefix, multi-slash remote name, the "feature/foo"
  local-branch trap, the pathological local "origin/foo", and
  ambiguous refs.
- TestTryBranchReview: prefers branch upstream over default branch,
  skips when upstream configured but unresolvable, blocks local
  main tracking non-origin upstream.
- TestGetBranchFiles: prefers non-origin upstream, blocks on local
  main tracking non-origin upstream.
- TestValidateRefineContext: prefers non-origin upstream, refuses
  local main tracking non-origin upstream, --since bypasses
  upstream resolution, UpstreamMissingError surfaces without
  --since.

Author: cpcloud

cmd/roborev/analyze.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -967,6 +968,19 @@ func getBranchFiles(cmd *cobra.Command, repoRoot string, opts analyzeOptions) (m
 
 	// Determine base branch
 	base := opts.baseBranch
+	if base == "" {
+		// Prefer the branch's upstream tracking ref so "ahead of upstream"
+		// semantics match `git status` and avoid pulling in commits that
+		// were merged to the parent branch upstream of this one.
+		upstream, uerr := git.GetUpstream(repoRoot, targetRef)
+		var missing *git.UpstreamMissingError
+		if errors.As(uerr, &missing) {
+			return nil, fmt.Errorf("%w (or pass --base <ref>)", missing)
+		}
+		if uerr == nil && upstream != "" {
+			base = upstream
+		}
+	}
 	if base == "" {
 		var err error
 		base, err = git.GetDefaultBranch(repoRoot)
@@ -978,8 +992,8 @@ func getBranchFiles(cmd *cobra.Command, repoRoot string, opts analyzeOptions) (m
 	// Validate not on base branch (only when analyzing current branch)
 	if targetRef == "HEAD" {
 		currentBranch := git.GetCurrentBranch(repoRoot)
-		if currentBranch == git.LocalBranchName(base) {
-			return nil, fmt.Errorf("already on %s - switch to a feature branch first", git.LocalBranchName(base))
+		if git.IsOnBaseBranch(repoRoot, currentBranch, base) {
+			return nil, fmt.Errorf("already on %s - switch to a feature branch first", currentBranch)
 		}
 	}
 

cmd/roborev/analyze_test.go

@@ -570,6 +570,51 @@ func TestGetBranchFiles(t *testing.T) {
 		require.Error(t, err, "expected error when on base branch")
 		assert.Contains(t, err.Error(), "already on main", "unexpected error")
 	})
+
+	t.Run("prefers non-origin upstream as base", func(t *testing.T) {
+		// Fork workflow: origin (stale fork) and upstream (real repo).
+		// Feature branch tracks upstream/main; merge-base must be taken
+		// against upstream/main, not origin/main, so commits already
+		// merged upstream aren't re-analyzed.
+		remote := newBareTestGitRepo(t)
+		stale := newBareTestGitRepo(t)
+		repo := newTestGitRepo(t)
+		repo.Run("symbolic-ref", "HEAD", "refs/heads/main")
+		repo.CommitFile("a.go", "package main", "u1")
+		repo.CommitFile("b.go", "package main", "u2")
+		repo.Run("remote", "add", "upstream", remote.Dir)
+		repo.Run("push", "-u", "upstream", "main")
+
+		// origin lags upstream by 1 commit.
+		repo.Run("remote", "add", "origin", stale.Dir)
+		repo.Run("push", "origin", "HEAD~1:refs/heads/main")
+		repo.Run("fetch", "origin")
+		repo.Run("remote", "set-head", "origin", "main")
+
+		repo.Run("checkout", "-b", "feature", "--track", "upstream/main")
+		repo.CommitFile("only-new.go", "package main\nfunc New() {}", "feature work")
+
+		files, err := getBranchFiles(cmd, repo.Dir, analyzeOptions{branch: "HEAD"})
+		require.NoError(t, err)
+		// b.go was already merged to upstream/main; must not be analyzed.
+		assert.NotContains(t, files, "b.go",
+			"already-merged upstream commit must not be analyzed")
+		assert.Contains(t, files, "only-new.go",
+			"new feature file should be analyzed")
+	})
+
+	t.Run("blocks local main tracking non-origin upstream", func(t *testing.T) {
+		remote := newBareTestGitRepo(t)
+		repo := newTestGitRepo(t)
+		repo.Run("symbolic-ref", "HEAD", "refs/heads/main")
+		repo.CommitFile("a.go", "package main", "initial")
+		repo.Run("remote", "add", "upstream", remote.Dir)
+		repo.Run("push", "-u", "upstream", "main")
+
+		_, err := getBranchFiles(cmd, repo.Dir, analyzeOptions{branch: "HEAD"})
+		require.Error(t, err, "expected error when on base branch")
+		assert.Contains(t, err.Error(), "already on main", "unexpected error")
+	})
 }
 
 func TestAnalyzeBranchSpaceSeparated(t *testing.T) {

cmd/roborev/helpers_test.go

@@ -43,6 +43,21 @@ func newTestGitRepo(t *testing.T) *TestGitRepo {
 	return r
 }
 
+// newBareTestGitRepo creates a bare git repository for use as a remote.
+func newBareTestGitRepo(t *testing.T) *TestGitRepo {
+	t.Helper()
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+	dir := t.TempDir()
+	resolved, err := filepath.EvalSymlinks(dir)
+	require.NoError(t, err, "Failed to resolve symlinks: %v")
+
+	r := &TestGitRepo{Dir: resolved, t: t}
+	r.Run("init", "--bare")
+	return r
+}
+
 // chdir changes to dir and registers a t.Cleanup to restore the original directory.
 func chdir(t *testing.T, dir string) {
 	t.Helper()

cmd/roborev/refine.go

@@ -207,12 +207,14 @@ func (t *stepTimer) stopLive() {
 }
 
 // validateRefineContext validates git and branch preconditions for refine.
-// Returns repoPath, currentBranch, defaultBranch, mergeBase, or an error.
+// Returns repoPath, currentBranch, base, mergeBase, or an error.
+// base is the ref that HEAD was diverged from — the branch's upstream
+// tracking ref when configured, otherwise the repository's default branch.
 // If branchFlag is non-empty, validates that the user is on that branch.
 // This validation happens before any daemon interaction.
 func validateRefineContext(
 	cwd, since, branchFlag string,
-) (repoPath, currentBranch, defaultBranch, mergeBase string, err error) {
+) (repoPath, currentBranch, base, mergeBase string, err error) {
 	repoPath, err = git.GetRepoRoot(cwd)
 	if err != nil {
 		return "", "", "", "",
@@ -235,14 +237,6 @@ func validateRefineContext(
 			)
 	}
 
-	defaultBranch, err = git.GetDefaultBranch(repoPath)
-	if err != nil {
-		return "", "", "", "",
-			fmt.Errorf(
-				"cannot determine default branch: %w", err,
-			)
-	}
-
 	currentBranch = git.GetCurrentBranch(repoPath)
 
 	// --branch: validate the user is on the expected branch
@@ -255,6 +249,9 @@ func validateRefineContext(
 	}
 
 	if since != "" {
+		// --since provides an explicit merge base, so upstream/default-branch
+		// resolution is unnecessary. Skip it so a misconfigured or unfetched
+		// upstream doesn't block an otherwise-valid --since invocation.
 		mergeBase, err = git.ResolveSHA(repoPath, since)
 		if err != nil {
 			return "", "", "", "",
@@ -281,27 +278,51 @@ func validateRefineContext(
 				)
 		}
 	} else {
-		if currentBranch == git.LocalBranchName(defaultBranch) {
+		// Prefer the current branch's upstream tracking ref (matches
+		// `git status` "ahead of upstream" semantics); fall back to the
+		// repository default branch when no upstream is configured.
+		upstream, uerr := git.GetUpstream(repoPath, "HEAD")
+		var missing *git.UpstreamMissingError
+		if errors.As(uerr, &missing) {
+			return "", "", "", "",
+				fmt.Errorf(
+					"%w (run 'git fetch' or pass --since)", missing,
+				)
+		}
+		if uerr == nil && upstream != "" {
+			base = upstream
+		}
+		if base == "" {
+			base, err = git.GetDefaultBranch(repoPath)
+			if err != nil {
+				return "", "", "", "",
+					fmt.Errorf(
+						"cannot determine default branch: %w", err,
+					)
+			}
+		}
+
+		if git.IsOnBaseBranch(repoPath, currentBranch, base) {
 			return "", "", "", "", fmt.Errorf(
 				"refusing to refine on %s branch "+
 					"without --since flag",
-				git.LocalBranchName(defaultBranch),
+				currentBranch,
 			)
 		}
 
 		mergeBase, err = git.GetMergeBase(
-			repoPath, defaultBranch, "HEAD",
+			repoPath, base, "HEAD",
 		)
 		if err != nil {
 			return "", "", "", "",
 				fmt.Errorf(
 					"cannot find merge-base with %s: %w",
-					defaultBranch, err,
+					base, err,
 				)
 		}
 	}
 
-	return repoPath, currentBranch, defaultBranch, mergeBase, nil
+	return repoPath, currentBranch, base, mergeBase, nil
 }
 
 // RunContext encapsulates the runtime context for the refine command,
@@ -314,7 +335,7 @@ type RunContext struct {
 
 func runRefine(ctx RunContext, opts refineOptions) error {
 	// 1. Validate git and branch context (before touching daemon)
-	repoPath, currentBranch, defaultBranch, mergeBase, err := validateRefineContext(
+	repoPath, currentBranch, base, mergeBase, err := validateRefineContext(
 		ctx.WorkingDir, opts.since, opts.branch,
 	)
 	if err != nil {
@@ -368,7 +389,7 @@ func runRefine(ctx RunContext, opts refineOptions) error {
 	if opts.since != "" {
 		fmt.Printf("Refining commits since %s on branch %q\n", git.ShortSHA(mergeBase), currentBranch)
 	} else {
-		fmt.Printf("Refining branch %q (diverged from %s at %s)\n", currentBranch, defaultBranch, git.ShortSHA(mergeBase))
+		fmt.Printf("Refining branch %q (diverged from %s at %s)\n", currentBranch, base, git.ShortSHA(mergeBase))
 	}
 
 	allowUnsafe := resolveAllowUnsafeAgents(opts.allowUnsafeAgents, opts.unsafeFlagChanged, cfg)

cmd/roborev/refine_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/roborev-dev/roborev/internal/agent"
 	"github.com/roborev-dev/roborev/internal/config"
 	"github.com/roborev-dev/roborev/internal/daemon"
+	"github.com/roborev-dev/roborev/internal/git"
 	"github.com/roborev-dev/roborev/internal/storage"
 	"github.com/roborev-dev/roborev/internal/testutil"
 	"github.com/stretchr/testify/assert"
@@ -563,6 +564,54 @@ func TestValidateRefineContext_AllowsMainBranchWithSince(t *testing.T) {
 	assert.Equal(t, mergeBase, baseSHA)
 }
 
+func TestValidateRefineContext_SinceIgnoresUpstreamMissing(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+
+	// --since provides the merge base explicitly, so an unfetched or
+	// missing @{upstream} must not block a valid --since invocation.
+	repo := testutil.InitTestRepo(t)
+	baseSHA := repo.RevParse("HEAD")
+	repo.CommitFile("second.txt", "second", "second commit")
+
+	// Configure tracking against an upstream that never resolves locally.
+	repo.Run("config", "branch.main.remote", "upstream")
+	repo.Run("config", "branch.main.merge", "refs/heads/main")
+
+	chdirForTest(t, repo.Root)
+
+	repoPath, currentBranch, _, mergeBase, err := validateRefineContext("", baseSHA, "")
+	require.NoError(t, err, "--since should bypass upstream resolution: %v", err)
+
+	assert.NotEmpty(t, repoPath)
+	assert.Equal(t, "main", currentBranch)
+	assert.Equal(t, baseSHA, mergeBase)
+}
+
+func TestValidateRefineContext_UpstreamMissingErrorsWithoutSince(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+
+	// Without --since, a configured-but-unfetched upstream must surface
+	// as UpstreamMissingError rather than silently falling back to the
+	// repository default branch (which could yield the wrong range).
+	repo := testutil.InitTestRepo(t)
+	repo.RunGit("checkout", "-b", "feature")
+	repo.CommitFile("feature.txt", "feature", "feature commit")
+	repo.Run("config", "branch.feature.remote", "upstream")
+	repo.Run("config", "branch.feature.merge", "refs/heads/main")
+
+	chdirForTest(t, repo.Root)
+
+	_, _, _, _, err := validateRefineContext("", "", "")
+	require.Error(t, err, "expected missing-upstream error without --since")
+
+	var missing *git.UpstreamMissingError
+	assert.ErrorAs(t, err, &missing, "expected UpstreamMissingError, got: %v", err)
+}
+
 func TestValidateRefineContext_SinceWorksOnFeatureBranch(t *testing.T) {
 	if _, err := exec.LookPath("git"); err != nil {
 		t.Skip("git not available")
@@ -619,6 +668,69 @@ func TestValidateRefineContext_SinceNotAncestorOfHEAD(t *testing.T) {
 	assert.Contains(t, err.Error(), "not an ancestor of HEAD")
 }
 
+func TestValidateRefineContext_PrefersNonOriginUpstream(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+
+	// Fork workflow: origin (stale fork) lags behind upstream (real repo).
+	// Feature branch tracks upstream/main. validateRefineContext must
+	// pick upstream/main as the base so commits already merged upstream
+	// are not refined.
+	repo := testutil.InitTestRepo(t)
+	repo.CommitFile("upstream_c2.go", "package main", "upstream c2")
+	upstreamSHA := repo.RevParse("HEAD")
+
+	bareRemote := t.TempDir()
+	bareStale := t.TempDir()
+	repo.Run("init", "--bare", bareRemote)
+	repo.Run("init", "--bare", bareStale)
+	repo.Run("remote", "add", "upstream", bareRemote)
+	repo.Run("push", "-u", "upstream", "main")
+
+	repo.Run("remote", "add", "origin", bareStale)
+	repo.Run("push", "origin", "HEAD~1:refs/heads/main")
+	repo.Run("fetch", "origin")
+	repo.Run("remote", "set-head", "origin", "main")
+
+	repo.RunGit("checkout", "-b", "feature", "--track", "upstream/main")
+	repo.CommitFile("feature.go", "package main", "feature commit")
+
+	chdirForTest(t, repo.Root)
+
+	repoPath, currentBranch, base, mergeBase, err := validateRefineContext("", "", "")
+	require.NoError(t, err, "validation should pass on feature tracking upstream/main")
+
+	assert.NotEmpty(t, repoPath)
+	assert.Equal(t, "feature", currentBranch)
+	assert.Equal(t, "upstream/main", base,
+		"base must resolve to the branch's upstream, not origin/main")
+	assert.Equal(t, upstreamSHA, mergeBase,
+		"merge-base must be upstream/main's tip so already-merged commits aren't refined")
+}
+
+func TestValidateRefineContext_RefusesLocalMainTrackingNonOriginUpstream(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not available")
+	}
+
+	// Local main tracking upstream/main must still hit the "refusing to
+	// refine on main" guardrail even though base is "upstream/main"
+	// (not "origin/main"). Regression for IsOnBaseBranch's non-origin
+	// remote handling.
+	repo := testutil.InitTestRepo(t)
+	bareRemote := t.TempDir()
+	repo.Run("init", "--bare", bareRemote)
+	repo.Run("remote", "add", "upstream", bareRemote)
+	repo.Run("push", "-u", "upstream", "main")
+
+	chdirForTest(t, repo.Root)
+
+	_, _, _, _, err := validateRefineContext("", "", "")
+	require.Error(t, err, "expected refuse-on-base error for main tracking upstream/main")
+	assert.Contains(t, err.Error(), "refusing to refine on main")
+}
+
 func TestValidateRefineContext_FeatureBranchWithoutSinceStillWorks(t *testing.T) {
 	if _, err := exec.LookPath("git"); err != nil {
 		t.Skip("git not available")