Skip to content

feat: prompt before cleaning up stale sync state #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
javoire merged 7 commits into from
Dec 19, 2025
Merged

feat: prompt before cleaning up stale sync state #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99a1270
fix: explicitly fetch base branch before rebase in sync
javoire Dec 16, 2025
e719028
fix: add FetchBranch mock expectations for base branch fetch
javoire Dec 16, 2025
39141df
feat: prompt before cleaning up stale sync state
javoire Dec 17, 2025
83f7e9a
fix: add missing mock expectations for unique commit detection
javoire Dec 18, 2025
2cb6ee1
fix: remove test for old auto-cleanup behavior
javoire Dec 19, 2025
d057552
fix: add missing FetchBranch mock for merged parent test
javoire Dec 19, 2025
1cd5ef5
feat: add injectable stdinReader for testable prompts
javoire Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions cmd/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bufio"
"errors"
"fmt"
"io"
"os"
"strings"
"sync"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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/<parent>
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
Expand Down
40 changes: 38 additions & 2 deletions cmd/sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cmd

import (
"fmt"
"os"
"strings"
"testing"

"github.com/javoire/stackinator/internal/github"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down