From e20c2c0e8b714def1f99fd596399d08a0a8e7f5f Mon Sep 17 00:00:00 2001 From: rios0rios0 Date: Thu, 2 Apr 2026 00:53:17 -0300 Subject: [PATCH 1/3] fix(sync): changed restore to stay on default branch after sync - changed `RestoreAfterSync` to checkout the default branch instead of the original branch after rebasing the WIP branch - removed the `else if` block that checked out the original branch for clean repos on non-default branches (already on default from the sync step) - this prevents `prune` from failing with "cannot delete branch used by worktree" when the merged branch is currently checked out Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/repo/sync.go | 4 +--- internal/repo/sync_test.go | 14 +++++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/repo/sync.go b/internal/repo/sync.go index 19351bf..586d143 100644 --- a/internal/repo/sync.go +++ b/internal/repo/sync.go @@ -173,10 +173,8 @@ func RestoreAfterSync( if err := runner.Run(repoPath, "rebase", defaultBranch); err != nil { _ = runner.Run(repoPath, "rebase", "--abort") } - _ = runner.Run(repoPath, "checkout", currentBranch) + _ = runner.Run(repoPath, "checkout", defaultBranch) status = fmt.Sprintf("synced (wip: %s)", wipBranch) - } else if currentBranch != defaultBranch { - _ = runner.Run(repoPath, "checkout", currentBranch) } return SyncResult{Name: name, Status: status} } diff --git a/internal/repo/sync_test.go b/internal/repo/sync_test.go index 08ffc1a..583b1f3 100644 --- a/internal/repo/sync_test.go +++ b/internal/repo/sync_test.go @@ -257,7 +257,7 @@ func TestRestoreAfterSync(t *testing.T) { assert.Equal(t, "synced", result.Status) }) - t.Run("should checkout original branch when not on default", func(t *testing.T) { + t.Run("should stay on default branch when clean repo was on non-default branch", func(t *testing.T) { t.Parallel() // given var checkedOut string @@ -274,19 +274,27 @@ func TestRestoreAfterSync(t *testing.T) { // then assert.Equal(t, "synced", result.Status) - assert.Equal(t, "feat/x", checkedOut) + assert.Empty(t, checkedOut) }) - t.Run("should rebase WIP branch and return wip status for dirty repo", func(t *testing.T) { + t.Run("should rebase WIP branch and checkout default branch for dirty repo", func(t *testing.T) { t.Parallel() // given + var lastCheckedOut string runner := doubles.NewGitRunnerStub() + runner.RunFunc = func(_ string, args ...string) error { + if len(args) > 0 && args[0] == "checkout" { + lastCheckedOut = args[1] + } + return nil + } // when result := repo.RestoreAfterSync("/repo", "my-repo", "main", "feat/x", "wip/feat/x", true, runner) // then assert.Contains(t, result.Status, "synced (wip: wip/feat/x)") + assert.Equal(t, "main", lastCheckedOut) }) } From f8844bba9a3748e8b209d572919bb3935d47120b Mon Sep 17 00:00:00 2001 From: rios0rios0 Date: Thu, 2 Apr 2026 15:08:12 -0300 Subject: [PATCH 2/3] fix(sync): removed unused `currentBranch` parameter from `RestoreAfterSync` and added changelog entry Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 + internal/repo/sync.go | 4 ++-- internal/repo/sync_test.go | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e7baa..ae94e94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Exceptions are acceptable depending on the circumstances (critical bug fixes tha - changed the Go module dependencies to their latest versions - changed the Go module dependencies to their latest versions - changed per-repo logging in parallel operations to run inside goroutines for progressive feedback +- changed `RestoreAfterSync` to stay on the default branch after a successful sync ## [0.4.0] - 2026-03-31 diff --git a/internal/repo/sync.go b/internal/repo/sync.go index 586d143..1bb1382 100644 --- a/internal/repo/sync.go +++ b/internal/repo/sync.go @@ -151,7 +151,7 @@ func SyncAndRestore( return SyncResult{Name: name, Status: fmt.Sprintf("FAIL (pull --rebase: %v)", err)} } - return RestoreAfterSync(repoPath, name, defaultBranch, currentBranch, wipBranch, isDirty, runner) + return RestoreAfterSync(repoPath, name, defaultBranch, wipBranch, isDirty, runner) } // RestoreBranch restores the appropriate branch after a failure. @@ -165,7 +165,7 @@ func RestoreBranch(repoPath, currentBranch, wipBranch string, isDirty bool, runn // RestoreAfterSync restores the original branch state after a successful sync. func RestoreAfterSync( - repoPath, name, defaultBranch, currentBranch, wipBranch string, isDirty bool, runner GitRunner, + repoPath, name, defaultBranch, wipBranch string, isDirty bool, runner GitRunner, ) SyncResult { status := "synced" if isDirty { diff --git a/internal/repo/sync_test.go b/internal/repo/sync_test.go index 583b1f3..1dce817 100644 --- a/internal/repo/sync_test.go +++ b/internal/repo/sync_test.go @@ -251,7 +251,7 @@ func TestRestoreAfterSync(t *testing.T) { runner := doubles.NewGitRunnerStub() // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "main", "wip/main", false, runner) + result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/main", false, runner) // then assert.Equal(t, "synced", result.Status) @@ -270,7 +270,7 @@ func TestRestoreAfterSync(t *testing.T) { } // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "feat/x", "wip/feat/x", false, runner) + result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/feat/x", false, runner) // then assert.Equal(t, "synced", result.Status) @@ -290,7 +290,7 @@ func TestRestoreAfterSync(t *testing.T) { } // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "feat/x", "wip/feat/x", true, runner) + result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/feat/x", true, runner) // then assert.Contains(t, result.Status, "synced (wip: wip/feat/x)") From e1ea4d09428b44bad660b222be4ddfc9c1c395fd Mon Sep 17 00:00:00 2001 From: rios0rios0 Date: Thu, 2 Apr 2026 15:21:03 -0300 Subject: [PATCH 3/3] fix(pr-review): addressed PR review comments - renamed `RestoreAfterSync` to `FinalizeSync` and updated doc comment - handled checkout error in `FinalizeSync` instead of ignoring it - added `SyncAndRestore` test for non-default branch stay behavior - moved changelog entry from Changed to Fixed section Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 5 +++- internal/repo/sync.go | 10 ++++---- internal/repo/sync_test.go | 47 ++++++++++++++++++++++++++++++++++---- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae94e94..8eccd3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,10 @@ Exceptions are acceptable depending on the circumstances (critical bug fixes tha - changed the Go module dependencies to their latest versions - changed the Go module dependencies to their latest versions - changed per-repo logging in parallel operations to run inside goroutines for progressive feedback -- changed `RestoreAfterSync` to stay on the default branch after a successful sync + +### Fixed + +- fixed `RestoreAfterSync` to stay on the default branch after a successful sync ## [0.4.0] - 2026-03-31 diff --git a/internal/repo/sync.go b/internal/repo/sync.go index 1bb1382..eab3742 100644 --- a/internal/repo/sync.go +++ b/internal/repo/sync.go @@ -151,7 +151,7 @@ func SyncAndRestore( return SyncResult{Name: name, Status: fmt.Sprintf("FAIL (pull --rebase: %v)", err)} } - return RestoreAfterSync(repoPath, name, defaultBranch, wipBranch, isDirty, runner) + return FinalizeSync(repoPath, name, defaultBranch, wipBranch, isDirty, runner) } // RestoreBranch restores the appropriate branch after a failure. @@ -163,8 +163,8 @@ func RestoreBranch(repoPath, currentBranch, wipBranch string, isDirty bool, runn } } -// RestoreAfterSync restores the original branch state after a successful sync. -func RestoreAfterSync( +// FinalizeSync rebases the WIP branch (if dirty) and ensures the repo ends on the default branch. +func FinalizeSync( repoPath, name, defaultBranch, wipBranch string, isDirty bool, runner GitRunner, ) SyncResult { status := "synced" @@ -173,7 +173,9 @@ func RestoreAfterSync( if err := runner.Run(repoPath, "rebase", defaultBranch); err != nil { _ = runner.Run(repoPath, "rebase", "--abort") } - _ = runner.Run(repoPath, "checkout", defaultBranch) + if err := runner.Run(repoPath, "checkout", defaultBranch); err != nil { + return SyncResult{Name: name, Status: fmt.Sprintf("FAIL (checkout %s: %v)", defaultBranch, err)} + } status = fmt.Sprintf("synced (wip: %s)", wipBranch) } return SyncResult{Name: name, Status: status} diff --git a/internal/repo/sync_test.go b/internal/repo/sync_test.go index 1dce817..19446f0 100644 --- a/internal/repo/sync_test.go +++ b/internal/repo/sync_test.go @@ -198,6 +198,26 @@ func TestSyncAndRestore(t *testing.T) { // then assert.Contains(t, result.Status, "FAIL (pull --rebase") }) + + t.Run("should stay on default branch when starting from non-default branch", func(t *testing.T) { + t.Parallel() + // given + var checkouts []string + runner := doubles.NewGitRunnerStub() + runner.RunFunc = func(_ string, args ...string) error { + if len(args) > 0 && args[0] == "checkout" { + checkouts = append(checkouts, args[1]) + } + return nil + } + + // when + result := repo.SyncAndRestore("/repo", "my-repo", "main", "feat/x", "wip/feat/x", false, runner) + + // then + assert.Equal(t, "synced", result.Status) + assert.Equal(t, []string{"main"}, checkouts) + }) } func TestRestoreBranch(t *testing.T) { @@ -242,7 +262,7 @@ func TestRestoreBranch(t *testing.T) { }) } -func TestRestoreAfterSync(t *testing.T) { +func TestFinalizeSync(t *testing.T) { t.Parallel() t.Run("should return synced for clean repo on default branch", func(t *testing.T) { @@ -251,7 +271,7 @@ func TestRestoreAfterSync(t *testing.T) { runner := doubles.NewGitRunnerStub() // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/main", false, runner) + result := repo.FinalizeSync("/repo", "my-repo", "main", "wip/main", false, runner) // then assert.Equal(t, "synced", result.Status) @@ -270,7 +290,7 @@ func TestRestoreAfterSync(t *testing.T) { } // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/feat/x", false, runner) + result := repo.FinalizeSync("/repo", "my-repo", "main", "wip/feat/x", false, runner) // then assert.Equal(t, "synced", result.Status) @@ -290,12 +310,31 @@ func TestRestoreAfterSync(t *testing.T) { } // when - result := repo.RestoreAfterSync("/repo", "my-repo", "main", "wip/feat/x", true, runner) + result := repo.FinalizeSync("/repo", "my-repo", "main", "wip/feat/x", true, runner) // then assert.Contains(t, result.Status, "synced (wip: wip/feat/x)") assert.Equal(t, "main", lastCheckedOut) }) + + t.Run("should return FAIL when checkout to default branch fails for dirty repo", func(t *testing.T) { + t.Parallel() + // given + runner := doubles.NewGitRunnerStub() + runner.RunFunc = func(_ string, args ...string) error { + if len(args) >= 2 && args[0] == "checkout" && args[1] == "main" { + return errors.New("checkout failed") + } + return nil + } + + // when + result := repo.FinalizeSync("/repo", "my-repo", "main", "wip/feat/x", true, runner) + + // then + assert.Contains(t, result.Status, "FAIL") + assert.Contains(t, result.Status, "checkout main") + }) } func TestRunSync(t *testing.T) {