diff --git a/README.md b/README.md index f1cef0a4..06ec23c4 100644 --- a/README.md +++ b/README.md @@ -381,7 +381,7 @@ A clean remote-ahead update (PRs added on top of your local stack) is pulled dow 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. +- **Unstack on GitHub and keep local** — unstacks eligible PRs and stops sync without changing local branches or their order. Merged PRs may remain in the old stack; recreate the active stack with `gh stack submit` (or run `gh stack modify` first to change its structure). If unmerged PRs remain stacked, the local association is kept and the command reports a conflict. Unlike `sync`, `submit` also creates PRs for 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. diff --git a/cmd/checkout.go b/cmd/checkout.go index 98016f45..4fe24f2c 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -580,13 +580,19 @@ func handleCompositionConflict( p := prompter.New(cfg.In, cfg.Out, cfg.Err) options := []string{ "Replace local stack with remote version", - "Delete remote stack and keep local version", + "Unstack on GitHub and keep local version", "Cancel", } - selected, err := p.Select("How would you like to resolve this?", "", options) + selectFn := p.Select + if cfg.SelectFn != nil { + selectFn = cfg.SelectFn + } + selected, err := selectFn("How would you like to resolve this?", "", options) if err != nil { if isInterruptError(err) { - clearSelectPrompt(cfg, len(options)) + if cfg.SelectFn == nil { + clearSelectPrompt(cfg, len(options)) + } printInterrupt(cfg) return nil, errInterrupt } @@ -619,28 +625,8 @@ func handleCompositionConflict( return s, nil case 1: - // Unstack the remote stack, keep local - _, dissolved, err := client.Unstack(remoteStack.Number) - if err != nil { - var httpErr *api.HTTPError - if errors.As(err, &httpErr) && httpErr.StatusCode == 404 { - cfg.Warningf("Remote stack already removed") - } else if errors.As(err, &httpErr) && httpErr.StatusCode == 422 { - cfg.Errorf("Cannot unstack remote stack: %s", httpErr.Message) - return nil, ErrAPIFailure - } else { - cfg.Errorf("failed to unstack remote stack: %v", err) - return nil, ErrAPIFailure - } - } else if dissolved { - cfg.Successf("Remote stack removed") - } else { - cfg.Warningf("Some pull requests could not be unstacked and remain on GitHub") - } - localStack.ID = "" - localStack.Number = 0 - if err := stack.Save(gitDir, sf); err != nil { - return nil, stackSaveError(cfg, err) + if err := unstackAndKeepLocal(cfg, client, sf, localStack, remoteStack, gitDir); err != nil { + return nil, err } return localStack, nil diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index e62de03b..f6623838 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -1323,6 +1323,82 @@ func TestCheckout_NumericTarget_AlreadyOnTarget(t *testing.T) { // --- Helper tests --- +func TestCheckout_CompositionConflict_KeepLocal(t *testing.T) { + for _, tc := range []struct { + name string + remaining *github.RemoteStack + apiErr error + alreadyUnstacked bool + wantErr error + }{ + {name: "dissolved"}, + {name: "merged", remaining: &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{101}}}, + {name: "already unstacked", remaining: &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{101}}, alreadyUnstacked: true}, + {name: "queued", remaining: &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{102}}, wantErr: ErrConflict}, + {name: "server rejection", apiErr: &api.HTTPError{StatusCode: 422, Message: "Pull requests cannot be removed"}, wantErr: ErrAPIFailure}, + {name: "already gone", apiErr: &api.HTTPError{StatusCode: 404}}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeStackFileMulti(t, dir, stack.Stack{ + ID: "9", Number: 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}}, + }, + }, stack.Stack{ + ID: "20", Number: 20, Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "other"}}, + }) + sf, err := stack.Load(dir) + require.NoError(t, err) + local := &sf.Stacks[0] + remote := &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{101, 102, 104}} + if tc.alreadyUnstacked { + remote = tc.remaining + } + unstacked := false + client := &github.MockClient{ + UnstackFn: func(number int) (*github.RemoteStack, bool, error) { + assert.Equal(t, 9, number) + unstacked = true + return tc.remaining, tc.remaining == nil, tc.apiErr + }, + } + cfg, outR, errR := config.NewTestConfig() + cfg.ForceInteractive = true + cfg.SelectFn = func(_, _ string, _ []string) (int, error) { return 1, nil } + prs := []*github.PullRequest{{HeadRefName: "b1"}, {HeadRefName: "b2"}, {HeadRefName: "b4"}} + result, err := handleCompositionConflict(cfg, client, sf, local, remote, prs, dir, "main") + _, output := commandOutput(t, cfg, outR, errR) + assert.Equal(t, !tc.alreadyUnstacked, unstacked) + saved, loadErr := stack.Load(dir) + require.NoError(t, loadErr) + require.Len(t, saved.Stacks, 2) + assert.Equal(t, []string{"b1", "b2", "b3"}, saved.Stacks[0].BranchNames()) + assert.Equal(t, "20", saved.Stacks[1].ID) + assert.Equal(t, []string{"other"}, saved.Stacks[1].BranchNames()) + if tc.wantErr != nil { + assert.ErrorIs(t, err, tc.wantErr) + assert.Nil(t, result) + assert.Equal(t, "9", saved.Stacks[0].ID) + assert.Equal(t, 9, saved.Stacks[0].Number) + } else { + require.NoError(t, err) + assert.Same(t, local, result) + assert.Empty(t, saved.Stacks[0].ID) + assert.Zero(t, saved.Stacks[0].Number) + if tc.remaining != nil { + assert.Contains(t, output, "remain stacked on GitHub") + assert.NotContains(t, output, "Remote stack removed") + assert.NotContains(t, output, "could not be unstacked") + } + } + }) + } +} + func TestStackCompositionMatches(t *testing.T) { tests := []struct { name string diff --git a/cmd/sync_test.go b/cmd/sync_test.go index eb23322b..1060fdd0 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2473,52 +2473,87 @@ func TestSync_Divergent_UseRemote_DirtyBlocked(t *testing.T) { assert.Equal(t, "9", sf.Stacks[0].ID) } -// 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. +// The keep-local resolution stops before pushing or recreating the stack. func TestSync_Divergent_DeleteRemote(t *testing.T) { - tmpDir := t.TempDir() - divergentStack(t, tmpDir) - - deleted := false - var deletedNumber int - var pushed bool - ghMock := divergentRemoteMock() - ghMock.UnstackFn = func(number int) (*github.RemoteStack, bool, error) { - deleted = true - deletedNumber = number - return nil, true, nil - } - ghMock.CreateStackFn = func([]int) (*github.RemoteStack, error) { - t.Fatal("CreateStack must not be called") - return nil, nil - } - ghMock.AddToStackFn = func(int, []int) (*github.RemoteStack, error) { - t.Fatal("AddToStack must not be called") - return nil, nil + mergedAt := "2026-01-01T00:00:00Z" + for _, tc := range []struct { + name string + remaining *github.RemoteStack + apiErr error + lookupFails bool + wantErr error + }{ + {name: "dissolved"}, + {name: "merged", remaining: &github.RemoteStack{ + ID: 9, Number: 9, PullRequests: []int{101}, + PRDetails: []github.RemoteStackPR{{Number: 101, MergedAt: &mergedAt}}, + }}, + {name: "queued", remaining: &github.RemoteStack{ID: 9, Number: 9, PullRequests: []int{102}}, wantErr: ErrConflict}, + {name: "server rejection", apiErr: &api.HTTPError{StatusCode: 422, Message: "Pull requests cannot be removed"}, wantErr: ErrAPIFailure}, + {name: "lookup failure", lookupFails: true, wantErr: ErrAPIFailure}, + } { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + divergentStack(t, tmpDir) + unstacked, pushed := false, false + ghMock := divergentRemoteMock() + listStacks := ghMock.ListStacksFn + lookups := 0 + ghMock.ListStacksFn = func() ([]github.RemoteStack, error) { + lookups++ + if tc.lookupFails && lookups > 1 { + return nil, assert.AnError + } + return listStacks() + } + ghMock.UnstackFn = func(number int) (*github.RemoteStack, bool, error) { + assert.Equal(t, 9, number) + unstacked = true + return tc.remaining, tc.remaining == nil, tc.apiErr + } + ghMock.CreateStackFn = func([]int) (*github.RemoteStack, error) { + t.Fatal("CreateStack must not be called") + return nil, nil + } + ghMock.AddToStackFn = func(int, []int) (*github.RemoteStack, error) { + t.Fatal("AddToStack must not be called") + return nil, 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 1, nil } + }) + + assert.Equal(t, !tc.lookupFails, unstacked) + assert.False(t, pushed) + assert.NotContains(t, output, "Stack synced") + assert.NotContains(t, output, "Branches synced") + sf, loadErr := stack.Load(tmpDir) + require.NoError(t, loadErr) + assert.Equal(t, []string{"b1", "b2", "b3"}, sf.Stacks[0].BranchNames()) + if tc.wantErr != nil { + assert.ErrorIs(t, err, tc.wantErr) + assert.Equal(t, "9", sf.Stacks[0].ID) + assert.NotContains(t, output, "gh stack submit") + } else { + require.NoError(t, err) + assert.Empty(t, sf.Stacks[0].ID) + assert.Zero(t, sf.Stacks[0].Number) + assert.Contains(t, output, "gh stack submit") + if tc.remaining != nil { + assert.Contains(t, output, "remain stacked on GitHub") + assert.NotContains(t, output, "Remote stack removed") + assert.NotContains(t, output, "only the stack on GitHub was removed") + assert.True(t, sf.Stacks[0].Branches[0].IsMerged()) + } else { + assert.Contains(t, output, "Remote stack removed") + } + } + }) } - 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 1, nil } - }) - require.NoError(t, err) - - assert.True(t, deleted, "remote stack should be deleted") - assert.Equal(t, 9, deletedNumber) - 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, "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. diff --git a/cmd/utils.go b/cmd/utils.go index 4dec9a45..c1a8f1a8 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -2000,7 +2000,7 @@ func resolveStackDivergence(cfg *config.Config, client github.ClientOps, sf *sta options := []string{ "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", + "Unstack pull requests on GitHub — keep your local stack and recreate on remote later", "Cancel — make no changes", } p := prompter.New(cfg.In, cfg.Out, cfg.Err) @@ -2136,40 +2136,54 @@ func nearestBranchAfterReplace(oldBranches []string, currentBranch string, newSt return newStack.Trunk.Branch } -// 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). +// resolveDivergenceDeleteRemote unstacks the remote PRs while keeping the local +// branch layout. It stops sync so submit can recreate the active stack. func resolveDivergenceDeleteRemote(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir string) (remoteReconcileResult, error) { res := remoteReconcileResult{stop: true} - number, err := ensureStackNumber(client, s) - if err != nil || number == 0 { - cfg.Warningf("Remote stack already deleted") - } else if _, dissolved, unstackErr := client.Unstack(number); unstackErr != nil { + remote, err := remoteStackByID(client, s.ID) + if err != nil { + cfg.Errorf("failed to look up remote stack: %v", err) + return res, ErrAPIFailure + } + if err := unstackAndKeepLocal(cfg, client, sf, s, remote, gitDir); err != nil { + return res, err + } + + cfg.Printf("") + cfg.Printf("Your local branches and stack order are unchanged.") + 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 +} + +func unstackAndKeepLocal(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, remote *github.RemoteStack, gitDir string) error { + if remote == nil { + cfg.Warningf("Remote stack already removed") + } else if prepareStackRecreation(s, remote) { + cfg.Infof("Merged pull requests remain stacked on GitHub%s", stackLabel(remote.Number)) + } else if remaining, dissolved, unstackErr := client.Unstack(remote.Number); unstackErr != nil { var httpErr *api.HTTPError if errors.As(unstackErr, &httpErr) && httpErr.StatusCode == 404 { - cfg.Warningf("Remote stack already deleted") + cfg.Warningf("Remote stack already removed") } else { - cfg.Errorf("failed to delete remote stack: %v", unstackErr) - return res, ErrAPIFailure + cfg.Errorf("failed to unstack remote stack: %v", unstackErr) + return ErrAPIFailure } + } else if !dissolved && !prepareStackRecreation(s, remaining) { + cfg.Errorf("Some pull requests still belong to the remote stack; it cannot be recreated yet") + return ErrConflict } else if dissolved { - cfg.Successf("Deleted the stack on GitHub") + cfg.Successf("Remote stack removed%s", stackLabel(remote.Number)) } else { - cfg.Warningf("Some pull requests could not be unstacked and remain on GitHub") + cfg.Successf("Pull requests unstacked on GitHub%s", stackLabel(remote.Number)) + cfg.Infof("Merged pull requests remain stacked on GitHub%s", stackLabel(remote.Number)) } s.ID = "" s.Number = 0 if err := stack.Save(gitDir, sf); err != nil { - return res, handleSaveError(cfg, err) + return stackSaveError(cfg, err) } - - 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 + return nil } diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 90d9987d..297b666d 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -250,7 +250,7 @@ If PRs are added to the stack on GitHub by someone else, `gh stack sync` fetches 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. +- **Unstack on GitHub and keep local** — unstacks eligible PRs and stops sync without changing local branches or their order. Merged PRs may remain in the old stack; recreate the active stack with `gh stack submit` (or run `gh stack modify` first to change its structure). If unmerged PRs remain stacked, the local association is kept and the command reports a conflict. Unlike `sync`, `submit` also creates PRs for 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. diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 75447576..01fb1f93 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -395,7 +395,7 @@ A clean remote-ahead update (PRs added on top of your local stack) is pulled dow 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. +- **Unstack on GitHub and keep local** — unstacks eligible PRs and stops sync without changing local branches or their order. Merged PRs may remain in the old stack; recreate the active stack with `gh stack submit` (or run `gh stack modify` first to change its structure). If unmerged PRs remain stacked, the local association is kept and the command reports a conflict. Unlike `sync`, `submit` also creates PRs for 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.