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 — but only when @{upstream}
resolves to trunk, not when it's the branch's own remote counterpart
— and falls back to the default branch otherwise.
Helpers in internal/git:
- GetUpstream(repoPath, ref) returns ("", nil) when no @{upstream} is
set, the resolved short name when the configured ref resolves, and
("", *UpstreamMissingError) when @{upstream} is configured but the
referenced ref does not resolve. Verification uses the fully-
qualified ref (refs/heads/<merge> for branch.<name>.remote = ".",
refs/remotes/<remote>/<merge> otherwise) so a lookalike tag or
local branch with the same short name can't shadow a missing
remote-tracking ref.
- UpstreamIsTrunk(repoPath, ref) reports whether ref's @{upstream}
points to the repository's trunk. Only refs/remotes/... qualified
upstreams can be trunk; refs/heads/... (local-branch tracking via
branch.<name>.remote = ".") is rejected even if the short name
happens to match the default branch's. Compares the full branch
name after stripping the longest configured-remote prefix so
"origin/team/main" (branch "team/main") never matches
"origin/main" (branch "main").
- IsOnBaseBranch(repoPath, currentBranch, base) classifies bases by
namespace: bare local names match directly, unambiguous remote-
tracking refs strip the configured-remote prefix, unambiguous
local branches match verbatim, and ambiguous names (refs/heads/
and refs/remotes/ both resolve) refuse to match either way.
- readUpstreamConfig accepts fully-qualified local branch refs
("refs/heads/<name>") so callers passing a ResolveSHA-style ref
still hit the correct branch.<name>.* config keys.
- stripRemotePrefix removes the longest matching configured-remote
prefix (handling multi-slash names like "company/fork/main").
- listRemotes uses strings.Fields so CRLF output on Windows doesn't
leave stray "\r" on remote names.
Callers (review, analyze, refine, tryBranchReview):
- Propagate any GetUpstream error: interactive commands surface
UpstreamMissingError as a user-visible error with a "pass --base
<ref>" / "pass --since" hint, and generic errors as "resolve
upstream for <ref>: ..."; the post-commit hook path
(tryBranchReview) skips ("", false) on any GetUpstream error
rather than silently falling back — falling back with a missing-
but-configured upstream would enqueue a review against the wrong
commit range in fork workflows.
- Pass the ref (not the resolved upstream) to UpstreamIsTrunk so it
can consult branch config to distinguish local vs remote tracking.
- refine's validateRefineContext only resolves @{upstream} when
--since is empty, so an unresolvable upstream doesn't block an
otherwise-valid --since invocation.
Tests:
- TestGetUpstream covers: no upstream; remote tracking branch; named
ref; empty ref defaults to HEAD; local-branch upstream; missing-
ref surfaces UpstreamMissingError; configured-but-never-fetched
tracking; colliding local ref at the same short name as a missing
remote-tracking ref; dotted branch names (release/1.2.3).
- TestIsOnBaseBranch covers: bare local; origin/-prefixed;
non-origin remote prefix; multi-slash remote name; the
"feature/foo" local-branch trap; pathological local "origin/foo";
pathological local "origin/main" with a matching remote-tracking
ref (ambiguous refuse); ambiguous slash refs.
- TestUpstreamIsTrunk covers: trunk-named upstream matches; self-
counterpart does not match; multi-remote trunk matches; no
upstream configured; default branch undetectable; feature branches
like "origin/team/main" are not trunk; local-branch upstream
literally named "origin/main" is not trunk; refs/heads/-qualified
ref input.
- TestTryBranchReview: prefers branch upstream over default branch;
allows feature tracking origin/feature (guardrail targets trunk,
not the branch's own counterpart); 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.
cpcloud
force-pushed
the
fix/branch-review-upstream
branch
from
Summary
review --branch,tryBranchReview,analyze --branch, andrefineresolved the merge-base against the repository's default branch (typicallyorigin/main). In fork workflows where the current branch's actual upstream (e.g.upstream/main) is ahead oforigin/main, this pulled in commits that were already merged upstream. Each command now prefers@{upstream}of the current branch and falls back toGetDefaultBranch()only when no upstream is configured.git.GetUpstream(repoPath, ref)(returns the upstream tracking branch viagit rev-parse --abbrev-ref <ref>@{upstream}, empty on no upstream).currentBranch == LocalBranchName(base)guardrails withgit.IsOnBaseBranch(repoPath, currentBranch, base), which generalizes theorigin/shortcut by verifyingrefs/remotes/<base>exists before stripping the remote prefix — so non-origin remotes are handled and local branches with slashes (e.g.feature/foo) are not misclassified.TestGetUpstream,TestIsOnBaseBranch, andTestTryBranchReviewcases for the non-origin upstream scenario and for the feature/foo misclassification guard.