diff --git a/README.md b/README.md index f2e6463..baa33d0 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Pull from remote and do a cascading rebase across the stack. gh stack rebase [flags] [branch] ``` -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. +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. 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. diff --git a/cmd/rebase.go b/cmd/rebase.go index 0d0ef41..caa3c32 100644 --- a/cmd/rebase.go +++ b/cmd/rebase.go @@ -188,7 +188,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { return ErrSilent } - // Track --onto rebase state for squash-merged branches. + // Track --onto rebase state for merged branches. needsOnto := false var ontoOldBase string @@ -201,7 +201,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { base = s.Branches[absIdx-1].Branch } - // Skip branches whose PRs have already been merged (e.g. via squash). + // Skip branches whose PRs have already been merged. // Record state so subsequent branches can use --onto rebase. if br.IsMerged() { ontoOldBase = originalRefs[br.Branch] @@ -252,7 +252,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error { return ErrConflict } - cfg.Successf("Rebased %s onto %s (squash-merge detected)", br.Branch, newBase) + cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", br.Branch, newBase) // Keep --onto mode; update old base for the next branch. ontoOldBase = originalRefs[br.Branch] } else { @@ -450,7 +450,7 @@ func continueRebase(cfg *config.Config, gitDir string) error { return ErrConflict } - cfg.Successf("Rebased %s onto %s (squash-merge detected)", branchName, newBase) + cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", branchName, newBase) state.OntoOldBase = state.OriginalRefs[branchName] } else { var rebaseErr error diff --git a/cmd/rebase_test.go b/cmd/rebase_test.go index 47255cf..cd77a90 100644 --- a/cmd/rebase_test.go +++ b/cmd/rebase_test.go @@ -106,10 +106,10 @@ func TestRebase_CascadeRebase(t *testing.T) { assert.Contains(t, output, "rebased locally") } -// TestRebase_SquashMergedBranch_UsesOnto verifies that when b1 has a merged PR, +// TestRebase_MergedBranch_UsesOnto verifies that when b1 has a merged PR, // it is skipped and b2 uses RebaseOnto with trunk as newBase and b1's original // SHA as oldBase. b3 also uses --onto (propagation). -func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) { +func TestRebase_MergedBranch_UsesOnto(t *testing.T) { s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{ @@ -171,7 +171,7 @@ func TestRebase_SquashMergedBranch_UsesOnto(t *testing.T) { } // TestRebase_OntoPropagatesToSubsequentBranches verifies that when multiple -// branches are squash-merged, --onto propagates correctly through the chain. +// branches are merged, --onto propagates correctly through the chain. func TestRebase_OntoPropagatesToSubsequentBranches(t *testing.T) { s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, @@ -651,7 +651,7 @@ func TestRebase_Continue_RebasesRemainingBranches(t *testing.T) { } // TestRebase_Continue_OntoMode verifies the --continue path when UseOnto is -// set (squash-merged branches upstream). With no remaining branches, only +// set (merged branches upstream). With no remaining branches, only // RebaseContinue runs and the state is cleaned up. func TestRebase_Continue_OntoMode(t *testing.T) { s := stack.Stack{ diff --git a/cmd/sync.go b/cmd/sync.go index c7035a6..a405817 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -129,10 +129,16 @@ func runSync(cfg *config.Config, opts *syncOptions) error { // Sync PR state to detect merged PRs before rebasing. syncStackPRs(cfg, s) - // Save original refs so we can restore on conflict - branchNames := make([]string, len(s.Branches)) - for i, b := range s.Branches { - branchNames[i] = b.Branch + // Save original refs so we can restore on conflict. + // Merged branches that no longer exist locally have no ref to + // resolve. They are always skipped during rebase but we must + // also exclude them here to avoid a rev-parse error. + branchNames := make([]string, 0, len(s.Branches)) + for _, b := range s.Branches { + if b.IsMerged() && !git.BranchExists(b.Branch) { + continue + } + branchNames = append(branchNames, b.Branch) } originalRefs, _ := git.RevParseMap(branchNames) @@ -191,7 +197,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error { break } - cfg.Successf("Rebased %s onto %s (squash-merge detected)", br.Branch, newBase) + cfg.Successf("Rebased %s onto %s (adjusted for merged PR)", br.Branch, newBase) ontoOldBase = originalRefs[br.Branch] } else { var rebaseErr error diff --git a/cmd/sync_test.go b/cmd/sync_test.go index 6704040..78a58b0 100644 --- a/cmd/sync_test.go +++ b/cmd/sync_test.go @@ -522,10 +522,10 @@ func TestSync_PushForceFlagDependsOnRebase(t *testing.T) { } } -// TestSync_SquashMergedBranch_UsesOnto verifies that when a squash-merged +// TestSync_MergedBranch_UsesOnto verifies that when a merged // branch exists in the stack, sync's cascade rebase correctly uses --onto // to skip the merged branch and rebase subsequent branches onto the right base. -func TestSync_SquashMergedBranch_UsesOnto(t *testing.T) { +func TestSync_MergedBranch_UsesOnto(t *testing.T) { s := stack.Stack{ Trunk: stack.BranchRef{Branch: "main"}, Branches: []stack.BranchRef{ @@ -549,6 +549,7 @@ func TestSync_SquashMergedBranch_UsesOnto(t *testing.T) { } mock := newSyncMock(tmpDir, "b2") + mock.BranchExistsFn = func(name string) bool { return true } // Trunk behind remote to trigger rebase mock.RevParseFn = func(ref string) (string, error) { if ref == "main" { @@ -765,8 +766,8 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) { writeStackFile(t, tmpDir, s) var updateBranchRefCalls []struct{ branch, sha string } - var rebaseCalls []rebaseCall - var pushCalls []pushCall + var rebaseCalls2 []rebaseCall + var pushCalls2 []pushCall mock := newSyncMock(tmpDir, "b1") // Trunk and b2 both behind remote @@ -803,15 +804,15 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) { } mock.CheckoutBranchFn = func(string) error { return nil } mock.RebaseFn = func(base string) error { - rebaseCalls = append(rebaseCalls, rebaseCall{branch: "(rebase)" + base}) + rebaseCalls2 = append(rebaseCalls2, rebaseCall{branch: "(rebase)" + base}) return nil } mock.RebaseOntoFn = func(newBase, oldBase, branch string) error { - rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch}) + rebaseCalls2 = append(rebaseCalls2, rebaseCall{newBase, oldBase, branch}) return nil } mock.PushFn = func(remote string, branches []string, force, atomic bool) error { - pushCalls = append(pushCalls, pushCall{remote, branches, force, atomic}) + pushCalls2 = append(pushCalls2, pushCall{remote, branches, force, atomic}) return nil } @@ -829,7 +830,6 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) { output := string(errOut) assert.NoError(t, err) - // Both trunk and b2 should be updated branchUpdates := make(map[string]string) for _, c := range updateBranchRefCalls { @@ -839,7 +839,83 @@ func TestSync_BranchFastForward_WithTrunkUpdate(t *testing.T) { assert.Equal(t, "b2-remote", branchUpdates["b2"], "b2 should be fast-forwarded") assert.Contains(t, output, "fast-forwarded") - assert.NotEmpty(t, rebaseCalls, "rebase should occur") - require.Len(t, pushCalls, 1) - assert.True(t, pushCalls[0].force, "push should use force after rebase") + assert.NotEmpty(t, rebaseCalls2, "rebase should occur") + require.Len(t, pushCalls2, 1) + assert.True(t, pushCalls2[0].force, "push should use force after rebase") +} + +func TestSync_MergedBranchDeletedFromRemote(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}}, + {Branch: "b2"}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + var rebaseOntoCalls []rebaseCall + + mock := newSyncMock(tmpDir, "b2") + mock.BranchExistsFn = func(name string) bool { + // b1 does not exist locally (deleted from remote after merge) + return name != "b1" + } + mock.RevParseMultiFn = func(refs []string) ([]string, error) { + shas := make([]string, len(refs)) + for i, r := range refs { + if r == "b1" { + t.Fatalf("RevParseMulti should not be called with non-existent branch b1") + } + if r == "main" { + shas[i] = "local-sha" + } else if r == "origin/main" { + shas[i] = "remote-sha" + } else { + shas[i] = "sha-" + r + } + } + return shas, nil + } + // Trunk behind remote to trigger rebase + mock.RevParseFn = func(ref string) (string, error) { + if ref == "main" { + return "local-sha", nil + } + if ref == "origin/main" { + return "remote-sha", nil + } + return "sha-" + ref, nil + } + mock.IsAncestorFn = func(a, d string) (bool, error) { + return a == "local-sha" && d == "remote-sha", nil + } + mock.UpdateBranchRefFn = func(string, string) error { return nil } + mock.CheckoutBranchFn = func(string) error { return nil } + mock.RebaseOntoFn = func(newBase, oldBase, branch string) error { + rebaseOntoCalls = append(rebaseOntoCalls, rebaseCall{newBase, oldBase, branch}) + return nil + } + + restore := git.SetOps(mock) + defer restore() + + cfg, _, errR := config.NewTestConfig() + cmd := SyncCmd(cfg) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Err.Close() + errOut, _ := io.ReadAll(errR) + output := string(errOut) + + assert.NoError(t, err) + assert.Contains(t, output, "Skipping b1") + + // Only b2 should be rebased + require.Len(t, rebaseOntoCalls, 1) + assert.Equal(t, "b2", rebaseOntoCalls[0].branch) } diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 5853d9f..75d845f 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -223,7 +223,7 @@ gh stack rebase [flags] [branch] 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. +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. 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. diff --git a/internal/git/git.go b/internal/git/git.go index 8af505d..b7b9941 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -179,7 +179,7 @@ func SaveRerereDeclined() error { // git rebase --onto // // This replays commits after oldBase from branch onto newBase. It is used -// when a prior branch was squash-merged and the normal rebase cannot detect +// when a prior branch was merged and the normal rebase cannot detect // which commits have already been applied. // If rerere resolves all conflicts automatically, the rebase continues // without user intervention. diff --git a/skills/gh-stack/SKILL.md b/skills/gh-stack/SKILL.md index 1dc3a51..6d58198 100644 --- a/skills/gh-stack/SKILL.md +++ b/skills/gh-stack/SKILL.md @@ -570,7 +570,7 @@ gh stack sync [flags] 1. **Fetch** latest changes from the remote 2. **Fast-forward trunk** to match remote (skips if already up to date, warns if diverged) -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 +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 4. **Push** all active branches atomically 5. **Sync PR state** from GitHub and report the status of each PR @@ -625,7 +625,7 @@ gh stack rebase --abort **Conflict handling:** See [Handle rebase conflicts](#handle-rebase-conflicts-agent-workflow) in the Workflows section for the full resolution workflow. -**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. +**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. **Rerere (conflict memory):** `git rerere` is enabled by `init` so previously resolved conflicts are auto-resolved in future rebases.