[Fix] worktree: use per-worktree upstream for diff stat base - #79
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2d9aa59bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve the diff base per worktree instead of using a single repo-wide
value. The previous approach called `resolveSyncBaseBranch` once and
fed the result to every worktree, which on fork setups (origin + upstream)
collapsed to the local short name and produced wildly inflated stats
(e.g. 3223 files when the real diff was 17).
The new resolver:
1. Uses the branch's own `@{upstream}` when set — git's recorded
truth for where this branch was forked from.
2. Falls back to `upstream/<base>` then `origin/<base>`, preferring
the upstream remote.
3. Falls back to the local short name as a last resort.
`gmc wt sync` is unaffected — its base resolution path is unchanged.
Tests: two new cases cover the upstream-tracked and no-upstream paths.
## Considered and deferred
- internal/worktree/diff_stat.go:45 [BOT-NIT]: `seen` map is built but never read; harmless dead code, removing would be a NIT-only edit.
- internal/worktree/worktree_test.go [BOT-NIT]: `FallsBackToRemoteRef` test only passes because the test setup does not create `refs/remotes/upstream/main`; with the new `upstream`-first ordering, the test could be tightened, but current behavior is correct.
Signed-off-by: samzong <samzong.lu@gmail.com>
c2d9aa5 to
e31141c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e31141c4cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if up := c.branchUpstream(path); up != "" { | ||
| return up, nil |
There was a problem hiding this comment.
When a feature worktree has been pushed with git push -u origin feature/foo, branchUpstream returns origin/feature/foo here, so WorktreeDiffStat compares the branch against its own remote-tracking copy; after a push the merge base is often HEAD, and gmc wt list reports no changed files even though the worktree differs from main/upstream/main. The --diff-base help describes a base branch/ref for diff stats, so the default resolver should not treat the branch's push/pull upstream as the PR/base branch unconditionally.
Useful? React with 👍 / 👎.
1 participant
What's changed?
ResolveDiffBaseForWorktree(path, override)that resolves the diff base per worktree.branchUpstream(path)readsbranch@{upstream}— the per-branch truth git itself records.upstream/<base>→origin/<base>→ local short name.loadWorktreeDiffStatsnow resolves the base inside the per-worktree loop instead of once for the whole repo.Why
gmc wt list(PR #77) was callingresolveSyncBaseBranchonce and feeding the resulting short name (e.g.main) to every worktree. In fork setups with bothoriginandupstream, this short name resolved to a stale localrefs/heads/main, so the diff stat was computed against an ancient ancestor and produced wildly inflated numbers — e.g. a 17-file real diff on~/git/openclawwas reported as 3223 files / +116k / -41k.The previous function
resolveSyncBaseBranchis unchanged and still backsgmc wt sync, where the upstream/remote selection is done in a separate step.