Skip to content

Commit 22c8ef4

Browse files
committed
fix for rev-parse error during sync with deleted branches (#42)
* fix for rev-parse error during sync * clearer info msg with rebasing over merged PRs
1 parent b038eed commit 22c8ef4

File tree

8 files changed

+111
-29
lines changed

8 files changed

+111
-29
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ Pull from remote and do a cascading rebase across the stack.
197197
gh stack rebase [flags] [branch]
198198
```
199199

200-
Fetches the latest changes from `origin`, then ensures each branch in the stack has the tip of the previous layer in its commit history. Rebases branches in order from trunk upward. If a branch's PR has been squash-merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
200+
Fetches the latest changes from `origin`, then ensures each branch in the stack has the tip of the previous layer in its commit history. Rebases branches in order from trunk upward. If a branch's PR has been merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
201201

202202
If a rebase conflict occurs, the operation pauses and prints the conflicted files with line numbers. Resolve the conflicts, stage with `git add`, and continue with `--continue`. To undo the entire rebase, use `--abort` to restore all branches to their pre-rebase state.
203203

cmd/rebase.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
188188
return ErrSilent
189189
}
190190

191-
// Track --onto rebase state for squash-merged branches.
191+
// Track --onto rebase state for merged branches.
192192
needsOnto := false
193193
var ontoOldBase string
194194

@@ -201,7 +201,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
201201
base = s.Branches[absIdx-1].Branch
202202
}
203203

204-
// Skip branches whose PRs have already been merged (e.g. via squash).
204+
// Skip branches whose PRs have already been merged.
205205
// Record state so subsequent branches can use --onto rebase.
206206
if br.IsMerged() {
207207
ontoOldBase = originalRefs[br.Branch]
@@ -252,7 +252,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
252252
return ErrConflict
253253
}
254254

255-
cfg.Successf("Rebased %s onto %s (squash-merge detected)", br.Branch, newBase)
255+
cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", br.Branch, newBase)
256256
// Keep --onto mode; update old base for the next branch.
257257
ontoOldBase = originalRefs[br.Branch]
258258
} else {
@@ -450,7 +450,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
450450
return ErrConflict
451451
}
452452

453-
cfg.Successf("Rebased %s onto %s (squash-merge detected)", branchName, newBase)
453+
cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", branchName, newBase)
454454
state.OntoOldBase = state.OriginalRefs[branchName]
455455
} else {
456456
var rebaseErr error

cmd/rebase_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ func TestRebase_CascadeRebase(t *testing.T) {
106106
assert.Contains(t, output, "rebased locally")
107107
}
108108

109-
// TestRebase_SquashMergedBranch_UsesOnto verifies that when b1 has a merged PR,
109+
// TestRebase_MergedBranch_UsesOnto verifies that when b1 has a merged PR,
110110
// it is skipped and b2 uses RebaseOnto with trunk as newBase and b1's original
111111
// SHA as oldBase. b3 also uses --onto (propagation).
112-
func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) {
112+
func TestRebase_MergedBranch_UsesOnto(t *testing.T) {
113113
s := stack.Stack{
114114
Trunk: stack.BranchRef{Branch: "main"},
115115
Branches: []stack.BranchRef{
@@ -171,7 +171,7 @@ func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) {
171171
}
172172

173173
// TestRebase_OntoPropagatesToSubsequentBranches verifies that when multiple
174-
// branches are squash-merged, --onto propagates correctly through the chain.
174+
// branches are merged, --onto propagates correctly through the chain.
175175
func TestRebase_OntoPropagatesToSubsequentBranches(t *testing.T) {
176176
s := stack.Stack{
177177
Trunk: stack.BranchRef{Branch: "main"},
@@ -651,7 +651,7 @@ func TestRebase_Continue_RebasesRemainingBranches(t *testing.T) {
651651
}
652652

653653
// TestRebase_Continue_OntoMode verifies the --continue path when UseOnto is
654-
// set (squash-merged branches upstream). With no remaining branches, only
654+
// set (merged branches upstream). With no remaining branches, only
655655
// RebaseContinue runs and the state is cleaned up.
656656
func TestRebase_Continue_OntoMode(t *testing.T) {
657657
s := stack.Stack{

cmd/sync.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,16 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
129129
// Sync PR state to detect merged PRs before rebasing.
130130
syncStackPRs(cfg, s)
131131

132-
// Save original refs so we can restore on conflict
133-
branchNames := make([]string, len(s.Branches))
134-
for i, b := range s.Branches {
135-
branchNames[i] = b.Branch
132+
// Save original refs so we can restore on conflict.
133+
// Merged branches that no longer exist locally have no ref to
134+
// resolve. They are always skipped during rebase but we must
135+
// also exclude them here to avoid a rev-parse error.
136+
branchNames := make([]string, 0, len(s.Branches))
137+
for _, b := range s.Branches {
138+
if b.IsMerged() && !git.BranchExists(b.Branch) {
139+
continue
140+
}
141+
branchNames = append(branchNames, b.Branch)
136142
}
137143
originalRefs, _ := git.RevParseMap(branchNames)
138144

@@ -191,7 +197,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
191197
break
192198
}
193199

194-
cfg.Successf("Rebased %s onto %s (squash-merge detected)", br.Branch, newBase)
200+
cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", br.Branch, newBase)
195201
ontoOldBase = originalRefs[br.Branch]
196202
} else {
197203
var rebaseErr error

cmd/sync_test.go

Lines changed: 87 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -522,10 +522,10 @@ func TestSync_PushForceFlagDependsOnRebase(t *testing.T) {
522522
}
523523
}
524524

525-
// TestSync_SquashMergedBranch_UsesOnto verifies that when a squash-merged
525+
// TestSync_MergedBranch_UsesOnto verifies that when a merged
526526
// branch exists in the stack, sync's cascade rebase correctly uses --onto
527527
// to skip the merged branch and rebase subsequent branches onto the right base.
528-
func TestSync_SquashMergedBranch_UsesOnto(t *testing.T) {
528+
func TestSync_MergedBranch_UsesOnto(t *testing.T) {
529529
s := stack.Stack{
530530
Trunk: stack.BranchRef{Branch: "main"},
531531
Branches: []stack.BranchRef{
@@ -549,6 +549,7 @@ func TestSync_SquashMergedBranch_UsesOnto(t *testing.T) {
549549
}
550550

551551
mock := newSyncMock(tmpDir, "b2")
552+
mock.BranchExistsFn = func(name string) bool { return true }
552553
// Trunk behind remote to trigger rebase
553554
mock.RevParseFn = func(ref string) (string, error) {
554555
if ref == "main" {
@@ -765,8 +766,8 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) {
765766
writeStackFile(t, tmpDir, s)
766767

767768
var updateBranchRefCalls []struct{ branch, sha string }
768-
var rebaseCalls []rebaseCall
769-
var pushCalls []pushCall
769+
var rebaseCalls2 []rebaseCall
770+
var pushCalls2 []pushCall
770771

771772
mock := newSyncMock(tmpDir, "b1")
772773
// Trunk and b2 both behind remote
@@ -803,15 +804,15 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) {
803804
}
804805
mock.CheckoutBranchFn = func(string) error { return nil }
805806
mock.RebaseFn = func(base string) error {
806-
rebaseCalls = append(rebaseCalls, rebaseCall{branch: "(rebase)" + base})
807+
rebaseCalls2 = append(rebaseCalls2, rebaseCall{branch: "(rebase)" + base})
807808
return nil
808809
}
809810
mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
810-
rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
811+
rebaseCalls2 = append(rebaseCalls2, rebaseCall{newBase, oldBase, branch})
811812
return nil
812813
}
813814
mock.PushFn = func(remote string, branches []string, force, atomic bool) error {
814-
pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic})
815+
pushCalls2 = append(pushCalls2, pushCall{remote, branches, force, atomic})
815816
return nil
816817
}
817818

@@ -829,7 +830,6 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) {
829830
output := string(errOut)
830831

831832
assert.NoError(t, err)
832-
833833
// Both trunk and b2 should be updated
834834
branchUpdates := make(map[string]string)
835835
for _, c := range updateBranchRefCalls {
@@ -839,7 +839,83 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) {
839839
assert.Equal(t, "b2-remote", branchUpdates["b2"], "b2 should be fast-forwarded")
840840

841841
assert.Contains(t, output, "fast-forwarded")
842-
assert.NotEmpty(t, rebaseCalls, "rebase should occur")
843-
require.Len(t, pushCalls, 1)
844-
assert.True(t, pushCalls[0].force, "push should use force after rebase")
842+
assert.NotEmpty(t, rebaseCalls2, "rebase should occur")
843+
require.Len(t, pushCalls2, 1)
844+
assert.True(t, pushCalls2[0].force, "push should use force after rebase")
845+
}
846+
847+
func TestSync_MergedBranchDeletedFromRemote(t *testing.T) {
848+
s := stack.Stack{
849+
Trunk: stack.BranchRef{Branch: "main"},
850+
Branches: []stack.BranchRef{
851+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
852+
{Branch: "b2"},
853+
},
854+
}
855+
856+
tmpDir := t.TempDir()
857+
writeStackFile(t, tmpDir, s)
858+
859+
var rebaseOntoCalls []rebaseCall
860+
861+
mock := newSyncMock(tmpDir, "b2")
862+
mock.BranchExistsFn = func(name string) bool {
863+
// b1 does not exist locally (deleted from remote after merge)
864+
return name != "b1"
865+
}
866+
mock.RevParseMultiFn = func(refs []string) ([]string, error) {
867+
shas := make([]string, len(refs))
868+
for i, r := range refs {
869+
if r == "b1" {
870+
t.Fatalf("RevParseMulti should not be called with non-existent branch b1")
871+
}
872+
if r == "main" {
873+
shas[i] = "local-sha"
874+
} else if r == "origin/main" {
875+
shas[i] = "remote-sha"
876+
} else {
877+
shas[i] = "sha-" + r
878+
}
879+
}
880+
return shas, nil
881+
}
882+
// Trunk behind remote to trigger rebase
883+
mock.RevParseFn = func(ref string) (string, error) {
884+
if ref == "main" {
885+
return "local-sha", nil
886+
}
887+
if ref == "origin/main" {
888+
return "remote-sha", nil
889+
}
890+
return "sha-" + ref, nil
891+
}
892+
mock.IsAncestorFn = func(a, d string) (bool, error) {
893+
return a == "local-sha" && d == "remote-sha", nil
894+
}
895+
mock.UpdateBranchRefFn = func(string, string) error { return nil }
896+
mock.CheckoutBranchFn = func(string) error { return nil }
897+
mock.RebaseOntoFn = func(newBase, oldBase, branch string) error {
898+
rebaseOntoCalls = append(rebaseOntoCalls, rebaseCall{newBase, oldBase, branch})
899+
return nil
900+
}
901+
902+
restore := git.SetOps(mock)
903+
defer restore()
904+
905+
cfg, _, errR := config.NewTestConfig()
906+
cmd := SyncCmd(cfg)
907+
cmd.SetOut(io.Discard)
908+
cmd.SetErr(io.Discard)
909+
err := cmd.Execute()
910+
911+
cfg.Err.Close()
912+
errOut, _ := io.ReadAll(errR)
913+
output := string(errOut)
914+
915+
assert.NoError(t, err)
916+
assert.Contains(t, output, "Skipping b1")
917+
918+
// Only b2 should be rebased
919+
require.Len(t, rebaseOntoCalls, 1)
920+
assert.Equal(t, "b2", rebaseOntoCalls[0].branch)
845921
}

docs/src/content/docs/reference/cli.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ gh stack rebase [flags] [branch]
223223

224224
Fetches the latest changes from `origin`, then ensures each branch in the stack has the tip of the previous layer in its commit history. Rebases branches in order from trunk upward.
225225

226-
If a branch's PR has been squash-merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
226+
If a branch's PR has been merged, the rebase automatically switches to `--onto` mode to correctly replay commits on top of the merge target.
227227

228228
If a rebase conflict occurs, the operation pauses and prints the conflicted files with line numbers. Resolve the conflicts, stage with `git add`, and continue with `--continue`. To undo the entire rebase, use `--abort` to restore all branches to their pre-rebase state.
229229

internal/git/git.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func SaveRerereDeclined() error {
179179
// git rebase --onto <newBase> <oldBase> <branch>
180180
//
181181
// This replays commits after oldBase from branch onto newBase. It is used
182-
// when a prior branch was squash-merged and the normal rebase cannot detect
182+
// when a prior branch was merged and the normal rebase cannot detect
183183
// which commits have already been applied.
184184
// If rerere resolves all conflicts automatically, the rebase continues
185185
// without user intervention.

skills/gh-stack/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ gh stack sync [flags]
570570

571571
1. **Fetch** latest changes from the remote
572572
2. **Fast-forward trunk** to match remote (skips if already up to date, warns if diverged)
573-
3. **Cascade rebase** all stack branches onto their updated parents (only if trunk moved). Handles squash-merged PRs automatically. If a conflict is detected, **all branches are restored** to their pre-rebase state and the command exits with code 3 — see [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) for the resolution workflow
573+
3. **Cascade rebase** all stack branches onto their updated parents (only if trunk moved). Handles merged PRs automatically. If a conflict is detected, **all branches are restored** to their pre-rebase state and the command exits with code 3 — see [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) for the resolution workflow
574574
4. **Push** all active branches atomically
575575
5. **Sync PR state** from GitHub and report the status of each PR
576576

@@ -625,7 +625,7 @@ gh stack rebase --abort
625625

626626
**Conflict handling:** See [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) in the Workflows section for the full resolution workflow.
627627

628-
**Squash-merge detection:** If a branch's PR was squash-merged on GitHub, the rebase automatically handles this and correctly replays commits on top of the merge target.
628+
**Merged PR detection:** If a branch's PR was merged on GitHub, the rebase automatically handles this using `--onto` mode and correctly replays commits on top of the merge target.
629629

630630
**Rerere (conflict memory):** `git rerere` is enabled by `init` so previously resolved conflicts are auto-resolved in future rebases.
631631

0 commit comments

Comments
 (0)