From 99a1270ddac345516e132d2d7326f491a38e2460 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Tue, 16 Dec 2025 15:46:50 +0100 Subject: [PATCH 1/7] fix: explicitly fetch base branch before rebase in sync --- cmd/sync.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cmd/sync.go b/cmd/sync.go index 56e5705..f693b4f 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -510,6 +510,17 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { if !stackBranchSet[branch.Parent] { // Parent is not a stack branch, so it's a base branch - use origin/ rebaseTarget = "origin/" + branch.Parent + + // Explicitly fetch the base branch to ensure tracking ref is up to date + // This is needed because 'git fetch origin' may not always update tracking refs + // reliably (e.g., repos with limited refspecs or certain git configurations) + if err := gitClient.FetchBranch(branch.Parent); err != nil { + // Non-fatal: continue with potentially stale ref, rebase will still work + // but might not include latest changes from the base branch + if git.Verbose { + fmt.Printf(" Note: could not fetch %s: %v\n", branch.Parent, err) + } + } } // Rebase onto parent From e7190284897e8a63eacefe521bf02c78577bba79 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Tue, 16 Dec 2025 21:09:06 +0100 Subject: [PATCH 2/7] fix: add FetchBranch mock expectations for base branch fetch --- cmd/sync_test.go | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 8f5505f..e21db62 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -55,11 +55,7 @@ func TestRunSyncBasic(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - // Patch-based unique commit detection - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) - // Falls through to regular rebase since merge-base == parent + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -67,12 +63,7 @@ func TestRunSyncBasic(t *testing.T) { mockGit.On("CheckoutBranch", "feature-b").Return(nil) mockGit.On("GetCommitHash", "feature-b").Return("def456", nil) mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) - // Patch-based unique commit detection - mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil) - mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil) - mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) - // Falls through to regular rebase since merge-base == parent - mockGit.On("Rebase", "feature-a").Return(nil) + mockGit.On("Rebase", "feature-a").Return(nil) // Parent is stack branch, no FetchBranch needed mockGit.On("FetchBranch", "feature-b").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) // Return to original branch @@ -207,9 +198,7 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -218,10 +207,7 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("CheckoutBranch", "feature-b").Return(nil) mockGit.On("GetCommitHash", "feature-b").Return("def456", nil) mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) - mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil) - mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil) - mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) - mockGit.On("Rebase", "feature-a").Return(nil) + mockGit.On("Rebase", "feature-a").Return(nil) // Parent is stack branch, no FetchBranch needed mockGit.On("FetchBranch", "feature-b").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) @@ -287,9 +273,7 @@ func TestRunSyncStashHandling(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -347,9 +331,7 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase // Rebase fails mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict")) // Note: StashPop is NOT called because rebaseConflict=true @@ -400,9 +382,7 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase // Rebase fails - stash should NOT be popped (preserved for --resume) mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict")) // Note: StashPop is NOT called because rebaseConflict=true @@ -545,9 +525,7 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -607,9 +585,7 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("CheckoutBranch", "feature-a").Return(nil) mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) From 39141dff27a482361e56f5c7418b4d7bc6d8913c Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Wed, 17 Dec 2025 14:13:33 +0100 Subject: [PATCH 3/7] feat: prompt before cleaning up stale sync state --- cmd/sync.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/cmd/sync.go b/cmd/sync.go index f693b4f..a64d60a 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -178,7 +178,22 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { if hasSavedState { fmt.Fprintf(os.Stderr, "Warning: found state from a previous interrupted sync\n") fmt.Fprintf(os.Stderr, "If you resolved rebase conflicts, run 'stack sync --resume'\n") - fmt.Fprintf(os.Stderr, "Otherwise, cleaning up stale state and starting fresh...\n\n") + fmt.Fprintf(os.Stderr, "\nStart fresh? [y/N] ") + + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return fmt.Errorf("failed to read input: %w", err) + } + + input = strings.TrimSpace(strings.ToLower(input)) + if input != "y" && input != "yes" { + fmt.Println("Aborted. Use 'stack sync --resume' or 'stack sync --abort' to handle the interrupted sync.") + return nil + } + + fmt.Println("Cleaning up stale state and starting fresh...") + fmt.Println() // Clean up stale state _ = gitClient.UnsetConfig(configSyncStashed) _ = gitClient.UnsetConfig(configSyncOriginalBranch) From 83f7e9ae58541a839ef95be22bba37957c3a3dee Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Fri, 19 Dec 2025 00:30:38 +0100 Subject: [PATCH 4/7] fix: add missing mock expectations for unique commit detection --- cmd/sync_test.go | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index e21db62..7890ae3 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -56,6 +56,11 @@ func TestRunSyncBasic(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + // Patch-based unique commit detection + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + // Falls through to regular rebase since merge-base == parent mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -63,7 +68,12 @@ func TestRunSyncBasic(t *testing.T) { mockGit.On("CheckoutBranch", "feature-b").Return(nil) mockGit.On("GetCommitHash", "feature-b").Return("def456", nil) mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) - mockGit.On("Rebase", "feature-a").Return(nil) // Parent is stack branch, no FetchBranch needed + // Patch-based unique commit detection + mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil) + mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil) + mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) + // Falls through to regular rebase since merge-base == parent + mockGit.On("Rebase", "feature-a").Return(nil) mockGit.On("FetchBranch", "feature-b").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) // Return to original branch @@ -199,6 +209,9 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -207,7 +220,10 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("CheckoutBranch", "feature-b").Return(nil) mockGit.On("GetCommitHash", "feature-b").Return("def456", nil) mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) - mockGit.On("Rebase", "feature-a").Return(nil) // Parent is stack branch, no FetchBranch needed + mockGit.On("GetUniqueCommitsByPatch", "feature-a", "feature-b").Return([]string{"def456"}, nil) + mockGit.On("GetMergeBase", "feature-b", "feature-a").Return("abc123", nil) + mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) + mockGit.On("Rebase", "feature-a").Return(nil) mockGit.On("FetchBranch", "feature-b").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) @@ -274,6 +290,9 @@ func TestRunSyncStashHandling(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -332,6 +351,9 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) // Rebase fails mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict")) // Note: StashPop is NOT called because rebaseConflict=true @@ -383,6 +405,9 @@ func TestRunSyncErrorHandling(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) // Rebase fails - stash should NOT be popped (preserved for --resume) mockGit.On("Rebase", "origin/main").Return(fmt.Errorf("rebase conflict")) // Note: StashPop is NOT called because rebaseConflict=true @@ -526,6 +551,9 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) @@ -586,6 +614,9 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) From 2cb6ee1787d5a19a82acc1415527124e27385f62 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Fri, 19 Dec 2025 01:00:37 +0100 Subject: [PATCH 5/7] fix: remove test for old auto-cleanup behavior --- cmd/sync_test.go | 61 +++--------------------------------------------- 1 file changed, 3 insertions(+), 58 deletions(-) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 7890ae3..3b334e3 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -575,64 +575,9 @@ func TestRunSyncResume(t *testing.T) { mockGH.AssertExpectations(t) }) - t.Run("orphaned state is cleaned up on fresh sync", func(t *testing.T) { - mockGit := new(testutil.MockGitClient) - mockGH := new(testutil.MockGitHubClient) - - // Orphaned state exists but --resume not passed - mockGit.On("GetConfig", "stack.sync.stashed").Return("true") - mockGit.On("GetConfig", "stack.sync.originalBranch").Return("old-branch") - // Clean up orphaned state - mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) - mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) - - mockGit.On("GetCurrentBranch").Return("feature-a", nil) - // Save original branch state - mockGit.On("SetConfig", "stack.sync.originalBranch", "feature-a").Return(nil) - mockGit.On("IsWorkingTreeClean").Return(true, nil) - mockGit.On("GetConfig", "branch.feature-a.stackparent").Return("main") - mockGit.On("GetConfig", "stack.baseBranch").Return("").Maybe() - mockGit.On("GetDefaultBranch").Return("main").Maybe() - - stackParents := map[string]string{ - "feature-a": "main", - } - mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe() - - mockGit.On("Fetch").Return(nil) - mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) - - mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) - mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) - mockGit.On("GetRemoteBranchesSet").Return(map[string]bool{ - "main": true, - "feature-a": true, - }) - - // Process feature-a - mockGit.On("CheckoutBranch", "feature-a").Return(nil) - mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) - mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) - mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase - mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) - mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) - mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) - mockGit.On("Rebase", "origin/main").Return(nil) - mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) - - // Return to original branch - mockGit.On("CheckoutBranch", "feature-a").Return(nil) - // Clean up sync state - mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) - mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) - - err := runSync(mockGit, mockGH) - - assert.NoError(t, err) - mockGit.AssertExpectations(t) - mockGH.AssertExpectations(t) - }) + // Note: The "orphaned state is cleaned up on fresh sync" test was removed + // because the behavior changed - now we prompt the user before cleaning up + // stale state, which requires stdin input that's difficult to mock in tests. } func TestRunSyncAbort(t *testing.T) { From d0575524f63931537d5d5ea2fe65b34fcb52e3fb Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Fri, 19 Dec 2025 01:12:37 +0100 Subject: [PATCH 6/7] fix: add missing FetchBranch mock for merged parent test --- cmd/sync_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 3b334e3..73e7105 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -142,6 +142,7 @@ func TestRunSyncMergedParent(t *testing.T) { mockGit.On("CheckoutBranch", "feature-b").Return(nil) mockGit.On("GetCommitHash", "feature-b").Return("def456", nil) mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase mockGit.On("RebaseOnto", "origin/main", "feature-a", "feature-b").Return(nil) mockGit.On("FetchBranch", "feature-b").Return(nil) mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) From 1cd5ef5b811e88cbdda22ebc0e80f4cc9950a822 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Fri, 19 Dec 2025 14:07:49 +0100 Subject: [PATCH 7/7] feat: add injectable stdinReader for testable prompts --- cmd/sync.go | 7 ++-- cmd/sync_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index a64d60a..a2af5a2 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -4,6 +4,7 @@ import ( "bufio" "errors" "fmt" + "io" "os" "strings" "sync" @@ -22,6 +23,8 @@ var ( syncForce bool syncResume bool syncAbort bool + // stdinReader allows tests to inject mock input for prompts + stdinReader io.Reader = os.Stdin ) // Git config keys for sync state persistence @@ -180,7 +183,7 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { fmt.Fprintf(os.Stderr, "If you resolved rebase conflicts, run 'stack sync --resume'\n") fmt.Fprintf(os.Stderr, "\nStart fresh? [y/N] ") - reader := bufio.NewReader(os.Stdin) + reader := bufio.NewReader(stdinReader) input, err := reader.ReadString('\n') if err != nil { return fmt.Errorf("failed to read input: %w", err) @@ -260,7 +263,7 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { fmt.Printf("Branch '%s' is not in a stack.\n", originalBranch) fmt.Printf("Add it with parent '%s'? [Y/n] ", baseBranch) - reader := bufio.NewReader(os.Stdin) + reader := bufio.NewReader(stdinReader) input, err := reader.ReadString('\n') if err != nil { return fmt.Errorf("failed to read input: %w", err) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 73e7105..3f52931 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2,6 +2,8 @@ package cmd import ( "fmt" + "os" + "strings" "testing" "github.com/javoire/stackinator/internal/github" @@ -576,9 +578,90 @@ func TestRunSyncResume(t *testing.T) { mockGH.AssertExpectations(t) }) - // Note: The "orphaned state is cleaned up on fresh sync" test was removed - // because the behavior changed - now we prompt the user before cleaning up - // stale state, which requires stdin input that's difficult to mock in tests. + t.Run("stale state cleaned up when user confirms", func(t *testing.T) { + mockGit := new(testutil.MockGitClient) + mockGH := new(testutil.MockGitHubClient) + + // Inject "y" input for the prompt + stdinReader = strings.NewReader("y\n") + defer func() { stdinReader = os.Stdin }() + + // Orphaned state exists but --resume not passed + mockGit.On("GetConfig", "stack.sync.stashed").Return("true") + mockGit.On("GetConfig", "stack.sync.originalBranch").Return("old-branch") + // Clean up orphaned state (user confirmed) + mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) + mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) + + mockGit.On("GetCurrentBranch").Return("feature-a", nil) + // Save original branch state + mockGit.On("SetConfig", "stack.sync.originalBranch", "feature-a").Return(nil) + mockGit.On("IsWorkingTreeClean").Return(true, nil) + mockGit.On("GetConfig", "branch.feature-a.stackparent").Return("main") + mockGit.On("GetConfig", "stack.baseBranch").Return("").Maybe() + mockGit.On("GetDefaultBranch").Return("main").Maybe() + + stackParents := map[string]string{ + "feature-a": "main", + } + mockGit.On("GetAllStackParents").Return(stackParents, nil).Maybe() + + mockGit.On("Fetch").Return(nil) + mockGH.On("GetAllPRs").Return(make(map[string]*github.PRInfo), nil) + + mockGit.On("GetWorktreeBranches").Return(make(map[string]string), nil) + mockGit.On("GetCurrentWorktreePath").Return("/Users/test/repo", nil) + mockGit.On("GetRemoteBranchesSet").Return(map[string]bool{ + "main": true, + "feature-a": true, + }) + + // Process feature-a + mockGit.On("CheckoutBranch", "feature-a").Return(nil) + mockGit.On("GetCommitHash", "feature-a").Return("abc123", nil) + mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) + mockGit.On("FetchBranch", "main").Return(nil) // Fetch base branch before rebase + mockGit.On("GetUniqueCommitsByPatch", "origin/main", "feature-a").Return([]string{"abc123"}, nil) + mockGit.On("GetMergeBase", "feature-a", "origin/main").Return("main123", nil) + mockGit.On("GetCommitHash", "origin/main").Return("main123", nil) + mockGit.On("Rebase", "origin/main").Return(nil) + mockGit.On("FetchBranch", "feature-a").Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) + + // Return to original branch + mockGit.On("CheckoutBranch", "feature-a").Return(nil) + // Clean up sync state + mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) + mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) + + err := runSync(mockGit, mockGH) + + assert.NoError(t, err) + mockGit.AssertExpectations(t) + mockGH.AssertExpectations(t) + }) + + t.Run("sync aborted when user declines stale state cleanup", func(t *testing.T) { + mockGit := new(testutil.MockGitClient) + mockGH := new(testutil.MockGitHubClient) + + // Inject "n" input for the prompt (user declines) + stdinReader = strings.NewReader("n\n") + defer func() { stdinReader = os.Stdin }() + + // Orphaned state exists but --resume not passed + mockGit.On("GetConfig", "stack.sync.stashed").Return("true") + mockGit.On("GetConfig", "stack.sync.originalBranch").Return("old-branch") + + // User declined, so sync should abort without calling any other methods + + err := runSync(mockGit, mockGH) + + // Should return nil (not an error) since user chose to abort + assert.NoError(t, err) + mockGit.AssertExpectations(t) + mockGH.AssertExpectations(t) + }) } func TestRunSyncAbort(t *testing.T) {