From e4e128d0c96e183aae3335ce9fa689a6e606c4d8 Mon Sep 17 00:00:00 2001 From: Jonatan Dahl Date: Mon, 8 Dec 2025 13:26:04 -0500 Subject: [PATCH] fix: remove overly cautious divergence check and fix stale info push errors --- cmd/sync.go | 43 ++++++++++++++++++++------------------ cmd/sync_test.go | 16 +++++++------- docs/commands.md | 6 +++++- internal/git/git.go | 15 +++++++++++++ internal/git/interface.go | 2 +- internal/testutil/mocks.go | 6 +++++- 6 files changed, 57 insertions(+), 31 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 4a06119..149e89f 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -77,7 +77,7 @@ Uncommitted changes are automatically stashed and reapplied (using --autostash). } func init() { - syncCmd.Flags().BoolVarP(&syncForce, "force", "f", false, "Force push even if local and remote have diverged (use with caution)") + syncCmd.Flags().BoolVarP(&syncForce, "force", "f", false, "Use --force instead of --force-with-lease for push (bypasses safety checks)") syncCmd.Flags().BoolVarP(&syncResume, "resume", "r", false, "Resume a sync after resolving rebase conflicts") } @@ -412,17 +412,11 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { return fmt.Errorf("failed to fast-forward: %w", err) } } else { - // Branches have diverged - this is NOT safe to auto-resolve - fmt.Fprintf(os.Stderr, " Error: local and remote have diverged for %s\n", branch.Name) - fmt.Fprintf(os.Stderr, "\n This usually means:\n") - fmt.Fprintf(os.Stderr, " - Someone else force-pushed to the remote, OR\n") - fmt.Fprintf(os.Stderr, " - You have local commits that differ from remote\n") - fmt.Fprintf(os.Stderr, "\n To resolve:\n") - fmt.Fprintf(os.Stderr, " 1. Check what's on remote: git log origin/%s\n", branch.Name) - fmt.Fprintf(os.Stderr, " 2. Check what's local: git log %s\n", branch.Name) - fmt.Fprintf(os.Stderr, " 3. If remote is correct: git reset --hard origin/%s, then run 'stack sync'\n", branch.Name) - fmt.Fprintf(os.Stderr, " 4. If local is correct: stack sync --force\n") - return errAlreadyPrinted + // Branches have diverged - this is normal after rebasing onto an updated parent + // --force-with-lease will safely handle this during push + if git.Verbose { + fmt.Printf(" Local and remote have diverged (normal after rebase)\n") + } } } else if git.Verbose { fmt.Printf(" Local branch is up-to-date with origin/%s\n", branch.Name) @@ -485,28 +479,37 @@ func runSync(gitClient git.GitClient, githubClient github.GitHubClient) error { return gitClient.ForcePush(branch.Name) } - // Fetch one more time right before push to ensure --force-with-lease has fresh tracking info - // This prevents "stale info" errors if the remote was updated during our rebase + // Fetch one more time right before push to get the current remote SHA if git.Verbose { fmt.Printf(" Refreshing remote tracking ref before push...\n") } if err := gitClient.FetchBranch(branch.Name); err != nil { - // Non-fatal, continue with push + // Non-fatal, continue with push using plain --force-with-lease if git.Verbose { fmt.Fprintf(os.Stderr, " Note: could not refresh tracking ref: %v\n", err) } + return gitClient.Push(branch.Name, true) + } + + // Get the remote SHA to use with explicit --force-with-lease + // This avoids "stale info" errors that can occur with plain --force-with-lease + remoteSha, err := gitClient.GetCommitHash("origin/" + branch.Name) + if err != nil { + // Fall back to plain --force-with-lease + if git.Verbose { + fmt.Fprintf(os.Stderr, " Note: could not get remote SHA, using plain force-with-lease: %v\n", err) + } + return gitClient.Push(branch.Name, true) } - // Use --force-with-lease (safe force push) - return gitClient.Push(branch.Name, true) + return gitClient.PushWithExpectedRemote(branch.Name, remoteSha) }, ) if pushErr != nil { if !syncForce { - fmt.Fprintf(os.Stderr, "\nPossible causes:\n") - fmt.Fprintf(os.Stderr, " 1. Remote branch was updated by someone else - try running 'stack sync' again\n") - fmt.Fprintf(os.Stderr, " 2. Your local branch has diverged from remote - use 'stack sync --force'\n") + fmt.Fprintf(os.Stderr, "\nPossible cause:\n") + fmt.Fprintf(os.Stderr, " Remote branch was updated after fetch - try running 'stack sync' again\n") } return fmt.Errorf("push failed for %s", branch.Name) } diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 11db639..427d6b8 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -55,14 +55,14 @@ func TestRunSyncBasic(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("Push", "feature-a", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) // Process feature-b 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) mockGit.On("FetchBranch", "feature-b").Return(nil) - mockGit.On("Push", "feature-b", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) // Return to original branch mockGit.On("CheckoutBranch", "feature-b").Return(nil) @@ -126,7 +126,7 @@ func TestRunSyncMergedParent(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) mockGit.On("RebaseOnto", "origin/main", "feature-a", "feature-b").Return(nil) mockGit.On("FetchBranch", "feature-b").Return(nil) - mockGit.On("Push", "feature-b", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) // Return to original branch mockGit.On("CheckoutBranch", "feature-b").Return(nil) @@ -187,7 +187,7 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("Push", "feature-a", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) // Process feature-b mockGit.On("CheckoutBranch", "feature-b").Return(nil) @@ -195,7 +195,7 @@ func TestRunSyncUpdatePRBase(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-b").Return("def456", nil) mockGit.On("Rebase", "feature-a").Return(nil) mockGit.On("FetchBranch", "feature-b").Return(nil) - mockGit.On("Push", "feature-b", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-b", "def456").Return(nil) // Update PR base! mockGH.On("UpdatePRBase", 2, "feature-a").Return(nil) @@ -256,7 +256,7 @@ func TestRunSyncStashHandling(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("Push", "feature-a", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) mockGit.On("CheckoutBranch", "feature-a").Return(nil) @@ -502,7 +502,7 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("Push", "feature-a", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) // Return to original branch (called twice: once for return, once for displayStatusAfterSync) mockGit.On("CheckoutBranch", "feature-a").Return(nil) @@ -559,7 +559,7 @@ func TestRunSyncResume(t *testing.T) { mockGit.On("GetCommitHash", "origin/feature-a").Return("abc123", nil) mockGit.On("Rebase", "origin/main").Return(nil) mockGit.On("FetchBranch", "feature-a").Return(nil) - mockGit.On("Push", "feature-a", true).Return(nil) + mockGit.On("PushWithExpectedRemote", "feature-a", "abc123").Return(nil) // Return to original branch mockGit.On("CheckoutBranch", "feature-a").Return(nil) diff --git a/docs/commands.md b/docs/commands.md index 5ff4769..dd3228e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -28,6 +28,7 @@ stack status --no-pr ``` Flags: + - `--no-pr` - Skip fetching PR information (faster) ## `stack sync` @@ -51,7 +52,8 @@ stack sync --force ``` Flags: -- `--force`, `-f` - Force push even if local and remote have diverged (use with caution) + +- `--force`, `-f` - Use `--force` instead of `--force-with-lease` for push (bypasses safety checks) ## `stack parent` @@ -80,6 +82,7 @@ stack prune --dry-run ``` Flags: + - `--all`, `-a` - Check all local branches, not just stack branches - `--force`, `-f` - Force delete branches even if they have unmerged commits @@ -132,6 +135,7 @@ stack worktree --prune ``` Flags: + - `--prune` - Remove worktrees for branches with merged PRs ## `stack version` diff --git a/internal/git/git.go b/internal/git/git.go index da15dd6..a467d45 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -216,6 +216,21 @@ func (c *gitClient) Push(branch string, forceWithLease bool) error { return err } +// PushWithExpectedRemote pushes a branch using --force-with-lease with an explicit expected SHA. +// This avoids "stale info" errors that can occur with plain --force-with-lease. +func (c *gitClient) PushWithExpectedRemote(branch string, expectedRemoteSha string) error { + leaseArg := fmt.Sprintf("--force-with-lease=refs/heads/%s:%s", branch, expectedRemoteSha) + args := []string{"push", leaseArg, "origin", branch} + + if DryRun { + fmt.Printf(" [DRY RUN] git %s\n", strings.Join(args, " ")) + return nil + } + + _, err := c.runCmd(args...) + return err +} + // ForcePush force pushes a branch to origin (bypasses --force-with-lease safety) func (c *gitClient) ForcePush(branch string) error { args := []string{"push", "--force", "origin", branch} diff --git a/internal/git/interface.go b/internal/git/interface.go index 42df7c2..4605ecb 100644 --- a/internal/git/interface.go +++ b/internal/git/interface.go @@ -16,6 +16,7 @@ type GitClient interface { RebaseOnto(newBase, oldBase, currentBranch string) error FetchBranch(branch string) error Push(branch string, forceWithLease bool) error + PushWithExpectedRemote(branch string, expectedRemoteSha string) error ForcePush(branch string) error IsWorkingTreeClean() (bool, error) Fetch() error @@ -40,4 +41,3 @@ type GitClient interface { RemoveWorktree(path string) error ListWorktrees() ([]string, error) } - diff --git a/internal/testutil/mocks.go b/internal/testutil/mocks.go index f24ea36..e01e288 100644 --- a/internal/testutil/mocks.go +++ b/internal/testutil/mocks.go @@ -80,6 +80,11 @@ func (m *MockGitClient) Push(branch string, forceWithLease bool) error { return args.Error(0) } +func (m *MockGitClient) PushWithExpectedRemote(branch string, expectedRemoteSha string) error { + args := m.Called(branch, expectedRemoteSha) + return args.Error(0) +} + func (m *MockGitClient) ForcePush(branch string) error { args := m.Called(branch) return args.Error(0) @@ -222,4 +227,3 @@ func (m *MockGitHubClient) IsPRMerged(prNumber int) (bool, error) { args := m.Called(prNumber) return args.Bool(0), args.Error(1) } -