Fix rebase treating queued PRs as merged

cmd/rebase.go

@@ -198,14 +198,14 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
 		return fmt.Errorf("resolving branch refs: %w", err)
 	}
 
-	// Get --onto state from merged/queued branches below the rebase range.
-	// Ensures that when --upstack excludes skipped branches, we still check
-	// the immediate predecessor and use --onto if needed.
+	// Get --onto state from a merged branch immediately below the rebase range.
+	// Ensures that when --upstack excludes merged branches, we still check the
+	// immediate predecessor and use --onto if needed.
 	needsOnto := false
 	var ontoOldBase string
 	if startIdx > 0 {
 		prev := s.Branches[startIdx-1]
-		if prev.IsSkipped() {
+		if prev.IsMerged() {
 			if sha, ok := originalRefs[prev.Branch]; ok {
 				needsOnto = true
 				ontoOldBase = sha
@@ -315,6 +315,14 @@ func continueRebase(cfg *config.Config, gitDir string) error {
 		return fmt.Errorf("no stack found for branch %s", state.OriginalBranch)
 	}
 
+	// Refresh PR state before selecting the base and cascading the remaining
+	// branches. The queued flag is transient (not persisted), so it was lost
+	// when the stack was reloaded from disk above. Without this, a queued
+	// branch in the remaining cascade would be treated as active and its
+	// frozen merge-queue branch would be rebased. Mirrors the syncStackPRs
+	// call in runRebase before its cascade.
+	_ = syncStackPRs(cfg, s)
+
 	// The branch that had the conflict is stored in state; fall back to
 	// looking it up by index for backwards compatibility with older state files.
 	conflictBranch := state.ConflictBranch
@@ -334,10 +342,10 @@ func continueRebase(cfg *config.Config, gitDir string) error {
 
 	var baseBranch string
 	if state.UseOnto {
-		// The --onto path targets the first non-skipped ancestor, or trunk.
+		// The --onto path targets the first non-merged ancestor, or trunk.
 		baseBranch = s.Trunk.Branch
 		for j := state.CurrentBranchIndex - 1; j >= 0; j-- {
-			if !s.Branches[j].IsSkipped() {
+			if !s.Branches[j].IsMerged() {
 				baseBranch = s.Branches[j].Branch
 				break
 			}

cmd/rebase_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/github/gh-stack/internal/config"
 	"github.com/github/gh-stack/internal/git"
+	"github.com/github/gh-stack/internal/github"
 	"github.com/github/gh-stack/internal/stack"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -680,6 +681,188 @@ func TestRebase_SkipsMergedBranches(t *testing.T) {
 	assert.Equal(t, "b2", rebaseCalls[0].branch)
 }
 
+// queuedPRClient returns a MockClient whose FindPRByNumber reports the given PR
+// numbers as queued (in a merge queue, open, not merged) and finds no PR by
+// branch name. Used to drive the transient Queued state through syncStackPRs in
+// rebase/sync tests.
+func queuedPRClient(headByNumber map[int]string) *github.MockClient {
+	return &github.MockClient{
+		FindPRByNumberFn: func(n int) (*github.PullRequest, error) {
+			head, ok := headByNumber[n]
+			if !ok {
+				return nil, nil
+			}
+			return &github.PullRequest{
+				Number:          n,
+				HeadRefName:     head,
+				State:           "OPEN",
+				Merged:          false,
+				MergeQueueEntry: &github.MergeQueueEntry{ID: fmt.Sprintf("MQ_%d", n)},
+			}, nil
+		},
+		FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil },
+	}
+}
+
+// TestRebase_QueuedBranch_DownstreamStaysStacked verifies the #144 fix: a queued
+// PR is NOT treated as merged. Its branch is skipped (frozen in the merge queue),
+// but downstream branches stay stacked on top of it — they rebase onto the queued
+// branch, not --onto trunk with the queued commits dropped.
+func TestRebase_QueuedBranch_DownstreamStaysStacked(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2"},
+			{Branch: "b3"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var rebaseCalls []rebaseCall
+
+	mock := newRebaseMock(tmpDir, "b2")
+	mock.BranchExistsFn = func(name string) bool { return true }
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error {
+		rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = queuedPRClient(map[int]string{10: "b1"})
+	cmd := RebaseCmd(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")
+	assert.Contains(t, output, "queued")
+	assert.NotContains(t, output, "adjusted for merged PR",
+		"queued branches must not trigger the merged --onto path")
+
+	// b2 stays stacked on the queued b1 (not rebased --onto main); b3 onto b2.
+	require.Len(t, rebaseCalls, 2)
+	assert.Equal(t, rebaseCall{"b1", "sha-b1", "b2"}, rebaseCalls[0],
+		"b2 should rebase onto the queued branch b1, keeping its commits")
+	assert.Equal(t, rebaseCall{"b2", "sha-b2", "b3"}, rebaseCalls[1],
+		"b3 should rebase onto b2")
+}
+
+// TestRebase_MergedBelowQueued_KeepsStackedOnQueued verifies that when a merged
+// branch sits below a queued branch, the branch above the queued one stays
+// stacked on the queued branch. The queued branch is frozen and still carries the
+// merged branch's commits, so downstream cannot drop them via --onto.
+func TestRebase_MergedBelowQueued_KeepsStackedOnQueued(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10, Merged: true}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 11}},
+			{Branch: "b3"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var rebaseCalls []rebaseCall
+
+	mock := newRebaseMock(tmpDir, "b3")
+	mock.BranchExistsFn = func(name string) bool { return true }
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error {
+		rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = queuedPRClient(map[int]string{11: "b2"})
+	cmd := RebaseCmd(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")
+	assert.Contains(t, output, "PR #10 merged")
+	assert.Contains(t, output, "Skipping b2")
+	assert.Contains(t, output, "queued")
+
+	// b1 merged and b2 queued are both skipped. b3 stays stacked on the queued
+	// b2 — it must NOT be rebased --onto main (which would drop b2's + b1's
+	// commits while b2 is frozen).
+	require.Len(t, rebaseCalls, 1)
+	assert.Equal(t, rebaseCall{"b2", "sha-b2", "b3"}, rebaseCalls[0],
+		"b3 should rebase onto the queued b2, not --onto main")
+	assert.NotContains(t, output, "adjusted for merged PR")
+}
+
+// TestRebase_UpstackAboveQueuedBranch verifies the onto-seed fix: with --upstack
+// starting just above a queued branch, the first in-range branch rebases normally
+// onto the queued predecessor rather than dropping its commits via --onto.
+func TestRebase_UpstackAboveQueuedBranch(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 10}},
+			{Branch: "b2"},
+			{Branch: "b3"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	var rebaseCalls []rebaseCall
+
+	mock := newRebaseMock(tmpDir, "b2")
+	mock.BranchExistsFn = func(name string) bool { return true }
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error {
+		rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = queuedPRClient(map[int]string{10: "b1"})
+	cmd := RebaseCmd(cfg)
+	cmd.SetArgs([]string{"--upstack"})
+	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)
+	// upstack from b2 = [b2, b3]; b1 (queued) is below the range.
+	require.Len(t, rebaseCalls, 2)
+	assert.Equal(t, rebaseCall{"b1", "sha-b1", "b2"}, rebaseCalls[0],
+		"b2 should rebase onto the queued predecessor b1, not --onto main")
+	assert.Equal(t, rebaseCall{"b2", "sha-b2", "b3"}, rebaseCalls[1],
+		"b3 should rebase onto b2")
+	assert.NotContains(t, output, "adjusted for merged PR")
+}
+
 // TestRebase_StateRoundTrip verifies that rebase state can be saved and loaded
 // back with all fields preserved, including the --onto fields.
 func TestRebase_StateRoundTrip(t *testing.T) {
@@ -791,6 +974,80 @@ func TestRebase_Continue_RebasesRemainingBranches(t *testing.T) {
 	assert.Contains(t, checkouts, "b1", "should checkout original branch")
 }
 
+// TestRebase_Continue_QueuedBranchBelowConflict verifies that a queued branch is
+// still skipped when the cascade resumes via --continue after a conflict below
+// it. The Queued flag is transient and lost when continueRebase reloads the
+// stack from disk, so it must be refreshed before the remaining cascade — else
+// the frozen merge-queue branch would be rebased.
+func TestRebase_Continue_QueuedBranchBelowConflict(t *testing.T) {
+	s := stack.Stack{
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1"},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 20}},
+			{Branch: "b3"},
+		},
+	}
+
+	tmpDir := t.TempDir()
+	writeStackFile(t, tmpDir, s)
+
+	// State: b1 (below the queued b2) conflicted; b2 and b3 remain.
+	state := &rebaseState{
+		CurrentBranchIndex: 0,
+		ConflictBranch:     "b1",
+		RemainingBranches:  []string{"b2", "b3"},
+		OriginalBranch:     "b3",
+		OriginalRefs: map[string]string{
+			"main": "main-orig-sha",
+			"b1":   "sha-b1",
+			"b2":   "sha-b2",
+			"b3":   "sha-b3",
+		},
+	}
+	stateData, _ := json.MarshalIndent(state, "", "  ")
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "gh-stack-rebase-state"), stateData, 0644))
+
+	var rebaseCalls []rebaseCall
+
+	mock := newRebaseMock(tmpDir, "b1")
+	mock.BranchExistsFn = func(name string) bool { return true }
+	mock.IsRebaseInProgressFn = func() bool { return true }
+	mock.RebaseContinueFn = func(opts git.RebaseOpts) error { return nil }
+	mock.RebaseOntoFn = func(newBase, oldBase, branch string, opts git.RebaseOpts) error {
+		rebaseCalls = append(rebaseCalls, rebaseCall{newBase, oldBase, branch})
+		return nil
+	}
+	mock.CheckoutBranchFn = func(string) error { return nil }
+
+	restore := git.SetOps(mock)
+	defer restore()
+
+	cfg, _, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = queuedPRClient(map[int]string{20: "b2"})
+	cmd := RebaseCmd(cfg)
+	cmd.SetArgs([]string{"--continue"})
+	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 b2")
+	assert.Contains(t, output, "queued")
+
+	// Only b3 is rebased, onto the queued b2. The queued b2 itself must not be
+	// rebased (its branch is frozen in the merge queue).
+	require.Len(t, rebaseCalls, 1)
+	assert.Equal(t, rebaseCall{"b2", "sha-b2", "b3"}, rebaseCalls[0])
+	for _, c := range rebaseCalls {
+		assert.NotEqual(t, "b2", c.branch, "the frozen queued branch must not be rebased")
+	}
+}
+
 // TestRebase_Continue_OntoMode verifies the --continue path when UseOnto is
 // set (merged branches upstream). With no remaining branches, only
 // RebaseContinue runs and the state is cleaned up.