diff --git a/cmd/sync.go b/cmd/sync.go index 56e5705..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 @@ -178,7 +181,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(stdinReader) + 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) @@ -245,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) @@ -510,6 +528,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 diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 8f5505f..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" @@ -55,6 +57,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) + 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) @@ -141,6 +144,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) @@ -207,6 +211,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("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) @@ -287,6 +292,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("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) @@ -347,6 +353,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("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) @@ -400,6 +407,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("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) @@ -545,6 +553,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("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) @@ -569,14 +578,18 @@ func TestRunSyncResume(t *testing.T) { mockGH.AssertExpectations(t) }) - t.Run("orphaned state is cleaned up on fresh sync", func(t *testing.T) { + 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 + // Clean up orphaned state (user confirmed) mockGit.On("UnsetConfig", "stack.sync.stashed").Return(nil) mockGit.On("UnsetConfig", "stack.sync.originalBranch").Return(nil) @@ -607,6 +620,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("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) @@ -626,6 +640,28 @@ func TestRunSyncResume(t *testing.T) { 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) {