Skip to content

Commit dcb5885

Browse files
skarimCopilot
andcommitted
Skip pushing branches with queued PRs
Detect merge queue status via the GitHub GraphQL API's mergeQueueEntry field and temporarily skip queued branches in push, sync, and submit commands. Unlike merged state (which is persisted permanently), queued state is transient — held in-memory only via a json:"-" tagged field on BranchRef. Each command run re-checks queue status from the API, so if a PR is ejected from the queue it becomes active again on next run. Changes: - Add MergeQueueEntry to PullRequest GraphQL struct and PRDetails - Add IsQueued()/IsSkipped()/QueuedBranches() to stack model - Update ActiveBranches() family to exclude queued branches - Skip queued branches in push, sync (rebase + push), and submit - Add queued icon, style, and QUEUED state label in TUI view - Add comprehensive tests for queued state handling Closes github/pull-requests#24019 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 0ba54a4 commit dcb5885

File tree

14 files changed

+597
-64
lines changed

14 files changed

+597
-64
lines changed

cmd/push.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,20 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
7171
}
7272
return ErrSilent
7373
}
74+
// Sync PR state to detect merged/queued PRs before pushing.
75+
syncStackPRs(cfg, s)
76+
7477
merged := s.MergedBranches()
7578
if len(merged) > 0 {
7679
cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches"))
7780
}
81+
queued := s.QueuedBranches()
82+
if len(queued) > 0 {
83+
cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches"))
84+
}
7885
activeBranches := activeBranchNames(s)
7986
if len(activeBranches) == 0 {
80-
cfg.Printf("No active branches to push (all merged)")
87+
cfg.Printf("No active branches to push (all merged or queued)")
8188
return nil
8289
}
8390
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)

cmd/submit.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
7777
return ErrAPIFailure
7878
}
7979

80+
// Sync PR state to detect merged/queued PRs before pushing.
81+
syncStackPRs(cfg, s)
82+
8083
// Push all active branches atomically
8184
remote, err := pickRemote(cfg, currentBranch, opts.remote)
8285
if err != nil {
@@ -89,9 +92,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
8992
if len(merged) > 0 {
9093
cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches"))
9194
}
95+
queued := s.QueuedBranches()
96+
if len(queued) > 0 {
97+
cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches"))
98+
}
9299
activeBranches := activeBranchNames(s)
93100
if len(activeBranches) == 0 {
94-
cfg.Printf("All branches are merged, nothing to submit")
101+
cfg.Printf("All branches are merged or queued, nothing to submit")
95102
return nil
96103
}
97104
cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote)
@@ -104,7 +111,7 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
104111
// correct base branch. This makes submit idempotent: running it again
105112
// fills gaps and fixes base branches before syncing the stack.
106113
for i, b := range s.Branches {
107-
if s.Branches[i].IsMerged() {
114+
if s.Branches[i].IsMerged() || s.Branches[i].IsQueued() {
108115
continue
109116
}
110117
baseBranch := s.ActiveBaseBranch(b.Branch)

cmd/sync.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,20 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
152152
continue
153153
}
154154

155+
// Skip branches whose PRs are currently in a merge queue.
156+
if br.IsQueued() {
157+
ontoOldBase = originalRefs[br.Branch]
158+
needsOnto = true
159+
cfg.Successf("Skipping %s (PR %s queued)", br.Branch, cfg.PRLink(br.PullRequest.Number, br.PullRequest.URL))
160+
continue
161+
}
162+
155163
if needsOnto {
156-
// Find --onto target: first non-merged ancestor, or trunk.
164+
// Find --onto target: first non-merged/queued ancestor, or trunk.
157165
newBase := trunk
158166
for j := i - 1; j >= 0; j-- {
159167
b := s.Branches[j]
160-
if !b.IsMerged() {
168+
if !b.IsSkipped() {
161169
newBase = b.Branch
162170
break
163171
}
@@ -233,6 +241,9 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
233241
if mergedCount := len(s.MergedBranches()); mergedCount > 0 {
234242
cfg.Printf("Skipping %d merged %s", mergedCount, plural(mergedCount, "branch", "branches"))
235243
}
244+
if queuedCount := len(s.QueuedBranches()); queuedCount > 0 {
245+
cfg.Printf("Skipping %d queued %s", queuedCount, plural(queuedCount, "branch", "branches"))
246+
}
236247

237248
if len(branches) == 0 {
238249
cfg.Printf("No active branches to push (all merged)")
@@ -265,6 +276,10 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
265276
if b.IsMerged() {
266277
continue
267278
}
279+
if b.IsQueued() {
280+
cfg.Successf("PR %s (%s) — Queued", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch)
281+
continue
282+
}
268283
if b.PullRequest != nil {
269284
cfg.Successf("PR %s (%s) — Open", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch)
270285
} else {

cmd/utils.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac
214214
// syncStackPRs discovers and updates pull request metadata for branches in a stack.
215215
// For each branch, it queries GitHub for the most recent PR and updates the
216216
// PullRequestRef including merge status. Branches with already-merged PRs are skipped.
217+
// The transient Queued flag is also populated from the API response.
217218
func syncStackPRs(cfg *config.Config, s *stack.Stack) {
218219
client, err := cfg.GitHubClient()
219220
if err != nil {
@@ -238,6 +239,7 @@ func syncStackPRs(cfg *config.Config, s *stack.Stack) {
238239
URL: pr.URL,
239240
Merged: pr.Merged,
240241
}
242+
b.Queued = pr.IsQueued()
241243
}
242244
}
243245

cmd/view.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,14 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
7676
for i := len(s.Branches) - 1; i >= 0; i-- {
7777
b := s.Branches[i]
7878
merged := b.IsMerged()
79+
queued := b.IsQueued()
7980

80-
// Insert separator when transitioning from active to merged section
81+
// Insert separator when transitioning from active to queued section
82+
if queued && !merged && (i == len(s.Branches)-1 || (!s.Branches[i+1].IsQueued() && !s.Branches[i+1].IsMerged())) {
83+
cfg.Outf("├─── %s ────\n", cfg.ColorWarning("queued"))
84+
}
85+
86+
// Insert separator when transitioning from active/queued to merged section
8187
if merged && (i == len(s.Branches)-1 || !s.Branches[i+1].IsMerged()) {
8288
cfg.Outf("├─── %s ────\n", cfg.ColorMagenta("merged"))
8389
}
@@ -88,6 +94,8 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
8894
cfg.Outf("» %s%s%s %s\n", cfg.ColorBold(b.Branch), indicator, prSuffix, cfg.ColorCyan("(current)"))
8995
} else if merged {
9096
cfg.Outf("│ %s%s%s\n", cfg.ColorGray(b.Branch), indicator, prSuffix)
97+
} else if queued {
98+
cfg.Outf("│ %s%s%s\n", cfg.ColorWarning(b.Branch), indicator, prSuffix)
9199
} else {
92100
cfg.Outf("├ %s%s%s\n", b.Branch, indicator, prSuffix)
93101
}
@@ -98,13 +106,18 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error {
98106

99107
// branchStatusIndicator returns a colored status icon for a branch:
100108
// - ✓ (purple) if the PR has been merged
109+
// - ◎ (yellow) if the PR is queued in a merge queue
101110
// - ⚠ (yellow) if the branch needs rebasing (non-linear history)
102111
// - ○ (green) if there is an open PR
103112
func branchStatusIndicator(cfg *config.Config, s *stack.Stack, b stack.BranchRef) string {
104113
if b.IsMerged() {
105114
return " " + cfg.ColorMagenta("✓")
106115
}
107116

117+
if b.IsQueued() {
118+
return " " + cfg.ColorWarning("◎")
119+
}
120+
108121
baseBranch := s.ActiveBaseBranch(b.Branch)
109122
if needsRebase, err := git.IsAncestor(baseBranch, b.Branch); err == nil && !needsRebase {
110123
return " " + cfg.ColorWarning("⚠")
@@ -131,6 +144,7 @@ type viewJSONBranch struct {
131144
Base string `json:"base,omitempty"`
132145
IsCurrent bool `json:"isCurrent"`
133146
IsMerged bool `json:"isMerged"`
147+
IsQueued bool `json:"isQueued"`
134148
NeedsRebase bool `json:"needsRebase"`
135149
PR *viewJSONPR `json:"pr,omitempty"`
136150
}
@@ -156,6 +170,7 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error {
156170
Base: b.Base,
157171
IsCurrent: b.Branch == currentBranch,
158172
IsMerged: b.IsMerged(),
173+
IsQueued: b.IsQueued(),
159174
}
160175

161176
// Check if the branch needs rebasing (base not ancestor of branch).
@@ -170,6 +185,8 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error {
170185
state := "OPEN"
171186
if b.PullRequest.Merged {
172187
state = "MERGED"
188+
} else if b.IsQueued() {
189+
state = "QUEUED"
173190
}
174191
jb.PR = &viewJSONPR{
175192
Number: b.PullRequest.Number,

cmd/view_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/github/gh-stack/internal/config"
1010
"github.com/github/gh-stack/internal/git"
11+
"github.com/github/gh-stack/internal/github"
1112
"github.com/github/gh-stack/internal/stack"
1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
@@ -283,3 +284,135 @@ func TestViewShort_FullyMergedStack(t *testing.T) {
283284
assert.Contains(t, output, "b1")
284285
assert.Contains(t, output, "b2")
285286
}
287+
288+
// TestViewShort_QueuedStack verifies that --short output shows queued
289+
// branches with a "queued" separator and the ◎ icon.
290+
func TestViewShort_QueuedStack(t *testing.T) {
291+
s := stack.Stack{
292+
Trunk: stack.BranchRef{Branch: "main"},
293+
Branches: []stack.BranchRef{
294+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1}},
295+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2}},
296+
{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}},
297+
},
298+
}
299+
300+
tmpDir := t.TempDir()
301+
writeStackFile(t, tmpDir, s)
302+
303+
restore := git.SetOps(&git.MockOps{
304+
GitDirFn: func() (string, error) { return tmpDir, nil },
305+
CurrentBranchFn: func() (string, error) { return "b3", nil },
306+
IsAncestorFn: func(string, string) (bool, error) { return true, nil },
307+
RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
308+
})
309+
defer restore()
310+
311+
// Mock GitHub client to return b1 as queued (MergeQueueEntry set)
312+
cfg, outR, _ := config.NewTestConfig()
313+
cfg.GitHubClientOverride = &github.MockClient{
314+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
315+
switch branch {
316+
case "b1":
317+
return &github.PullRequest{
318+
Number: 1,
319+
ID: "PR_1",
320+
MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_1"},
321+
}, nil
322+
case "b2":
323+
return &github.PullRequest{Number: 2, ID: "PR_2"}, nil
324+
case "b3":
325+
return &github.PullRequest{Number: 3, ID: "PR_3"}, nil
326+
}
327+
return nil, nil
328+
},
329+
}
330+
331+
cmd := ViewCmd(cfg)
332+
cmd.SetArgs([]string{"--short"})
333+
cmd.SetOut(io.Discard)
334+
cmd.SetErr(io.Discard)
335+
err := cmd.Execute()
336+
337+
cfg.Out.Close()
338+
raw, _ := io.ReadAll(outR)
339+
output := string(raw)
340+
341+
assert.NoError(t, err)
342+
assert.Contains(t, output, "b1")
343+
assert.Contains(t, output, "b2")
344+
assert.Contains(t, output, "b3")
345+
assert.Contains(t, output, "queued", "should show queued separator")
346+
assert.Contains(t, output, "◎", "should show queued icon for b1")
347+
}
348+
349+
// TestViewShort_MixedQueuedAndMerged verifies that --short output shows
350+
// both "queued" and "merged" separators in the correct order.
351+
func TestViewShort_MixedQueuedAndMerged(t *testing.T) {
352+
s := stack.Stack{
353+
Trunk: stack.BranchRef{Branch: "main"},
354+
Branches: []stack.BranchRef{
355+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
356+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2}},
357+
{Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}},
358+
},
359+
}
360+
361+
tmpDir := t.TempDir()
362+
writeStackFile(t, tmpDir, s)
363+
364+
restore := git.SetOps(&git.MockOps{
365+
GitDirFn: func() (string, error) { return tmpDir, nil },
366+
CurrentBranchFn: func() (string, error) { return "b3", nil },
367+
IsAncestorFn: func(string, string) (bool, error) { return true, nil },
368+
RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil },
369+
})
370+
defer restore()
371+
372+
// b1 is merged (persisted), b2 is queued (from API)
373+
cfg, outR, _ := config.NewTestConfig()
374+
cfg.GitHubClientOverride = &github.MockClient{
375+
FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) {
376+
switch branch {
377+
case "b2":
378+
return &github.PullRequest{
379+
Number: 2,
380+
ID: "PR_2",
381+
MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_2"},
382+
}, nil
383+
case "b3":
384+
return &github.PullRequest{Number: 3, ID: "PR_3"}, nil
385+
}
386+
return nil, nil
387+
},
388+
}
389+
390+
cmd := ViewCmd(cfg)
391+
cmd.SetArgs([]string{"--short"})
392+
cmd.SetOut(io.Discard)
393+
cmd.SetErr(io.Discard)
394+
err := cmd.Execute()
395+
396+
cfg.Out.Close()
397+
raw, _ := io.ReadAll(outR)
398+
output := string(raw)
399+
400+
assert.NoError(t, err)
401+
assert.Contains(t, output, "queued", "should show queued separator")
402+
assert.Contains(t, output, "merged", "should show merged separator")
403+
404+
// "merged" section (b1) should appear below "queued" section (b2) in output
405+
// Since we render top-to-bottom: b3 (active) -> queued separator -> b2 -> merged separator -> b1
406+
queuedIdx := indexOf(output, "queued")
407+
mergedIdx := indexOf(output, "merged")
408+
assert.Less(t, queuedIdx, mergedIdx, "queued separator should appear before merged separator")
409+
}
410+
411+
func indexOf(s, substr string) int {
412+
for i := 0; i <= len(s)-len(substr); i++ {
413+
if s[i:i+len(substr)] == substr {
414+
return i
415+
}
416+
}
417+
return -1
418+
}

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ require (
1313
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
1414
github.com/spf13/cobra v1.10.2
1515
github.com/stretchr/testify v1.11.1
16+
golang.org/x/sys v0.39.0
1617
golang.org/x/text v0.32.0
1718
)
1819

@@ -45,7 +46,6 @@ require (
4546
github.com/spf13/pflag v1.0.10 // indirect
4647
github.com/thlib/go-timezone-local v0.0.6 // indirect
4748
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
48-
golang.org/x/sys v0.39.0 // indirect
4949
golang.org/x/term v0.38.0 // indirect
5050
gopkg.in/yaml.v3 v3.0.1 // indirect
5151
)

0 commit comments

Comments
 (0)