Skip to content

Commit a5cae7b

Browse files
skarimskarim
authored
Rebase stacks onto the latest remote trunk (#330)
* 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. * Restore stacks after incomplete cascade rebases Roll back branches already rewritten when a later rebase cannot start or final ancestry verification fails, preventing retries from replaying stale history. Preserve retryable modify state without repeating completed work, and add regression coverage for remote-qualified trunk normalization.
1 parent 53ed88c commit a5cae7b

13 files changed

Lines changed: 1133 additions & 130 deletions

File tree

‎cmd/rebase.go‎

Lines changed: 61 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,16 @@ 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+
if rebaseResult.Rebased {
230+
restoreRebaseRefs(cfg, currentBranch, originalRefs)
231+
} else {
232+
_ = git.CheckoutBranch(currentBranch)
233+
}
229234
return ErrSilent
230235
}
231236

@@ -242,6 +247,10 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
242247
OntoOldBase: rebaseResult.OntoOldBase,
243248
CommitterDateIsAuthorDate: opts.committerDateIsAuthorDate,
244249
NoTrunk: opts.noTrunk,
250+
TrunkRef: trunk.Ref,
251+
TrunkSHA: trunk.SHA,
252+
StartIndex: startIdx,
253+
EndIndex: endIdx,
245254
}
246255
if err := saveRebaseState(gitDir, state); err != nil {
247256
cfg.Warningf("failed to save rebase state: %s", err)
@@ -259,6 +268,14 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
259268

260269
_ = git.CheckoutBranch(currentBranch)
261270

271+
if unstacked := verifyStacked(s, trunk.Ref, startIdx, endIdx); len(unstacked) > 0 {
272+
reportUnstacked(cfg, trunk.Ref, unstacked)
273+
if rebaseResult.Rebased {
274+
restoreRebaseRefs(cfg, currentBranch, originalRefs)
275+
}
276+
return ErrSilent
277+
}
278+
262279
updateBaseSHAs(s)
263280

264281
_ = syncStackPRs(cfg, s)
@@ -284,7 +301,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
284301
if opts.noTrunk {
285302
cfg.Printf("%s rebased locally (without trunk)", rangeDesc)
286303
} else {
287-
cfg.Printf("%s rebased locally with %s", rangeDesc, s.Trunk.Branch)
304+
cfg.Printf("%s rebased locally with %s", rangeDesc, trunk.Describe())
288305
}
289306
cfg.Printf("To push up your changes, run `%s`",
290307
cfg.ColorCyan("gh stack push"))
@@ -314,6 +331,14 @@ func continueRebase(cfg *config.Config, gitDir string) error {
314331
if s == nil {
315332
return fmt.Errorf("no stack found for branch %s", state.OriginalBranch)
316333
}
334+
trunkRef := state.TrunkRef
335+
if trunkRef == "" {
336+
trunkRef = s.Trunk.Branch
337+
}
338+
trunkBase := state.TrunkSHA
339+
if trunkBase == "" {
340+
trunkBase = trunkRef
341+
}
317342

318343
// Refresh PR state before selecting the base and cascading the remaining
319344
// branches. The queued flag is transient (not persisted), so it was lost
@@ -343,7 +368,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
343368
var baseBranch string
344369
if state.UseOnto {
345370
// The --onto path targets the first non-merged ancestor, or trunk.
346-
baseBranch = s.Trunk.Branch
371+
baseBranch = trunkRef
347372
for j := state.CurrentBranchIndex - 1; j >= 0; j-- {
348373
if !s.Branches[j].IsMerged() {
349374
baseBranch = s.Branches[j].Branch
@@ -353,7 +378,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
353378
} else if state.CurrentBranchIndex > 0 {
354379
baseBranch = s.Branches[state.CurrentBranchIndex-1].Branch
355380
} else {
356-
baseBranch = s.Trunk.Branch
381+
baseBranch = trunkRef
357382
}
358383
cfg.Successf("Rebased %s onto %s", conflictBranch, baseBranch)
359384

@@ -385,10 +410,13 @@ func continueRebase(cfg *config.Config, gitDir string) error {
385410
NeedsOnto: state.UseOnto,
386411
OntoOldBase: state.OntoOldBase,
387412
CommitterDateIsAuthorDate: state.CommitterDateIsAuthorDate,
413+
TrunkRef: trunkBase,
388414
})
389415

390416
if result.Err != nil {
391417
cfg.Errorf("%v", result.Err)
418+
restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs)
419+
clearRebaseState(gitDir)
392420
return ErrSilent
393421
}
394422

@@ -414,9 +442,23 @@ func continueRebase(cfg *config.Config, gitDir string) error {
414442
}
415443
}
416444

417-
clearRebaseState(gitDir)
418445
_ = git.CheckoutBranch(state.OriginalBranch)
419446

447+
verifyStart, verifyEnd := state.StartIndex, state.EndIndex
448+
if verifyEnd <= verifyStart {
449+
verifyStart, verifyEnd = 0, len(s.Branches)
450+
if state.NoTrunk {
451+
verifyStart = 1
452+
}
453+
}
454+
if unstacked := verifyStacked(s, trunkBase, verifyStart, verifyEnd); len(unstacked) > 0 {
455+
reportUnstacked(cfg, trunkRef, unstacked)
456+
restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs)
457+
clearRebaseState(gitDir)
458+
return ErrSilent
459+
}
460+
461+
clearRebaseState(gitDir)
420462
updateBaseSHAs(s)
421463

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

426468
if state.NoTrunk {
427469
cfg.Printf("All branches in stack rebased locally (without trunk)")
470+
} else if state.TrunkSHA != "" {
471+
cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA))
428472
} else {
429-
cfg.Printf("All branches in stack rebased locally with %s", s.Trunk.Branch)
473+
cfg.Printf("All branches in stack rebased locally with %s", trunkRef)
430474
}
431475
cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`",
432476
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: 43 additions & 13 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,27 +141,27 @@ 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
156+
var originalRefs map[string]string
155157
if needsRebase {
156158
cfg.Printf("")
157159
cfg.Printf("Rebasing stack ...")
158160

159161
// Sync PR state to detect merged PRs before rebasing.
160162
_ = syncStackPRs(cfg, s)
161163

162-
originalRefs, err := resolveOriginalRefs(s)
164+
originalRefs, err = resolveOriginalRefs(s)
163165
if err != nil {
164166
cfg.Warningf("Could not resolve branch SHAs — skipping rebase: %v", err)
165167
} else {
@@ -169,11 +171,16 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
169171
Branches: s.Branches,
170172
StartAbsIdx: 0,
171173
OriginalRefs: originalRefs,
174+
TrunkRef: trunk.Ref,
172175
})
173176

174177
if result.Err != nil {
175178
cfg.Errorf("%v", result.Err)
176-
_ = git.CheckoutBranch(currentBranch)
179+
if result.Rebased {
180+
restoreRebaseRefs(cfg, currentBranch, originalRefs)
181+
} else {
182+
_ = git.CheckoutBranch(currentBranch)
183+
}
177184
stack.SaveNonBlocking(gitDir, sf)
178185
return ErrSilent
179186
}
@@ -204,6 +211,16 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
204211
_ = git.CheckoutBranch(currentBranch)
205212
}
206213

214+
if unstacked := verifyStacked(s, trunk.Ref, 0, len(s.Branches)); len(unstacked) > 0 {
215+
_ = git.CheckoutBranch(currentBranch)
216+
reportUnstacked(cfg, trunk.Ref, unstacked)
217+
if rebased && originalRefs != nil {
218+
restoreRebaseRefs(cfg, currentBranch, originalRefs)
219+
}
220+
stack.SaveNonBlocking(gitDir, sf)
221+
return ErrSilent
222+
}
223+
207224
// --- Step 4: Push ---
208225
cfg.Printf("")
209226
branches := activeBranchNames(s)
@@ -329,7 +346,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
329346
}
330347
}
331348
if needsSwitch {
332-
switchTarget := trunk
349+
switchTarget := trunk.Branch
333350
for _, b := range s.Branches {
334351
if !b.IsSkipped() {
335352
switchTarget = b.Branch
@@ -385,13 +402,20 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
385402
// unavailable, or a divergence). Report only what actually happened.
386403
cfg.Successf("Branches synced")
387404
}
405+
cfg.Printf(" Stacked on %s", trunk.Describe())
388406
return nil
389407
}
390408

391409
// restoreBranches resets each branch to its original SHA, collecting any errors.
392410
func restoreBranches(originalRefs map[string]string) []string {
393411
var errors []string
394412
for branch, sha := range originalRefs {
413+
if !git.BranchExists(branch) {
414+
continue
415+
}
416+
if currentSHA, err := git.RevParse(branch); err == nil && currentSHA == sha {
417+
continue
418+
}
395419
if err := git.CheckoutBranch(branch); err != nil {
396420
errors = append(errors, fmt.Sprintf("checkout %s: %s", branch, err))
397421
continue
@@ -403,6 +427,12 @@ func restoreBranches(originalRefs map[string]string) []string {
403427
return errors
404428
}
405429

430+
func restoreRebaseRefs(cfg *config.Config, originalBranch string, originalRefs map[string]string) {
431+
restoreErrors := restoreBranches(originalRefs)
432+
_ = git.CheckoutBranch(originalBranch)
433+
reportRestoreStatus(cfg, restoreErrors)
434+
}
435+
406436
// reportRestoreStatus prints whether branch restoration succeeded or partially failed.
407437
func reportRestoreStatus(cfg *config.Config, restoreErrors []string) {
408438
if len(restoreErrors) > 0 {

0 commit comments

Comments
 (0)