Skip to content

Commit dde62a5

Browse files
skarimskarim
committed
Rebase stacks onto the latest remote trunk
Fetch the configured trunk explicitly before sync or rebase and use that fetched ref whenever the local trunk cannot be safely updated, while preserving local-only and locally-ahead trunks. Fail instead of reporting success when the fetch or rebase never starts, carry the resolved trunk through conflict recovery, and verify the resulting ancestry before sync pushes or either command reports success.
1 parent 53ed88c commit dde62a5

12 files changed

Lines changed: 719 additions & 119 deletions

File tree

‎cmd/rebase.go‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ type rebaseState struct {
3636
OntoOldBase string `json:"ontoOldBase,omitempty"`
3737
CommitterDateIsAuthorDate bool `json:"committerDateIsAuthorDate,omitempty"`
3838
NoTrunk bool `json:"noTrunk,omitempty"`
39+
TrunkRef string `json:"trunkRef,omitempty"`
40+
TrunkSHA string `json:"trunkSha,omitempty"`
41+
StartIndex int `json:"startIndex,omitempty"`
42+
EndIndex int `json:"endIndex,omitempty"`
3943
}
4044

4145
const rebaseStateFile = "gh-stack-rebase-state"
@@ -125,6 +129,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
125129
return ErrSilent
126130
}
127131

132+
var trunk trunkTarget
128133
if !opts.noTrunk {
129134
// Resolve remote for fetch and trunk comparison
130135
remote, err := pickRemote(cfg, currentBranch, opts.remote)
@@ -135,22 +140,16 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
135140
return ErrSilent
136141
}
137142

138-
if err := git.Fetch(remote); err != nil {
139-
cfg.Warningf("Failed to fetch %s: %v", remote, err)
140-
} else {
141-
cfg.Successf("Fetched %s", remote)
143+
trunk, err = resolveTrunkTarget(cfg, s, remote, currentBranch)
144+
if err != nil {
145+
return err
142146
}
143147

144-
// Ensure trunk exists locally before fast-forward or cascade rebase.
145-
if err := ensureLocalTrunk(cfg, s.Trunk.Branch, remote); err != nil {
146-
cfg.Errorf("%s", err)
148+
// Fast-forward stack branches that are behind their remote tracking branch.
149+
if err := git.FetchBranches(remote, activeBranchNames(s)); err != nil {
150+
cfg.Errorf("failed to fetch stack branches from %s: %v", remote, err)
147151
return ErrSilent
148152
}
149-
150-
// Fast-forward trunk so the cascade rebase targets the latest upstream.
151-
fastForwardTrunk(cfg, s.Trunk.Branch, remote, currentBranch)
152-
153-
// Fast-forward stack branches that are behind their remote tracking branch.
154153
fastForwardBranches(cfg, s, remote, currentBranch)
155154
}
156155

@@ -222,10 +221,12 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
222221
NeedsOnto: needsOnto,
223222
OntoOldBase: ontoOldBase,
224223
CommitterDateIsAuthorDate: opts.committerDateIsAuthorDate,
224+
TrunkRef: trunk.Ref,
225225
})
226226

227227
if rebaseResult.Err != nil {
228228
cfg.Errorf("%v", rebaseResult.Err)
229+
_ = git.CheckoutBranch(currentBranch)
229230
return ErrSilent
230231
}
231232

@@ -242,6 +243,10 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
242243
OntoOldBase: rebaseResult.OntoOldBase,
243244
CommitterDateIsAuthorDate: opts.committerDateIsAuthorDate,
244245
NoTrunk: opts.noTrunk,
246+
TrunkRef: trunk.Ref,
247+
TrunkSHA: trunk.SHA,
248+
StartIndex: startIdx,
249+
EndIndex: endIdx,
245250
}
246251
if err := saveRebaseState(gitDir, state); err != nil {
247252
cfg.Warningf("failed to save rebase state: %s", err)
@@ -259,6 +264,11 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
259264

260265
_ = git.CheckoutBranch(currentBranch)
261266

267+
if unstacked := verifyStacked(s, trunk.Ref, startIdx, endIdx); len(unstacked) > 0 {
268+
reportUnstacked(cfg, trunk.Ref, unstacked)
269+
return ErrSilent
270+
}
271+
262272
updateBaseSHAs(s)
263273

264274
_ = syncStackPRs(cfg, s)
@@ -284,7 +294,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
284294
if opts.noTrunk {
285295
cfg.Printf("%s rebased locally (without trunk)", rangeDesc)
286296
} else {
287-
cfg.Printf("%s rebased locally with %s", rangeDesc, s.Trunk.Branch)
297+
cfg.Printf("%s rebased locally with %s", rangeDesc, trunk.Describe())
288298
}
289299
cfg.Printf("To push up your changes, run `%s`",
290300
cfg.ColorCyan("gh stack push"))
@@ -314,6 +324,14 @@ func continueRebase(cfg *config.Config, gitDir string) error {
314324
if s == nil {
315325
return fmt.Errorf("no stack found for branch %s", state.OriginalBranch)
316326
}
327+
trunkRef := state.TrunkRef
328+
if trunkRef == "" {
329+
trunkRef = s.Trunk.Branch
330+
}
331+
trunkBase := state.TrunkSHA
332+
if trunkBase == "" {
333+
trunkBase = trunkRef
334+
}
317335

318336
// Refresh PR state before selecting the base and cascading the remaining
319337
// branches. The queued flag is transient (not persisted), so it was lost
@@ -343,7 +361,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
343361
var baseBranch string
344362
if state.UseOnto {
345363
// The --onto path targets the first non-merged ancestor, or trunk.
346-
baseBranch = s.Trunk.Branch
364+
baseBranch = trunkRef
347365
for j := state.CurrentBranchIndex - 1; j >= 0; j-- {
348366
if !s.Branches[j].IsMerged() {
349367
baseBranch = s.Branches[j].Branch
@@ -353,7 +371,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
353371
} else if state.CurrentBranchIndex > 0 {
354372
baseBranch = s.Branches[state.CurrentBranchIndex-1].Branch
355373
} else {
356-
baseBranch = s.Trunk.Branch
374+
baseBranch = trunkRef
357375
}
358376
cfg.Successf("Rebased %s onto %s", conflictBranch, baseBranch)
359377

@@ -385,6 +403,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
385403
NeedsOnto: state.UseOnto,
386404
OntoOldBase: state.OntoOldBase,
387405
CommitterDateIsAuthorDate: state.CommitterDateIsAuthorDate,
406+
TrunkRef: trunkBase,
388407
})
389408

390409
if result.Err != nil {
@@ -414,9 +433,21 @@ func continueRebase(cfg *config.Config, gitDir string) error {
414433
}
415434
}
416435

417-
clearRebaseState(gitDir)
418436
_ = git.CheckoutBranch(state.OriginalBranch)
419437

438+
verifyStart, verifyEnd := state.StartIndex, state.EndIndex
439+
if verifyEnd <= verifyStart {
440+
verifyStart, verifyEnd = 0, len(s.Branches)
441+
if state.NoTrunk {
442+
verifyStart = 1
443+
}
444+
}
445+
if unstacked := verifyStacked(s, trunkBase, verifyStart, verifyEnd); len(unstacked) > 0 {
446+
reportUnstacked(cfg, trunkRef, unstacked)
447+
return ErrSilent
448+
}
449+
450+
clearRebaseState(gitDir)
420451
updateBaseSHAs(s)
421452

422453
_ = syncStackPRs(cfg, s)
@@ -425,8 +456,10 @@ func continueRebase(cfg *config.Config, gitDir string) error {
425456

426457
if state.NoTrunk {
427458
cfg.Printf("All branches in stack rebased locally (without trunk)")
459+
} else if state.TrunkSHA != "" {
460+
cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA))
428461
} else {
429-
cfg.Printf("All branches in stack rebased locally with %s", s.Trunk.Branch)
462+
cfg.Printf("All branches in stack rebased locally with %s", trunkRef)
430463
}
431464
cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`",
432465
cfg.ColorCyan("gh stack submit"))

‎cmd/rebase_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ func newRebaseMock(tmpDir string, currentBranch string) *git.MockOps {
4444
}
4545
return "sha-" + ref, nil
4646
},
47-
IsAncestorFn: func(a, d string) (bool, error) { return true, nil },
48-
FetchFn: func(string) error { return nil },
49-
EnableRerereFn: func() error { return nil },
47+
IsAncestorFn: func(a, d string) (bool, error) { return true, nil },
48+
FetchFn: func(string) error { return nil },
49+
EnableRerereFn: func() error { return nil },
5050
IsRebaseInProgressFn: func() bool { return false },
5151
}
5252
}
@@ -1293,11 +1293,7 @@ func TestRebase_FastForwardsBranchFromRemote(t *testing.T) {
12931293
return "sha-" + ref, nil
12941294
}
12951295
mock.IsAncestorFn = func(a, d string) (bool, error) {
1296-
// b1-local is ancestor of b1-remote → can fast-forward
1297-
if a == "b1-local-sha" && d == "b1-remote-sha" {
1298-
return true, nil
1299-
}
1300-
return false, nil
1296+
return true, nil
13011297
}
13021298
mock.UpdateBranchRefFn = func(branch, sha string) error {
13031299
updateBranchRefCalls = append(updateBranchRefCalls, struct{ branch, sha string }{branch, sha})
@@ -1415,7 +1411,11 @@ func TestRebase_BranchDiverged_NoFF(t *testing.T) {
14151411
}
14161412
// Neither is ancestor of the other — diverged
14171413
mock.IsAncestorFn = func(a, d string) (bool, error) {
1418-
return false, nil
1414+
if (a == "b1-local-sha" && d == "b1-remote-sha") ||
1415+
(a == "b1-remote-sha" && d == "b1-local-sha") {
1416+
return false, nil
1417+
}
1418+
return true, nil
14191419
}
14201420
mock.UpdateBranchRefFn = func(string, string) error {
14211421
updateBranchRefCalls++

‎cmd/sync.go‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,11 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
107107

108108
// Fetch trunk + active branches so tracking refs are current for
109109
// fast-forward detection (Step 2) and --force-with-lease (Step 4).
110-
fetchTargets := append([]string{s.Trunk.Branch}, activeBranchNames(s)...)
111-
_ = git.FetchBranches(remote, fetchTargets)
112-
cfg.Successf("Fetched latest changes from %s", remote)
110+
normalizeStackTrunk(cfg, s, remote)
111+
if err := git.FetchBranches(remote, activeBranchNames(s)); err != nil {
112+
cfg.Errorf("failed to fetch stack branches from %s: %v", remote, err)
113+
return ErrSilent
114+
}
113115

114116
// --- Step 1b: Reconcile remote-ahead stack changes ---
115117
// Pull in branches for PRs that were added to the stack on GitHub, or
@@ -139,18 +141,17 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
139141
currentBranch = cb
140142
}
141143

142-
// --- Step 2: Fast-forward trunk ---
143-
trunk := s.Trunk.Branch
144-
trunkUpdated := fastForwardTrunk(cfg, trunk, remote, currentBranch)
144+
// --- Step 2: Resolve trunk ---
145+
trunk, err := resolveTrunkTarget(cfg, s, remote, currentBranch)
146+
if err != nil {
147+
return err
148+
}
145149

146150
// --- Step 2b: Fast-forward stack branches behind their remote tracking branch ---
147151
updatedBranches := fastForwardBranches(cfg, s, remote, currentBranch)
148-
branchesUpdated := len(updatedBranches) > 0
149152

150153
// --- Step 3: Cascade rebase ---
151-
// Rebase if trunk or any branch moved, or if the stack is stale
152-
// (branches not yet rebased onto their parent's current tip).
153-
needsRebase := trunkUpdated || branchesUpdated || stackNeedsRebase(s)
154+
needsRebase := trunk.Moved || len(updatedBranches) > 0 || stackNeedsRebase(s, trunk.Ref)
154155
rebased := false
155156
if needsRebase {
156157
cfg.Printf("")
@@ -169,6 +170,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
169170
Branches: s.Branches,
170171
StartAbsIdx: 0,
171172
OriginalRefs: originalRefs,
173+
TrunkRef: trunk.Ref,
172174
})
173175

174176
if result.Err != nil {
@@ -204,6 +206,13 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
204206
_ = git.CheckoutBranch(currentBranch)
205207
}
206208

209+
if unstacked := verifyStacked(s, trunk.Ref, 0, len(s.Branches)); len(unstacked) > 0 {
210+
_ = git.CheckoutBranch(currentBranch)
211+
reportUnstacked(cfg, trunk.Ref, unstacked)
212+
stack.SaveNonBlocking(gitDir, sf)
213+
return ErrSilent
214+
}
215+
207216
// --- Step 4: Push ---
208217
cfg.Printf("")
209218
branches := activeBranchNames(s)
@@ -329,7 +338,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
329338
}
330339
}
331340
if needsSwitch {
332-
switchTarget := trunk
341+
switchTarget := trunk.Branch
333342
for _, b := range s.Branches {
334343
if !b.IsSkipped() {
335344
switchTarget = b.Branch
@@ -385,6 +394,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
385394
// unavailable, or a divergence). Report only what actually happened.
386395
cfg.Successf("Branches synced")
387396
}
397+
cfg.Printf(" Stacked on %s", trunk.Describe())
388398
return nil
389399
}
390400

‎cmd/sync_test.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -139,21 +139,23 @@ func TestSync_TrunkUpToDate_StackStale(t *testing.T) {
139139
}
140140
return "sha-" + ref, nil
141141
}
142-
// Stack branches are NOT rebased onto trunk — parent is not an ancestor.
142+
// Stack branches are NOT rebased onto trunk until the cascade runs.
143+
rebased := false
143144
mock.IsAncestorFn = func(a, d string) (bool, error) {
144-
// main is NOT an ancestor of b1 → stack is stale
145-
if a == "main" && d == "b1" {
145+
if a == "main" && d == "b1" && !rebased {
146146
return false, nil
147147
}
148148
return true, nil
149149
}
150150
mock.CheckoutBranchFn = func(string) error { return nil }
151151
mock.RebaseFn = func(base string, opts git.RebaseOpts) error {
152152
rebaseCalls = append(rebaseCalls, rebaseCall{branch: "(rebase)" + base})
153+
rebased = true
153154
return nil
154155
}
155156
mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error {
156157
rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
158+
rebased = true
157159
return nil
158160
}
159161
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
@@ -298,10 +300,13 @@ func TestSync_TrunkFastForward_WhenOnTrunk(t *testing.T) {
298300
if ref == "origin/main" {
299301
return "remote-sha", nil
300302
}
303+
if strings.HasPrefix(ref, "origin/") {
304+
return "sha-" + strings.TrimPrefix(ref, "origin/"), nil
305+
}
301306
return "sha-" + ref, nil
302307
}
303308
mock.IsAncestorFn = func(a, d string) (bool, error) {
304-
return a == "local-sha" && d == "remote-sha", nil
309+
return true, nil
305310
}
306311
mock.MergeFFFn = func(target string) error {
307312
mergeFFCalls = append(mergeFFCalls, target)
@@ -625,7 +630,7 @@ func TestSync_PushForceFlagDependsOnRebase(t *testing.T) {
625630
return "sha-" + ref, nil
626631
}
627632
mock.IsAncestorFn = func(a, d string) (bool, error) {
628-
return a == "local-sha" && d == "remote-sha", nil
633+
return true, nil
629634
}
630635
mock.UpdateBranchRefFn = func(string, string) error { return nil }
631636
} else {
@@ -938,7 +943,7 @@ func TestSync_PushFailureAfterRebase(t *testing.T) {
938943
return "sha-" + ref, nil
939944
}
940945
mock.IsAncestorFn = func(a, d string) (bool, error) {
941-
return a == "local-sha" && d == "remote-sha", nil
946+
return true, nil
942947
}
943948
mock.UpdateBranchRefFn = func(string, string) error { return nil }
944949
mock.CheckoutBranchFn = func(string) error { return nil }
@@ -1006,10 +1011,7 @@ func TestSync_BranchFastForward_TriggersRebase(t *testing.T) {
10061011
return "sha-" + ref, nil
10071012
}
10081013
mock.IsAncestorFn = func(a, d string) (bool, error) {
1009-
if a == "b1-local-sha" && d == "b1-remote-sha" {
1010-
return true, nil
1011-
}
1012-
return false, nil
1014+
return true, nil
10131015
}
10141016
mock.MergeFFFn = func(target string) error {
10151017
mergeFFCalls = append(mergeFFCalls, target)
@@ -1096,13 +1098,7 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) {
10961098
return "sha-" + ref, nil
10971099
}
10981100
mock.IsAncestorFn = func(a, d string) (bool, error) {
1099-
if a == "trunk-local" && d == "trunk-remote" {
1100-
return true, nil
1101-
}
1102-
if a == "b2-local" && d == "b2-remote" {
1103-
return true, nil
1104-
}
1105-
return false, nil
1101+
return true, nil
11061102
}
11071103
mock.UpdateBranchRefFn = func(branch, sha string) error {
11081104
updateBranchRefCalls = append(updateBranchRefCalls, struct{ branch, sha string }{branch, sha})

0 commit comments

Comments
 (0)