Fix rebase treating queued PRs as merged (#173)

* Don't treat queued PRs as merged when rebasing the stack

gh stack rebase and gh stack sync share cascadeRebase, which skipped
branches via IsSkipped() (merged or queued) and then switched to a
`git rebase --onto` that drops the skipped branch's commits from every
downstream branch. That is right for a merged PR — its commits are
already in trunk — but wrong for a queued PR: its commits only exist on
its own branch, which is frozen in the merge queue, so the branches
above it were rebased onto trunk and lost work they depend on.

Handle the two cases separately. A merged branch still activates --onto
so its commits are dropped. A queued branch is still skipped (its branch
is frozen and is not rebased or pushed), but onto mode is reset so
downstream branches rebase normally onto the queued branch, keeping its
commits underneath. The --onto target search, the runRebase --onto seed,
and the continueRebase display base now key on IsMerged() instead of
IsSkipped(), so a queued predecessor no longer forces downstream
branches onto trunk. gh stack sync is fixed through the same shared
helper.

Add rebase coverage for a queued branch mid-stack, a merged branch below
a queued branch, and --upstack above a queued branch, plus a sync test
that also asserts the queued branch is excluded from the push. The
transient queued state is injected through the GitHub mock's merge-queue
entry.

* Refresh queued PR state when continuing a stack rebase

continueRebase reloads the stack from disk, where the Queued flag is
transient (json:"-") and therefore lost, and it only called syncStackPRs
after the cascade. So if the initial rebase conflicted on a branch below
a queued branch, `gh stack rebase --continue` resumed with that branch
seen as active: it rebased the frozen merge-queue branch and rebuilt the
downstream branches on a local history that differs from the queued
branch.

Call syncStackPRs right after resolving the stack — before selecting the
base and cascading the remaining branches — mirroring the refresh
runRebase already does before its cascade. The queued flag is
repopulated, so queued branches stay skipped and downstream branches
stay stacked on them.

Add TestRebase_Continue_QueuedBranchBelowConflict, which conflicts below
a queued branch and asserts the frozen branch is not rebased and the
branch above stays stacked on it. Verified to fail without the refresh.

Author: skarim

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.