Skip to content

Commit 18e9026

Browse files
authored
[fix] Check against origin/<mainBranch> in clean --merged (#31)
* fix: use origin/<mainBranch> after successful fetch in clean --merged After git fetch updates remote-tracking refs, check merged status against origin/<mainBranch> instead of local <mainBranch>. This correctly detects branches merged via GitHub PRs, where local main is typically behind. Falls back to local <mainBranch> when fetch fails (no remote). * test: verify merge ref selection in clean --merged - Rewrite TestCleanMergedFetchFails to assert local main is used as merge ref when fetch fails - Add TestCleanMergedFetchSucceeds to assert origin/main is used when fetch succeeds - Update findMergedCandidates calls for new two-param signature - Remove unused fetchFailRunner helper
1 parent a3d8120 commit 18e9026

5 files changed

Lines changed: 106 additions & 29 deletions

File tree

cmd/clean.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,17 @@ func cleanMerged(cmd *cobra.Command, r git.Runner) error {
9999
defer s.Stop()
100100

101101
// Fetch latest (non-fatal)
102+
mergeRef := mainBranch
102103
s.Start("Fetching from origin...")
103104
if err := git.Fetch(r, "origin"); err != nil {
104105
s.Stop()
105106
fmt.Fprintf(cmd.OutOrStdout(), "Warning: fetch failed (no remote?): continuing with local state\n")
107+
} else {
108+
mergeRef = "origin/" + mainBranch
106109
}
107110

108111
s.Update("Analyzing branches...")
109-
candidates, err := findMergedCandidates(r, mainBranch)
112+
candidates, err := findMergedCandidates(r, mergeRef, mainBranch)
110113
if err != nil {
111114
return err
112115
}
@@ -134,8 +137,8 @@ func cleanMerged(cmd *cobra.Command, r git.Runner) error {
134137
return nil
135138
}
136139

137-
func findMergedCandidates(r git.Runner, mainBranch string) ([]mergedCandidate, error) {
138-
mergedList, err := git.MergedBranches(r, mainBranch)
140+
func findMergedCandidates(r git.Runner, mergeRef, mainBranch string) ([]mergedCandidate, error) {
141+
mergedList, err := git.MergedBranches(r, mergeRef)
139142
if err != nil {
140143
return nil, fmt.Errorf("failed to list merged branches: %w", err)
141144
}

cmd/clean_test.go

Lines changed: 91 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ func TestFindMergedCandidatesFound(t *testing.T) {
3333
"HEAD abc123",
3434
"branch refs/heads/main",
3535
"",
36-
"worktree " + pathWtDone,
36+
wtDone,
3737
"HEAD def456",
38-
"branch refs/heads/" + branchDone,
38+
branchRefPrefix + branchDone,
3939
"",
4040
"worktree /wt/feature-active",
4141
"HEAD ghi789",
@@ -44,9 +44,35 @@ func TestFindMergedCandidatesFound(t *testing.T) {
4444
}, "\n")
4545

4646
r := mergedWorktreeRunner(" "+branchDone+"\n bugfix/old", worktreeOut)
47-
candidates, err := findMergedCandidates(r, branchMain)
47+
candidates, err := findMergedCandidates(r, branchMain, branchMain)
4848
if err != nil {
49-
t.Fatalf("findMergedCandidates: %v", err)
49+
t.Fatalf(fatalFindMerged, err)
50+
}
51+
if len(candidates) != 1 {
52+
t.Fatalf("got %d candidates, want 1", len(candidates))
53+
}
54+
if candidates[0].branch != branchDone {
55+
t.Errorf("branch = %q, want %q", candidates[0].branch, branchDone)
56+
}
57+
}
58+
59+
func TestFindMergedCandidatesRemoteRef(t *testing.T) {
60+
worktreeOut := strings.Join([]string{
61+
wtRepo,
62+
headABC123,
63+
branchRefMain,
64+
"",
65+
wtDone,
66+
headDEF456,
67+
branchRefPrefix + branchDone,
68+
"",
69+
}, "\n")
70+
71+
// mergeRef differs from mainBranch — simulates post-fetch scenario
72+
r := mergedWorktreeRunner(" "+branchDone+"\n main", worktreeOut)
73+
candidates, err := findMergedCandidates(r, "origin/"+branchMain, branchMain)
74+
if err != nil {
75+
t.Fatalf(fatalFindMerged, err)
5076
}
5177
if len(candidates) != 1 {
5278
t.Fatalf("got %d candidates, want 1", len(candidates))
@@ -58,9 +84,9 @@ func TestFindMergedCandidatesFound(t *testing.T) {
5884

5985
func TestFindMergedCandidatesNone(t *testing.T) {
6086
r := mergedWorktreeRunner("", wtRepo + headMainBlock)
61-
candidates, err := findMergedCandidates(r, branchMain)
87+
candidates, err := findMergedCandidates(r, branchMain, branchMain)
6288
if err != nil {
63-
t.Fatalf("findMergedCandidates: %v", err)
89+
t.Fatalf(fatalFindMerged, err)
6490
}
6591
if len(candidates) != 0 {
6692
t.Errorf("expected 0 candidates, got %d", len(candidates))
@@ -78,7 +104,7 @@ func TestFindMergedCandidatesError(t *testing.T) {
78104
runInDir: noopRunInDir,
79105
}
80106

81-
_, err := findMergedCandidates(r, branchMain)
107+
_, err := findMergedCandidates(r, branchMain, branchMain)
82108
if err == nil {
83109
t.Fatal(errExpected)
84110
}
@@ -98,50 +124,94 @@ func TestFindMergedCandidatesWorktreeError(t *testing.T) {
98124
runInDir: noopRunInDir,
99125
}
100126

101-
_, err := findMergedCandidates(r, branchMain)
127+
_, err := findMergedCandidates(r, branchMain, branchMain)
102128
if err == nil {
103129
t.Fatal("expected error from ListWorktrees failure")
104130
}
105131
}
106132

107-
func fetchFailRunner(topLevel, worktreeOut string) *mockRunner {
108-
return &mockRunner{
133+
func TestCleanMergedFetchFails(t *testing.T) {
134+
worktreeOut := cleanMergedWorktreeOut()
135+
cmd, buf := newCleanMergedCmd()
136+
_ = cmd.Flags().Set(flagForce, "true")
137+
138+
var mergedRef string
139+
r := &mockRunner{
109140
run: func(args ...string) (string, error) {
110141
if len(args) >= 2 && args[1] == cmdShowToplevel {
111-
return topLevel, nil
142+
return t.TempDir(), nil
112143
}
113144
if args[0] == cmdSymbolicRef {
114145
return refsRemotesOriginMain, nil
115146
}
116-
if len(args) >= 1 && args[0] == "fetch" {
147+
if args[0] == cmdFetch {
117148
return "", errors.New("no remote")
118149
}
119-
if len(args) >= 1 && args[0] == cmdBranch {
150+
if args[0] == cmdBranch {
151+
mergedRef = args[len(args)-1]
120152
return "", nil
121153
}
122-
if len(args) >= 1 && args[0] == cmdWorktreeTest {
154+
if args[0] == cmdWorktreeTest {
123155
return worktreeOut, nil
124156
}
125157
return "", nil
126158
},
127159
runInDir: noopRunInDir,
128160
}
161+
162+
err := cleanMerged(cmd, r)
163+
if err != nil {
164+
t.Fatalf(fatalCleanMerged, err)
165+
}
166+
out := buf.String()
167+
if !strings.Contains(out, "Warning: fetch failed") {
168+
t.Errorf("expected fetch warning, got %q", out)
169+
}
170+
if mergedRef != branchMain {
171+
t.Errorf("merged ref = %q, want %q (local fallback)", mergedRef, branchMain)
172+
}
129173
}
130174

131-
func TestCleanMergedFetchFails(t *testing.T) {
175+
func TestCleanMergedFetchSucceeds(t *testing.T) {
132176
worktreeOut := cleanMergedWorktreeOut()
133177
cmd, buf := newCleanMergedCmd()
134178
_ = cmd.Flags().Set(flagForce, "true")
135179

136-
r := fetchFailRunner(t.TempDir(), worktreeOut)
180+
var mergedRef string
181+
r := &mockRunner{
182+
run: func(args ...string) (string, error) {
183+
if len(args) >= 2 && args[1] == cmdShowToplevel {
184+
return t.TempDir(), nil
185+
}
186+
if args[0] == cmdSymbolicRef {
187+
return refsRemotesOriginMain, nil
188+
}
189+
if args[0] == cmdFetch {
190+
return "", nil // fetch succeeds
191+
}
192+
if args[0] == cmdBranch {
193+
mergedRef = args[len(args)-1]
194+
return "", nil
195+
}
196+
if args[0] == cmdWorktreeTest {
197+
return worktreeOut, nil
198+
}
199+
return "", nil
200+
},
201+
runInDir: noopRunInDir,
202+
}
137203

138204
err := cleanMerged(cmd, r)
139205
if err != nil {
140206
t.Fatalf(fatalCleanMerged, err)
141207
}
142208
out := buf.String()
143-
if !strings.Contains(out, "Warning: fetch failed") {
144-
t.Errorf("expected fetch warning, got %q", out)
209+
if strings.Contains(out, "Warning: fetch failed") {
210+
t.Errorf("unexpected fetch warning in output: %q", out)
211+
}
212+
wantRef := "origin/" + branchMain
213+
if mergedRef != wantRef {
214+
t.Errorf("merged ref = %q, want %q", mergedRef, wantRef)
145215
}
146216
}
147217

@@ -343,7 +413,7 @@ func cleanMergedTestRunner(t *testing.T, mergedOut, worktreeOut string) *mockRun
343413
if len(args) >= 1 && args[0] == cmdWorktreeTest {
344414
return worktreeOut, nil
345415
}
346-
if len(args) >= 1 && args[0] == "fetch" {
416+
if len(args) >= 1 && args[0] == cmdFetch {
347417
return "", errors.New("no remote")
348418
}
349419
return "", nil
@@ -366,9 +436,9 @@ func cleanMergedWorktreeOut() string {
366436
"HEAD abc123",
367437
"branch refs/heads/main",
368438
"",
369-
"worktree " + pathWtDone,
439+
wtDone,
370440
"HEAD def456",
371-
"branch refs/heads/" + branchDone,
441+
branchRefPrefix + branchDone,
372442
"",
373443
}, "\n")
374444
}

cmd/completions_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func TestCompleteWorktreeTasks(t *testing.T) {
1515
"",
1616
wtFeatureLogin,
1717
"HEAD def456",
18-
"branch refs/heads/" + branchFeature,
18+
branchRefPrefix + branchFeature,
1919
"",
2020
"worktree /wt/bugfix-typo",
2121
"HEAD ghi789",
@@ -114,7 +114,7 @@ func TestCompleteWorktreeTasksSkipsBare(t *testing.T) {
114114
"",
115115
wtFeatureLogin,
116116
"HEAD def456",
117-
"branch refs/heads/" + branchFeature,
117+
branchRefPrefix + branchFeature,
118118
"",
119119
}, "\n")
120120

cmd/helpers_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func TestListWorktreeInfos(t *testing.T) {
8080
"",
8181
"worktree /repo-worktrees/feature-login",
8282
"HEAD def456",
83-
"branch refs/heads/" + branchFeature,
83+
branchRefPrefix + branchFeature,
8484
"",
8585
}, "\n")
8686

@@ -122,7 +122,7 @@ func TestFindWorktree(t *testing.T) {
122122
"",
123123
"worktree /repo-worktrees/feature-login",
124124
"HEAD def456",
125-
"branch refs/heads/" + branchFeature,
125+
branchRefPrefix + branchFeature,
126126
"",
127127
}, "\n")
128128

cmd/mock_runner_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,14 @@ const (
3434
branchBugfixTypo = "bugfix/typo"
3535
msgRemovedWorktree = "Removed worktree"
3636

37+
branchRefPrefix = "branch refs/heads/"
38+
wtDone = "worktree " + pathWtDone
3739
wtFeatureLogin = "worktree /wt/feature-login"
3840
pathWorktreesFeatureLogin = "/worktrees/feature-login"
3941
cmdRevList = "rev-list"
4042

4143
// Shared git command and value constants
44+
cmdFetch = "fetch"
4245
cmdRemove = "remove"
4346
cmdSymbolicRef = "symbolic-ref"
4447
cmdStatus = "status"
@@ -53,7 +56,8 @@ const (
5356
useTestCmd = "test-cmd"
5457

5558
// Shared fatalf format strings
56-
fatalMergeRunE = "mergeCmd.RunE: %v"
59+
fatalFindMerged = "findMergedCandidates: %v"
60+
fatalMergeRunE = "mergeCmd.RunE: %v"
5761
fatalCleanPrune = "cleanPrune: %v"
5862
fatalCleanMerged = "cleanMerged: %v"
5963
fatalListRunE = "listCmd.RunE: %v"

0 commit comments

Comments
 (0)