Skip to content

Commit 3f8c957

Browse files
authored
Base retargeting support (#590)
* Follow server base changes in sync and rebase * Handle retargets across stack commands * Document automatic base retargeting * Validate retarget cutoffs and recovery * Normalize retarget comparisons in view * Avoid false retarget consent prompts * Keep view's retarget check best-effort * 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. * 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.
1 parent c00989a commit 3f8c957

16 files changed

Lines changed: 788 additions & 16 deletions

File tree

‎cmd/checkout.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,12 @@ func reconcileAndImportRemoteStack(cfg *config.Config, client github.ClientOps,
475475
if err := stack.Save(gitDir, sf); err != nil {
476476
return nil, "", stackSaveError(cfg, err)
477477
}
478-
cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number))
478+
if base := remoteStack.Base.Ref; base != "" && localStack.Trunk.Branch != base {
479+
cfg.Warningf("GitHub retargeted this stack%s from %s to %s", linkedStackLabel(cfg, remoteStack.Number), localStack.Trunk.Branch, base)
480+
cfg.Printf(" Run `%s` to rebase the stack onto %s", cfg.ColorCyan("gh stack sync"), base)
481+
} else {
482+
cfg.Successf("Local stack matches remote%s", linkedStackLabel(cfg, remoteStack.Number))
483+
}
479484
return localStack, targetBranch, nil
480485
}
481486

‎cmd/checkout_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,3 +1667,47 @@ func TestCheckout_ByPRURL_Remote(t *testing.T) {
16671667
assert.Equal(t, "feat-2", checkedOut)
16681668
assert.Contains(t, output, "Imported stack with 2 branches")
16691669
}
1670+
1671+
func TestCheckout_BaseRetarget(t *testing.T) {
1672+
t.Run("reports a tracked stack", func(t *testing.T) {
1673+
repo := setupBaseRetargetRepo(t, "main", false)
1674+
withIssue250Repo(t, repo.dir)
1675+
cfg, outR, errR := config.NewTestConfig()
1676+
cfg.GitHubClientOverride = repo.client
1677+
1678+
require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "9"}))
1679+
_, diagnostics := commandOutput(t, cfg, outR, errR)
1680+
assert.Contains(t, diagnostics, "from parent to main")
1681+
assert.NotContains(t, diagnostics, "matches remote")
1682+
sf, err := stack.Load(repo.gitDir)
1683+
require.NoError(t, err)
1684+
assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch)
1685+
})
1686+
1687+
t.Run("keeps the old base until rebased", func(t *testing.T) {
1688+
// The child merged into the parent, which was deleted without merging.
1689+
repo := setupBaseRetargetRepo(t, "main", false)
1690+
withIssue250Repo(t, repo.dir)
1691+
sf, err := stack.Load(repo.gitDir)
1692+
require.NoError(t, err)
1693+
sf.Stacks = nil
1694+
require.NoError(t, stack.Save(repo.gitDir, sf))
1695+
child := issue250Git(t, repo.dir, "rev-parse", "child")
1696+
repo.client.FindPRByNumberFn = func(n int) (*github.PullRequest, error) {
1697+
if n == 11 {
1698+
return &github.PullRequest{Number: 11, HeadRefName: "child", BaseRefName: "parent", State: "MERGED", Merged: true, HeadRefOID: child}, nil
1699+
}
1700+
return &github.PullRequest{Number: 12, HeadRefName: "top", BaseRefName: "main", State: "OPEN"}, nil
1701+
}
1702+
cfg := issue250TestConfig(t)
1703+
cfg.GitHubClientOverride = repo.client
1704+
1705+
require.NoError(t, runCheckout(cfg, &checkoutOptions{target: "9"}))
1706+
sf, err = stack.Load(repo.gitDir)
1707+
require.NoError(t, err)
1708+
assert.Equal(t, "parent", sf.Stacks[0].Trunk.Branch)
1709+
top := issue250Git(t, repo.dir, "rev-parse", "top")
1710+
require.Error(t, runRebase(cfg, &rebaseOptions{remote: "origin"}), "dropping the unmerged parent needs consent")
1711+
assert.Equal(t, top, issue250Git(t, repo.dir, "rev-parse", "top"))
1712+
})
1713+
}

‎cmd/push.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
8484
}
8585
return ErrSilent
8686
}
87+
if err := requireCurrentBase(cfg, s, remote); err != nil {
88+
return err
89+
}
8790
// Sync PR state to detect merged/queued PRs before pushing.
8891
_ = syncStackPRs(cfg, s)
8992

‎cmd/push_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,3 +404,23 @@ func TestPickRemote_OverrideTakesPrecedence(t *testing.T) {
404404
assert.Equal(t, "custom", remote)
405405
assert.False(t, resolveCalled, "ResolveRemote should not be called when override is provided")
406406
}
407+
408+
func TestPush_BaseRetarget(t *testing.T) {
409+
dir := t.TempDir()
410+
writeStackFile(t, dir, stack.Stack{ID: "9", Number: 9, Trunk: stack.BranchRef{Branch: "parent"}, Branches: []stack.BranchRef{{Branch: "child"}}})
411+
mock := newPushMock(dir, "child")
412+
mock.PushFn = func(string, []string, bool, bool) error {
413+
t.Fatal("must not push before rebasing onto the new base")
414+
return nil
415+
}
416+
restore := git.SetOps(mock)
417+
defer restore()
418+
cfg, _, _ := config.NewTestConfig()
419+
cfg.GitHubClientOverride = &github.MockClient{
420+
GetStackFn: func(int) (*github.RemoteStack, error) {
421+
return &github.RemoteStack{ID: 9, Base: github.RemoteStackBase{Ref: "main"}}, nil
422+
},
423+
}
424+
425+
assert.ErrorIs(t, runPush(cfg, &pushOptions{remote: "origin"}), ErrSilent)
426+
}

‎cmd/rebase.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type rebaseOptions struct {
2626
noTrunk bool
2727
remote string
2828
committerDateIsAuthorDate bool
29+
acceptBaseChange bool
2930
}
3031

3132
type rebaseState struct {
@@ -34,6 +35,7 @@ type rebaseState struct {
3435
Worktrees *worktree.Context `json:"worktrees,omitempty"`
3536
StackID string `json:"stackId,omitempty"`
3637
StackTrunk string `json:"stackTrunk,omitempty"`
38+
TargetTrunk string `json:"targetTrunk,omitempty"`
3739
StackBranches []string `json:"stackBranches,omitempty"`
3840
OriginalStack *stack.Stack `json:"originalStack,omitempty"`
3941
RebaseBase string `json:"rebaseBase,omitempty"`
@@ -72,7 +74,11 @@ layer in its commit history, rebasing if necessary.
7274
7375
Use --no-trunk to skip fetching and rebasing with the trunk branch.
7476
Only the inter-branch rebases are performed (branch 2 onto branch 1,
75-
branch 3 onto branch 2, etc.).`,
77+
branch 3 onto branch 2, etc.).
78+
79+
If GitHub retargeted the stack because its base branch was deleted, the
80+
stack is rebased onto the new base. Removing commits from an old base that
81+
did not merge requires confirmation, or --accept-base-change.`,
7682
Example: ` # Rebase the entire stack
7783
$ gh stack rebase
7884
@@ -102,6 +108,7 @@ branch 3 onto branch 2, etc.).`,
102108
cmd.Flags().BoolVar(&opts.downstack, "downstack", false, "Only rebase branches from trunk to current branch")
103109
cmd.Flags().BoolVar(&opts.upstack, "upstack", false, "Only rebase branches from current branch to top")
104110
cmd.Flags().BoolVar(&opts.noTrunk, "no-trunk", false, "Skip trunk — only rebase stack branches onto each other")
111+
cmd.Flags().BoolVar(&opts.acceptBaseChange, "accept-base-change", false, "Rebase onto a retargeted base even if the old base did not merge")
105112
cmd.Flags().BoolVar(&opts.cont, "continue", false, "Continue rebase after resolving conflicts")
106113
cmd.Flags().BoolVar(&opts.abort, "abort", false, "Abort rebase and restore all branches")
107114
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 {
192199
}
193200

194201
var trunk trunkTarget
202+
var newBase, cutoff string
195203
if !opts.noTrunk {
196204
// Resolve remote for fetch and trunk comparison
197205
remote, err := pickRemote(cfg, anchor, opts.remote)
@@ -201,6 +209,14 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
201209
}
202210
return ErrSilent
203211
}
212+
if newBase, err = retargetedBase(cfg, s, remote); err != nil {
213+
cfg.Errorf("failed to read the stack's base on GitHub: %v", err)
214+
return ErrAPIFailure
215+
}
216+
if i := s.FirstActiveBranchIndex(); newBase != "" && i >= 0 && (i < startIdx || i >= endIdx) {
217+
cfg.Errorf("rebase the whole stack to move it onto %s", newBase)
218+
return ErrInvalidArgs
219+
}
204220

205221
// Fast-forward stack branches that are behind their remote tracking branch.
206222
if err := git.FetchBranches(remote, activeBranchNames(s)); err != nil {
@@ -215,10 +231,15 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
215231
cfg.Errorf("%s", err)
216232
return ErrSilent
217233
}
218-
trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{Worktrees: ctx})
234+
trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch, trunkResolveOptions{Worktrees: ctx, Branch: newBase})
219235
if err != nil {
220236
return err
221237
}
238+
if newBase != "" {
239+
if cutoff, err = retargetCutoff(cfg, s, trunk, planned, opts.acceptBaseChange); err != nil {
240+
return err
241+
}
242+
}
222243
if _, err := fastForwardBranches(cfg, planned, ctx); err != nil {
223244
cfg.Errorf("%s", err)
224245
return ErrSilent
@@ -254,11 +275,15 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
254275
ontoOldBase = originalRefs[prev.Branch]
255276
}
256277
}
278+
if cutoff != "" {
279+
needsOnto, ontoOldBase = true, cutoff
280+
}
257281

258282
state := newWorktreeRebaseState(s, ctx, currentBranch, originalRefs, trunk, startIdx, endIdx)
259283
state.CommitterDateIsAuthorDate = opts.committerDateIsAuthorDate
260284
state.NoTrunk = opts.noTrunk
261285
state.UseOnto, state.OntoOldBase = needsOnto, ontoOldBase
286+
state.TargetTrunk = newBase
262287
if s.Trunk.Branch != originalTrunk {
263288
if err := stack.Save(gitDir, sf); err != nil {
264289
return handleSaveError(cfg, err)

0 commit comments

Comments
 (0)