From 9d299c67c659c900d57e2980eb0299dc03269d15 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 09:32:51 -0400 Subject: [PATCH 1/9] Follow server base changes in sync and rebase --- cmd/rebase.go | 29 ++++++- cmd/rebase_test.go | 193 +++++++++++++++++++++++++++++++++++++++++ cmd/rebase_worktree.go | 8 +- cmd/sync.go | 29 ++++++- cmd/sync_test.go | 41 +++++++++ cmd/utils.go | 98 +++++++++++++++++++++ 6 files changed, 390 insertions(+), 8 deletions(-) diff --git a/cmd/rebase.go b/cmd/rebase.go index a567c119..bc9cc29a 100644 --- a/cmd/rebase.go +++ b/cmd/rebase.go @@ -26,6 +26,7 @@ type rebaseOptions struct { noTrunk bool remote string committerDateIsAuthorDate bool + acceptBaseChange bool } type rebaseState struct { @@ -34,6 +35,7 @@ type rebaseState struct { Worktrees *worktree.Context `json:"worktrees,omitempty"` StackID string `json:"stackId,omitempty"` StackTrunk string `json:"stackTrunk,omitempty"` + TargetTrunk string `json:"targetTrunk,omitempty"` StackBranches []string `json:"stackBranches,omitempty"` OriginalStack *stack.Stack `json:"originalStack,omitempty"` RebaseBase string `json:"rebaseBase,omitempty"` @@ -72,7 +74,11 @@ layer in its commit history, rebasing if necessary. Use --no-trunk to skip fetching and rebasing with the trunk branch. Only the inter-branch rebases are performed (branch 2 onto branch 1, -branch 3 onto branch 2, etc.).`, +branch 3 onto branch 2, etc.). + +If GitHub retargeted the stack because its base branch was deleted, the +stack is rebased onto the new base. Removing commits from an old base that +did not merge requires confirmation, or --accept-base-change.`, Example: ` # Rebase the entire stack $ gh stack rebase @@ -102,6 +108,7 @@ branch 3 onto branch 2, etc.).`, cmd.Flags().BoolVar(&opts.downstack, "downstack", false, "Only rebase branches from trunk to current branch") cmd.Flags().BoolVar(&opts.upstack, "upstack", false, "Only rebase branches from current branch to top") cmd.Flags().BoolVar(&opts.noTrunk, "no-trunk", false, "Skip trunk — only rebase stack branches onto each other") + cmd.Flags().BoolVar(&opts.acceptBaseChange, "accept-base-change", false, "Rebase onto a retargeted base even if the old base did not merge") cmd.Flags().BoolVar(&opts.cont, "continue", false, "Continue rebase after resolving conflicts") cmd.Flags().BoolVar(&opts.abort, "abort", false, "Abort rebase and restore all branches") cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to fetch from (defaults to auto-detected remote)") @@ -192,6 +199,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { } var trunk trunkTarget + var newBase, cutoff string if !opts.noTrunk { // Resolve remote for fetch and trunk comparison remote, err := pickRemote(cfg, anchor, opts.remote) @@ -201,6 +209,14 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { } return ErrSilent } + if newBase, err = retargetedBase(cfg, s, remote); err != nil { + cfg.Errorf("failed to read the stack's base on GitHub: %v", err) + return ErrAPIFailure + } + if i := s.FirstActiveBranchIndex(); newBase != "" && i >= 0 && (i < startIdx || i >= endIdx) { + cfg.Errorf("rebase the whole stack to move it onto %s", newBase) + return ErrInvalidArgs + } // Fast-forward stack branches that are behind their remote tracking branch. if err := git.FetchBranches(remote, activeBranchNames(s)); err != nil { @@ -215,10 +231,15 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { cfg.Errorf("%s", err) return ErrSilent } - trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{Worktrees: ctx}) + trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{Worktrees: ctx, Branch: newBase}) if err != nil { return err } + if newBase != "" { + if cutoff, err = retargetCutoff(cfg, s, trunk, opts.acceptBaseChange); err != nil { + return err + } + } if _, err := fastForwardBranches(cfg, planned, ctx); err != nil { cfg.Errorf("%s", err) return ErrSilent @@ -254,11 +275,15 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { ontoOldBase = originalRefs[prev.Branch] } } + if cutoff != "" { + needsOnto, ontoOldBase = true, cutoff + } state := newWorktreeRebaseState(s, ctx, currentBranch, originalRefs, trunk, startIdx, endIdx) state.CommitterDateIsAuthorDate = opts.committerDateIsAuthorDate state.NoTrunk = opts.noTrunk state.UseOnto, state.OntoOldBase = needsOnto, ontoOldBase + state.TargetTrunk = newBase if s.Trunk.Branch != originalTrunk { if err := stack.Save(gitDir, sf); err != nil { return handleSaveError(cfg, err) diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 308aa34a..1139dfa0 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "github.com/AlecAivazis/survey/v2/terminal" "github.com/github/gh-stack/internal/config" "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/github" @@ -3822,3 +3823,195 @@ func TestRebase_LegacyCompletionRetainsRetryState(t *testing.T) { }) } } + +type baseRetargetRepo struct { + amendedParentRepo + target string + remote *github.RemoteStack + client *github.MockClient +} + +// setupBaseRetargetRepo builds child <- top on a deleted parent branch whose +// stack GitHub retargeted to target. When merged, target has the squashed parent. +func setupBaseRetargetRepo(t *testing.T, target string, merged bool) baseRetargetRepo { + t.Helper() + repo := setupAmendedParentRepo(t, false) + issue250Git(t, repo.dir, "checkout", "-b", "top") + issue250WriteFile(t, repo.dir, "top.txt", "top\n") + issue250Git(t, repo.dir, "add", ".") + issue250Git(t, repo.dir, "commit", "-m", "top commit") + issue250Git(t, repo.dir, "push", "-u", "origin", "top") + issue250Git(t, repo.dir, "checkout", "-B", target, "main") + if merged { + issue250Git(t, repo.dir, "merge", "--squash", "origin/parent") + issue250Git(t, repo.dir, "commit", "-m", "squashed parent") + } + issue250Git(t, repo.dir, "push", "origin", target) + issue250Git(t, repo.dir, "push", "origin", "--delete", "parent") + issue250Git(t, repo.dir, "checkout", "top") + child := issue250Git(t, repo.dir, "rev-parse", "child") + writeStackFile(t, repo.gitDir, stack.Stack{ + ID: "9", Number: 9, + Trunk: stack.BranchRef{Branch: "parent", Head: repo.oldParent}, + Branches: []stack.BranchRef{ + {Branch: "child", Base: repo.oldParent, Head: child, PullRequest: &stack.PullRequestRef{Number: 11}}, + {Branch: "top", Base: child, Head: issue250Git(t, repo.dir, "rev-parse", "top"), PullRequest: &stack.PullRequestRef{Number: 12}}, + }, + }) + remote := &github.RemoteStack{ID: 9, Number: 9, Base: github.RemoteStackBase{Ref: target}, PullRequests: []int{11, 12}} + forbid := func(string) { t.Fatal("following a retarget must not restructure the stack on GitHub") } + client := &github.MockClient{ + ListStacksFn: func() ([]github.RemoteStack, error) { return []github.RemoteStack{*remote}, nil }, + GetStackFn: func(int) (*github.RemoteStack, error) { return remote, nil }, + FindPRByNumberFn: func(n int) (*github.PullRequest, error) { + heads := map[int]string{11: "child", 12: "top"} + return &github.PullRequest{Number: n, HeadRefName: heads[n], State: "OPEN"}, nil + }, + FindPRDetailsForBranchFn: func(branch string) (*github.PRDetails, error) { + return &github.PRDetails{Number: 10, Merged: merged && branch == "parent"}, nil + }, + UnstackFn: func(int) (*github.RemoteStack, bool, error) { forbid("unstack"); return nil, false, nil }, + UpdatePRBaseFn: func(int, string) error { forbid("base"); return nil }, + CreateStackFn: func([]int) (*github.RemoteStack, error) { forbid("create"); return nil, nil }, + } + return baseRetargetRepo{repo, target, remote, client} +} + +func assertBaseRetargetHistory(t *testing.T, repo baseRetargetRepo) { + t.Helper() + assert.Equal(t, "top commit\nchild commit", issue250Git(t, repo.dir, "log", "--format=%s", repo.target+"..top")) + assert.Error(t, issue250GitMayFail(t, repo.dir, "merge-base", "--is-ancestor", repo.oldParent, "child")) + assert.Equal(t, repo.newParent, issue250Git(t, repo.dir, "rev-parse", "parent"), "the old local base is kept") + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + s := sf.Stacks[0] + assert.Equal(t, "9", s.ID) + assert.Equal(t, repo.target, s.Trunk.Branch) + target := issue250Git(t, repo.dir, "rev-parse", repo.target) + assert.Equal(t, target, s.Trunk.Head) + assert.Equal(t, target, s.Branches[0].Base) +} + +func TestRebase_BaseRetarget(t *testing.T) { + for _, target := range []string{"main", "release"} { + t.Run(target, func(t *testing.T) { + repo := setupBaseRetargetRepo(t, target, true) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + + require.NoError(t, runRebase(cfg, &rebaseOptions{remote: "origin"})) + assertBaseRetargetHistory(t, repo) + top := issue250Git(t, repo.dir, "rev-parse", "top") + require.NoError(t, runRebase(cfg, &rebaseOptions{remote: "origin"})) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) + }) + } +} + +func TestRebase_BaseRetargetConsent(t *testing.T) { + for _, mode := range []string{"non-interactive", "decline", "interrupt", "upstack", "accept", "flag"} { + t.Run(mode, func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", false) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + cfg.ForceInteractive = mode == "decline" || mode == "interrupt" || mode == "accept" + cfg.ConfirmFn = func(_ string, defaultValue bool) (bool, error) { + assert.False(t, defaultValue) + if mode == "interrupt" { + return false, terminal.InterruptErr + } + return mode == "accept", nil + } + catalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + top := issue250Git(t, repo.dir, "rev-parse", "top") + + err = runRebase(cfg, &rebaseOptions{remote: "origin", upstack: mode == "upstack", acceptBaseChange: mode == "flag"}) + if mode == "accept" || mode == "flag" { + require.NoError(t, err) + assertBaseRetargetHistory(t, repo) + assert.NoFileExists(t, filepath.Join(repo.dir, "old-parent.txt")) + return + } + require.Error(t, err) + after, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, string(catalog), string(after)) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + }) + } +} + +func TestRebase_BaseRetargetRecovery(t *testing.T) { + for _, action := range []string{"continue", "abort"} { + t.Run(action, func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", true) + issue250Git(t, repo.dir, "checkout", "main") + issue250WriteFile(t, repo.dir, "child.txt", "conflict\n") + issue250Git(t, repo.dir, "add", ".") + issue250Git(t, repo.dir, "commit", "-m", "conflict") + issue250Git(t, repo.dir, "push", "origin", "main") + issue250Git(t, repo.dir, "checkout", "top") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + top := issue250Git(t, repo.dir, "rev-parse", "top") + + require.ErrorIs(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), ErrConflict) + state, err := loadRebaseState(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "main", state.TargetTrunk) + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch, "the trunk changes only after the rebase completes") + + // Recovery keeps the target accepted when the rebase started. + repo.remote.Base.Ref = "later-base" + if action == "continue" { + issue250WriteFile(t, repo.dir, "child.txt", "resolved\n") + issue250Git(t, repo.dir, "add", "child.txt") + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) + assertBaseRetargetHistory(t, repo) + } else { + require.NoError(t, runRebase(cfg, &rebaseOptions{abort: true})) + sf, err = stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch) + assert.Equal(t, repo.oldParent, sf.Stacks[0].Branches[0].Base) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) + } + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + }) + } +} + +// A crash after publishing the new trunk leaves a completed journal behind. +func TestRebase_BaseRetargetCompletedJournal(t *testing.T) { + for _, action := range []string{"continue", "abort"} { + t.Run(action, func(t *testing.T) { + dir := t.TempDir() + original := stack.Stack{Trunk: stack.BranchRef{Branch: "parent", Head: "old-parent"}, Branches: []stack.BranchRef{{Branch: "b1", Base: "old-parent"}}} + published := original + published.Trunk.Branch = "main" + writeStackFile(t, dir, published) + restore := git.SetOps(newRebaseMock(dir, "b1")) + defer restore() + ctx, err := worktree.New() + require.NoError(t, err) + state := newWorktreeRebaseState(&original, ctx, "b1", map[string]string{"b1": "sha-b1"}, trunkTarget{Branch: "main", Ref: "main", SHA: "sha-main"}, 0, 1) + state.TargetTrunk = "main" + state.Phase, state.CurrentBranchIndex = "complete", state.EndIndex + require.NoError(t, saveRebaseState(dir, state)) + + require.NoError(t, runRebase(issue250TestConfig(t), &rebaseOptions{cont: action == "continue", abort: action == "abort"})) + sf, err := stack.Load(dir) + require.NoError(t, err) + want := map[string]string{"continue": "main", "abort": "parent"}[action] + assert.Equal(t, want, sf.Stacks[0].Trunk.Branch) + assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) + }) + } +} diff --git a/cmd/rebase_worktree.go b/cmd/rebase_worktree.go index cbb36c2e..f078f108 100644 --- a/cmd/rebase_worktree.go +++ b/cmd/rebase_worktree.go @@ -125,7 +125,7 @@ func restoreWorktreeRebaseMetadata(dir string, state *rebaseState) error { if !slices.Equal(state.OriginalStack.BranchNames(), target.BranchNames()) { return fmt.Errorf("original stack snapshot does not match the recovery target") } - target.Trunk.Head = state.OriginalStack.Trunk.Head + target.Trunk = state.OriginalStack.Trunk for i, before := range state.OriginalStack.Branches { target.Branches[i].Base = before.Base target.Branches[i].Head = before.Head @@ -147,7 +147,8 @@ func rebaseStackFromState(sf *stack.StackFile, state *rebaseState) (*stack.Stack var target *stack.Stack for i := range sf.Stacks { s := &sf.Stacks[i] - if s.Trunk.Branch != state.StackTrunk || !slices.Equal(s.BranchNames(), state.StackBranches) { + trunk := s.Trunk.Branch + if (trunk != state.StackTrunk && trunk != state.TargetTrunk) || !slices.Equal(s.BranchNames(), state.StackBranches) { continue } if (state.Phase == "applying" || state.Phase == "conflict") && state.StackID != "" && s.ID != "" && state.StackID != s.ID { @@ -328,6 +329,9 @@ func finishWorktreeRebase(cfg *config.Config, dir string, state *rebaseState, sf } func publishCompletedRebase(cfg *config.Config, dir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error { + if state.TargetTrunk != "" { + s.Trunk.Branch = state.TargetTrunk + } updateBaseSHAsWithTrunk(s, state.TrunkSHA) state.Phase = "complete" state.CurrentBranchIndex = state.EndIndex diff --git a/cmd/sync.go b/cmd/sync.go index d3800a13..dfffc3a4 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -15,8 +15,9 @@ import ( ) type syncOptions struct { - remote string - prune bool + remote string + prune bool + acceptBaseChange bool } func SyncCmd(cfg *config.Config) *cobra.Command { @@ -60,6 +61,10 @@ 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). +If GitHub retargeted the stack because its base branch was deleted, sync +rebases the stack onto the new base. Removing commits from an old base that +did not merge requires confirmation, or --accept-base-change. + Use --prune to delete local branches for merged PRs. Stack metadata is preserved so that rebase and display logic continue to work correctly. If you are on a branch that would be pruned, your checkout is moved to @@ -71,6 +76,7 @@ the first active branch in the stack, or the trunk if all are merged.`, cmd.Flags().StringVar(&opts.remote, "remote", "", "Remote to fetch from and push to (defaults to auto-detected remote)") cmd.Flags().BoolVar(&opts.prune, "prune", false, "Delete local branches for merged PRs") + cmd.Flags().BoolVar(&opts.acceptBaseChange, "accept-base-change", false, "Rebase onto a retargeted base even if the old base did not merge") return cmd } @@ -151,6 +157,11 @@ func runSync(cfg *config.Config, opts *syncOptions) error { currentBranch = cb } _ = syncStackPRs(cfg, s) + newBase, err := retargetedBase(cfg, s, remote) + if err != nil { + cfg.Errorf("failed to read the stack's base on GitHub: %v", err) + return ErrAPIFailure + } ctx, err := worktree.New() if err != nil { cfg.Errorf("%s", err) @@ -161,12 +172,13 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // --- Step 2: Resolve trunk --- trunk, err := resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{ Worktrees: ctx, + Branch: newBase, Preflight: func(sha string, moved bool) error { var required []string for _, forward := range planned { required = append(required, forward.Branch) } - if moved || len(planned) > 0 || stackNeedsRebase(s, sha) { + if newBase != "" || moved || len(planned) > 0 || stackNeedsRebase(s, sha) { required = append(required, activeBranchNames(s)...) } if err := ctx.Preflight(required); err != nil { @@ -179,6 +191,12 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if err != nil { return err } + var cutoff string + if newBase != "" { + if cutoff, err = retargetCutoff(cfg, s, trunk, opts.acceptBaseChange); err != nil { + return err + } + } // --- Step 2b: Fast-forward stack branches behind their remote tracking branch --- updatedBranches, err := fastForwardBranches(cfg, planned, ctx) @@ -188,7 +206,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { } // --- Step 3: Cascade rebase --- - needsRebase := trunk.Moved || len(updatedBranches) > 0 || stackNeedsRebase(s, trunk.Ref) + needsRebase := newBase != "" || trunk.Moved || len(updatedBranches) > 0 || stackNeedsRebase(s, trunk.Ref) rebased := false var originalRefs map[string]string var state *rebaseState @@ -202,6 +220,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { return ErrSilent } else { state = newWorktreeRebaseState(s, ctx, currentBranch, originalRefs, trunk, 0, len(s.Branches)) + state.TargetTrunk = newBase if s.Trunk.Branch != originalTrunk { if err := stack.Save(gitDir, sf); err != nil { return handleSaveError(cfg, err) @@ -217,6 +236,8 @@ func runSync(cfg *config.Config, opts *syncOptions) error { Branches: s.Branches, StartAbsIdx: 0, OriginalRefs: originalRefs, + NeedsOnto: cutoff != "", + OntoOldBase: cutoff, TrunkRef: trunk.Ref, TrunkSHA: trunk.SHA, Worktrees: ctx, diff --git a/cmd/sync_test.go b/cmd/sync_test.go index b8e81a70..3158345a 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2857,3 +2857,44 @@ func TestSync_RollbackFailureRetainsRecovery(t *testing.T) { assert.Equal(t, "b2", current) assert.NoFileExists(t, filepath.Join(dir, rebaseStateFile)) } + +func TestSync_BaseRetarget(t *testing.T) { + for _, merged := range []bool{true, false} { + t.Run(fmt.Sprintf("merged=%t", merged), func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", merged) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + + require.NoError(t, runSync(cfg, &syncOptions{remote: "origin", acceptBaseChange: !merged})) + assertBaseRetargetHistory(t, repo) + assert.Equal(t, issue250Git(t, repo.dir, "rev-parse", "top"), issue250Git(t, repo.dir, "rev-parse", "origin/top")) + top := issue250Git(t, repo.dir, "rev-parse", "top") + require.NoError(t, runSync(cfg, &syncOptions{remote: "origin"})) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) + }) + } +} + +func TestSync_BaseRetargetConflictKeepsOldBase(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", true) + issue250Git(t, repo.dir, "checkout", "main") + issue250WriteFile(t, repo.dir, "child.txt", "conflict\n") + issue250Git(t, repo.dir, "add", ".") + issue250Git(t, repo.dir, "commit", "-m", "conflict") + issue250Git(t, repo.dir, "push", "origin", "main") + issue250Git(t, repo.dir, "checkout", "top") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + top := issue250Git(t, repo.dir, "rev-parse", "top") + + require.ErrorIs(t, runSync(cfg, &syncOptions{remote: "origin"}), ErrConflict) + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch) + assert.Equal(t, repo.oldParent, sf.Stacks[0].Branches[0].Base) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "origin/top")) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) +} diff --git a/cmd/utils.go b/cmd/utils.go index 70cf45de..5ea62491 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1087,6 +1087,100 @@ func normalizeStackTrunk(cfg *config.Config, s *stack.Stack, remote string) erro return nil } +// retargetedBase returns the branch GitHub moved a tracked stack onto, or "" +// if the base is unchanged. GitHub retargets a stack when its base branch is +// deleted but does not rebase the stack's branches. +func retargetedBase(cfg *config.Config, s *stack.Stack, remote string) (string, error) { + if s.ID == "" { + return "", nil + } + client, err := cfg.GitHubClient() + if err != nil { + return "", err + } + number, err := ensureStackNumber(client, s) + if err != nil || number == 0 { + return "", err + } + rs, err := client.GetStack(number) + if isNotFound(err) { + return "", nil + } + if err != nil || rs == nil || rs.Base.Ref == "" { + return "", err + } + trunk, err := normalizeTrunkBranch(s.Trunk.Branch, remote) + if err != nil || rs.Base.Ref == trunk { + return "", err + } + cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, s.Number), s.Trunk.Branch, rs.Base.Ref) + return rs.Base.Ref, nil +} + +// requireCurrentBase stops publishing a stack that GitHub retargeted until it +// has been rebased onto its new base. +func requireCurrentBase(cfg *config.Config, s *stack.Stack, remote string) error { + newBase, err := retargetedBase(cfg, s, remote) + if err != nil { + cfg.Errorf("failed to read the stack's base on GitHub: %v", err) + return ErrAPIFailure + } + if newBase != "" { + cfg.Errorf("run `%s` to rebase the stack onto %s before publishing", cfg.ColorCyan("gh stack sync"), newBase) + return ErrSilent + } + return nil +} + +// retargetCutoff returns the old base commit for rebasing the bottom branch +// onto a retargeted base, or "" when merged branches below it already set the +// cutoff. Dropping commits that are not known to have landed in the new base +// requires consent. +func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, accept bool) (string, error) { + i := s.FirstActiveBranchIndex() + if i < 0 { + return "", nil + } + var cutoff string + landed := false + if i == 0 { + b := s.Branches[0] + oldTip, _ := git.RevParse(s.Trunk.Branch) + var err error + if cutoff, err = resolveRebaseOldBase(oldTip, b.Base, target.Ref, b.Branch); err == nil { + landed, err = git.IsAncestor(cutoff, target.SHA) + } + if err != nil { + cfg.Errorf("%s", err) + return "", ErrSilent + } + } + if !landed && !accept { + if client, err := cfg.GitHubClient(); err == nil { + pr, err := client.FindPRDetailsForBranch(s.Trunk.Branch) + landed = err == nil && pr != nil && pr.Merged + } + } + if landed || accept { + return cutoff, nil + } + cfg.Warningf("%s is not known to have merged into %s; rebasing onto %s removes its commits from this stack", + s.Trunk.Branch, target.Branch, target.Branch) + if cfg.IsInteractive() { + ok, err := confirmPrune(cfg, "Rebase onto "+target.Branch+" without them?", false) + if isInterruptError(err) { + printInterrupt(cfg) + return "", ErrSilent + } + if err == nil && ok { + return cutoff, nil + } + cfg.Canceled = true + } + cfg.Errorf("stack not rebased; rerun with --accept-base-change to rebase onto %s", target.Branch) + return "", ErrSilent +} + type trunkTarget struct { Branch string Ref string @@ -1097,6 +1191,7 @@ type trunkTarget struct { type trunkResolveOptions struct { Worktrees *worktree.Context Preflight func(string, bool) error + Branch string // overrides the stack's trunk } func (t trunkTarget) Describe() string { @@ -1123,6 +1218,9 @@ func resolveTrunkTarget(cfg *config.Config, s *stack.Stack, remote, currentBranc return trunkTarget{}, err } trunk := s.Trunk.Branch + if opts.Branch != "" { + trunk = opts.Branch + } remoteRef := remote + "/" + trunk if err := git.FetchBranch(remote, trunk); err != nil { From 274f0a03a6cd001f27cf06899102eb24a9fa7485 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 09:32:55 -0400 Subject: [PATCH 2/9] Handle retargets across stack commands --- cmd/checkout.go | 11 ++++++++++- cmd/checkout_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ cmd/push.go | 3 +++ cmd/push_test.go | 20 ++++++++++++++++++++ cmd/submit.go | 3 +++ cmd/submit_test.go | 30 ++++++++++++++++++++++++++++++ cmd/view.go | 23 +++++++++++++++++++---- cmd/view_test.go | 24 ++++++++++++++++++++++-- 8 files changed, 148 insertions(+), 7 deletions(-) diff --git a/cmd/checkout.go b/cmd/checkout.go index 61c28b5b..d9888576 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -415,6 +415,10 @@ func checkoutStackByNumber(cfg *config.Config, sf *stack.StackFile, gitDir strin // importing the stack from the remote — and returns the resolved local stack // and the branch to check out. func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, gitDir string, remoteStack *github.RemoteStack, prs []*github.PullRequest, trunk, targetBranch string) (*stack.Stack, string, error) { + // A merged first PR keeps its old base after GitHub retargets the stack. + if remoteStack.Base.Ref != "" { + trunk = remoteStack.Base.Ref + } allMerged := true for _, pr := range prs { if !pr.Merged { @@ -475,7 +479,12 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, if err := stack.Save(gitDir, sf); err != nil { return nil, "", stackSaveError(cfg, err) } - cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number)) + if localStack.Trunk.Branch != trunk { + cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, trunk) + cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), trunk) + } else { + cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number)) + } return localStack, targetBranch, nil } diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index 4fb08707..35c3df36 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -1667,3 +1667,44 @@ func TestCheckout_ByPRURL_Remote(t *testing.T) { assert.Equal(t, "feat-2", checkedOut) assert.Contains(t, output, "Imported stack with 2 branches") } + +func TestCheckout_BaseRetarget(t *testing.T) { + t.Run("reports a tracked stack", func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", false) + withIssue250Repo(t, repo.dir) + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = repo.client + + require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "9"})) + _, diagnostics := commandOutput(t, cfg, outR, errR) + assert.Contains(t, diagnostics, "from parent to main") + assert.NotContains(t, diagnostics, "matches remote") + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch) + }) + + t.Run("imports onto the stack base", func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", true) + withIssue250Repo(t, repo.dir) + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + sf.Stacks = nil + require.NoError(t, stack.Save(repo.gitDir, sf)) + repo.remote.PullRequests = []int{10, 11, 12} + repo.client.FindPRByNumberFn = func(n int) (*github.PullRequest, error) { + if n == 10 { + return &github.PullRequest{Number: 10, HeadRefName: "parent-old", BaseRefName: "deleted", State: "MERGED", Merged: true}, nil + } + heads := map[int]string{11: "child", 12: "top"} + return &github.PullRequest{Number: n, HeadRefName: heads[n], BaseRefName: "parent", State: "OPEN"}, nil + } + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + + require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "9"})) + sf, err = stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, "main", sf.Stacks[0].Trunk.Branch) + }) +} diff --git a/cmd/push.go b/cmd/push.go index 9455a309..38529f41 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -84,6 +84,9 @@ func runPush(cfg *config.Config, opts *pushOptions) error { } return ErrSilent } + if err := requireCurrentBase(cfg, s, remote); err != nil { + return err + } // Sync PR state to detect merged/queued PRs before pushing. _ = syncStackPRs(cfg, s) diff --git a/cmd/push_test.go b/cmd/push_test.go index 059b003d..15fa1875 100644 --- a/cmd/push_test.go +++ b/cmd/push_test.go @@ -404,3 +404,23 @@ func TestPickRemote_OverrideTakesPrecedence(t *testing.T) { assert.Equal(t, "custom", remote) assert.False(t, resolveCalled, "ResolveRemote should not be called when override is provided") } + +func TestPush_BaseRetarget(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "parent"}, Branches: []stack.BranchRef{{Branch: "child"}}}) + mock := newPushMock(dir, "child") + mock.PushFn = func(string, []string, bool, bool) error { + t.Fatal("must not push before rebasing onto the new base") + return nil + } + restore := git.SetOps(mock) + defer restore() + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + GetStackFn: func(int) (*github.RemoteStack, error) { + return &github.RemoteStack{ID: 9, Base: github.RemoteStackBase{Ref: "main"}}, nil + }, + } + + assert.ErrorIs(t, runPush(cfg, &pushOptions{remote: "origin"}), ErrSilent) +} diff --git a/cmd/submit.go b/cmd/submit.go index 36fe8c82..4151efdf 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -156,6 +156,9 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { } return ErrSilent } + if err := requireCurrentBase(cfg, s, remote); err != nil { + return err + } merged := s.MergedBranches() if len(merged) > 0 { cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches")) diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 2f4dfa96..62868390 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -3145,3 +3145,33 @@ func TestEnsurePR_DeselectedMissingPRIsIncompleteWithCachedAssociation(t *testin assert.False(t, submitted, "a cached PR count must not turn a deselected missing PR into success") commandOutput(t, cfg, outR, errR) } + +func TestSubmit_BaseRetarget(t *testing.T) { + for name, tt := range map[string]struct { + err error + want error + }{ + "retargeted": {want: ErrSilent}, + "lookup failure": {err: fmt.Errorf("unavailable"), want: ErrAPIFailure}, + } { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "parent"}, Branches: []stack.BranchRef{{Branch: "child"}}}) + mock := newSubmitMock(dir, "child") + mock.PushFn = func(string, []string, bool, bool) error { + t.Fatal("must not push before rebasing onto the new base") + return nil + } + restore := git.SetOps(mock) + defer restore() + cfg, _, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + GetStackFn: func(int) (*github.RemoteStack, error) { + return &github.RemoteStack{ID: 9, Base: github.RemoteStackBase{Ref: "main"}}, tt.err + }, + } + + assert.ErrorIs(t, runSubmit(cfg, &submitOptions{auto: true, remote: "origin"}), tt.want) + }) + } +} diff --git a/cmd/view.go b/cmd/view.go index 548f0488..f78eb3a7 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -75,6 +75,7 @@ func runView(cfg *config.Config, opts *viewOptions) error { sf := result.StackFile s := result.Stack currentBranch := result.CurrentBranch + warnRetargeted(cfg, s) // Show loading indicator for interactive TUI mode. showingLoader := false @@ -130,10 +131,20 @@ func runViewJSON(cfg *config.Config) error { } s := stacks[0] + retargeted := warnRetargeted(cfg, s) syncStackPRs(cfg, s) stack.SaveNonBlocking(gitDir, sf) - return viewJSON(cfg, s, currentBranch) + return viewJSON(cfg, s, currentBranch, retargeted) +} + +// warnRetargeted reports a GitHub retarget that has not been rebased onto yet. +func warnRetargeted(cfg *config.Config, s *stack.Stack) bool { + newBase, _ := retargetedBase(cfg, s, "") + if newBase != "" { + cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), newBase) + } + return newBase != "" } func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { @@ -229,14 +240,18 @@ type viewJSONPR struct { State string `json:"state"` } -func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error { +func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string, retargeted bool) error { out := viewJSONOutput{ Trunk: s.Trunk.Branch, CurrentBranch: currentBranch, Branches: make([]viewJSONBranch, 0, len(s.Branches)), } + bottom := -1 + if retargeted { + bottom = s.FirstActiveBranchIndex() + } - for _, b := range s.Branches { + for i, b := range s.Branches { jb := viewJSONBranch{ Name: b.Branch, Head: b.Head, @@ -249,7 +264,7 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error { // Check if the branch needs rebasing (base not ancestor of branch). if !jb.IsMerged { baseBranch := s.ActiveBaseBranch(b.Branch) - if isAnc, err := git.IsAncestor(baseBranch, b.Branch); err == nil && !isAnc { + if isAnc, err := git.IsAncestor(baseBranch, b.Branch); i == bottom || (err == nil && !isAnc) { jb.NeedsRebase = true } } diff --git a/cmd/view_test.go b/cmd/view_test.go index d8410f8f..a4546dae 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -148,7 +148,7 @@ func TestViewJSON(t *testing.T) { defer outR.Close() tt.stack.Number = 7 - err := viewJSON(cfg, tt.stack, tt.currentBranch) + err := viewJSON(cfg, tt.stack, tt.currentBranch, false) require.NoError(t, err) cfg.Out.Close() @@ -206,7 +206,7 @@ func TestViewJSON_BranchFields(t *testing.T) { cfg, outR, _ := config.NewTestConfig() defer outR.Close() - err := viewJSON(cfg, s, "feat/02") + err := viewJSON(cfg, s, "feat/02", false) require.NoError(t, err) cfg.Out.Close() @@ -622,3 +622,23 @@ func TestRunViewJSON_SharedCatalogDuringMutation(t *testing.T) { assert.Equal(t, before, after, "read-only metadata refresh cannot overwrite a mutation's catalog") assert.NoFileExists(t, filepath.Join(local, "gh-stack")) } + +func TestView_BaseRetarget(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", false) + withIssue250Repo(t, repo.dir) + catalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = repo.client + + require.NoError(t, runView(cfg, &viewOptions{asJSON: true})) + out, diagnostics := commandOutput(t, cfg, outR, errR) + var data viewJSONOutput + require.NoError(t, json.Unmarshal([]byte(out), &data)) + assert.Equal(t, "parent", data.Trunk) + assert.True(t, data.Branches[0].NeedsRebase, "the bottom branch must be rebased onto the new base") + assert.Contains(t, diagnostics, "from parent to main") + after, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + assert.Equal(t, string(catalog), string(after)) +} From 0a4590ff7679d26d97a44a59c72b349af8afa676 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 09:32:55 -0400 Subject: [PATCH 3/9] Document automatic base retargeting --- docs/src/content/docs/reference/cli.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 97047dd9..6368d0c5 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -384,6 +384,7 @@ gh stack sync [flags] |------|-------------| | `--remote ` | Remote to fetch from and push to (defaults to auto-detected remote) | | `--prune` | Delete local branches for merged PRs | +| `--accept-base-change` | Rebase onto a base GitHub retargeted the stack to, even if the old base did not merge | Performs a synchronization of the entire stack: @@ -396,7 +397,11 @@ Performs a synchronization of the entire stack: 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. +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, or before removing an unmerged base's commits (see **Retargeted stacks** below). + +**Retargeted stacks** + +When a stack's base branch is deleted, GitHub moves the stack onto a new base but does not rebase its branches. `sync` and `rebase` rebase the stack onto that base, keep its stack and PR numbers, and drop the old base's commits. If the old base did not merge into the new base, they ask before dropping its commits; in a non-interactive terminal, pass `--accept-base-change`. Until the stack is rebased, `push` and `submit` stop, and `view` reports the new base. **Diverged stacks** @@ -430,6 +435,7 @@ gh stack rebase [flags] [branch] | `--downstack` | Only rebase branches from trunk to the current branch | | `--upstack` | Only rebase branches from the current branch to the top | | `--no-trunk` | Skip trunk — only rebase stack branches onto each other (no fetch, no trunk rebase) | +| `--accept-base-change` | Rebase onto a base GitHub retargeted the stack to, even if the old base did not merge (see [`gh stack sync`](#gh-stack-sync)) | | `--continue` | Continue the rebase after resolving conflicts | | `--abort` | Abort the rebase and restore all branches to their pre-rebase state | | `--remote ` | Remote to fetch from (defaults to auto-detected remote) | From ad5706f301491c270903d6d77845d792ccafc13d Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 10:29:40 -0400 Subject: [PATCH 4/9] Validate retarget cutoffs and recovery --- cmd/rebase.go | 2 +- cmd/rebase_test.go | 127 ++++++++++++++++++++++++++++++++- cmd/sync.go | 3 +- cmd/sync_test.go | 44 ++++++++++++ cmd/utils.go | 127 +++++++++++++++++++++++++++++---- internal/github/github.go | 54 ++++++++------ internal/github/github_test.go | 46 ++++++++++++ 7 files changed, 363 insertions(+), 40 deletions(-) diff --git a/cmd/rebase.go b/cmd/rebase.go index bc9cc29a..6e29d65e 100644 --- a/cmd/rebase.go +++ b/cmd/rebase.go @@ -236,7 +236,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { return err } if newBase != "" { - if cutoff, err = retargetCutoff(cfg, s, trunk, opts.acceptBaseChange); err != nil { + if cutoff, err = retargetCutoff(cfg, s, trunk, planned, opts.acceptBaseChange); err != nil { return err } } diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 1139dfa0..86770c4d 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -3846,6 +3846,7 @@ func setupBaseRetargetRepo(t *testing.T, target string, merged bool) baseRetarge issue250Git(t, repo.dir, "merge", "--squash", "origin/parent") issue250Git(t, repo.dir, "commit", "-m", "squashed parent") } + mergeCommit := issue250Git(t, repo.dir, "rev-parse", target) issue250Git(t, repo.dir, "push", "origin", target) issue250Git(t, repo.dir, "push", "origin", "--delete", "parent") issue250Git(t, repo.dir, "checkout", "top") @@ -3868,7 +3869,10 @@ func setupBaseRetargetRepo(t *testing.T, target string, merged bool) baseRetarge return &github.PullRequest{Number: n, HeadRefName: heads[n], State: "OPEN"}, nil }, FindPRDetailsForBranchFn: func(branch string) (*github.PRDetails, error) { - return &github.PRDetails{Number: 10, Merged: merged && branch == "parent"}, nil + return &github.PRDetails{ + Number: 10, Merged: merged && branch == "parent", + HeadRefOID: repo.oldParent, MergeCommitOID: mergeCommit, + }, nil }, UnstackFn: func(int) (*github.RemoteStack, bool, error) { forbid("unstack"); return nil, false, nil }, UpdatePRBaseFn: func(int, string) error { forbid("base"); return nil }, @@ -4015,3 +4019,124 @@ func TestRebase_BaseRetargetCompletedJournal(t *testing.T) { }) } } + +func TestRebase_BaseRetargetReviewCases(t *testing.T) { + testBaseRetargetReviewCases(t, func(cfg *config.Config, accept bool) error { + return runRebase(cfg, &rebaseOptions{remote: "origin", acceptBaseChange: accept}) + }) +} + +func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) error) { + t.Helper() + for _, scenario := range []string{ + "stale local trunk", "old trunk reflog", "missing old trunk", + "missing cutoff", "planned fast-forward", "merged prefix", + "merged elsewhere", "commits after merge", + } { + t.Run(scenario, func(t *testing.T) { + target := "main" + if scenario == "merged elsewhere" { + target = "release" + } + repo := setupBaseRetargetRepo(t, target, scenario == "merged elsewhere" || scenario == "commits after merge") + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + s := &sf.Stacks[0] + child := issue250Git(t, repo.dir, "rev-parse", "child") + wantConsent := false + accept := false + switch scenario { + case "stale local trunk": + issue250Git(t, repo.dir, "branch", "-f", "parent", "main") + wantConsent = true + case "old trunk reflog", "missing cutoff": + s.Trunk.Head = repo.newParent + s.Branches[0].Base = repo.newParent + accept = true + if scenario == "missing cutoff" { + issue250Git(t, repo.dir, "reflog", "expire", "--expire=now", "--all") + } + case "missing old trunk": + issue250Git(t, repo.dir, "branch", "-D", "parent") + accept = true + case "planned fast-forward": + // The fetched child contains a parent update that the local child + // has not seen yet; consent must cover the post-fast-forward history. + issue250Git(t, repo.dir, "branch", "-f", "main", repo.oldParent) + issue250Git(t, repo.dir, "push", "origin", "main") + issue250Git(t, repo.dir, "checkout", "-B", "parent", repo.oldParent) + issue250WriteFile(t, repo.dir, "parent-extra.txt", "unmerged parent update\n") + issue250Git(t, repo.dir, "add", ".") + issue250Git(t, repo.dir, "commit", "-m", "parent extra") + issue250Git(t, repo.dir, "checkout", "child") + issue250Git(t, repo.dir, "merge", "--no-edit", "parent") + issue250Git(t, repo.dir, "push", "origin", "child") + issue250Git(t, repo.dir, "checkout", "top") + issue250Git(t, repo.dir, "branch", "-f", "child", child) + wantConsent = true + case "merged prefix": + issue250Git(t, repo.dir, "checkout", "main") + issue250Git(t, repo.dir, "merge", "--no-ff", "-m", "merge parent and child", "child") + issue250Git(t, repo.dir, "push", "origin", "main") + issue250Git(t, repo.dir, "checkout", "top") + s.Branches[0].PullRequest.Merged = true + s.Branches[0].PullRequest.MergedHead = child + lookup := repo.client.FindPRByNumberFn + repo.client.FindPRByNumberFn = func(n int) (*github.PullRequest, error) { + pr, err := lookup(n) + if n == 11 { + pr.Merged, pr.State, pr.HeadRefOID = true, "MERGED", child + } + return pr, err + } + case "merged elsewhere": + repo.remote.Base.Ref = "main" + target = "main" + wantConsent = true + case "commits after merge": + issue250Git(t, repo.dir, "checkout", "-B", "parent", repo.oldParent) + issue250WriteFile(t, repo.dir, "parent-extra.txt", "not in the merged PR\n") + issue250Git(t, repo.dir, "add", ".") + issue250Git(t, repo.dir, "commit", "-m", "parent extra") + issue250Git(t, repo.dir, "rebase", "--onto", "parent", repo.oldParent, "child") + issue250Git(t, repo.dir, "rebase", "--onto", "child", child, "top") + s.Trunk.Head = issue250Git(t, repo.dir, "rev-parse", "parent") + s.Branches[0].Base = s.Trunk.Head + s.Branches[1].Base = issue250Git(t, repo.dir, "rev-parse", "child") + wantConsent = true + } + require.NoError(t, stack.Save(repo.gitDir, sf)) + before, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + refs := issue250Git(t, repo.dir, "rev-parse", "child", "top", "origin/child", "origin/top") + + err = run(cfg, accept) + if wantConsent || scenario == "missing cutoff" { + require.Error(t, err) + after, readErr := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, readErr) + assert.Equal(t, before, after) + assert.Equal(t, refs, issue250Git(t, repo.dir, "rev-parse", "child", "top", "origin/child", "origin/top")) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) + if scenario == "missing cutoff" { + return + } + err = run(cfg, true) + } + require.NoError(t, err) + wantCommits := "top commit\nchild commit" + if scenario == "merged prefix" { + wantCommits = "top commit" + } + assert.Equal(t, wantCommits, issue250Git(t, repo.dir, "log", "--format=%s", target+"..top")) + assert.NoFileExists(t, filepath.Join(repo.dir, "parent-extra.txt")) + after, err := stack.Load(repo.gitDir) + require.NoError(t, err) + assert.Equal(t, target, after.Stacks[0].Trunk.Branch) + assert.Equal(t, "9", after.Stacks[0].ID) + }) + } +} diff --git a/cmd/sync.go b/cmd/sync.go index dfffc3a4..c2c058a2 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -193,7 +193,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { } var cutoff string if newBase != "" { - if cutoff, err = retargetCutoff(cfg, s, trunk, opts.acceptBaseChange); err != nil { + if cutoff, err = retargetCutoff(cfg, s, trunk, planned, opts.acceptBaseChange); err != nil { return err } } @@ -221,6 +221,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { } else { state = newWorktreeRebaseState(s, ctx, currentBranch, originalRefs, trunk, 0, len(s.Branches)) state.TargetTrunk = newBase + state.UseOnto, state.OntoOldBase = cutoff != "", cutoff if s.Trunk.Branch != originalTrunk { if err := stack.Save(gitDir, sf); err != nil { return handleSaveError(cfg, err) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 3158345a..f7db77df 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2898,3 +2898,47 @@ func TestSync_BaseRetargetConflictKeepsOldBase(t *testing.T) { assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "origin/top")) assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) } + +type interruptAfterSyncJournal struct { + git.Ops + path string +} + +func (o interruptAfterSyncJournal) IsAncestor(ancestor, descendant string) (bool, error) { + if _, err := os.Stat(o.path); err == nil { + panic("interrupted after initial journal") + } else if !errors.Is(err, os.ErrNotExist) { + return false, err + } + return o.Ops.IsAncestor(ancestor, descendant) +} + +func TestSync_BaseRetargetInitialJournal(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", false) + withIssue250Repo(t, repo.dir) + cfg := issue250TestConfig(t) + cfg.GitHubClientOverride = repo.client + restore := git.SetOps(interruptAfterSyncJournal{ + Ops: git.CurrentOps(), path: filepath.Join(repo.gitDir, rebaseStateFile), + }) + defer restore() + + require.PanicsWithValue(t, "interrupted after initial journal", func() { + _ = runSync(cfg, &syncOptions{remote: "origin", acceptBaseChange: true}) + }) + restore() + state, err := loadRebaseState(repo.gitDir) + require.NoError(t, err) + assert.True(t, state.UseOnto) + assert.Equal(t, repo.oldParent, state.OntoOldBase) + + require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) + assertBaseRetargetHistory(t, repo) + assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) +} + +func TestSync_BaseRetargetReviewCases(t *testing.T) { + testBaseRetargetReviewCases(t, func(cfg *config.Config, accept bool) error { + return runSync(cfg, &syncOptions{remote: "origin", acceptBaseChange: accept}) + }) +} diff --git a/cmd/utils.go b/cmd/utils.go index 5ea62491..c9ecfb22 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1136,31 +1136,53 @@ func requireCurrentBase(cfg *config.Config, s *stack.Stack, remote string) error // onto a retargeted base, or "" when merged branches below it already set the // cutoff. Dropping commits that are not known to have landed in the new base // requires consent. -func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, accept bool) (string, error) { +func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, planned []branchFastForward, accept bool) (string, error) { i := s.FirstActiveBranchIndex() if i < 0 { return "", nil } + b := s.Branches[i] + head := b.Branch + for _, forward := range planned { + if forward.Branch == b.Branch { + head = forward.NewSHA + break + } + } var cutoff string - landed := false + var err error if i == 0 { - b := s.Branches[0] - oldTip, _ := git.RevParse(s.Trunk.Branch) - var err error - if cutoff, err = resolveRebaseOldBase(oldTip, b.Base, target.Ref, b.Branch); err == nil { - landed, err = git.IsAncestor(cutoff, target.SHA) - } - if err != nil { - cfg.Errorf("%s", err) + cutoff, err = resolveRetargetOldBase(s.Trunk, b.Base, head) + } else { + parent := s.Branches[i-1] + if !parent.IsMerged() { + cfg.Errorf("the retargeted bottom PR is queued; wait for it to leave the merge queue before rebasing") return "", ErrSilent } + refs, lookupErr := resolveOriginalRefs(s) + if lookupErr != nil { + return "", lookupErr + } + cutoff, err = resolveMergedRebaseOldBase(parent.PullRequest.MergedHead, refs[parent.Branch], b.Base, target.SHA, head) + } + if err != nil { + cfg.Errorf("%s", err) + return "", ErrSilent + } + landed, err := git.IsAncestor(cutoff, target.SHA) + if err != nil { + return "", err } if !landed && !accept { - if client, err := cfg.GitHubClient(); err == nil { - pr, err := client.FindPRDetailsForBranch(s.Trunk.Branch) - landed = err == nil && pr != nil && pr.Merged + landed, err = retargetBaseMerged(cfg, s.Trunk.Branch, cutoff, target.SHA) + if err != nil { + cfg.Warningf("could not verify whether the old base landed in %s: %v", target.Branch, err) } } + if i > 0 { + // The existing merged-predecessor cascade selects this same boundary. + cutoff = "" + } if landed || accept { return cutoff, nil } @@ -1172,7 +1194,10 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, acce printInterrupt(cfg) return "", ErrSilent } - if err == nil && ok { + if err != nil { + return "", err + } + if ok { return cutoff, nil } cfg.Canceled = true @@ -1181,6 +1206,80 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, acce return "", ErrSilent } +// Only old-parent evidence can identify the cutoff; the new base's fork point +// may sit below commits that must be excluded from the retargeted stack. +func resolveRetargetOldBase(trunk stack.BranchRef, recordedBase, head string) (string, error) { + candidates := []string{recordedBase, trunk.Head} + exists, err := git.BranchExists(trunk.Branch) + if err != nil { + return "", err + } + if exists { + tip, err := git.RevParse(trunk.Branch) + if err != nil { + return "", err + } + forkPoint, err := git.MergeBaseForkPoint(trunk.Branch, head) + if err != nil { + return "", err + } + candidates = append(candidates, tip, forkPoint) + } + var cutoff string + for _, candidate := range candidates { + valid, err := rebaseBoundaryContains(candidate, head) + if err != nil { + return "", err + } + if !valid || candidate == cutoff { + continue + } + if cutoff == "" { + cutoff = candidate + continue + } + newer, err := git.IsAncestor(cutoff, candidate) + if err != nil { + return "", err + } + if newer { + cutoff = candidate + continue + } + older, err := git.IsAncestor(candidate, cutoff) + if err != nil { + return "", err + } + if !older { + return "", fmt.Errorf("conflicting previous bases of %s; rebase this branch manually", trunk.Branch) + } + } + if cutoff == "" { + return "", fmt.Errorf("could not determine the previous base from %s; rebase this branch manually", trunk.Branch) + } + return cutoff, nil +} + +func retargetBaseMerged(cfg *config.Config, oldBranch, cutoff, target string) (bool, error) { + client, err := cfg.GitHubClient() + if err != nil { + return false, err + } + pr, err := client.FindPRDetailsForBranch(oldBranch) + if err != nil || pr == nil || !pr.Merged || pr.HeadRefOID == "" || pr.MergeCommitOID == "" { + return false, err + } + exists, err := git.CommitExists(pr.HeadRefOID) + if err != nil || !exists { + return false, err + } + covered, err := git.IsAncestor(cutoff, pr.HeadRefOID) + if err != nil || !covered { + return false, err + } + return rebaseBoundaryContains(pr.MergeCommitOID, target) +} + type trunkTarget struct { Branch string Ref string diff --git a/internal/github/github.go b/internal/github/github.go index b6a96e4a..f6bc4a90 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -290,30 +290,36 @@ func (c *Client) repositoryID() (string, error) { return query.Repository.ID, nil } -// PRDetails holds enriched pull request data for display in the TUI. +// PRDetails holds pull request status and commit metadata. type PRDetails struct { - Number int - State string // OPEN, CLOSED, MERGED - URL string - Title string - Body string - IsDraft bool - Merged bool - IsQueued bool -} - -// FindPRDetailsForBranch fetches enriched PR data for display purposes. + Number int + State string // OPEN, CLOSED, MERGED + URL string + Title string + Body string + IsDraft bool + Merged bool + IsQueued bool + HeadRefOID string + MergeCommitOID string +} + +// FindPRDetailsForBranch fetches status and commit metadata for the latest PR. // Returns nil without error if no PR exists for the branch. func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { var query struct { Repository struct { PullRequests struct { Nodes []struct { - Number int `graphql:"number"` - State string `graphql:"state"` - URL string `graphql:"url"` - IsDraft bool `graphql:"isDraft"` - Merged bool `graphql:"merged"` + Number int `graphql:"number"` + State string `graphql:"state"` + URL string `graphql:"url"` + IsDraft bool `graphql:"isDraft"` + Merged bool `graphql:"merged"` + HeadRefOID string `graphql:"headRefOid"` + MergeCommit struct { + OID string `graphql:"oid"` + } `graphql:"mergeCommit"` MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"` } } `graphql:"pullRequests(headRefName: $head, last: 1)"` @@ -337,12 +343,14 @@ func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { n := nodes[0] return &PRDetails{ - Number: n.Number, - State: n.State, - URL: n.URL, - IsDraft: n.IsDraft, - Merged: n.Merged, - IsQueued: n.MergeQueueEntry != nil && n.MergeQueueEntry.ID != "", + Number: n.Number, + State: n.State, + URL: n.URL, + IsDraft: n.IsDraft, + Merged: n.Merged, + IsQueued: n.MergeQueueEntry != nil && n.MergeQueueEntry.ID != "", + HeadRefOID: n.HeadRefOID, + MergeCommitOID: n.MergeCommit.OID, }, nil } diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 0df92613..2b3a558d 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -154,6 +154,52 @@ func TestFindPRByNumber_MergedHead(t *testing.T) { } } +func TestFindPRDetailsForBranch_MergeEvidence(t *testing.T) { + for _, merged := range []bool{true, false} { + t.Run(strconv.FormatBool(merged), func(t *testing.T) { + var mergeCommit any + if merged { + mergeCommit = map[string]any{"oid": "merge-sha"} + } + body, err := json.Marshal(map[string]any{ + "data": map[string]any{"repository": map[string]any{"pullRequests": map[string]any{ + "nodes": []any{map[string]any{ + "number": 42, "merged": merged, "headRefOid": "head-sha", "mergeCommit": mergeCommit, + }}, + }}}, + }) + require.NoError(t, err) + transport := roundTripFunc(func(r *http.Request) (*http.Response, error) { + var request struct { + Query string `json:"query"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + assert.Contains(t, request.Query, "headRefOid") + assert.Contains(t, request.Query, "mergeCommit{oid}") + return &http.Response{ + StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body)), + Header: http.Header{"Content-Type": []string{"application/json"}}, Request: r, + }, nil + }) + gql, err := api.NewGraphQLClient(api.ClientOptions{ + Host: "github.com", AuthToken: "x", Transport: transport, + }) + require.NoError(t, err) + client := &Client{gql: gql, owner: "o", repo: "r"} + got, err := client.FindPRDetailsForBranch("parent") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "head-sha", got.HeadRefOID) + assert.Equal(t, merged, got.Merged) + if merged { + assert.Equal(t, "merge-sha", got.MergeCommitOID) + } else { + assert.Empty(t, got.MergeCommitOID) + } + }) + } +} + // TestRemoteStack_UnmarshalJSON verifies the custom decoder that flattens the // Stacks REST API's pull_requests object array into ordered PullRequests // numbers while preserving the full entries in PRDetails. This is the only path From c09f7969d1da4cfdc8c5624f83823bffbbea9130 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 10:29:40 -0400 Subject: [PATCH 5/9] Normalize retarget comparisons in view --- cmd/view.go | 14 +++++++++++++- cmd/view_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/cmd/view.go b/cmd/view.go index f78eb3a7..4ee99950 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -140,7 +140,19 @@ func runViewJSON(cfg *config.Config) error { // warnRetargeted reports a GitHub retarget that has not been rebased onto yet. func warnRetargeted(cfg *config.Config, s *stack.Stack) bool { - newBase, _ := retargetedBase(cfg, s, "") + if s.ID == "" { + return false + } + remote, err := git.ResolveRemote(s.Trunk.Branch) + if err != nil { + cfg.Warningf("could not check the stack's base: %v", err) + return false + } + newBase, err := retargetedBase(cfg, s, remote) + if err != nil { + cfg.Warningf("could not check the stack's base: %v", err) + return false + } if newBase != "" { cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), newBase) } diff --git a/cmd/view_test.go b/cmd/view_test.go index a4546dae..3bdb9560 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -642,3 +642,50 @@ func TestView_BaseRetarget(t *testing.T) { require.NoError(t, err) assert.Equal(t, string(catalog), string(after)) } + +func TestView_BaseRetargetRemoteQualifiedTrunk(t *testing.T) { + for _, localBranch := range []bool{false, true} { + name := "tracking ref" + if localBranch { + name = "real local branch" + } + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + writeStackFile(t, dir, stack.Stack{ + ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "origin/main"}, + Branches: []stack.BranchRef{{Branch: "child"}}, + }) + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return dir, nil }, + CurrentBranchFn: func() (string, error) { return "child", nil }, + ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, + BranchExistsFn: func(branch string) (bool, error) { return branch != "origin/main" || localBranch, nil }, + IsAncestorFn: func(string, string) (bool, error) { return true, nil }, + }) + defer restore() + cfg, outR, errR := config.NewTestConfig() + cfg.ForceInteractive = true + cfg.SelectFn = func(string, string, []string) (int, error) { + t.Fatal("view must not prompt for a remote") + return 0, nil + } + cfg.GitHubClientOverride = &github.MockClient{ + GetStackFn: func(int) (*github.RemoteStack, error) { + return &github.RemoteStack{ID: 9, Number: 9, Base: github.RemoteStackBase{Ref: "main"}}, nil + }, + } + require.NoError(t, runView(cfg, &viewOptions{asJSON: true})) + out, diagnostics := commandOutput(t, cfg, outR, errR) + var result viewJSONOutput + require.NoError(t, json.Unmarshal([]byte(out), &result)) + assert.Equal(t, "origin/main", result.Trunk) + assert.Equal(t, localBranch, result.Branches[0].NeedsRebase) + if !localBranch { + assert.NotContains(t, diagnostics, "retargeted") + } + sf, err := stack.Load(dir) + require.NoError(t, err) + assert.Equal(t, "origin/main", sf.Stacks[0].Trunk.Branch) + }) + } +} From 30e438de07863ebc77158188e11fda35a84b4344 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 13:30:01 -0400 Subject: [PATCH 6/9] Avoid false retarget consent prompts --- cmd/rebase_test.go | 18 +++-- cmd/sync_test.go | 38 --------- cmd/utils.go | 136 ++++++++++----------------------- internal/github/github.go | 51 +++++++------ internal/github/github_test.go | 46 ----------- 5 files changed, 79 insertions(+), 210 deletions(-) diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 86770c4d..5c4ddc2c 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -4031,14 +4031,16 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er for _, scenario := range []string{ "stale local trunk", "old trunk reflog", "missing old trunk", "missing cutoff", "planned fast-forward", "merged prefix", - "merged elsewhere", "commits after merge", + "merged elsewhere", "commits after merge", "amended before merge", } { t.Run(scenario, func(t *testing.T) { target := "main" if scenario == "merged elsewhere" { target = "release" } - repo := setupBaseRetargetRepo(t, target, scenario == "merged elsewhere" || scenario == "commits after merge") + merged := scenario == "merged prefix" || scenario == "merged elsewhere" || + scenario == "commits after merge" || scenario == "amended before merge" + repo := setupBaseRetargetRepo(t, target, merged) withIssue250Repo(t, repo.dir) cfg := issue250TestConfig(t) cfg.GitHubClientOverride = repo.client @@ -4078,10 +4080,6 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er issue250Git(t, repo.dir, "branch", "-f", "child", child) wantConsent = true case "merged prefix": - issue250Git(t, repo.dir, "checkout", "main") - issue250Git(t, repo.dir, "merge", "--no-ff", "-m", "merge parent and child", "child") - issue250Git(t, repo.dir, "push", "origin", "main") - issue250Git(t, repo.dir, "checkout", "top") s.Branches[0].PullRequest.Merged = true s.Branches[0].PullRequest.MergedHead = child lookup := repo.client.FindPRByNumberFn @@ -4107,6 +4105,14 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er s.Branches[0].Base = s.Trunk.Head s.Branches[1].Base = issue250Git(t, repo.dir, "rev-parse", "child") wantConsent = true + case "amended before merge": + // The parent was amended after the child last synced, then merged. + details := repo.client.FindPRDetailsForBranchFn + repo.client.FindPRDetailsForBranchFn = func(branch string) (*github.PRDetails, error) { + pr, err := details(branch) + pr.HeadRefOID = repo.newParent + return pr, err + } } require.NoError(t, stack.Save(repo.gitDir, sf)) before, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) diff --git a/cmd/sync_test.go b/cmd/sync_test.go index f7db77df..2682c761 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -2899,44 +2899,6 @@ func TestSync_BaseRetargetConflictKeepsOldBase(t *testing.T) { assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) } -type interruptAfterSyncJournal struct { - git.Ops - path string -} - -func (o interruptAfterSyncJournal) IsAncestor(ancestor, descendant string) (bool, error) { - if _, err := os.Stat(o.path); err == nil { - panic("interrupted after initial journal") - } else if !errors.Is(err, os.ErrNotExist) { - return false, err - } - return o.Ops.IsAncestor(ancestor, descendant) -} - -func TestSync_BaseRetargetInitialJournal(t *testing.T) { - repo := setupBaseRetargetRepo(t, "main", false) - withIssue250Repo(t, repo.dir) - cfg := issue250TestConfig(t) - cfg.GitHubClientOverride = repo.client - restore := git.SetOps(interruptAfterSyncJournal{ - Ops: git.CurrentOps(), path: filepath.Join(repo.gitDir, rebaseStateFile), - }) - defer restore() - - require.PanicsWithValue(t, "interrupted after initial journal", func() { - _ = runSync(cfg, &syncOptions{remote: "origin", acceptBaseChange: true}) - }) - restore() - state, err := loadRebaseState(repo.gitDir) - require.NoError(t, err) - assert.True(t, state.UseOnto) - assert.Equal(t, repo.oldParent, state.OntoOldBase) - - require.NoError(t, runRebase(cfg, &rebaseOptions{cont: true})) - assertBaseRetargetHistory(t, repo) - assert.NoFileExists(t, filepath.Join(repo.gitDir, rebaseStateFile)) -} - func TestSync_BaseRetargetReviewCases(t *testing.T) { testBaseRetargetReviewCases(t, func(cfg *config.Config, accept bool) error { return runSync(cfg, &syncOptions{remote: "origin", acceptBaseChange: accept}) diff --git a/cmd/utils.go b/cmd/utils.go index c9ecfb22..c525ada0 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1141,47 +1141,27 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, plan if i < 0 { return "", nil } - b := s.Branches[i] - head := b.Branch - for _, forward := range planned { - if forward.Branch == b.Branch { - head = forward.NewSHA - break - } - } var cutoff string - var err error + landed := false if i == 0 { - cutoff, err = resolveRetargetOldBase(s.Trunk, b.Base, head) - } else { - parent := s.Branches[i-1] - if !parent.IsMerged() { - cfg.Errorf("the retargeted bottom PR is queued; wait for it to leave the merge queue before rebasing") - return "", ErrSilent + b := s.Branches[0] + head := b.Branch + for _, forward := range planned { + if forward.Branch == b.Branch { + head = forward.NewSHA + } } - refs, lookupErr := resolveOriginalRefs(s) - if lookupErr != nil { - return "", lookupErr + var err error + if cutoff, err = resolveRetargetOldBase(s.Trunk.Branch, b.Base, head); err == nil { + landed, err = git.IsAncestor(cutoff, target.SHA) } - cutoff, err = resolveMergedRebaseOldBase(parent.PullRequest.MergedHead, refs[parent.Branch], b.Base, target.SHA, head) - } - if err != nil { - cfg.Errorf("%s", err) - return "", ErrSilent - } - landed, err := git.IsAncestor(cutoff, target.SHA) - if err != nil { - return "", err - } - if !landed && !accept { - landed, err = retargetBaseMerged(cfg, s.Trunk.Branch, cutoff, target.SHA) if err != nil { - cfg.Warningf("could not verify whether the old base landed in %s: %v", target.Branch, err) + cfg.Errorf("%s", err) + return "", ErrSilent } } - if i > 0 { - // The existing merged-predecessor cascade selects this same boundary. - cutoff = "" + if !landed && !accept { + landed = retargetBaseMerged(cfg, s.Trunk.Branch, cutoff, target.SHA) } if landed || accept { return cutoff, nil @@ -1194,10 +1174,7 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, plan printInterrupt(cfg) return "", ErrSilent } - if err != nil { - return "", err - } - if ok { + if err == nil && ok { return cutoff, nil } cfg.Canceled = true @@ -1206,78 +1183,45 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, plan return "", ErrSilent } -// Only old-parent evidence can identify the cutoff; the new base's fork point -// may sit below commits that must be excluded from the retargeted stack. -func resolveRetargetOldBase(trunk stack.BranchRef, recordedBase, head string) (string, error) { - candidates := []string{recordedBase, trunk.Head} - exists, err := git.BranchExists(trunk.Branch) +// resolveRetargetOldBase finds the old base from the recorded base and the old +// trunk's reflog. The new base's fork point may sit below the old base's +// commits, so it is never used. +func resolveRetargetOldBase(oldTrunk, recordedBase, head string) (string, error) { + exists, err := git.BranchExists(oldTrunk) if err != nil { return "", err } if exists { - tip, err := git.RevParse(trunk.Branch) - if err != nil { - return "", err - } - forkPoint, err := git.MergeBaseForkPoint(trunk.Branch, head) - if err != nil { - return "", err - } - candidates = append(candidates, tip, forkPoint) + return resolveRebaseOldBase("", recordedBase, oldTrunk, head) } - var cutoff string - for _, candidate := range candidates { - valid, err := rebaseBoundaryContains(candidate, head) - if err != nil { - return "", err - } - if !valid || candidate == cutoff { - continue - } - if cutoff == "" { - cutoff = candidate - continue - } - newer, err := git.IsAncestor(cutoff, candidate) - if err != nil { - return "", err - } - if newer { - cutoff = candidate - continue - } - older, err := git.IsAncestor(candidate, cutoff) - if err != nil { - return "", err - } - if !older { - return "", fmt.Errorf("conflicting previous bases of %s; rebase this branch manually", trunk.Branch) - } + valid, err := rebaseBoundaryContains(recordedBase, head) + if err != nil { + return "", err } - if cutoff == "" { - return "", fmt.Errorf("could not determine the previous base from %s; rebase this branch manually", trunk.Branch) + if !valid { + return "", fmt.Errorf("could not determine the previous base from %s; rebase this branch manually", oldTrunk) } - return cutoff, nil + return recordedBase, nil } -func retargetBaseMerged(cfg *config.Config, oldBranch, cutoff, target string) (bool, error) { +// retargetBaseMerged reports whether the old base's PR merged into the new base +// before the stack built on any later commits to the old base. +func retargetBaseMerged(cfg *config.Config, oldBase, cutoff, target string) bool { client, err := cfg.GitHubClient() if err != nil { - return false, err - } - pr, err := client.FindPRDetailsForBranch(oldBranch) - if err != nil || pr == nil || !pr.Merged || pr.HeadRefOID == "" || pr.MergeCommitOID == "" { - return false, err + return false } - exists, err := git.CommitExists(pr.HeadRefOID) - if err != nil || !exists { - return false, err + pr, err := client.FindPRDetailsForBranch(oldBase) + if err != nil || pr == nil || !pr.Merged { + return false } - covered, err := git.IsAncestor(cutoff, pr.HeadRefOID) - if err != nil || !covered { - return false, err + if cutoff != "" && cutoff != pr.HeadRefOID { + if after, err := rebaseBoundaryContains(pr.HeadRefOID, cutoff); err != nil || after { + return false + } } - return rebaseBoundaryContains(pr.MergeCommitOID, target) + landed, err := rebaseBoundaryContains(pr.MergeCommitOID, target) + return err == nil && landed } type trunkTarget struct { diff --git a/internal/github/github.go b/internal/github/github.go index f6bc4a90..88d3f6f5 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -290,37 +290,39 @@ func (c *Client) repositoryID() (string, error) { return query.Repository.ID, nil } -// PRDetails holds pull request status and commit metadata. +// PRDetails holds enriched pull request data for display in the TUI. type PRDetails struct { - Number int - State string // OPEN, CLOSED, MERGED - URL string - Title string - Body string - IsDraft bool - Merged bool - IsQueued bool + Number int + State string // OPEN, CLOSED, MERGED + URL string + Title string + Body string + IsDraft bool + Merged bool + IsQueued bool + + // Merge evidence for a stack retargeted off this PR's branch. HeadRefOID string MergeCommitOID string } -// FindPRDetailsForBranch fetches status and commit metadata for the latest PR. +// FindPRDetailsForBranch fetches enriched PR data for display purposes. // Returns nil without error if no PR exists for the branch. func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { var query struct { Repository struct { PullRequests struct { Nodes []struct { - Number int `graphql:"number"` - State string `graphql:"state"` - URL string `graphql:"url"` - IsDraft bool `graphql:"isDraft"` - Merged bool `graphql:"merged"` - HeadRefOID string `graphql:"headRefOid"` - MergeCommit struct { + Number int `graphql:"number"` + State string `graphql:"state"` + URL string `graphql:"url"` + IsDraft bool `graphql:"isDraft"` + Merged bool `graphql:"merged"` + MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"` + HeadRefOID string `graphql:"headRefOid"` + MergeCommit struct { OID string `graphql:"oid"` } `graphql:"mergeCommit"` - MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"` } } `graphql:"pullRequests(headRefName: $head, last: 1)"` } `graphql:"repository(owner: $owner, name: $name)"` @@ -343,12 +345,13 @@ func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { n := nodes[0] return &PRDetails{ - Number: n.Number, - State: n.State, - URL: n.URL, - IsDraft: n.IsDraft, - Merged: n.Merged, - IsQueued: n.MergeQueueEntry != nil && n.MergeQueueEntry.ID != "", + Number: n.Number, + State: n.State, + URL: n.URL, + IsDraft: n.IsDraft, + Merged: n.Merged, + IsQueued: n.MergeQueueEntry != nil && n.MergeQueueEntry.ID != "", + HeadRefOID: n.HeadRefOID, MergeCommitOID: n.MergeCommit.OID, }, nil diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 2b3a558d..0df92613 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -154,52 +154,6 @@ func TestFindPRByNumber_MergedHead(t *testing.T) { } } -func TestFindPRDetailsForBranch_MergeEvidence(t *testing.T) { - for _, merged := range []bool{true, false} { - t.Run(strconv.FormatBool(merged), func(t *testing.T) { - var mergeCommit any - if merged { - mergeCommit = map[string]any{"oid": "merge-sha"} - } - body, err := json.Marshal(map[string]any{ - "data": map[string]any{"repository": map[string]any{"pullRequests": map[string]any{ - "nodes": []any{map[string]any{ - "number": 42, "merged": merged, "headRefOid": "head-sha", "mergeCommit": mergeCommit, - }}, - }}}, - }) - require.NoError(t, err) - transport := roundTripFunc(func(r *http.Request) (*http.Response, error) { - var request struct { - Query string `json:"query"` - } - require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) - assert.Contains(t, request.Query, "headRefOid") - assert.Contains(t, request.Query, "mergeCommit{oid}") - return &http.Response{ - StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body)), - Header: http.Header{"Content-Type": []string{"application/json"}}, Request: r, - }, nil - }) - gql, err := api.NewGraphQLClient(api.ClientOptions{ - Host: "github.com", AuthToken: "x", Transport: transport, - }) - require.NoError(t, err) - client := &Client{gql: gql, owner: "o", repo: "r"} - got, err := client.FindPRDetailsForBranch("parent") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, "head-sha", got.HeadRefOID) - assert.Equal(t, merged, got.Merged) - if merged { - assert.Equal(t, "merge-sha", got.MergeCommitOID) - } else { - assert.Empty(t, got.MergeCommitOID) - } - }) - } -} - // TestRemoteStack_UnmarshalJSON verifies the custom decoder that flattens the // Stacks REST API's pull_requests object array into ordered PullRequests // numbers while preserving the full entries in PRDetails. This is the only path From 12daf5db2f19fe38cdad68163ff7418de13c468f Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Sun, 25 Oct 2026 13:31:10 -0400 Subject: [PATCH 7/9] Keep view's retarget check best-effort --- cmd/view.go | 15 ++------- cmd/view_test.go | 81 +++++++++++++++--------------------------------- 2 files changed, 27 insertions(+), 69 deletions(-) diff --git a/cmd/view.go b/cmd/view.go index 4ee99950..eca65e8f 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -140,19 +140,8 @@ func runViewJSON(cfg *config.Config) error { // warnRetargeted reports a GitHub retarget that has not been rebased onto yet. func warnRetargeted(cfg *config.Config, s *stack.Stack) bool { - if s.ID == "" { - return false - } - remote, err := git.ResolveRemote(s.Trunk.Branch) - if err != nil { - cfg.Warningf("could not check the stack's base: %v", err) - return false - } - newBase, err := retargetedBase(cfg, s, remote) - if err != nil { - cfg.Warningf("could not check the stack's base: %v", err) - return false - } + remote, _ := git.ResolveRemote(s.Trunk.Branch) + newBase, _ := retargetedBase(cfg, s, remote) if newBase != "" { cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), newBase) } diff --git a/cmd/view_test.go b/cmd/view_test.go index 3bdb9560..086952ee 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -624,68 +624,37 @@ func TestRunViewJSON_SharedCatalogDuringMutation(t *testing.T) { } func TestView_BaseRetarget(t *testing.T) { - repo := setupBaseRetargetRepo(t, "main", false) - withIssue250Repo(t, repo.dir) - catalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) - require.NoError(t, err) - cfg, outR, errR := config.NewTestConfig() - cfg.GitHubClientOverride = repo.client - - require.NoError(t, runView(cfg, &viewOptions{asJSON: true})) - out, diagnostics := commandOutput(t, cfg, outR, errR) - var data viewJSONOutput - require.NoError(t, json.Unmarshal([]byte(out), &data)) - assert.Equal(t, "parent", data.Trunk) - assert.True(t, data.Branches[0].NeedsRebase, "the bottom branch must be rebased onto the new base") - assert.Contains(t, diagnostics, "from parent to main") - after, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) - require.NoError(t, err) - assert.Equal(t, string(catalog), string(after)) -} - -func TestView_BaseRetargetRemoteQualifiedTrunk(t *testing.T) { - for _, localBranch := range []bool{false, true} { - name := "tracking ref" - if localBranch { - name = "real local branch" - } - t.Run(name, func(t *testing.T) { - dir := t.TempDir() - writeStackFile(t, dir, stack.Stack{ - ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "origin/main"}, - Branches: []stack.BranchRef{{Branch: "child"}}, - }) - restore := git.SetOps(&git.MockOps{ - GitDirFn: func() (string, error) { return dir, nil }, - CurrentBranchFn: func() (string, error) { return "child", nil }, - ResolveRemoteFn: func(string) (string, error) { return "origin", nil }, - BranchExistsFn: func(branch string) (bool, error) { return branch != "origin/main" || localBranch, nil }, - IsAncestorFn: func(string, string) (bool, error) { return true, nil }, - }) - defer restore() - cfg, outR, errR := config.NewTestConfig() - cfg.ForceInteractive = true - cfg.SelectFn = func(string, string, []string) (int, error) { - t.Fatal("view must not prompt for a remote") - return 0, nil - } - cfg.GitHubClientOverride = &github.MockClient{ - GetStackFn: func(int) (*github.RemoteStack, error) { - return &github.RemoteStack{ID: 9, Number: 9, Base: github.RemoteStackBase{Ref: "main"}}, nil - }, + for _, trunk := range []string{"parent", "origin/main"} { + t.Run(trunk, func(t *testing.T) { + repo := setupBaseRetargetRepo(t, "main", false) + withIssue250Repo(t, repo.dir) + retargeted := trunk == "parent" + if !retargeted { + // A remote-qualified trunk matching GitHub's base is not a retarget. + sf, err := stack.Load(repo.gitDir) + require.NoError(t, err) + sf.Stacks[0].Trunk.Branch = trunk + require.NoError(t, stack.Save(repo.gitDir, sf)) } + catalog, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) + require.NoError(t, err) + cfg, outR, errR := config.NewTestConfig() + cfg.GitHubClientOverride = repo.client + require.NoError(t, runView(cfg, &viewOptions{asJSON: true})) out, diagnostics := commandOutput(t, cfg, outR, errR) - var result viewJSONOutput - require.NoError(t, json.Unmarshal([]byte(out), &result)) - assert.Equal(t, "origin/main", result.Trunk) - assert.Equal(t, localBranch, result.Branches[0].NeedsRebase) - if !localBranch { + var data viewJSONOutput + require.NoError(t, json.Unmarshal([]byte(out), &data)) + assert.Equal(t, trunk, data.Trunk) + assert.Equal(t, retargeted, data.Branches[0].NeedsRebase, "only a retargeted bottom branch needs a rebase") + if retargeted { + assert.Contains(t, diagnostics, "from parent to main") + } else { assert.NotContains(t, diagnostics, "retargeted") } - sf, err := stack.Load(dir) + after, err := os.ReadFile(filepath.Join(repo.gitDir, "gh-stack")) require.NoError(t, err) - assert.Equal(t, "origin/main", sf.Stacks[0].Trunk.Branch) + assert.Equal(t, string(catalog), string(after)) }) } } From a7db03d9d3161bb59ce26c2d78705aaec48ad054 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Mon, 26 Oct 2026 02:20:33 -0400 Subject: [PATCH 8/9] Require consent before dropping unlanded merged branches A merged branch below a retargeted stack can land in the old base after that base merged. Check the boundary the cascade drops, and skip the prompt only when those commits reached the new base. --- cmd/rebase_test.go | 31 +++++++++++++++++++++++++++---- cmd/utils.go | 44 ++++++++++++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 5c4ddc2c..3e2d4e79 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -4030,7 +4030,8 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er t.Helper() for _, scenario := range []string{ "stale local trunk", "old trunk reflog", "missing old trunk", - "missing cutoff", "planned fast-forward", "merged prefix", + "missing cutoff", "planned fast-forward", "prefix merged before base", + "prefix merged after base", "prefix merged into target", "merged elsewhere", "commits after merge", "amended before merge", } { t.Run(scenario, func(t *testing.T) { @@ -4038,7 +4039,8 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er if scenario == "merged elsewhere" { target = "release" } - merged := scenario == "merged prefix" || scenario == "merged elsewhere" || + prefix := strings.HasPrefix(scenario, "prefix") + merged := prefix || scenario == "merged elsewhere" || scenario == "commits after merge" || scenario == "amended before merge" repo := setupBaseRetargetRepo(t, target, merged) withIssue250Repo(t, repo.dir) @@ -4079,7 +4081,7 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er issue250Git(t, repo.dir, "checkout", "top") issue250Git(t, repo.dir, "branch", "-f", "child", child) wantConsent = true - case "merged prefix": + case "prefix merged before base", "prefix merged after base", "prefix merged into target": s.Branches[0].PullRequest.Merged = true s.Branches[0].PullRequest.MergedHead = child lookup := repo.client.FindPRByNumberFn @@ -4090,6 +4092,27 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er } return pr, err } + wantConsent = scenario == "prefix merged after base" + if wantConsent { + break + } + // The child's changes reached main through the parent or directly. + issue250Git(t, repo.dir, "checkout", "main") + issue250Git(t, repo.dir, "merge", "--squash", "origin/child") + issue250Git(t, repo.dir, "commit", "-m", "squashed child") + issue250Git(t, repo.dir, "push", "origin", "main") + issue250Git(t, repo.dir, "checkout", "top") + squash := issue250Git(t, repo.dir, "rev-parse", "main") + details := repo.client.FindPRDetailsForBranchFn + repo.client.FindPRDetailsForBranchFn = func(branch string) (*github.PRDetails, error) { + pr, err := details(branch) + if scenario == "prefix merged before base" && branch == "parent" { + pr.HeadRefOID = repo.newParent // the parent took the child before merging + } else if scenario == "prefix merged into target" && branch == "child" { + pr.Merged, pr.HeadRefOID, pr.MergeCommitOID = true, child, squash + } + return pr, err + } case "merged elsewhere": repo.remote.Base.Ref = "main" target = "main" @@ -4134,7 +4157,7 @@ func testBaseRetargetReviewCases(t *testing.T, run func(*config.Config, bool) er } require.NoError(t, err) wantCommits := "top commit\nchild commit" - if scenario == "merged prefix" { + if prefix { wantCommits = "top commit" } assert.Equal(t, wantCommits, issue250Git(t, repo.dir, "log", "--format=%s", target+"..top")) diff --git a/cmd/utils.go b/cmd/utils.go index c525ada0..d8aa476b 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1141,27 +1141,39 @@ func retargetCutoff(cfg *config.Config, s *stack.Stack, target trunkTarget, plan if i < 0 { return "", nil } + b := s.Branches[i] + head := b.Branch + for _, forward := range planned { + if forward.Branch == b.Branch { + head = forward.NewSHA + } + } var cutoff string - landed := false + var err error if i == 0 { - b := s.Branches[0] - head := b.Branch - for _, forward := range planned { - if forward.Branch == b.Branch { - head = forward.NewSHA - } - } - var err error - if cutoff, err = resolveRetargetOldBase(s.Trunk.Branch, b.Base, head); err == nil { - landed, err = git.IsAncestor(cutoff, target.SHA) - } - if err != nil { - cfg.Errorf("%s", err) - return "", ErrSilent + cutoff, err = resolveRetargetOldBase(s.Trunk.Branch, b.Base, head) + } else if prev := s.Branches[i-1]; prev.IsMerged() { + // The cascade also drops the merged branches below; check that boundary. + var refs map[string]string + if refs, err = resolveOriginalRefs(s); err == nil { + cutoff, err = resolveMergedRebaseOldBase(prev.PullRequest.MergedHead, refs[prev.Branch], b.Base, target.Ref, head) } } + landed := false + if err == nil && cutoff != "" { + landed, err = git.IsAncestor(cutoff, target.SHA) + } + if err != nil { + cfg.Errorf("%s", err) + return "", ErrSilent + } if !landed && !accept { - landed = retargetBaseMerged(cfg, s.Trunk.Branch, cutoff, target.SHA) + // Merged branches below may have landed directly in the new base. + landed = retargetBaseMerged(cfg, s.Trunk.Branch, cutoff, target.SHA) || + i > 0 && retargetBaseMerged(cfg, s.Branches[i-1].Branch, cutoff, target.SHA) + } + if i > 0 { + cutoff = "" // the merged-branch cascade selects this boundary itself } if landed || accept { return cutoff, nil From a7914a9329ffbaa9f66641f969935681313eaac4 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Mon, 26 Oct 2026 02:23:04 -0400 Subject: [PATCH 9/9] Keep the old base when importing a retargeted stack Importing onto GitHub's new base hid the pending retarget, so the next rebase skipped consent. Keep the merged first PR's base so sync repairs the stack, and compare GitHub's base only for the tracked-stack warning. --- cmd/checkout.go | 10 +++------- cmd/checkout_test.go | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cmd/checkout.go b/cmd/checkout.go index d9888576..64675b9b 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -415,10 +415,6 @@ func checkoutStackByNumber(cfg *config.Config, sf *stack.StackFile, gitDir strin // importing the stack from the remote — and returns the resolved local stack // and the branch to check out. func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, gitDir string, remoteStack *github.RemoteStack, prs []*github.PullRequest, trunk, targetBranch string) (*stack.Stack, string, error) { - // A merged first PR keeps its old base after GitHub retargets the stack. - if remoteStack.Base.Ref != "" { - trunk = remoteStack.Base.Ref - } allMerged := true for _, pr := range prs { if !pr.Merged { @@ -479,9 +475,9 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps, if err := stack.Save(gitDir, sf); err != nil { return nil, "", stackSaveError(cfg, err) } - if localStack.Trunk.Branch != trunk { - cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, trunk) - cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), trunk) + if base := remoteStack.Base.Ref; base != "" && localStack.Trunk.Branch != base { + cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, base) + cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), base) } else { cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number)) } diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go index 35c3df36..7c3fcd1a 100644 --- a/cmd/checkout_test.go +++ b/cmd/checkout_test.go @@ -1684,20 +1684,20 @@ func TestCheckout_BaseRetarget(t *testing.T) { assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch) }) - t.Run("imports onto the stack base", func(t *testing.T) { - repo := setupBaseRetargetRepo(t, "main", true) + t.Run("keeps the old base until rebased", func(t *testing.T) { + // The child merged into the parent, which was deleted without merging. + repo := setupBaseRetargetRepo(t, "main", false) withIssue250Repo(t, repo.dir) sf, err := stack.Load(repo.gitDir) require.NoError(t, err) sf.Stacks = nil require.NoError(t, stack.Save(repo.gitDir, sf)) - repo.remote.PullRequests = []int{10, 11, 12} + child := issue250Git(t, repo.dir, "rev-parse", "child") repo.client.FindPRByNumberFn = func(n int) (*github.PullRequest, error) { - if n == 10 { - return &github.PullRequest{Number: 10, HeadRefName: "parent-old", BaseRefName: "deleted", State: "MERGED", Merged: true}, nil + if n == 11 { + return &github.PullRequest{Number: 11, HeadRefName: "child", BaseRefName: "parent", State: "MERGED", Merged: true, HeadRefOID: child}, nil } - heads := map[int]string{11: "child", 12: "top"} - return &github.PullRequest{Number: n, HeadRefName: heads[n], BaseRefName: "parent", State: "OPEN"}, nil + return &github.PullRequest{Number: 12, HeadRefName: "top", BaseRefName: "main", State: "OPEN"}, nil } cfg := issue250TestConfig(t) cfg.GitHubClientOverride = repo.client @@ -1705,6 +1705,9 @@ func TestCheckout_BaseRetarget(t *testing.T) { require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "9"})) sf, err = stack.Load(repo.gitDir) require.NoError(t, err) - assert.Equal(t, "main", sf.Stacks[0].Trunk.Branch) + assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch) + top := issue250Git(t, repo.dir, "rev-parse", "top") + require.Error(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), "dropping the unmerged parent needs consent") + assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top")) }) }