From cbe1e9441e24b9d0edf7a1f2d93b675a77403a23 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sat, 11 Jul 2026 14:00:42 -0400 Subject: [PATCH 1/7] pull in remote updates during sync --- cmd/checkout.go | 16 +- cmd/sync.go | 47 ++++-- cmd/sync_test.go | 410 ++++++++++++++++++++++++++++++++++++++++++++++- cmd/utils.go | 371 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 818 insertions(+), 26 deletions(-) diff --git a/cmd/checkout.go b/cmd/checkout.go index 0ceed6a..526e18f 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -491,21 +491,9 @@ func importRemoteStack( // Skip merged PRs whose branches were deleted from the remote — // these no longer exist upstream and can't be created locally. for _, pr := range prs { - branch := pr.HeadRefName - if git.BranchExists(branch) { - continue - } - remoteRef := remote + "/" + branch - if err := git.CreateBranch(branch, remoteRef); err != nil { - if pr.Merged { - cfg.Infof("Skipping merged branch %s", branch) - continue - } - cfg.Errorf("failed to pull branch %s from %s: %v", branch, remoteRef, err) - return nil, ErrSilent + if _, err := ensureLocalBranchFromRemote(cfg, remote, pr); err != nil { + return nil, err } - _ = git.SetUpstreamTracking(branch, remote) - cfg.Successf("Pulled branch %s", branch) } // Build the stack diff --git a/cmd/sync.go b/cmd/sync.go index 70bd831..18931a7 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -26,16 +26,27 @@ func SyncCmd(cfg *config.Config) *cobra.Command { Short: "Sync the current stack with the remote", Long: `Fetch, rebase, push, and sync PR state for the current stack. -This command performs a safe, non-interactive synchronization: +This command performs a safe synchronization: 1. Fetches the latest changes from the remote - 2. Fast-forwards the trunk branch to match the remote - 3. Cascade-rebases stack branches onto their updated parents - 4. Pushes all branches atomically (using --force-with-lease --atomic) - 5. Syncs PR state from GitHub - 6. Links the stack's open PRs into a stack on GitHub (creating or updating + 2. Reconciles the stack on GitHub with your local stack: pulls down + branches for any PRs added to the stack on GitHub, or prompts you to + resolve a divergence in an interactive terminal + 3. Fast-forwards the trunk branch to match the remote + 4. Cascade-rebases stack branches onto their updated parents + 5. Pushes all branches atomically (using --force-with-lease --atomic) + 6. Syncs PR state from GitHub + 7. Links the stack's open PRs into a stack on GitHub (creating or updating the remote stack object) when two or more PRs exist +If PRs have been added to the stack on GitHub, their branches are pulled +down and appended to your local stack so it mirrors the remote. A clean +"remote is ahead" update happens automatically without prompting. If the +local and remote stacks have diverged, sync prompts (in an interactive +terminal) to use the remote as the source of truth, use your local stack +as the source of truth, disassociate them, or cancel. In a non-interactive +terminal a divergence is reported and left untouched. + If a rebase conflict is detected, all branches are restored to their original state and you are advised to run "gh stack rebase" to resolve conflicts interactively. @@ -45,7 +56,7 @@ links PRs that already exist. The final message reflects what happened: "Stack synced" means the stack object on GitHub now matches your local stack, while "Branches synced" means the branches were rebased and pushed but no remote stack object was created or updated (for example, when fewer -than two PRs exist yet). +than two PRs exist yet, or a divergence was left unresolved). Use --prune to delete local branches for merged PRs. Stack metadata is preserved so that rebase and display logic continue to work correctly. @@ -99,6 +110,22 @@ func runSync(cfg *config.Config, opts *syncOptions) error { _ = git.FetchBranches(remote, fetchTargets) cfg.Successf("Fetched latest changes from %s", remote) + // --- Step 1b: Reconcile remote-ahead stack changes --- + // Pull in branches for PRs that were added to the stack on GitHub, or + // resolve a divergence, before rebasing and pushing so pulled branches + // participate in the normal flow. Best-effort for stacks tracked on the + // remote; a no-op otherwise. + reconcileRes, err := reconcileRemoteStack(cfg, sf, s, gitDir, remote) + if err != nil { + if errors.Is(err, errInterrupt) { + return ErrSilent + } + return err + } + if reconcileRes.stack != nil { + s = reconcileRes.stack + } + // --- Step 2: Fast-forward trunk --- trunk := s.Trunk.Branch trunkUpdated := fastForwardTrunk(cfg, trunk, remote, currentBranch) @@ -237,8 +264,10 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // stack object actually reflects the local stack, which determines the final // summary message below. stackSynced := false - if client, err := cfg.GitHubClient(); err == nil { - stackSynced = syncStack(cfg, client, s) + if !reconcileRes.skipStackObjectSync { + if client, err := cfg.GitHubClient(); err == nil { + stackSynced = syncStack(cfg, client, s) + } } // --- Step 6: Prune merged branches (optional) --- diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 6eaebcb..5630628 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -38,9 +38,9 @@ func newSyncMock(tmpDir string, currentBranch string) *git.MockOps { } return "sha-" + ref, nil }, - IsAncestorFn: func(a, d string) (bool, error) { return true, nil }, - FetchFn: func(string) error { return nil }, - EnableRerereFn: func() error { return nil }, + IsAncestorFn: func(a, d string) (bool, error) { return true, nil }, + FetchFn: func(string) error { return nil }, + EnableRerereFn: func() error { return nil }, IsRebaseInProgressFn: func() bool { return false }, PushFn: func(string, []string, bool, bool) error { return nil }, } @@ -1949,3 +1949,407 @@ func TestSync_PRsSpanMultipleStacks_BranchesSynced(t *testing.T) { assert.Contains(t, output, "Branches synced") assert.NotContains(t, output, "Stack synced") } + +// --- Remote-ahead pull & divergence reconciliation --- + +// runSyncCfg runs sync against tmpDir with the given git mock, allowing the +// caller to configure the Config (GitHub override, interactivity, SelectFn). +// It returns the captured stderr output and the command's error. +func runSyncCfg(t *testing.T, gitMock *git.MockOps, configure func(*config.Config)) (string, error) { + t.Helper() + restore := git.SetOps(gitMock) + defer restore() + + cfg, _, errR := config.NewTestConfig() + if configure != nil { + configure(cfg) + } + cmd := SyncCmd(cfg) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + return string(errOut), err +} + +// prByNumberFinder returns a FindPRByNumber func that reports OPEN PRs for the +// given number->branch map and nil for any other number. +func prByNumberFinder(branchByNum map[int]string) func(int) (*github.PullRequest, error) { + return func(n int) (*github.PullRequest, error) { + b, ok := branchByNum[n] + if !ok { + return nil, nil + } + return &github.PullRequest{ + Number: n, + ID: fmt.Sprintf("PR_%d", n), + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + HeadRefName: b, + State: "OPEN", + }, nil + } +} + +func TestClassifyRemoteStack(t *testing.T) { + tests := []struct { + name string + localActive []string + remoteActive []string + want remoteStackClass + }{ + {"identical", []string{"b1", "b2"}, []string{"b1", "b2"}, remoteStackInSync}, + {"clean append on top", []string{"b1", "b2"}, []string{"b1", "b2", "b3"}, remoteStackCleanAhead}, + {"local ahead", []string{"b1", "b2", "b3"}, []string{"b1", "b2"}, remoteStackLocalAhead}, + {"divergent tip", []string{"b1", "b2", "b3"}, []string{"b1", "b2", "b4"}, remoteStackDivergent}, + {"divergent reorder", []string{"b1", "b2"}, []string{"b2", "b1"}, remoteStackDivergent}, + {"empty local", nil, []string{"b1"}, remoteStackCleanAhead}, + {"empty remote", []string{"b1"}, nil, remoteStackLocalAhead}, + {"both empty", nil, nil, remoteStackInSync}, + {"divergent middle", []string{"b1", "x"}, []string{"b1", "y", "z"}, remoteStackDivergent}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, classifyRemoteStack(tt.localActive, tt.remoteActive)) + }) + } +} + +// TestSync_RemoteAhead_PullsNewBranches verifies the core new behavior: when the +// remote stack has PRs appended on top of the local stack, sync pulls the new +// branches down and adds them to the local stack. +func TestSync_RemoteAhead_PullsNewBranches(t *testing.T) { + s := stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + {Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 103}}, + }, + } + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + var created, fetched []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.BranchExistsFn = func(name string) bool { return name != "b4" && name != "b5" } + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.FetchBranchesFn = func(_ string, branches []string) error { fetched = append(fetched, branches...); return nil } + mock.SetUpstreamTrackingFn = func(string, string) error { return nil } + + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 103, 104, 105}}}, nil + }, + FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3", 104: "b4", 105: "b5"}), + } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock }) + require.NoError(t, err) + + assert.Contains(t, created, "b4") + assert.Contains(t, created, "b5") + assert.Subset(t, fetched, []string{"b4", "b5"}) + assert.Contains(t, output, "Pulling 2 new branches from the remote stack") + assert.Contains(t, output, "Pulled 2 new branches into the stack") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, []string{"b1", "b2", "b3", "b4", "b5"}, sf.Stacks[0].BranchNames()) +} + +// TestSync_RemoteInSync_NoPull verifies that when local and remote match, no +// branches are pulled and no divergence is reported. +func TestSync_RemoteInSync_NoPull(t *testing.T) { + s := stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + }, + } + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102}}}, nil + }, + FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2"}), + } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock }) + require.NoError(t, err) + + assert.Empty(t, created, "no branches should be pulled when in sync") + assert.NotContains(t, output, "Pulling") + assert.NotContains(t, output, "diverged") + assert.Contains(t, output, "Stack synced") +} + +// divergentStack returns a stack file (ID 9) and GitHub mock configured so that +// the local stack [b1,b2,b3] diverges from the remote stack [b1,b2,b4]. +func divergentStack(t *testing.T, tmpDir string) { + t.Helper() + s := stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + {Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 103}}, + }, + } + writeStackFile(t, tmpDir, s) +} + +func divergentRemoteMock() *github.MockClient { + return &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 104}}}, nil + }, + FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3", 104: "b4"}), + } +} + +// TestSync_Divergent_NonInteractive_SafeNoOp verifies that a divergence in a +// non-interactive terminal is a safe no-op: no stack API mutations, guidance is +// printed, the association is preserved, and it exits success. +func TestSync_Divergent_NonInteractive_SafeNoOp(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } + ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } + ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock }) + require.NoError(t, err) + + assert.Empty(t, created) + assert.Contains(t, output, "diverged") + assert.Contains(t, output, "Branches synced") + assert.NotContains(t, output, "Stack synced") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, "9", sf.Stacks[0].ID, "association is preserved") + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames()) +} + +// TestSync_Divergent_UseRemote replaces the local stack with the remote version. +func TestSync_Divergent_UseRemote(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.BranchExistsFn = func(name string) bool { return name != "b4" } + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.SetUpstreamTrackingFn = func(string, string) error { return nil } + mock.HasUncommittedChangesFn = func() (bool, error) { return false, nil } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + }) + require.NoError(t, err) + + assert.Contains(t, created, "b4") + assert.Contains(t, output, "replaced with the remote version") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, []string{"b1", "b2", "b4"}, sf.Stacks[0].BranchNames()) + assert.Equal(t, "9", sf.Stacks[0].ID) +} + +// TestSync_Divergent_UseRemote_DirtyBlocked refuses to replace the local stack +// when the working tree has uncommitted changes. +func TestSync_Divergent_UseRemote_DirtyBlocked(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.HasUncommittedChangesFn = func() (bool, error) { return true, nil } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + }) + + assert.Error(t, err) + assert.Contains(t, output, "uncommitted changes") + assert.Empty(t, created) + + sf, loadErr := stack.Load(tmpDir) + require.NoError(t, loadErr) + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames(), "local stack untouched") + assert.Equal(t, "9", sf.Stacks[0].ID) +} + +// TestSync_Divergent_UseLocal deletes the diverged remote stack and recreates it +// from the local stack. +func TestSync_Divergent_UseLocal(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + deleted := false + var deletedID string + var createdWith []int + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + if deleted { + return nil, nil + } + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 104}}}, nil + }, + FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3", 104: "b4"}), + FindPRForBranchFn: openPRFinder(map[string]int{"b1": 101, "b2": 102, "b3": 103}), + DeleteStackFn: func(id string) error { deleted = true; deletedID = id; return nil }, + CreateStackFn: func(prs []int) (int, error) { createdWith = prs; return 12, nil }, + } + mock := newSyncMockNoRebase(tmpDir, "b1") + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 1, nil } + }) + require.NoError(t, err) + + assert.True(t, deleted, "remote stack should be deleted") + assert.Equal(t, "9", deletedID) + assert.Equal(t, []int{101, 102, 103}, createdWith, "remote recreated from local PRs") + assert.Contains(t, output, "Stack synced") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, "12", sf.Stacks[0].ID, "adopts the recreated stack ID") +} + +// TestSync_Divergent_Disassociate clears the remote stack ID without touching +// either the remote stack object or the local branches. +func TestSync_Divergent_Disassociate(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } + ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } + ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } + mock := newSyncMockNoRebase(tmpDir, "b1") + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 2, nil } + }) + require.NoError(t, err) + + assert.Contains(t, output, "independent") + assert.Contains(t, output, "Branches synced") + assert.NotContains(t, output, "Stack synced") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, "", sf.Stacks[0].ID, "remote stack ID is cleared") + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames()) +} + +// TestSync_Divergent_Cancel makes no changes and preserves the association. +func TestSync_Divergent_Cancel(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } + ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } + ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } + mock := newSyncMockNoRebase(tmpDir, "b1") + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 3, nil } + }) + require.NoError(t, err) + + assert.Contains(t, output, "still differ") + assert.Contains(t, output, "Branches synced") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, "9", sf.Stacks[0].ID, "association is preserved") + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames()) +} + +// TestSync_MergedBranchPruned_NoFalseDivergence verifies that a merged branch +// (still tracked locally but reported merged by the remote) does not classify as +// a divergence. +func TestSync_MergedBranchPruned_NoFalseDivergence(t *testing.T) { + s := stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101, Merged: true}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + {Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 103}}, + }, + } + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + var created []string + mock := newSyncMockNoRebase(tmpDir, "b2") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 103}}}, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + branch := map[int]string{101: "b1", 102: "b2", 103: "b3"}[n] + if branch == "" { + return nil, nil + } + return &github.PullRequest{ + Number: n, ID: fmt.Sprintf("PR_%d", n), + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + HeadRefName: branch, + State: map[bool]string{true: "MERGED", false: "OPEN"}[n == 101], + Merged: n == 101, + }, nil + }, + } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { + t.Fatal("no prompt expected when merged branch is pruned") + return 0, nil + } + }) + require.NoError(t, err) + + assert.Empty(t, created) + assert.NotContains(t, output, "diverged") +} diff --git a/cmd/utils.go b/cmd/utils.go index 4266461..c1068c3 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/AlecAivazis/survey/v2/terminal" + "github.com/cli/go-gh/v2/pkg/api" "github.com/cli/go-gh/v2/pkg/prompter" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" @@ -1164,3 +1165,373 @@ func confirmSaveRemote(cfg *config.Config, remote string) (bool, error) { } return ok, nil } + +// ensureLocalBranchFromRemote creates a local branch tracking remote/ +// if it does not already exist. Merged PRs whose remote ref has been deleted +// are skipped (returns skipped=true, err=nil). A non-merged branch that cannot +// be created is a hard failure (returns ErrSilent). Shared by importRemoteStack +// and the sync remote-ahead pull. +func ensureLocalBranchFromRemote(cfg *config.Config, remote string, pr *github.PullRequest) (skipped bool, err error) { + branch := pr.HeadRefName + if git.BranchExists(branch) { + return false, nil + } + remoteRef := remote + "/" + branch + if createErr := git.CreateBranch(branch, remoteRef); createErr != nil { + if pr.Merged { + cfg.Infof("Skipping merged branch %s", branch) + return true, nil + } + cfg.Errorf("failed to pull branch %s from %s: %v", branch, remoteRef, createErr) + return false, ErrSilent + } + _ = git.SetUpstreamTracking(branch, remote) + cfg.Successf("Pulled branch %s", branch) + return false, nil +} + +// remoteReconcileResult reports how reconcileRemoteStack resolved the +// relationship between the local stack and its tracked remote stack. +type remoteReconcileResult struct { + // stack, when non-nil, is the stack the caller should continue with. It + // differs from the input stack only when "use remote as source of truth" + // rebuilt the stack (which reslices StackFile.Stacks and invalidates the + // original pointer). + stack *stack.Stack + + // skipStackObjectSync tells runSync to skip the later syncStack step so we + // never push the local stack over a divergent remote. Set for cancel, + // disassociate, and unresolved (non-interactive) divergences. + skipStackObjectSync bool +} + +// remoteStackClass classifies the relationship between the local stack's active +// (non-merged) branches and its tracked remote stack's active branches. +type remoteStackClass int + +const ( + remoteStackInSync remoteStackClass = iota // sequences are identical + remoteStackCleanAhead // local is a strict prefix of remote (remote appended on top) + remoteStackLocalAhead // remote is a strict prefix of local (local appended on top) + remoteStackDivergent // neither is a prefix of the other +) + +// reconcileRemoteStack brings remote-ahead stack changes into the local stack +// and resolves divergences. It runs early in `sync` (after fetch, before +// rebase/push) so any pulled branches participate in the normal flow. +// +// It only acts on stacks tracked on the remote (s.ID != ""). It is best-effort: +// a missing client, stacked PRs being unavailable, or any API error causes it +// to skip so the rest of sync still runs. Untracked stacks (s.ID == "") are +// left to the syncStack/reconcileUntrackedStack path. +func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir, remote string) (remoteReconcileResult, error) { + var res remoteReconcileResult + + if s.ID == "" { + return res, nil + } + + client, err := cfg.GitHubClient() + if err != nil { + return res, nil + } + + stacks, err := client.ListStacks() + if err != nil { + // Covers 404 (stacked PRs unavailable) and transient API errors. + return res, nil + } + + var remotePRNumbers []int + found := false + for _, rs := range stacks { + if strconv.Itoa(rs.ID) == s.ID { + remotePRNumbers = rs.PullRequests + found = true + break + } + } + if !found { + // The remote stack was deleted; the existing updateStack 404 → recreate + // path in syncStack handles this. + return res, nil + } + + prs, err := fetchStackPRDetails(client, remotePRNumbers) + if err != nil { + return res, nil + } + + localActive, remoteActive := activeStackSequences(s, prs) + + switch classifyRemoteStack(localActive, remoteActive) { + case remoteStackInSync, remoteStackLocalAhead: + // Nothing to pull; the existing flow pushes/updates the remote. + return res, nil + case remoteStackCleanAhead: + return pullRemoteAdditions(cfg, sf, s, gitDir, remote, prs) + default: + return resolveStackDivergence(cfg, client, sf, s, gitDir, remote, prs, remoteActive) + } +} + +// activeStackSequences returns the ordered active (non-merged) branch-name +// sequences for the local stack and the fetched remote PRs. Merged state is +// taken from the freshly fetched remote PRs (by branch name) when available so +// that a locally pruned merged branch does not look like a divergence. +func activeStackSequences(s *stack.Stack, prs []*github.PullRequest) (localActive, remoteActive []string) { + remoteMerged := make(map[string]bool, len(prs)) + for _, pr := range prs { + remoteMerged[pr.HeadRefName] = pr.Merged + if !pr.Merged { + remoteActive = append(remoteActive, pr.HeadRefName) + } + } + for _, b := range s.Branches { + merged := b.IsMerged() + if m, ok := remoteMerged[b.Branch]; ok { + merged = m + } + if !merged { + localActive = append(localActive, b.Branch) + } + } + return localActive, remoteActive +} + +// classifyRemoteStack compares the active local and remote branch sequences. +func classifyRemoteStack(localActive, remoteActive []string) remoteStackClass { + if slicesEqualStr(localActive, remoteActive) { + return remoteStackInSync + } + if isStrictPrefix(localActive, remoteActive) { + return remoteStackCleanAhead + } + if isStrictPrefix(remoteActive, localActive) { + return remoteStackLocalAhead + } + return remoteStackDivergent +} + +// isStrictPrefix reports whether a is a strict prefix of b: a is shorter than b +// and every element of a equals the element at the same position in b. +func isStrictPrefix(a, b []string) bool { + if len(a) >= len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// slicesEqualStr reports whether two string slices are element-wise equal. +func slicesEqualStr(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// pullRemoteAdditions handles the clean append-on-top case: it fetches and +// creates local branches for the remote PRs not yet tracked locally, appends +// them (in remote order) to the stack, and persists. Merged remote PRs with no +// local branch are ignored (they are not part of ongoing local work). +func pullRemoteAdditions(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest) (remoteReconcileResult, error) { + var res remoteReconcileResult + + existing := make(map[string]bool, len(s.Branches)) + for _, b := range s.Branches { + existing[b.Branch] = true + } + + var newPRs []*github.PullRequest + for _, pr := range prs { + if pr.Merged || existing[pr.HeadRefName] { + continue + } + newPRs = append(newPRs, pr) + } + if len(newPRs) == 0 { + return res, nil + } + + newBranchNames := make([]string, len(newPRs)) + for i, pr := range newPRs { + newBranchNames[i] = pr.HeadRefName + } + _ = git.FetchBranches(remote, newBranchNames) + + cfg.Printf("") + cfg.Printf("Pulling %d new %s from the remote stack ...", + len(newPRs), plural(len(newPRs), "branch", "branches")) + + added := 0 + for _, pr := range newPRs { + skipped, err := ensureLocalBranchFromRemote(cfg, remote, pr) + if err != nil { + return res, err + } + if skipped { + continue + } + s.Branches = append(s.Branches, stack.BranchRef{ + Branch: pr.HeadRefName, + PullRequest: &stack.PullRequestRef{ + Number: pr.Number, + ID: pr.ID, + URL: pr.URL, + Merged: pr.Merged, + }, + }) + added++ + } + + if added > 0 { + updateBaseSHAs(s) + if err := stack.Save(gitDir, sf); err != nil { + return res, handleSaveError(cfg, err) + } + cfg.Successf("Pulled %d new %s into the stack from the remote", + added, plural(added, "branch", "branches")) + } + return res, nil +} + +// resolveStackDivergence handles a stack whose local composition has diverged +// from the remote (neither is a prefix of the other). In an interactive +// terminal it prompts the user; otherwise it is a safe no-op that reports the +// divergence and leaves everything untouched. +func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest, remoteActive []string) (remoteReconcileResult, error) { + res := remoteReconcileResult{skipStackObjectSync: true} + + cfg.Printf("") + cfg.Warningf("Your local stack has diverged from the stack on GitHub") + cfg.Printf(" Local: %s", s.DisplayChain()) + cfg.Printf(" Remote: (%s) <- %s", s.Trunk.Branch, strings.Join(remoteActive, " <- ")) + + if !cfg.IsInteractive() { + cfg.Printf(" Re-run in an interactive terminal to resolve, or import the remote stack with `%s`.", + cfg.ColorCyan("gh stack checkout ")) + return res, nil + } + + options := []string{ + "Use the remote stack as the source of truth (update local to match)", + "Use your local stack as the source of truth (update remote to match)", + "Disassociate — stop tracking this stack on GitHub", + "Cancel — make no changes for now", + } + p := prompter.New(cfg.In, cfg.Out, cfg.Err) + selectFn := func(prompt, def string, opts []string) (int, error) { + if cfg.SelectFn != nil { + return cfg.SelectFn(prompt, def, opts) + } + return p.Select(prompt, def, opts) + } + selected, err := selectFn("How would you like to resolve this?", "", options) + if err != nil { + if isInterruptError(err) { + if cfg.SelectFn == nil { + clearSelectPrompt(cfg, len(options)) + } + printInterrupt(cfg) + return res, errInterrupt + } + cfg.Errorf("selection failed: %v", err) + return res, ErrSilent + } + + switch selected { + case 0: + return resolveDivergenceUseRemote(cfg, sf, s, gitDir, remote, prs) + case 1: + return resolveDivergenceUseLocal(cfg, client, sf, s, gitDir) + case 2: + return resolveDivergenceDisassociate(cfg, sf, s, gitDir) + default: + cfg.Infof("No changes made — your local and remote stacks still differ") + return res, nil + } +} + +// resolveDivergenceUseRemote replaces the local stack composition with the +// remote's. It refuses when the working tree is dirty (the rebuild is +// destructive to stack tracking). Local-only branches remain as git refs but +// are no longer part of the stack. Returns the rebuilt stack pointer so the +// caller can continue with it (importRemoteStack reslices StackFile.Stacks). +func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest) (remoteReconcileResult, error) { + var res remoteReconcileResult + + if dirty, err := git.HasUncommittedChanges(); err == nil && dirty { + cfg.Errorf("You have uncommitted changes — commit or stash them before replacing your local stack with the remote") + res.skipStackObjectSync = true + return res, ErrSilent + } + + trunk := s.Trunk.Branch + remoteStackID := s.ID + + removeLocalStack(sf, s) + newStack, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID) + if err != nil { + res.skipStackObjectSync = true + return res, err + } + if err := stack.Save(gitDir, sf); err != nil { + res.skipStackObjectSync = true + return res, handleSaveError(cfg, err) + } + + res.stack = newStack + cfg.Successf("Local stack replaced with the remote version") + return res, nil +} + +// resolveDivergenceUseLocal makes the remote match the local stack by deleting +// the diverged remote stack object and clearing the local ID. The normal +// syncStack step then recreates the stack from the local PR list (when two or +// more PRs exist). Remote PRs dropped from the stack remain open but unstacked. +func resolveDivergenceUseLocal(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { + var res remoteReconcileResult + + if err := client.DeleteStack(s.ID); err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { + cfg.Warningf("Remote stack already deleted") + } else { + cfg.Errorf("failed to delete remote stack: %v", err) + res.skipStackObjectSync = true + return res, ErrAPIFailure + } + } else { + cfg.Successf("Deleted the diverged stack on GitHub — it will be recreated from your local stack") + } + + s.ID = "" + if err := stack.Save(gitDir, sf); err != nil { + res.skipStackObjectSync = true + return res, handleSaveError(cfg, err) + } + return res, nil +} + +// resolveDivergenceDisassociate removes the local stack's remote stack ID so +// the local and remote stacks become independent. The remote stack object is +// left untouched; syncStack is skipped so nothing is pushed over it. +func resolveDivergenceDisassociate(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { + res := remoteReconcileResult{skipStackObjectSync: true} + s.ID = "" + if err := stack.Save(gitDir, sf); err != nil { + return res, handleSaveError(cfg, err) + } + cfg.Successf("Stopped tracking this stack on GitHub — your local and remote stacks are now independent") + return res, nil +} From efa0a9e6571515c627b0f45cdf5d6031c97dbf12 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sat, 11 Jul 2026 14:01:12 -0400 Subject: [PATCH 2/7] cancel aborts operation --- cmd/sync.go | 14 +++++++++++--- cmd/sync_test.go | 23 ++++++++++++++++------- cmd/utils.go | 31 +++++++++++++++++-------------- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 18931a7..b8c7427 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -44,8 +44,9 @@ down and appended to your local stack so it mirrors the remote. A clean "remote is ahead" update happens automatically without prompting. If the local and remote stacks have diverged, sync prompts (in an interactive terminal) to use the remote as the source of truth, use your local stack -as the source of truth, disassociate them, or cancel. In a non-interactive -terminal a divergence is reported and left untouched. +as the source of truth, disassociate them, or cancel. Cancelling — or a +divergence in a non-interactive terminal — aborts the sync without pushing +branches or updating PRs. If a rebase conflict is detected, all branches are restored to their original state and you are advised to run "gh stack rebase" to resolve @@ -56,7 +57,7 @@ links PRs that already exist. The final message reflects what happened: "Stack synced" means the stack object on GitHub now matches your local stack, while "Branches synced" means the branches were rebased and pushed but no remote stack object was created or updated (for example, when fewer -than two PRs exist yet, or a divergence was left unresolved). +than two PRs exist yet). Use --prune to delete local branches for merged PRs. Stack metadata is preserved so that rebase and display logic continue to work correctly. @@ -125,6 +126,13 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if reconcileRes.stack != nil { s = reconcileRes.stack } + if reconcileRes.abort { + // The user cancelled a divergence (or it can't be resolved + // non-interactively). Stop before touching any branches or PRs. + cfg.Printf("") + cfg.Infof("Sync aborted — no changes were made") + return nil + } // --- Step 2: Fast-forward trunk --- trunk := s.Trunk.Branch diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 5630628..58a0a27 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2119,17 +2119,20 @@ func divergentRemoteMock() *github.MockClient { } } -// TestSync_Divergent_NonInteractive_SafeNoOp verifies that a divergence in a -// non-interactive terminal is a safe no-op: no stack API mutations, guidance is -// printed, the association is preserved, and it exits success. -func TestSync_Divergent_NonInteractive_SafeNoOp(t *testing.T) { +// TestSync_Divergent_NonInteractive_Aborts verifies that a divergence in a +// non-interactive terminal aborts the sync: no branches are pushed, no stack API +// mutations occur, guidance is printed, the association is preserved, and it +// exits successfully. +func TestSync_Divergent_NonInteractive_Aborts(t *testing.T) { tmpDir := t.TempDir() divergentStack(t, tmpDir) ghMock := divergentRemoteMock() var created []string + var pushed bool mock := newSyncMockNoRebase(tmpDir, "b1") mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.PushFn = func(string, []string, bool, bool) error { pushed = true; return nil } ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } @@ -2138,8 +2141,10 @@ func TestSync_Divergent_NonInteractive_SafeNoOp(t *testing.T) { require.NoError(t, err) assert.Empty(t, created) + assert.False(t, pushed, "branches must not be pushed when sync aborts") assert.Contains(t, output, "diverged") - assert.Contains(t, output, "Branches synced") + assert.Contains(t, output, "Sync aborted") + assert.NotContains(t, output, "Branches synced") assert.NotContains(t, output, "Stack synced") sf, err := stack.Load(tmpDir) @@ -2283,7 +2288,9 @@ func TestSync_Divergent_Cancel(t *testing.T) { ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } + var pushed bool mock := newSyncMockNoRebase(tmpDir, "b1") + mock.PushFn = func(string, []string, bool, bool) error { pushed = true; return nil } output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock @@ -2292,8 +2299,10 @@ func TestSync_Divergent_Cancel(t *testing.T) { }) require.NoError(t, err) - assert.Contains(t, output, "still differ") - assert.Contains(t, output, "Branches synced") + assert.False(t, pushed, "branches must not be pushed when the user cancels") + assert.Contains(t, output, "Sync aborted") + assert.NotContains(t, output, "Branches synced") + assert.NotContains(t, output, "Stack synced") sf, err := stack.Load(tmpDir) require.NoError(t, err) diff --git a/cmd/utils.go b/cmd/utils.go index c1068c3..96d82b9 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1200,9 +1200,14 @@ type remoteReconcileResult struct { stack *stack.Stack // skipStackObjectSync tells runSync to skip the later syncStack step so we - // never push the local stack over a divergent remote. Set for cancel, - // disassociate, and unresolved (non-interactive) divergences. + // never push the local stack over a divergent remote. Set when disassociating. skipStackObjectSync bool + + // abort tells runSync to stop the whole sync immediately (before any + // fast-forward, rebase, or push) and exit successfully without changes. Set + // when the user cancels an interactive divergence prompt, or when a + // divergence is detected in a non-interactive terminal. + abort bool } // remoteStackClass classifies the relationship between the local stack's active @@ -1407,11 +1412,9 @@ func pullRemoteAdditions(cfg *config.Config, sf *stack.StackFile, s *stack.Stack // resolveStackDivergence handles a stack whose local composition has diverged // from the remote (neither is a prefix of the other). In an interactive -// terminal it prompts the user; otherwise it is a safe no-op that reports the -// divergence and leaves everything untouched. +// terminal it prompts the user to resolve it; otherwise (or when the user +// cancels) it aborts the sync so nothing is pushed or updated. func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest, remoteActive []string) (remoteReconcileResult, error) { - res := remoteReconcileResult{skipStackObjectSync: true} - cfg.Printf("") cfg.Warningf("Your local stack has diverged from the stack on GitHub") cfg.Printf(" Local: %s", s.DisplayChain()) @@ -1420,14 +1423,14 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta if !cfg.IsInteractive() { cfg.Printf(" Re-run in an interactive terminal to resolve, or import the remote stack with `%s`.", cfg.ColorCyan("gh stack checkout ")) - return res, nil + return remoteReconcileResult{abort: true}, nil } options := []string{ "Use the remote stack as the source of truth (update local to match)", "Use your local stack as the source of truth (update remote to match)", - "Disassociate — stop tracking this stack on GitHub", - "Cancel — make no changes for now", + "Disassociate — unlink local stack with remote on GitHub", + "Cancel — abort the sync and do not make any changes", } p := prompter.New(cfg.In, cfg.Out, cfg.Err) selectFn := func(prompt, def string, opts []string) (int, error) { @@ -1436,17 +1439,17 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta } return p.Select(prompt, def, opts) } - selected, err := selectFn("How would you like to resolve this?", "", options) + selected, err := selectFn("How would you like to resolve?", "", options) if err != nil { if isInterruptError(err) { if cfg.SelectFn == nil { clearSelectPrompt(cfg, len(options)) } printInterrupt(cfg) - return res, errInterrupt + return remoteReconcileResult{abort: true}, errInterrupt } cfg.Errorf("selection failed: %v", err) - return res, ErrSilent + return remoteReconcileResult{}, ErrSilent } switch selected { @@ -1457,8 +1460,8 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta case 2: return resolveDivergenceDisassociate(cfg, sf, s, gitDir) default: - cfg.Infof("No changes made — your local and remote stacks still differ") - return res, nil + // Cancel: abort the whole sync without touching branches or PRs. + return remoteReconcileResult{abort: true}, nil } } From 4cb3f22a5ba0e1e8bc2949e4840f0c15a3976bf3 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sat, 11 Jul 2026 14:35:37 -0400 Subject: [PATCH 3/7] switch to nearest surviving branch --- cmd/sync.go | 7 +++- cmd/sync_test.go | 66 ++++++++++++++++++++++++++++++++++-- cmd/utils.go | 51 +++++++++++++++++++++++++--- internal/modify/apply.go | 29 ++++------------ internal/stack/stack.go | 33 ++++++++++++++++++ internal/stack/stack_test.go | 31 +++++++++++++++++ 6 files changed, 187 insertions(+), 30 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index b8c7427..78b5eb9 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -116,7 +116,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // resolve a divergence, before rebasing and pushing so pulled branches // participate in the normal flow. Best-effort for stacks tracked on the // remote; a no-op otherwise. - reconcileRes, err := reconcileRemoteStack(cfg, sf, s, gitDir, remote) + reconcileRes, err := reconcileRemoteStack(cfg, sf, s, currentBranch, gitDir, remote) if err != nil { if errors.Is(err, errInterrupt) { return ErrSilent @@ -133,6 +133,11 @@ func runSync(cfg *config.Config, opts *syncOptions) error { cfg.Infof("Sync aborted — no changes were made") return nil } + // Reconciling "use remote as source of truth" may have moved us off a + // branch that is no longer in the stack, so re-read the current branch. + if cb, cbErr := git.CurrentBranch(); cbErr == nil { + currentBranch = cb + } // --- Step 2: Fast-forward trunk --- trunk := s.Trunk.Branch diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 58a0a27..456ac13 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2182,8 +2182,70 @@ func TestSync_Divergent_UseRemote(t *testing.T) { assert.Equal(t, "9", sf.Stacks[0].ID) } -// TestSync_Divergent_UseRemote_DirtyBlocked refuses to replace the local stack -// when the working tree has uncommitted changes. +func TestNearestBranchAfterReplace(t *testing.T) { + newStack := func(branches ...string) *stack.Stack { + s := &stack.Stack{Trunk: stack.BranchRef{Branch: "main"}} + for _, b := range branches { + s.Branches = append(s.Branches, stack.BranchRef{Branch: b}) + } + return s + } + tests := []struct { + name string + old []string + current string + newBranches []string + want string + }{ + {"still in stack", []string{"b1", "b2", "b3"}, "b2", []string{"b1", "b2", "b4"}, "b2"}, + {"dropped top prefers below", []string{"b1", "b2", "b3"}, "b3", []string{"b1", "b2", "b4"}, "b2"}, + {"dropped middle prefers above", []string{"b1", "b2", "b3"}, "b2", []string{"b1", "b3"}, "b3"}, + {"on trunk stays put", []string{"b1", "b2"}, "main", []string{"b1", "b2", "b4"}, "main"}, + {"none survive falls back to top", []string{"x", "y", "z"}, "y", []string{"a", "b", "c"}, "c"}, + {"empty new stack falls back to trunk", []string{"b1"}, "b1", nil, "main"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, nearestBranchAfterReplace(tt.old, tt.current, newStack(tt.newBranches...))) + }) + } +} + +// TestSync_Divergent_UseRemote_SwitchesOffDroppedBranch verifies that when the +// user is on a branch that the remote stack no longer contains, replacing the +// local stack with the remote moves them to the nearest surviving branch. +func TestSync_Divergent_UseRemote_SwitchesOffDroppedBranch(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) // local [b1,b2,b3], remote [b1,b2,b4]; user on b3 (dropped) + + ghMock := divergentRemoteMock() + current := "b3" + var checkouts []string + mock := newSyncMockNoRebase(tmpDir, "b3") + mock.CurrentBranchFn = func() (string, error) { return current, nil } + mock.CheckoutBranchFn = func(name string) error { current = name; checkouts = append(checkouts, name); return nil } + mock.BranchExistsFn = func(name string) bool { return name != "b4" } + mock.CreateBranchFn = func(string, string) error { return nil } + mock.SetUpstreamTrackingFn = func(string, string) error { return nil } + mock.HasUncommittedChangesFn = func() (bool, error) { return false, nil } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + }) + require.NoError(t, err) + + assert.Contains(t, checkouts, "b2", "should switch off dropped branch b3 to nearest surviving branch b2") + assert.NotContains(t, checkouts, "b3", "should never check the dropped branch back out") + assert.Contains(t, output, "Switched to b2") + assert.Contains(t, output, "no longer in the stack") + assert.Equal(t, "b2", current, "should end on b2, not the dropped b3") + + sf, err := stack.Load(tmpDir) + require.NoError(t, err) + assert.Equal(t, []string{"b1", "b2", "b4"}, sf.Stacks[0].BranchNames()) +} func TestSync_Divergent_UseRemote_DirtyBlocked(t *testing.T) { tmpDir := t.TempDir() divergentStack(t, tmpDir) diff --git a/cmd/utils.go b/cmd/utils.go index 96d82b9..928ccc0 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net/url" + "slices" "strconv" "strings" "sync" @@ -1229,7 +1230,7 @@ const ( // a missing client, stacked PRs being unavailable, or any API error causes it // to skip so the rest of sync still runs. Untracked stacks (s.ID == "") are // left to the syncStack/reconcileUntrackedStack path. -func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir, remote string) (remoteReconcileResult, error) { +func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, currentBranch, gitDir, remote string) (remoteReconcileResult, error) { var res remoteReconcileResult if s.ID == "" { @@ -1276,7 +1277,7 @@ func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stac case remoteStackCleanAhead: return pullRemoteAdditions(cfg, sf, s, gitDir, remote, prs) default: - return resolveStackDivergence(cfg, client, sf, s, gitDir, remote, prs, remoteActive) + return resolveStackDivergence(cfg, client, sf, s, currentBranch, gitDir, remote, prs, remoteActive) } } @@ -1414,7 +1415,7 @@ func pullRemoteAdditions(cfg *config.Config, sf *stack.StackFile, s *stack.Stack // from the remote (neither is a prefix of the other). In an interactive // terminal it prompts the user to resolve it; otherwise (or when the user // cancels) it aborts the sync so nothing is pushed or updated. -func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest, remoteActive []string) (remoteReconcileResult, error) { +func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, currentBranch, gitDir, remote string, prs []*github.PullRequest, remoteActive []string) (remoteReconcileResult, error) { cfg.Printf("") cfg.Warningf("Your local stack has diverged from the stack on GitHub") cfg.Printf(" Local: %s", s.DisplayChain()) @@ -1454,7 +1455,7 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta switch selected { case 0: - return resolveDivergenceUseRemote(cfg, sf, s, gitDir, remote, prs) + return resolveDivergenceUseRemote(cfg, sf, s, currentBranch, gitDir, remote, prs) case 1: return resolveDivergenceUseLocal(cfg, client, sf, s, gitDir) case 2: @@ -1470,7 +1471,7 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta // destructive to stack tracking). Local-only branches remain as git refs but // are no longer part of the stack. Returns the rebuilt stack pointer so the // caller can continue with it (importRemoteStack reslices StackFile.Stacks). -func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir, remote string, prs []*github.PullRequest) (remoteReconcileResult, error) { +func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, currentBranch, gitDir, remote string, prs []*github.PullRequest) (remoteReconcileResult, error) { var res remoteReconcileResult if dirty, err := git.HasUncommittedChanges(); err == nil && dirty { @@ -1481,6 +1482,7 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac trunk := s.Trunk.Branch remoteStackID := s.ID + oldBranches := s.BranchNames() removeLocalStack(sf, s) newStack, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID) @@ -1488,6 +1490,18 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac res.skipStackObjectSync = true return res, err } + + // If the user was on a branch that the remote stack no longer contains, + // move them to the nearest surviving branch so they don't end up detached + // from the stack. + if target := nearestBranchAfterReplace(oldBranches, currentBranch, newStack); target != currentBranch { + if err := git.CheckoutBranch(target); err != nil { + cfg.Warningf("Failed to switch from %s to %s: %v", currentBranch, target, err) + } else { + cfg.Printf("Switched to %s (original branch %s is no longer in the stack)", target, currentBranch) + } + } + if err := stack.Save(gitDir, sf); err != nil { res.skipStackObjectSync = true return res, handleSaveError(cfg, err) @@ -1498,6 +1512,33 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac return res, nil } +// nearestBranchAfterReplace decides which branch to check out after the local +// stack has been replaced with the remote version. If currentBranch still +// exists in newStack (or the user was on the trunk / a non-stack branch), it is +// returned unchanged. Otherwise it delegates to stack.NearestSurvivingBranch to +// pick the nearest branch from the pre-replacement ordering (neighbor above, +// then below), falling back to the top of the new stack, or the trunk if the new +// stack has no branches. Mirrors the checkout behavior of `gh stack modify`. +func nearestBranchAfterReplace(oldBranches []string, currentBranch string, newStack *stack.Stack) string { + // Still in the new stack, or never a stack branch (e.g. the trunk): stay put. + if newStack.IndexOf(currentBranch) >= 0 || slices.Index(oldBranches, currentBranch) < 0 { + return currentBranch + } + + if nearest := stack.NearestSurvivingBranch(oldBranches, currentBranch, func(name string) bool { + return newStack.IndexOf(name) >= 0 + }); nearest != "" { + return nearest + } + + // A dropped stack branch with no surviving neighbor — fall back to the top + // of the new stack, then the trunk. + if len(newStack.Branches) > 0 { + return newStack.Branches[len(newStack.Branches)-1].Branch + } + return newStack.Trunk.Branch +} + // resolveDivergenceUseLocal makes the remote match the local stack by deleting // the diverged remote stack object and clearing the local ID. The normal // syncStack step then recreates the stack from the local PR list (when two or diff --git a/internal/modify/apply.go b/internal/modify/apply.go index 9c70657..6aafc31 100644 --- a/internal/modify/apply.go +++ b/internal/modify/apply.go @@ -754,32 +754,17 @@ func adjacentSnapshotBranch(snapshot Snapshot, target string, direction int) str // still exists in the stack. Prefers the branch above (higher index), then below. // resolvedName translates snapshot names through any renames from the same operation. func nearestSurvivingBranch(snapshot Snapshot, dropped string, s *stack.Stack, resolvedName func(string) string) string { - pos := -1 + order := make([]string, len(snapshot.Branches)) for i, bs := range snapshot.Branches { - if bs.Name == dropped { - pos = i - break - } + order[i] = bs.Name } - if pos < 0 { + raw := stack.NearestSurvivingBranch(order, dropped, func(name string) bool { + return s.IndexOf(resolvedName(name)) >= 0 + }) + if raw == "" { return "" } - - // Search above first (higher indices = away from trunk) - for i := pos + 1; i < len(snapshot.Branches); i++ { - name := resolvedName(snapshot.Branches[i].Name) - if s.IndexOf(name) >= 0 { - return name - } - } - // Then below (lower indices = toward trunk) - for i := pos - 1; i >= 0; i-- { - name := resolvedName(snapshot.Branches[i].Name) - if s.IndexOf(name) >= 0 { - return name - } - } - return "" + return resolvedName(raw) } // ContinueApply resumes a modify operation after the user resolves a rebase conflict. diff --git a/internal/stack/stack.go b/internal/stack/stack.go index 7e4043a..a363820 100644 --- a/internal/stack/stack.go +++ b/internal/stack/stack.go @@ -193,6 +193,39 @@ func (s *Stack) IsFullyMerged() bool { return len(s.Branches) > 0 } +// NearestSurvivingBranch returns the branch nearest to target within the ordered +// branch-name list `order` for which `survives` reports true, preferring the +// neighbor above (later in the slice, away from the trunk) and then the neighbor +// below (earlier in the slice, toward the trunk). +// +// It returns "" when target is not present in order, or when no other branch in +// order survives. Callers layer their own fallback and any name translation +// around this search. It is shared by the checkout-branch selection in +// `gh stack modify` and `gh stack sync`. +func NearestSurvivingBranch(order []string, target string, survives func(string) bool) string { + pos := -1 + for i, name := range order { + if name == target { + pos = i + break + } + } + if pos < 0 { + return "" + } + for i := pos + 1; i < len(order); i++ { + if survives(order[i]) { + return order[i] + } + } + for i := pos - 1; i >= 0; i-- { + if survives(order[i]) { + return order[i] + } + } + return "" +} + // StackFile represents the JSON file stored in .git/gh-stack. type StackFile struct { SchemaVersion int `json:"schemaVersion"` diff --git a/internal/stack/stack_test.go b/internal/stack/stack_test.go index 987f25c..5bdfbb2 100644 --- a/internal/stack/stack_test.go +++ b/internal/stack/stack_test.go @@ -625,3 +625,34 @@ func TestIsFullyMerged_NotAffectedByQueued(t *testing.T) { assert.False(t, s.IsFullyMerged()) }) } + +func TestNearestSurvivingBranch(t *testing.T) { + // survivesIn returns a predicate reporting membership in the given set. + survivesIn := func(names ...string) func(string) bool { + set := make(map[string]bool, len(names)) + for _, n := range names { + set[n] = true + } + return func(name string) bool { return set[name] } + } + + tests := []struct { + name string + order []string + target string + survives func(string) bool + want string + }{ + {"prefers neighbor above", []string{"a", "b", "c"}, "b", survivesIn("a", "c"), "c"}, + {"falls to neighbor below", []string{"a", "b", "c"}, "c", survivesIn("a", "b"), "b"}, + {"skips dead neighbors above", []string{"a", "b", "c", "d"}, "b", survivesIn("a", "d"), "d"}, + {"target not in order", []string{"a", "b"}, "z", survivesIn("a", "b"), ""}, + {"no other survives", []string{"a", "b", "c"}, "b", survivesIn("b"), ""}, + {"empty order", nil, "a", survivesIn("a"), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, NearestSurvivingBranch(tt.order, tt.target, tt.survives)) + }) + } +} From 779793166bbd1ef600fe2b0a99e7c5c03c01c905 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 12 Jul 2026 12:04:10 -0400 Subject: [PATCH 4/7] simplified options to remote delete --- cmd/sync.go | 20 ++++++-------- cmd/sync_test.go | 65 ++++++++++++-------------------------------- cmd/utils.go | 71 ++++++++++++++++++------------------------------ 3 files changed, 53 insertions(+), 103 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 78b5eb9..34bbbc0 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -43,8 +43,8 @@ If PRs have been added to the stack on GitHub, their branches are pulled down and appended to your local stack so it mirrors the remote. A clean "remote is ahead" update happens automatically without prompting. If the local and remote stacks have diverged, sync prompts (in an interactive -terminal) to use the remote as the source of truth, use your local stack -as the source of truth, disassociate them, or cancel. Cancelling — or a +terminal) to use the remote as the source of truth, delete the stack on +GitHub and recreate it later with sync/submit, or cancel. Cancelling — or a divergence in a non-interactive terminal — aborts the sync without pushing branches or updating PRs. @@ -126,11 +126,11 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if reconcileRes.stack != nil { s = reconcileRes.stack } - if reconcileRes.abort { - // The user cancelled a divergence (or it can't be resolved - // non-interactively). Stop before touching any branches or PRs. - cfg.Printf("") - cfg.Infof("Sync aborted — no changes were made") + if reconcileRes.stop { + // The reconcile step resolved the situation and there is nothing more to + // do (the user cancelled or deleted the remote stack, or a divergence was + // detected non-interactively). The resolving path already reported the + // outcome, so just exit successfully. return nil } // Reconciling "use remote as source of truth" may have moved us off a @@ -277,10 +277,8 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // stack object actually reflects the local stack, which determines the final // summary message below. stackSynced := false - if !reconcileRes.skipStackObjectSync { - if client, err := cfg.GitHubClient(); err == nil { - stackSynced = syncStack(cfg, client, s) - } + if client, err := cfg.GitHubClient(); err == nil { + stackSynced = syncStack(cfg, client, s) } // --- Step 6: Prune merged branches (optional) --- diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 456ac13..d93aed1 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2272,73 +2272,42 @@ func TestSync_Divergent_UseRemote_DirtyBlocked(t *testing.T) { assert.Equal(t, "9", sf.Stacks[0].ID) } -// TestSync_Divergent_UseLocal deletes the diverged remote stack and recreates it -// from the local stack. -func TestSync_Divergent_UseLocal(t *testing.T) { +// TestSync_Divergent_DeleteRemote deletes the diverged remote stack, clears the +// local association, and stops the sync (pointing the user at submit) without +// recreating the stack or pushing. +func TestSync_Divergent_DeleteRemote(t *testing.T) { tmpDir := t.TempDir() divergentStack(t, tmpDir) deleted := false var deletedID string - var createdWith []int - ghMock := &github.MockClient{ - ListStacksFn: func() ([]github.RemoteStack, error) { - if deleted { - return nil, nil - } - return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 104}}}, nil - }, - FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3", 104: "b4"}), - FindPRForBranchFn: openPRFinder(map[string]int{"b1": 101, "b2": 102, "b3": 103}), - DeleteStackFn: func(id string) error { deleted = true; deletedID = id; return nil }, - CreateStackFn: func(prs []int) (int, error) { createdWith = prs; return 12, nil }, - } - mock := newSyncMockNoRebase(tmpDir, "b1") - - output, err := runSyncCfg(t, mock, func(cfg *config.Config) { - cfg.GitHubClientOverride = ghMock - cfg.ForceInteractive = true - cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 1, nil } - }) - require.NoError(t, err) - - assert.True(t, deleted, "remote stack should be deleted") - assert.Equal(t, "9", deletedID) - assert.Equal(t, []int{101, 102, 103}, createdWith, "remote recreated from local PRs") - assert.Contains(t, output, "Stack synced") - - sf, err := stack.Load(tmpDir) - require.NoError(t, err) - assert.Equal(t, "12", sf.Stacks[0].ID, "adopts the recreated stack ID") -} - -// TestSync_Divergent_Disassociate clears the remote stack ID without touching -// either the remote stack object or the local branches. -func TestSync_Divergent_Disassociate(t *testing.T) { - tmpDir := t.TempDir() - divergentStack(t, tmpDir) - + var pushed bool ghMock := divergentRemoteMock() - ghMock.DeleteStackFn = func(string) error { t.Fatal("DeleteStack must not be called"); return nil } + ghMock.DeleteStackFn = func(id string) error { deleted = true; deletedID = id; return nil } ghMock.CreateStackFn = func([]int) (int, error) { t.Fatal("CreateStack must not be called"); return 0, nil } ghMock.UpdateStackFn = func(string, []int) error { t.Fatal("UpdateStack must not be called"); return nil } mock := newSyncMockNoRebase(tmpDir, "b1") + mock.PushFn = func(string, []string, bool, bool) error { pushed = true; return nil } output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock cfg.ForceInteractive = true - cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 2, nil } + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 1, nil } }) require.NoError(t, err) - assert.Contains(t, output, "independent") - assert.Contains(t, output, "Branches synced") + assert.True(t, deleted, "remote stack should be deleted") + assert.Equal(t, "9", deletedID) + assert.False(t, pushed, "sync should stop after deleting the remote stack") + assert.Contains(t, output, "Deleted the stack on GitHub") + assert.Contains(t, output, "gh stack submit") assert.NotContains(t, output, "Stack synced") + assert.NotContains(t, output, "Branches synced") sf, err := stack.Load(tmpDir) require.NoError(t, err) - assert.Equal(t, "", sf.Stacks[0].ID, "remote stack ID is cleared") - assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames()) + assert.Equal(t, "", sf.Stacks[0].ID, "local association is cleared") + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames(), "local branches untouched") } // TestSync_Divergent_Cancel makes no changes and preserves the association. @@ -2357,7 +2326,7 @@ func TestSync_Divergent_Cancel(t *testing.T) { output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock cfg.ForceInteractive = true - cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 3, nil } + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 2, nil } }) require.NoError(t, err) diff --git a/cmd/utils.go b/cmd/utils.go index 928ccc0..7de02dd 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1200,15 +1200,11 @@ type remoteReconcileResult struct { // original pointer). stack *stack.Stack - // skipStackObjectSync tells runSync to skip the later syncStack step so we - // never push the local stack over a divergent remote. Set when disassociating. - skipStackObjectSync bool - - // abort tells runSync to stop the whole sync immediately (before any - // fast-forward, rebase, or push) and exit successfully without changes. Set - // when the user cancels an interactive divergence prompt, or when a - // divergence is detected in a non-interactive terminal. - abort bool + // stop tells runSync to end the sync immediately after reconcile (before any + // fast-forward, rebase, or push) and exit successfully. Set when the user + // cancels or deletes the remote stack, and when a divergence is detected in a + // non-interactive terminal. The resolving path prints its own outcome message. + stop bool } // remoteStackClass classifies the relationship between the local stack's active @@ -1424,14 +1420,14 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta if !cfg.IsInteractive() { cfg.Printf(" Re-run in an interactive terminal to resolve, or import the remote stack with `%s`.", cfg.ColorCyan("gh stack checkout ")) - return remoteReconcileResult{abort: true}, nil + cfg.Infof("Sync aborted — no changes were made") + return remoteReconcileResult{stop: true}, nil } options := []string{ - "Use the remote stack as the source of truth (update local to match)", - "Use your local stack as the source of truth (update remote to match)", - "Disassociate — unlink local stack with remote on GitHub", - "Cancel — abort the sync and do not make any changes", + "Update local to match remote — replace your local stack with the remote version", + "Delete the remote stack on GitHub — keep your local stack and recreate on remote later", + "Cancel — make no changes", } p := prompter.New(cfg.In, cfg.Out, cfg.Err) selectFn := func(prompt, def string, opts []string) (int, error) { @@ -1447,7 +1443,7 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta clearSelectPrompt(cfg, len(options)) } printInterrupt(cfg) - return remoteReconcileResult{abort: true}, errInterrupt + return remoteReconcileResult{stop: true}, errInterrupt } cfg.Errorf("selection failed: %v", err) return remoteReconcileResult{}, ErrSilent @@ -1457,12 +1453,11 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta case 0: return resolveDivergenceUseRemote(cfg, sf, s, currentBranch, gitDir, remote, prs) case 1: - return resolveDivergenceUseLocal(cfg, client, sf, s, gitDir) - case 2: - return resolveDivergenceDisassociate(cfg, sf, s, gitDir) + return resolveDivergenceDeleteRemote(cfg, client, sf, s, gitDir) default: - // Cancel: abort the whole sync without touching branches or PRs. - return remoteReconcileResult{abort: true}, nil + // Cancel: stop the sync without touching branches or PRs. + cfg.Infof("Sync aborted — no changes were made") + return remoteReconcileResult{stop: true}, nil } } @@ -1476,7 +1471,6 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac if dirty, err := git.HasUncommittedChanges(); err == nil && dirty { cfg.Errorf("You have uncommitted changes — commit or stash them before replacing your local stack with the remote") - res.skipStackObjectSync = true return res, ErrSilent } @@ -1487,7 +1481,6 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac removeLocalStack(sf, s) newStack, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID) if err != nil { - res.skipStackObjectSync = true return res, err } @@ -1503,7 +1496,6 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac } if err := stack.Save(gitDir, sf); err != nil { - res.skipStackObjectSync = true return res, handleSaveError(cfg, err) } @@ -1539,12 +1531,13 @@ func nearestBranchAfterReplace(oldBranches []string, currentBranch string, newSt return newStack.Trunk.Branch } -// resolveDivergenceUseLocal makes the remote match the local stack by deleting -// the diverged remote stack object and clearing the local ID. The normal -// syncStack step then recreates the stack from the local PR list (when two or -// more PRs exist). Remote PRs dropped from the stack remain open but unstacked. -func resolveDivergenceUseLocal(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { - var res remoteReconcileResult +// resolveDivergenceDeleteRemote deletes the diverged stack object on GitHub and +// removes the local association (clearing the stack ID). The PRs and local +// branches are left untouched — only the stack grouping on GitHub is removed. It +// stops the sync and points the user at `gh stack submit` to recreate the stack +// (which, unlike sync, also creates PRs for any un-submitted branches). +func resolveDivergenceDeleteRemote(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { + res := remoteReconcileResult{stop: true} if err := client.DeleteStack(s.ID); err != nil { var httpErr *api.HTTPError @@ -1552,30 +1545,20 @@ func resolveDivergenceUseLocal(cfg *config.Config, client github.ClientOps, sf * cfg.Warningf("Remote stack already deleted") } else { cfg.Errorf("failed to delete remote stack: %v", err) - res.skipStackObjectSync = true return res, ErrAPIFailure } } else { - cfg.Successf("Deleted the diverged stack on GitHub — it will be recreated from your local stack") + cfg.Successf("Deleted the stack on GitHub") } s.ID = "" if err := stack.Save(gitDir, sf); err != nil { - res.skipStackObjectSync = true return res, handleSaveError(cfg, err) } - return res, nil -} -// resolveDivergenceDisassociate removes the local stack's remote stack ID so -// the local and remote stacks become independent. The remote stack object is -// left untouched; syncStack is skipped so nothing is pushed over it. -func resolveDivergenceDisassociate(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { - res := remoteReconcileResult{skipStackObjectSync: true} - s.ID = "" - if err := stack.Save(gitDir, sf); err != nil { - return res, handleSaveError(cfg, err) - } - cfg.Successf("Stopped tracking this stack on GitHub — your local and remote stacks are now independent") + cfg.Printf("") + cfg.Printf("Your PRs and local branches are unchanged — only the stack on GitHub was removed.") + cfg.Printf(" Run `%s` to recreate the stack on GitHub.", cfg.ColorCyan("gh stack submit")) + cfg.Printf(" Run `%s` first if you want to change the stack's structure.", cfg.ColorCyan("gh stack modify")) return res, nil } From 1710098f5baf0d195a6cb38742d5d681fcf88011 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 12 Jul 2026 12:54:07 -0400 Subject: [PATCH 5/7] stack existing PRs from submit TUI --- cmd/submit.go | 18 ++-- cmd/submit_tui_test.go | 2 +- internal/tui/submitview/data.go | 14 +++ internal/tui/submitview/help.go | 1 + internal/tui/submitview/model.go | 20 ++++ internal/tui/submitview/mouse.go | 23 ++++ internal/tui/submitview/render.go | 10 +- internal/tui/submitview/screen.go | 49 +++++++-- internal/tui/submitview/stackbutton_test.go | 112 ++++++++++++++++++++ 9 files changed, 231 insertions(+), 18 deletions(-) create mode 100644 internal/tui/submitview/stackbutton_test.go diff --git a/cmd/submit.go b/cmd/submit.go index a1ba004..a8b5bf2 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -40,6 +40,10 @@ and draft each PR's title, description, and draft state, then submit them all at once with Ctrl+S. Pass --auto (or run in a non-interactive terminal) to skip the editor and use auto-generated titles. +If your branches already have open PRs but no stack on GitHub yet (for example, +after deleting the stack) and you deselect every new PR, press Ctrl+B (or click +the "STACK N PRs" button) to link the existing open PRs into a stack. + This command performs several steps: 1. Pushes all branches to the remote 2. Creates new PRs for the included branches @@ -202,7 +206,8 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { // auto-generated titles and bodies (today's behavior). var drafts map[string]*submitview.PRDraft if cfg.IsInteractive() && !opts.auto { - collected, cancelled, tuiErr := collectPRDrafts(cfg, client, s, currentBranch, prDetails, templateContent) + canCreateStack := stacksAvailable && s.ID == "" + collected, cancelled, tuiErr := collectPRDrafts(cfg, client, s, currentBranch, prDetails, templateContent, canCreateStack) if tuiErr != nil { cfg.Errorf("failed to run the submit editor: %s", tuiErr) return ErrSilent @@ -263,7 +268,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { // returns the per-branch overrides, whether the user cancelled, and any error. // When the stack contains no branches without a PR, it skips the TUI and // returns nil drafts so the normal push/relink path runs. -func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack, currentBranch string, prDetails map[string]*github.PRDetails, templateContent string) (map[string]*submitview.PRDraft, bool, error) { +func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack, currentBranch string, prDetails map[string]*github.PRDetails, templateContent string, canCreateStack bool) (map[string]*submitview.PRDraft, bool, error) { // Fill in the real title/description for existing PRs that were synced // without them (e.g. merged branches) so the read-only cards show API data. enrichPRContent(client, prDetails) @@ -290,10 +295,11 @@ func collectPRDrafts(cfg *config.Config, client github.ClientOps, s *stack.Stack } model := submitview.New(submitview.Options{ - Nodes: nodes, - Trunk: s.Trunk, - RepoLabel: repoLabel, - Version: Version, + Nodes: nodes, + Trunk: s.Trunk, + RepoLabel: repoLabel, + Version: Version, + CanCreateStack: canCreateStack, }) // Use cell-motion mouse mode (clicks, drag, and wheel) rather than all-motion. diff --git a/cmd/submit_tui_test.go b/cmd/submit_tui_test.go index 9cbac9f..a6dbea3 100644 --- a/cmd/submit_tui_test.go +++ b/cmd/submit_tui_test.go @@ -36,7 +36,7 @@ func TestCollectPRDrafts_SkipsWhenNoNewBranches(t *testing.T) { "b2": {Number: 2, State: "OPEN"}, } - drafts, cancelled, err := collectPRDrafts(cfg, nil, s, "b1", prDetails, "") + drafts, cancelled, err := collectPRDrafts(cfg, nil, s, "b1", prDetails, "", false) require.NoError(t, err) assert.False(t, cancelled) assert.Nil(t, drafts, "no NEW branches means the TUI is skipped and drafts are nil") diff --git a/internal/tui/submitview/data.go b/internal/tui/submitview/data.go index 8db39f1..05303b7 100644 --- a/internal/tui/submitview/data.go +++ b/internal/tui/submitview/data.go @@ -123,6 +123,20 @@ func CountSelected(nodes []SubmitNode) int { return n } +// CountOpenPRs returns the number of branches that already have an open pull +// request (open or draft). These are the PRs that can be linked into a stack via +// the "STACK N PRs" action. Queued, merged, and closed PRs are excluded because +// they cannot form a new stack. +func CountOpenPRs(nodes []SubmitNode) int { + n := 0 + for _, node := range nodes { + if node.State == StateOpen || node.State == StateDraft { + n++ + } + } + return n +} + // HasClosed reports whether any branch in the list has a closed PR, which blocks // the stack and triggers the closed-branch callout. func HasClosed(nodes []SubmitNode) bool { diff --git a/internal/tui/submitview/help.go b/internal/tui/submitview/help.go index ef85f0d..af0d609 100644 --- a/internal/tui/submitview/help.go +++ b/internal/tui/submitview/help.go @@ -53,6 +53,7 @@ var helpSections = []helpSection{ heading: "Existing PRs", entries: []helpEntry{ {"^o", "open the focused branch's PR on the web"}, + {"^b", "link the existing open PRs into a stack (when there's no stack on GitHub)"}, }, }, { diff --git a/internal/tui/submitview/model.go b/internal/tui/submitview/model.go index 7c6c679..2537a83 100644 --- a/internal/tui/submitview/model.go +++ b/internal/tui/submitview/model.go @@ -45,6 +45,11 @@ type Options struct { RepoLabel string // Version is the CLI version string. Version string + // CanCreateStack reports that the local stack has no remote stack object yet + // but one could be created (stacked PRs are available on the repo). When + // true, and once the user has deselected all new PRs, the TUI offers a + // "STACK N PRs" action to link the existing open PRs into a stack. + CanCreateStack bool } // Model is the Bubble Tea model backing the interactive `gh stack submit` TUI. @@ -54,6 +59,10 @@ type Model struct { repoLabel string version string + // canCreateStack mirrors Options.CanCreateStack: the local stack has no + // remote stack object yet, but one could be created. + canCreateStack bool + cursor int // index into nodes (the focused branch) width, height int @@ -139,6 +148,8 @@ func New(opts Options) Model { version: opts.Version, cursor: cursor, + canCreateStack: opts.CanCreateStack, + titleArea: tia, descArea: ta, focusedField: fieldTitle, @@ -263,6 +274,15 @@ func (m Model) anyEdited() bool { return false } +// canStackExistingPRs reports whether the "STACK N PRs" action should be offered: +// the local stack has no remote stack object yet, the user has deselected every +// new PR, and there are at least two existing open PRs to link into a stack. +func (m Model) canStackExistingPRs() bool { + return m.canCreateStack && + CountSelected(m.nodes) == 0 && + CountOpenPRs(m.nodes) >= 2 +} + // quit marks the session cancelled and exits. If the user has unsaved edits, it // first raises a discard-edits confirmation instead of quitting immediately. func (m Model) quit() (tea.Model, tea.Cmd) { diff --git a/internal/tui/submitview/mouse.go b/internal/tui/submitview/mouse.go index 7eb21c7..96588e2 100644 --- a/internal/tui/submitview/mouse.go +++ b/internal/tui/submitview/mouse.go @@ -137,6 +137,23 @@ func (m Model) leftCheckboxHit(x, y int) bool { return x >= leftW-5 && x < leftW-2 } +// leftStackButtonHit reports whether (x,y) lands on the bottom-left "STACK N PRs" +// button, when it is shown. The button occupies the last reserved row of the +// left panel (one blank separator row sits above it). +func (m Model) leftStackButtonHit(x, y int) bool { + if !m.canStackExistingPRs() { + return false + } + leftW, _ := m.panelWidths() + if x < 0 || x >= leftW { + return false + } + if y-m.panelTopRow() != m.leftVisibleHeight()+1 { + return false + } + return x < lipgloss.Width(m.renderStackButton(leftW-2)) +} + // handleClick routes a left click to a branch row (left map) or an editor // element (right panel). func (m Model) handleClick(x, y int) (tea.Model, tea.Cmd) { @@ -145,6 +162,12 @@ func (m Model) handleClick(x, y int) (tea.Model, tea.Cmd) { leftW, rightW := m.panelWidths() + // Bottom-left STACK button: link the existing open PRs into a stack. + if m.leftStackButtonHit(x, y) { + m.saveEditor() + return m.requestSubmit() + } + // Left panel: focus a branch, toggling include when its checkbox is clicked. if idx := m.branchRowAt(x, y); idx != -1 { onCheckbox := m.leftCheckboxHit(x, y) && m.nodes[idx].State == StateNew diff --git a/internal/tui/submitview/render.go b/internal/tui/submitview/render.go index ca92585..e1d8597 100644 --- a/internal/tui/submitview/render.go +++ b/internal/tui/submitview/render.go @@ -81,13 +81,19 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig { } // headerShortcuts returns the six primary single-screen keyboard shortcuts shown -// in the header (the help overlay lists the full set). +// in the header (the help overlay lists the full set). When the STACK action is +// offered (no stack yet and every new PR deselected), the submit hint is swapped +// for the "^b stack PRs" action, which is the relevant control in that state. func (m Model) headerShortcuts() []shared.ShortcutEntry { + primary := shared.ShortcutEntry{Key: "^s", Desc: "submit PRs"} + if m.canStackExistingPRs() { + primary = shared.ShortcutEntry{Key: "^b", Desc: "stack PRs"} + } return []shared.ShortcutEntry{ {Key: "↑↓", Desc: "select branch"}, {Key: "tab", Desc: "cycle field"}, {Key: "^x", Desc: "skip/include"}, - {Key: "^s", Desc: "submit PRs"}, + primary, {Key: "^h", Desc: "help"}, {Key: "esc", Desc: "quit"}, } diff --git a/internal/tui/submitview/screen.go b/internal/tui/submitview/screen.go index f323e37..a208855 100644 --- a/internal/tui/submitview/screen.go +++ b/internal/tui/submitview/screen.go @@ -13,6 +13,14 @@ import ( // updateScreen handles all key input on the single submit screen. func (m Model) updateScreen(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + // Ctrl+B links the existing open PRs into a stack. It is only intercepted + // while the STACK action is offered; otherwise it falls through so the + // editor's textarea keeps Ctrl+B (move cursor back) while editing. + if msg.Type == tea.KeyCtrlB && m.canStackExistingPRs() { + m.saveEditor() + return m.requestSubmit() + } + // Global keys, handled regardless of focus. switch msg.Type { case tea.KeyCtrlC: @@ -688,7 +696,8 @@ func (m Model) renderLeftPanel(width, height int) string { fullW = 6 } rows := m.buildLeftRows(fullW) - visH := height - 2 + buttonRows := m.leftButtonRows() + visH := height - 2 - buttonRows if visH < 1 { visH = 1 } @@ -698,14 +707,36 @@ func (m Model) renderLeftPanel(width, height int) string { end = len(rows) } - var b strings.Builder - for i, r := range rows[scroll:end] { - if i > 0 { - b.WriteString("\n") - } - b.WriteString(r.text) + lines := make([]string, 0, visH+buttonRows) + for _, r := range rows[scroll:end] { + lines = append(lines, r.text) + } + // Pad the scroll area to its full height so the button sits at the bottom. + for len(lines) < visH { + lines = append(lines, "") } - return leftPanelBox(b.String(), width, height) + if buttonRows > 0 { + lines = append(lines, "", m.renderStackButton(fullW)) + } + return leftPanelBox(strings.Join(lines, "\n"), width, height) +} + +// leftButtonRows returns the number of inner rows reserved at the bottom of the +// left panel for the "STACK N PRs" button (a blank separator plus the button), +// or 0 when the button is not shown. +func (m Model) leftButtonRows() int { + if m.canStackExistingPRs() { + return 2 + } + return 0 +} + +// renderStackButton renders the bottom-left "(^b) STACK N PRs" action, styled +// like the right-panel SUBMIT button. The keyboard hint sits to the left. +func (m Model) renderStackButton(fullW int) string { + n := CountOpenPRs(m.nodes) + label := hintStyle.Render("(^b) ") + submitButtonStyle.Render(fmt.Sprintf("STACK %d PRs", n)) + return pad(1, false) + label } // leftPanelBox frames the left panel with the shared rounded border but no inner @@ -892,7 +923,7 @@ func pad(n int, focused bool) string { // leftVisibleHeight is the number of timeline rows the left panel can show. func (m Model) leftVisibleHeight() int { - h := m.contentHeight() - 2 // panel border + h := m.contentHeight() - 2 - m.leftButtonRows() // panel border + reserved button rows if h < 1 { h = 1 } diff --git a/internal/tui/submitview/stackbutton_test.go b/internal/tui/submitview/stackbutton_test.go new file mode 100644 index 0000000..cf7fe54 --- /dev/null +++ b/internal/tui/submitview/stackbutton_test.go @@ -0,0 +1,112 @@ +package submitview + +import ( + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/github/gh-stack/internal/stack" + "github.com/stretchr/testify/assert" +) + +// deselectedNew builds a NEW branch node that the user has deselected. +func deselectedNew(branch string) SubmitNode { + n := newNode(branch, StateNew) + n.Included = false + return n +} + +// stackModel builds a sized model with the given CanCreateStack flag. +func stackModel(t *testing.T, nodes []SubmitNode, canCreateStack bool) Model { + t.Helper() + m := New(Options{ + Nodes: nodes, + Trunk: stack.BranchRef{Branch: "main"}, + RepoLabel: "myorg/myrepo", + Version: "1.0.0", + CanCreateStack: canCreateStack, + }) + m.openURL = func(string) {} + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 40}) + return updated.(Model) +} + +func TestCountOpenPRs(t *testing.T) { + nodes := []SubmitNode{ + newNode("a", StateNew), + newNode("b", StateOpen), + newNode("c", StateDraft), + newNode("d", StateQueued), + newNode("e", StateMerged), + newNode("f", StateClosed), + } + assert.Equal(t, 2, CountOpenPRs(nodes), "only open and draft PRs count") +} + +func TestCanStackExistingPRs(t *testing.T) { + twoOpen := []SubmitNode{newNode("a", StateOpen), newNode("b", StateDraft)} + tests := []struct { + name string + nodes []SubmitNode + canCreateStack bool + want bool + }{ + {"two open PRs, no stack, none selected", twoOpen, true, true}, + {"deselected NEW + two open PRs", []SubmitNode{deselectedNew("n"), newNode("a", StateOpen), newNode("b", StateDraft)}, true, true}, + {"has a remote stack already", twoOpen, false, false}, + {"a NEW branch is still selected", []SubmitNode{newNode("n", StateNew), newNode("a", StateOpen), newNode("b", StateDraft)}, true, false}, + {"only one open PR", []SubmitNode{newNode("a", StateOpen)}, true, false}, + {"open PRs but queued/merged do not count", []SubmitNode{newNode("a", StateOpen), newNode("b", StateQueued), newNode("c", StateMerged)}, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := stackModel(t, tt.nodes, tt.canCreateStack) + assert.Equal(t, tt.want, m.canStackExistingPRs()) + }) + } +} + +func TestStackButton_RendersWhenEligible(t *testing.T) { + m := stackModel(t, []SubmitNode{deselectedNew("feat/new"), newNode("a", StateOpen), newNode("b", StateDraft)}, true) + view := m.View() + assert.Contains(t, view, "STACK 2 PRs") + assert.Contains(t, view, "(^b)") +} + +func TestStackButton_HiddenWhenNotEligible(t *testing.T) { + // Has a remote stack already. + withStack := stackModel(t, []SubmitNode{newNode("a", StateOpen), newNode("b", StateDraft)}, false) + assert.NotContains(t, withStack.View(), "STACK 2 PRs") + assert.NotContains(t, withStack.View(), "(^b)") + + // A NEW branch is still selected for creation. + newSelected := stackModel(t, []SubmitNode{newNode("n", StateNew), newNode("a", StateOpen), newNode("b", StateDraft)}, true) + assert.NotContains(t, newSelected.View(), "STACK 2 PRs") + assert.NotContains(t, newSelected.View(), "(^b)") +} + +func TestStackButton_HeaderHintSwaps(t *testing.T) { + eligible := stackModel(t, []SubmitNode{deselectedNew("feat/new"), newNode("a", StateOpen), newNode("b", StateDraft)}, true) + view := eligible.View() + assert.Contains(t, view, "stack PRs", "header shows the stack hint when eligible") + assert.NotContains(t, view, "submit PRs", "the submit hint is swapped out in stack mode") +} + +func TestCtrlB_TriggersSubmitWhenEligible(t *testing.T) { + m := stackModel(t, []SubmitNode{deselectedNew("feat/new"), newNode("a", StateOpen), newNode("b", StateDraft)}, true) + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyCtrlB}) + assert.True(t, m.SubmitRequested(), "Ctrl+B requests submit when the STACK action is offered") +} + +func TestCtrlB_NoOpWhenNotEligible(t *testing.T) { + // A NEW branch is included (editing context) — Ctrl+B must not submit. + m := stackModel(t, []SubmitNode{newNode("n", StateNew), newNode("a", StateOpen)}, true) + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyCtrlB}) + assert.False(t, m.SubmitRequested(), "Ctrl+B does not submit when the STACK action is not offered") +} + +func TestStackButton_MouseClickTriggersSubmit(t *testing.T) { + m := stackModel(t, []SubmitNode{deselectedNew("feat/new"), newNode("a", StateOpen), newNode("b", StateDraft)}, true) + y := m.panelTopRow() + m.leftVisibleHeight() + 1 // the button's reserved bottom row + updated, _ := m.Update(tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonLeft, X: 2, Y: y}) + assert.True(t, updated.(Model).SubmitRequested(), "clicking the STACK button requests submit") +} From eb9ad81178571d3b2d04890e4739e861801ed23c Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 12 Jul 2026 14:25:35 -0400 Subject: [PATCH 6/7] docs updates --- README.md | 31 ++++++++--- docs/package-lock.json | 54 ------------------- docs/src/content/docs/guides/stacked-prs.md | 2 +- docs/src/content/docs/guides/workflows.md | 25 ++++++--- .../src/content/docs/introduction/overview.md | 2 +- docs/src/content/docs/reference/cli.md | 29 +++++++--- skills/gh-stack/SKILL.md | 22 +++++--- 7 files changed, 80 insertions(+), 85 deletions(-) diff --git a/README.md b/README.md index 4e63dc4..6c634eb 100644 --- a/README.md +++ b/README.md @@ -316,15 +316,28 @@ Fetch, rebase, push, and sync PR state in a single command. gh stack sync [flags] ``` -Performs a safe, non-interactive synchronization of the entire stack: +Performs a synchronization of the entire stack: -1. **Fetch** — fetches the latest changes from `origin` -2. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged) -3. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state and you are advised to run `gh stack rebase` to resolve conflicts interactively -4. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred) -5. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR -6. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. Only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that) -7. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically +1. **Fetch** — fetches the latest changes from `origin`. +2. **Reconcile the remote stack** — mirrors the GitHub stack locally. When PRs have been added to the stack on GitHub (the remote is ahead of your local stack), their branches are pulled down and appended to your local stack automatically. When the local and remote stacks have genuinely diverged (for example, you added a branch locally while different PRs were added to the stack on GitHub), you are prompted to resolve (see [Diverged stacks](#diverged-stacks) below). In a non-interactive terminal a divergence aborts the sync (nothing is pushed or updated). +3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged). +4. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state, and you are advised to run `gh stack rebase` to resolve conflicts interactively. +5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred). +6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR. +7. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. This only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that). +8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically. + +A clean remote-ahead update (PRs added on top of your local stack) is pulled down automatically without prompting, so `sync` is safe to run in automation. Sync only prompts when the stacks have truly diverged. + +#### Diverged stacks + +When neither stack is a clean prefix of the other — for example, you added a branch locally while separate PRs were added to the same stack on GitHub — sync cannot merge the two automatically. In an interactive terminal it offers three choices: + +- **Use the remote stack as the source of truth** — replaces your local stack composition with the remote's, pulling any missing branches. If you were on a branch that the remote stack no longer contains, you're moved to the nearest surviving branch. Requires a clean working state with no uncommitted changes. +- **Delete the stack on GitHub** — deletes the stack object on GitHub and stops the sync. Your PRs and local branches are untouched (only the stack on GitHub is removed); recreate the stack with `gh stack submit` (run `gh stack modify` first if you want to change its structure). This is the way to make GitHub match your local stack, because `submit` — unlike `sync` — also creates PRs for any branches you haven't submitted yet. +- **Cancel** — aborts the sync without pushing branches or updating any PRs. + +In a non-interactive terminal, a divergence aborts the sync (exit success) without pushing branches or updating PRs; resolve it by unstacking and recreating the stack. | Flag | Description | |------|-------------| @@ -377,6 +390,8 @@ If every PR in the stack has already been merged, that stack is complete and can In an interactive terminal, `submit` opens a full-screen, mouse- and keyboard-driven editor on a single screen. Every branch without a PR is included by default — deselect any you don't want on the left panel (Ctrl+X). Because each PR builds on the branch below it, deselecting a branch also deselects the ones stacked above it, and re-including a branch re-includes the ones below it. Draft each PR's title, description (with a markdown preview and `$EDITOR` escape), and choose ready-for-review or draft on the right, then submit them all at once with Ctrl+S. Pass `--auto` (or run in CI) to skip the editor and use auto-generated titles. +If the branches already have open PRs but no stack exists on GitHub, you will have the option to link the PRs into a stack with Ctrl+B. + In the editor, new PRs default to ready for review; flip any PR to draft with the ready ↔ draft toggle. With `--auto`, new PRs are created as drafts unless you pass `--open`. | Flag | Description | diff --git a/docs/package-lock.json b/docs/package-lock.json index acec4d8..8656976 100644 --- a/docs/package-lock.json +++ b/docs/package-lock.json @@ -72,9 +72,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -91,9 +88,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -110,9 +104,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -129,9 +120,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -447,9 +435,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "optional": true, "os": [ "linux" @@ -462,9 +447,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "optional": true, "os": [ "linux" @@ -477,9 +459,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "optional": true, "os": [ "linux" @@ -492,9 +471,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "optional": true, "os": [ "linux" @@ -1798,9 +1774,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1817,9 +1790,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1836,9 +1806,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1855,9 +1822,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1874,9 +1838,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1893,9 +1854,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3945,9 +3903,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3968,9 +3923,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3991,9 +3943,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -4014,9 +3963,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/docs/src/content/docs/guides/stacked-prs.md b/docs/src/content/docs/guides/stacked-prs.md index 23fe269..4336ce4 100644 --- a/docs/src/content/docs/guides/stacked-prs.md +++ b/docs/src/content/docs/guides/stacked-prs.md @@ -51,4 +51,4 @@ gh stack sync - **`gh stack push`** pushes branches only (uses `--force-with-lease` for safety). It does not create or update PRs. - **`gh stack submit`** pushes branches and creates or updates PRs, linking them as a Stack on GitHub. -- **`gh stack sync`** is the all-in-one command: fetch, rebase, push, sync PR state, link open PRs into a Stack on GitHub, and optionally prune local branches for merged PRs. +- **`gh stack sync`** is the all-in-one command: fetch, rebase, push, sync stack/PR state, link open PRs into a Stack on GitHub, and optionally prune local branches for merged PRs. If there is a divergence between local and remote stacks, you will be prompted to resolve. diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 65cd0be..515f77d 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -130,15 +130,28 @@ gh stack sync This command: 1. Fetches the latest changes from the remote -2. Fast-forwards the trunk branch -3. Rebases all remaining stack branches onto the updated trunk -4. Pushes the updated branches -5. Syncs PR state from GitHub -6. Links the open PRs into a Stack on GitHub (creating or updating the remote stack when two or more PRs exist) -7. Prompts to prune local branches for merged PRs (use `--prune` to prune automatically) +2. Reconciles the remote stack with your local stack +3. Fast-forwards the trunk branch +4. Rebases all remaining stack branches onto the updated trunk +5. Pushes the updated branches +6. Syncs PR state from GitHub +7. Links the open PRs into a Stack on GitHub (creating or updating the remote stack when two or more PRs exist) +8. Prompts to prune local branches for merged PRs (use `--prune` to prune automatically) If a conflict is detected during the rebase, all branches are restored to their original state, and you're advised to run `gh stack rebase` to resolve conflicts interactively. +### Pulling in PRs added to the stack on GitHub + +If PRs are added to the stack on GitHub by someone else, `gh stack sync` fetches the new PRs' branches and appends them to your local stack so it mirrors the remote. + +If your local and remote stacks have diverged — for example, you added a branch locally while different PRs/branches were added to the same stack on GitHub — sync can't merge them automatically. In an interactive terminal it offers three choices: + +- **Use the remote stack as the source of truth** — replaces your local stack composition with the remote's, pulling any missing branches. If you were on a branch that the remote stack no longer contains, you're moved to the nearest surviving branch. Requires a clean working state with no uncommitted changes. +- **Delete the stack on GitHub** — deletes the stack object on GitHub and stops the sync. Your PRs and local branches are untouched (only the stack on GitHub is removed); recreate the stack with `gh stack submit` (run `gh stack modify` first if you want to change its structure). This is the way to make GitHub match your local stack, because `submit` — unlike `sync` — also creates PRs for any branches you haven't submitted yet. +- **Cancel** — aborts the sync without pushing branches or updating any PRs. + +In a non-interactive terminal, a divergence aborts the sync (exit success) without pushing branches or updating PRs; resolve it by unstacking and recreating the stack. + ## Rebasing Your Stack Stacked PRs rely on rebasing rather than merge commits to keep each branch's diff clean and reviewable. If you're coming from a merge-commit workflow, the key difference is: instead of merging upstream changes into your branch (which creates a merge commit with multiple parents), you replay your commits on top of the latest base. The result is a linear history where each PR shows only its specific changes. diff --git a/docs/src/content/docs/introduction/overview.md b/docs/src/content/docs/introduction/overview.md index f3cb83a..11e5f07 100644 --- a/docs/src/content/docs/introduction/overview.md +++ b/docs/src/content/docs/introduction/overview.md @@ -88,7 +88,7 @@ While the PR UI provides the review and merge experience, the `gh stack` CLI han - **Pushing branches** — `gh stack push` pushes all branches to the remote. - **Creating PRs** — `gh stack submit` pushes branches and creates or updates PRs, linking them as a Stack on GitHub. - **Navigating the stack** — `gh stack up`, `down`, `top`, and `bottom` let you move between layers without remembering branch names. -- **Syncing everything** — `gh stack sync` fetches, rebases, pushes, updates PR state, and links open PRs into a Stack on GitHub in one command. +- **Syncing everything** — `gh stack sync` fetches, rebases, pushes, updates PR state, and links open PRs into a Stack on GitHub in one command. It also syncs the stack's remote state, pulling down branches for any PRs added to the stack on GitHub. - **Restructuring stacks** — `gh stack modify` opens an interactive terminal UI to drop, fold, insert, rename, and reorder branches in a stack. - **Tearing down stacks** — `gh stack unstack` removes a stack from GitHub and local tracking. - **Checking out a stack** — `gh stack checkout ` pulls down a stack, with all its branches, from GitHub to your local machine. diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 3cd99de..6a1bc3a 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -278,6 +278,8 @@ In an interactive terminal, `submit` opens a full-screen editor on a single scre Press Ctrl+S to submit all included PRs at once. The editor supports both keyboard and mouse input. Pass `--auto` (or run in a non-interactive terminal, such as CI) to skip the editor and use auto-generated titles. +If the branches already have open PRs but no stack exists on GitHub, you will have the option to link the PRs into a stack with Ctrl+B. + In the editor, new PRs default to **ready for review**; flip any PR to **draft** with the ready ↔ draft toggle. With `--auto`, new PRs are created as **drafts** unless you pass `--open`. | Flag | Description | @@ -302,15 +304,28 @@ Fetch, rebase, push, and sync PR state in a single command. gh stack sync [flags] ``` -Performs a safe, non-interactive synchronization of the entire stack: +Performs a synchronization of the entire stack: 1. **Fetch** — fetches the latest changes from `origin`. -2. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged). -3. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state, and you are advised to run `gh stack rebase` to resolve conflicts interactively. -4. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred). -5. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR. -6. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. This only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that). -7. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically. +2. **Reconcile the remote stack** — mirrors the GitHub stack locally. When PRs have been added to the stack on GitHub (the remote is ahead of your local stack), their branches are pulled down and appended to your local stack automatically. When the local and remote stacks have genuinely diverged (for example, you added a branch locally while different PRs were added to the stack on GitHub), you are prompted to resolve (see [Diverged stacks](#diverged-stacks) below). In a non-interactive terminal a divergence aborts the sync (nothing is pushed or updated). +3. **Fast-forward trunk** — fast-forwards the trunk branch to match the remote (skips if diverged). +4. **Cascade rebase** — rebases all stack branches onto their updated parents (only if trunk moved). If a conflict is detected, all branches are restored to their original state, and you are advised to run `gh stack rebase` to resolve conflicts interactively. +5. **Push** — pushes all branches (uses `--force-with-lease` if a rebase occurred). +6. **Sync PRs** — syncs PR state from GitHub and reports the status of each PR. +7. **Sync the stack** — links the stack's open PRs into a stack on GitHub, creating the remote stack object if it doesn't exist yet or updating it if it's partially formed. This only happens when two or more PRs exist; sync never opens PRs (use `gh stack submit` for that). +8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to prune automatically. + +A clean remote-ahead update (PRs added on top of your local stack) is pulled down automatically without prompting, so `sync` is safe to run in automation. Sync only prompts when the stacks have truly diverged. + +#### Diverged stacks + +When neither stack is a clean prefix of the other — for example, you added a branch locally while separate PRs were added to the same stack on GitHub — sync cannot merge the two automatically. In an interactive terminal it offers three choices: + +- **Use the remote stack as the source of truth** — replaces your local stack composition with the remote's, pulling any missing branches. If you were on a branch that the remote stack no longer contains, you're moved to the nearest surviving branch. Requires a clean working state with no uncommitted changes. +- **Delete the stack on GitHub** — deletes the stack object on GitHub and stops the sync. Your PRs and local branches are untouched (only the stack on GitHub is removed); recreate the stack with `gh stack submit` (run `gh stack modify` first if you want to change its structure). This is the way to make GitHub match your local stack, because `submit` — unlike `sync` — also creates PRs for any branches you haven't submitted yet. +- **Cancel** — aborts the sync without pushing branches or updating any PRs. + +In a non-interactive terminal, a divergence aborts the sync (exit success) without pushing branches or updating PRs; resolve it by unstacking and recreating the stack. | Flag | Description | |------|-------------| diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index ca528ea..aca1176 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -305,7 +305,7 @@ gh stack push ### Routine sync after merges ```bash -# Single command: fetch, rebase, push, sync PR state +# Single command: fetch, rebase, push, sync PR and stack state gh stack sync # Sync and automatically clean up local branches for merged PRs @@ -314,6 +314,8 @@ gh stack sync --prune > **Note for agents:** In non-interactive environments, the prune prompt is not shown. Use `--prune` explicitly to delete local branches for merged PRs. +> **Note for agents:** `sync` also mirrors the stack on GitHub locally. If PRs were added to the stack on github.com, their branches are pulled down and appended to the local stack automatically. If the local and remote stacks have **diverged** (you changed the local stack while the remote stack changed differently), sync can only prompt to resolve it in an interactive terminal — in non-interactive environments it aborts the sync (nothing is pushed or updated) and exits successfully with `ℹ Sync aborted`. Resolve a divergence by unstacking and recreating the stack. + ### Squash-merge recovery When a PR is squash-merged on GitHub, the original branch's commits no longer exist in the trunk history. `gh stack` detects this automatically and uses `git rebase --onto` to correctly replay remaining commits. @@ -628,16 +630,20 @@ gh stack sync [flags] **What it does (in order):** 1. **Fetch** latest changes from the remote -2. **Fast-forward trunk** to match remote (skips if already up to date, warns if diverged) -3. **Cascade rebase** all stack branches onto their updated parents (only if trunk moved). Handles merged PRs automatically. If a conflict is detected, **all branches are restored** to their pre-rebase state and the command exits with code 3 — see [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) for the resolution workflow -4. **Push** all active branches atomically -5. **Sync PR state** from GitHub and report the status of each PR -6. **Sync the stack object** — link the open PRs into a stack on GitHub. If the PRs are not yet in a stack, a new stack is created; if some PRs are already in a stack, it is updated (additive only). This only happens when two or more PRs exist. Sync **never opens PRs** — use `gh stack submit` for that -7. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to skip the prompt. In non-interactive environments, pruning only happens when `--prune` is passed explicitly +2. **Reconcile the remote stack** — mirror the GitHub stack locally. If PRs were added to the stack on GitHub, pull their branches down and append them to the local stack. If the local and remote stacks have diverged, aborts the sync in a non-interactive terminal. In an interactive terminal, offers prompts to resolve any divergence (replace local stack with remote version, delete stack on GitHub so it can be recreated, or cancel). +3. **Fast-forward trunk** to match remote (skips if already up to date, warns if diverged) +4. **Cascade rebase** all stack branches onto their updated parents (only if trunk moved). Handles merged PRs automatically. If a conflict is detected, **all branches are restored** to their pre-rebase state and the command exits with code 3 — see [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) for the resolution workflow +5. **Push** all active branches atomically +6. **Sync PR state** from GitHub and report the status of each PR +7. **Sync the stack object** — link the open PRs into a stack on GitHub. If the PRs are not yet in a stack, a new stack is created; if some PRs are already in a stack, it is updated (additive only). This only happens when two or more PRs exist. Sync **never opens PRs** — use `gh stack submit` for that +8. **Prune** — in interactive terminals, prompts to delete local branches for merged PRs. Use `--prune` to skip the prompt. In non-interactive environments, pruning only happens when `--prune` is passed explicitly **Output (stderr):** - `✓ Fetched latest changes from origin` +- `Pulling N new branches from the remote stack ...` then `✓ Pulled N new branches into the stack from the remote` (when the remote stack is ahead) +- `⚠ Your local stack has diverged from the stack on GitHub` (with `Local:` / `Remote:` chains) when the stacks have diverged +- `ℹ Sync aborted — no changes were made` when a sync is cancelled - `✓ Trunk main fast-forwarded to ` or `✓ Trunk main is already up to date` - `✓ Rebased onto ` per branch (if base moved) - `✓ Pushed N branches` @@ -645,7 +651,7 @@ gh stack sync [flags] - `Merged: #N, #M` for merged branches - `✓ Stack created on GitHub with N PRs` / `✓ Stack updated on GitHub with N PRs` / `✓ Linked to the existing stack on GitHub` (when two or more PRs exist) - `✓ Pruned (merged)` per pruned branch (when pruning) -- `✓ Stack synced` when the stack object on GitHub was created/updated to match local, or `✓ Branches synced` when only the branches were synced (fewer than two PRs, stacked PRs unavailable, or a divergence) +- `✓ Stack synced` when the stack object on GitHub was created/updated to match local, or `✓ Branches synced` when only the branches were synced (fewer than two PRs or stacked PRs unavailable) --- From 37505c73b3afe63fe7e465b043f0945f4d09e58a Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 12 Jul 2026 15:14:41 -0400 Subject: [PATCH 7/7] address review comments --- cmd/sync_test.go | 127 +++++++++++++++++++++++++++++++ cmd/utils.go | 49 +++++++++++- internal/tui/submitview/mouse.go | 4 +- 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index d93aed1..ca9acd7 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2060,6 +2060,133 @@ func TestSync_RemoteAhead_PullsNewBranches(t *testing.T) { assert.Equal(t, []string{"b1", "b2", "b3", "b4", "b5"}, sf.Stacks[0].BranchNames()) } +// TestSync_RemoteAhead_QueuedBranchNotPushed verifies that a pulled branch whose +// PR is in the merge queue has its transient queued state copied from the fresh +// PR details during reconciliation, so it is not force-pushed by the later push +// step. +func TestSync_RemoteAhead_QueuedBranchNotPushed(t *testing.T) { + s := stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + }, + } + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + var created []string + var pushes []pushCall + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.BranchExistsFn = func(name string) bool { return name != "b3" } + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.SetUpstreamTrackingFn = func(string, string) error { return nil } + mock.PushFn = func(remote string, branches []string, force, atomic bool) error { + pushes = append(pushes, pushCall{remote, branches, force, atomic}) + return nil + } + + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 103}}}, nil + }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + branch := map[int]string{101: "b1", 102: "b2", 103: "b3"}[n] + if branch == "" { + return nil, nil + } + pr := &github.PullRequest{ + Number: n, ID: fmt.Sprintf("PR_%d", n), + URL: fmt.Sprintf("https://github.com/o/r/pull/%d", n), + HeadRefName: branch, State: "OPEN", + } + if n == 103 { + pr.MergeQueueEntry = &github.MergeQueueEntry{ID: "MQ1"} + } + return pr, nil + }, + } + + _, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock }) + require.NoError(t, err) + + assert.Contains(t, created, "b3", "the queued branch is still pulled into the local stack") + for _, pc := range pushes { + assert.NotContains(t, pc.branches, "b3", "a merge-queued branch must not be pushed") + } +} + +// TestSync_RemoteAhead_DuplicateBranchAborts verifies that pulling a remote +// addition whose branch is already owned by another local stack aborts rather +// than writing the branch into two stacks. +func TestSync_RemoteAhead_DuplicateBranchAborts(t *testing.T) { + tmpDir := t.TempDir() + writeStackFileMulti(t, tmpDir, + stack.Stack{ + ID: "9", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}}, + }, + }, + stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b3"}}, // another stack already owns b3 + }, + ) + + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + + ghMock := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { + return []github.RemoteStack{{ID: 9, PullRequests: []int{101, 102, 103}}}, nil + }, + FindPRByNumberFn: prByNumberFinder(map[int]string{101: "b1", 102: "b2", 103: "b3"}), + } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { cfg.GitHubClientOverride = ghMock }) + + assert.Error(t, err) + assert.Contains(t, output, "Cannot pull b3") + assert.NotContains(t, created, "b3", "must not pull a branch owned by another stack") + + sf, loadErr := stack.Load(tmpDir) + require.NoError(t, loadErr) + assert.Equal(t, []string{"b1", "b2"}, sf.Stacks[0].BranchNames(), "tracked stack unchanged") +} + +// TestSync_Divergent_UseRemote_DirtyCheckErrorAborts verifies that when the +// working-tree status cannot be determined, "use remote" aborts instead of +// treating the tree as clean and running the destructive replace. +func TestSync_Divergent_UseRemote_DirtyCheckErrorAborts(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + + ghMock := divergentRemoteMock() + var created []string + mock := newSyncMockNoRebase(tmpDir, "b1") + mock.CreateBranchFn = func(name, base string) error { created = append(created, name); return nil } + mock.HasUncommittedChangesFn = func() (bool, error) { return false, fmt.Errorf("git status failed") } + + output, err := runSyncCfg(t, mock, func(cfg *config.Config) { + cfg.GitHubClientOverride = ghMock + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 0, nil } + }) + + assert.Error(t, err) + assert.Contains(t, output, "Could not determine whether the working tree is clean") + assert.Empty(t, created, "must not replace the local stack when the working-tree check fails") + + sf, loadErr := stack.Load(tmpDir) + require.NoError(t, loadErr) + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames(), "local stack untouched") +} + // TestSync_RemoteInSync_NoPull verifies that when local and remote match, no // branches are pulled and no divergence is reported. func TestSync_RemoteInSync_NoPull(t *testing.T) { diff --git a/cmd/utils.go b/cmd/utils.go index 7de02dd..61bd91e 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1268,7 +1268,11 @@ func reconcileRemoteStack(cfg *config.Config, sf *stack.StackFile, s *stack.Stac switch classifyRemoteStack(localActive, remoteActive) { case remoteStackInSync, remoteStackLocalAhead: - // Nothing to pull; the existing flow pushes/updates the remote. + // Nothing to pull; the existing flow pushes/updates the remote. Copy the + // freshly fetched PR state (merged/queued) onto the local branches so the + // fast-forward/rebase/push steps skip merged and merge-queued branches + // rather than rewriting them before the later PR-sync step runs. + syncRemotePRState(s, prs) return res, nil case remoteStackCleanAhead: return pullRemoteAdditions(cfg, sf, s, gitDir, remote, prs) @@ -1365,6 +1369,21 @@ func pullRemoteAdditions(cfg *config.Config, sf *stack.StackFile, s *stack.Stack return res, nil } + // A remote-added branch must not collide with a branch already tracked by + // another local stack, or with an existing local branch we would otherwise + // adopt as "pulled" without actually fetching it. Abort rather than persist + // duplicate ownership or a stale branch. + for _, pr := range newPRs { + if err := sf.ValidateNoDuplicateBranch(pr.HeadRefName); err != nil { + cfg.Errorf("Cannot pull %s from the remote stack: %s", pr.HeadRefName, err) + return res, ErrSilent + } + if git.BranchExists(pr.HeadRefName) { + cfg.Errorf("Cannot pull %s from the remote stack: a local branch with that name already exists", pr.HeadRefName) + return res, ErrSilent + } + } + newBranchNames := make([]string, len(newPRs)) for i, pr := range newPRs { newBranchNames[i] = pr.HeadRefName @@ -1397,6 +1416,9 @@ func pullRemoteAdditions(cfg *config.Config, sf *stack.StackFile, s *stack.Stack } if added > 0 { + // Copy the freshly fetched PR state (including the transient queued flag) + // onto the pulled branches so the rebase/push steps skip merge-queued ones. + syncRemotePRState(s, prs) updateBaseSHAs(s) if err := stack.Save(gitDir, sf); err != nil { return res, handleSaveError(cfg, err) @@ -1469,7 +1491,15 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stack.Stack, currentBranch, gitDir, remote string, prs []*github.PullRequest) (remoteReconcileResult, error) { var res remoteReconcileResult - if dirty, err := git.HasUncommittedChanges(); err == nil && dirty { + // Replacing the local stack is destructive, so require a known-clean working + // tree. Treat an inability to inspect the tree as a reason to abort (a failed + // status must not be read as "clean"). + dirty, err := git.HasUncommittedChanges() + if err != nil { + cfg.Errorf("Could not determine whether the working tree is clean: %v", err) + return res, ErrSilent + } + if dirty { cfg.Errorf("You have uncommitted changes — commit or stash them before replacing your local stack with the remote") return res, ErrSilent } @@ -1479,11 +1509,26 @@ func resolveDivergenceUseRemote(cfg *config.Config, sf *stack.StackFile, s *stac oldBranches := s.BranchNames() removeLocalStack(sf, s) + + // A remote PR branch must not already be owned by another local stack, or + // importing it would write the same branch into two stacks. Validate against + // the remaining stacks (the current one has been removed above). + for _, pr := range prs { + if err := sf.ValidateNoDuplicateBranch(pr.HeadRefName); err != nil { + cfg.Errorf("Cannot adopt the remote stack: %s", err) + return res, ErrSilent + } + } + newStack, err := importRemoteStack(cfg, sf, gitDir, remote, trunk, prs, remoteStackID) if err != nil { return res, err } + // Populate the transient queued/merged state so the rebase/push steps skip + // merge-queued or merged branches in the adopted stack. + syncRemotePRState(newStack, prs) + // If the user was on a branch that the remote stack no longer contains, // move them to the nearest surviving branch so they don't end up detached // from the stack. diff --git a/internal/tui/submitview/mouse.go b/internal/tui/submitview/mouse.go index 96588e2..924a762 100644 --- a/internal/tui/submitview/mouse.go +++ b/internal/tui/submitview/mouse.go @@ -151,7 +151,9 @@ func (m Model) leftStackButtonHit(x, y int) bool { if y-m.panelTopRow() != m.leftVisibleHeight()+1 { return false } - return x < lipgloss.Width(m.renderStackButton(leftW-2)) + // The button is rendered inside the panel's one-cell left border, so the + // hit target starts at screen column 1, not 0. + return x >= 1 && x < 1+lipgloss.Width(m.renderStackButton(leftW-2)) } // handleClick routes a left click to a branch row (left map) or an editor