From dcb588505342639feea3a45c69ec0c6796334810 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Wed, 8 Apr 2026 17:07:56 -0400 Subject: [PATCH] Skip pushing branches with queued PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- cmd/push.go | 9 +- cmd/submit.go | 11 +- cmd/sync.go | 19 ++- cmd/utils.go | 2 + cmd/view.go | 19 ++- cmd/view_test.go | 133 ++++++++++++++++ go.mod | 2 +- internal/github/github.go | 91 ++++++----- internal/github/github_test.go | 20 +++ internal/stack/stack.go | 46 ++++-- internal/stack/stack_test.go | 223 +++++++++++++++++++++++++++ internal/tui/stackview/model.go | 57 +++++-- internal/tui/stackview/model_test.go | 26 ++++ internal/tui/stackview/styles.go | 3 + 14 files changed, 597 insertions(+), 64 deletions(-) diff --git a/cmd/push.go b/cmd/push.go index e3d84da..f3d4ff4 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -71,13 +71,20 @@ func runPush(cfg *config.Config, opts *pushOptions) error { } return ErrSilent } + // Sync PR state to detect merged/queued PRs before pushing. + syncStackPRs(cfg, s) + merged := s.MergedBranches() if len(merged) > 0 { cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches")) } + queued := s.QueuedBranches() + if len(queued) > 0 { + cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches")) + } activeBranches := activeBranchNames(s) if len(activeBranches) == 0 { - cfg.Printf("No active branches to push (all merged)") + cfg.Printf("No active branches to push (all merged or queued)") return nil } cfg.Printf("Pushing %d %s to %s...", len(activeBranches), plural(len(activeBranches), "branch", "branches"), remote) diff --git a/cmd/submit.go b/cmd/submit.go index 1fc7ca6..84158a8 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -77,6 +77,9 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { return ErrAPIFailure } + // Sync PR state to detect merged/queued PRs before pushing. + syncStackPRs(cfg, s) + // Push all active branches atomically remote, err := pickRemote(cfg, currentBranch, opts.remote) if err != nil { @@ -89,9 +92,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error { if len(merged) > 0 { cfg.Printf("Skipping %d merged %s", len(merged), plural(len(merged), "branch", "branches")) } + queued := s.QueuedBranches() + if len(queued) > 0 { + cfg.Printf("Skipping %d queued %s", len(queued), plural(len(queued), "branch", "branches")) + } activeBranches := activeBranchNames(s) if len(activeBranches) == 0 { - cfg.Printf("All branches are merged, nothing to submit") + cfg.Printf("All branches are merged or queued, nothing to submit") return nil } 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 { // correct base branch. This makes submit idempotent: running it again // fills gaps and fixes base branches before syncing the stack. for i, b := range s.Branches { - if s.Branches[i].IsMerged() { + if s.Branches[i].IsMerged() || s.Branches[i].IsQueued() { continue } baseBranch := s.ActiveBaseBranch(b.Branch) diff --git a/cmd/sync.go b/cmd/sync.go index 52c8184..019059a 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -152,12 +152,20 @@ func runSync(cfg *config.Config, opts *syncOptions) error { continue } + // Skip branches whose PRs are currently in a merge queue. + if br.IsQueued() { + ontoOldBase = originalRefs[br.Branch] + needsOnto = true + cfg.Successf("Skipping %s (PR %s queued)", br.Branch, cfg.PRLink(br.PullRequest.Number, br.PullRequest.URL)) + continue + } + if needsOnto { - // Find --onto target: first non-merged ancestor, or trunk. + // Find --onto target: first non-merged/queued ancestor, or trunk. newBase := trunk for j := i - 1; j >= 0; j-- { b := s.Branches[j] - if !b.IsMerged() { + if !b.IsSkipped() { newBase = b.Branch break } @@ -233,6 +241,9 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if mergedCount := len(s.MergedBranches()); mergedCount > 0 { cfg.Printf("Skipping %d merged %s", mergedCount, plural(mergedCount, "branch", "branches")) } + if queuedCount := len(s.QueuedBranches()); queuedCount > 0 { + cfg.Printf("Skipping %d queued %s", queuedCount, plural(queuedCount, "branch", "branches")) + } if len(branches) == 0 { cfg.Printf("No active branches to push (all merged)") @@ -265,6 +276,10 @@ func runSync(cfg *config.Config, opts *syncOptions) error { if b.IsMerged() { continue } + if b.IsQueued() { + cfg.Successf("PR %s (%s) — Queued", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch) + continue + } if b.PullRequest != nil { cfg.Successf("PR %s (%s) — Open", cfg.PRLink(b.PullRequest.Number, b.PullRequest.URL), b.Branch) } else { diff --git a/cmd/utils.go b/cmd/utils.go index 815b8a8..b735ce2 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -214,6 +214,7 @@ func resolveStack(sf *stack.StackFile, branch string, cfg *config.Config) (*stac // syncStackPRs discovers and updates pull request metadata for branches in a stack. // For each branch, it queries GitHub for the most recent PR and updates the // PullRequestRef including merge status. Branches with already-merged PRs are skipped. +// The transient Queued flag is also populated from the API response. func syncStackPRs(cfg *config.Config, s *stack.Stack) { client, err := cfg.GitHubClient() if err != nil { @@ -238,6 +239,7 @@ func syncStackPRs(cfg *config.Config, s *stack.Stack) { URL: pr.URL, Merged: pr.Merged, } + b.Queued = pr.IsQueued() } } diff --git a/cmd/view.go b/cmd/view.go index fb4cc47..8ff3a35 100644 --- a/cmd/view.go +++ b/cmd/view.go @@ -76,8 +76,14 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { for i := len(s.Branches) - 1; i >= 0; i-- { b := s.Branches[i] merged := b.IsMerged() + queued := b.IsQueued() - // Insert separator when transitioning from active to merged section + // Insert separator when transitioning from active to queued section + if queued && !merged && (i == len(s.Branches)-1 || (!s.Branches[i+1].IsQueued() && !s.Branches[i+1].IsMerged())) { + cfg.Outf("├─── %s ────\n", cfg.ColorWarning("queued")) + } + + // Insert separator when transitioning from active/queued to merged section if merged && (i == len(s.Branches)-1 || !s.Branches[i+1].IsMerged()) { cfg.Outf("├─── %s ────\n", cfg.ColorMagenta("merged")) } @@ -88,6 +94,8 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { cfg.Outf("» %s%s%s %s\n", cfg.ColorBold(b.Branch), indicator, prSuffix, cfg.ColorCyan("(current)")) } else if merged { cfg.Outf("│ %s%s%s\n", cfg.ColorGray(b.Branch), indicator, prSuffix) + } else if queued { + cfg.Outf("│ %s%s%s\n", cfg.ColorWarning(b.Branch), indicator, prSuffix) } else { cfg.Outf("├ %s%s%s\n", b.Branch, indicator, prSuffix) } @@ -98,6 +106,7 @@ func viewShort(cfg *config.Config, s *stack.Stack, currentBranch string) error { // branchStatusIndicator returns a colored status icon for a branch: // - ✓ (purple) if the PR has been merged +// - ◎ (yellow) if the PR is queued in a merge queue // - ⚠ (yellow) if the branch needs rebasing (non-linear history) // - ○ (green) if there is an open PR func branchStatusIndicator(cfg *config.Config, s *stack.Stack, b stack.BranchRef) string { @@ -105,6 +114,10 @@ func branchStatusIndicator(cfg *config.Config, s *stack.Stack, b stack.BranchRef return " " + cfg.ColorMagenta("✓") } + if b.IsQueued() { + return " " + cfg.ColorWarning("◎") + } + baseBranch := s.ActiveBaseBranch(b.Branch) if needsRebase, err := git.IsAncestor(baseBranch, b.Branch); err == nil && !needsRebase { return " " + cfg.ColorWarning("⚠") @@ -131,6 +144,7 @@ type viewJSONBranch struct { Base string `json:"base,omitempty"` IsCurrent bool `json:"isCurrent"` IsMerged bool `json:"isMerged"` + IsQueued bool `json:"isQueued"` NeedsRebase bool `json:"needsRebase"` PR *viewJSONPR `json:"pr,omitempty"` } @@ -156,6 +170,7 @@ func viewJSON(cfg *config.Config, s *stack.Stack, currentBranch string) error { Base: b.Base, IsCurrent: b.Branch == currentBranch, IsMerged: b.IsMerged(), + IsQueued: b.IsQueued(), } // 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 { state := "OPEN" if b.PullRequest.Merged { state = "MERGED" + } else if b.IsQueued() { + state = "QUEUED" } jb.PR = &viewJSONPR{ Number: b.PullRequest.Number, diff --git a/cmd/view_test.go b/cmd/view_test.go index 865484d..8d1c507 100644 --- a/cmd/view_test.go +++ b/cmd/view_test.go @@ -8,6 +8,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" @@ -283,3 +284,135 @@ func TestViewShort_FullyMergedStack(t *testing.T) { assert.Contains(t, output, "b1") assert.Contains(t, output, "b2") } + +// TestViewShort_QueuedStack verifies that --short output shows queued +// branches with a "queued" separator and the ◎ icon. +func TestViewShort_QueuedStack(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1}}, + {Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2}}, + {Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return tmpDir, nil }, + CurrentBranchFn: func() (string, error) { return "b3", nil }, + IsAncestorFn: func(string, string) (bool, error) { return true, nil }, + RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil }, + }) + defer restore() + + // Mock GitHub client to return b1 as queued (MergeQueueEntry set) + cfg, outR, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) { + switch branch { + case "b1": + return &github.PullRequest{ + Number: 1, + ID: "PR_1", + MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_1"}, + }, nil + case "b2": + return &github.PullRequest{Number: 2, ID: "PR_2"}, nil + case "b3": + return &github.PullRequest{Number: 3, ID: "PR_3"}, nil + } + return nil, nil + }, + } + + cmd := ViewCmd(cfg) + cmd.SetArgs([]string{"--short"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Out.Close() + raw, _ := io.ReadAll(outR) + output := string(raw) + + assert.NoError(t, err) + assert.Contains(t, output, "b1") + assert.Contains(t, output, "b2") + assert.Contains(t, output, "b3") + assert.Contains(t, output, "queued", "should show queued separator") + assert.Contains(t, output, "◎", "should show queued icon for b1") +} + +// TestViewShort_MixedQueuedAndMerged verifies that --short output shows +// both "queued" and "merged" separators in the correct order. +func TestViewShort_MixedQueuedAndMerged(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", PullRequest: &stack.PullRequestRef{Number: 2}}, + {Branch: "b3", PullRequest: &stack.PullRequestRef{Number: 3}}, + }, + } + + tmpDir := t.TempDir() + writeStackFile(t, tmpDir, s) + + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return tmpDir, nil }, + CurrentBranchFn: func() (string, error) { return "b3", nil }, + IsAncestorFn: func(string, string) (bool, error) { return true, nil }, + RevParseFn: func(ref string) (string, error) { return "sha-" + ref, nil }, + }) + defer restore() + + // b1 is merged (persisted), b2 is queued (from API) + cfg, outR, _ := config.NewTestConfig() + cfg.GitHubClientOverride = &github.MockClient{ + FindAnyPRForBranchFn: func(branch string) (*github.PullRequest, error) { + switch branch { + case "b2": + return &github.PullRequest{ + Number: 2, + ID: "PR_2", + MergeQueueEntry: &github.MergeQueueEntry{ID: "MQE_2"}, + }, nil + case "b3": + return &github.PullRequest{Number: 3, ID: "PR_3"}, nil + } + return nil, nil + }, + } + + cmd := ViewCmd(cfg) + cmd.SetArgs([]string{"--short"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + + cfg.Out.Close() + raw, _ := io.ReadAll(outR) + output := string(raw) + + assert.NoError(t, err) + assert.Contains(t, output, "queued", "should show queued separator") + assert.Contains(t, output, "merged", "should show merged separator") + + // "merged" section (b1) should appear below "queued" section (b2) in output + // Since we render top-to-bottom: b3 (active) -> queued separator -> b2 -> merged separator -> b1 + queuedIdx := indexOf(output, "queued") + mergedIdx := indexOf(output, "merged") + assert.Less(t, queuedIdx, mergedIdx, "queued separator should appear before merged separator") +} + +func indexOf(s, substr string) int { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return i + } + } + return -1 +} diff --git a/go.mod b/go.mod index 2084dab..7933d90 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 + golang.org/x/sys v0.39.0 golang.org/x/text v0.32.0 ) @@ -45,7 +46,6 @@ require ( github.com/spf13/pflag v1.0.10 // indirect github.com/thlib/go-timezone-local v0.0.6 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - golang.org/x/sys v0.39.0 // indirect golang.org/x/term v0.38.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/internal/github/github.go b/internal/github/github.go index 54f8bdb..9237b12 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -9,17 +9,29 @@ import ( graphql "github.com/cli/shurcooL-graphql" ) +// MergeQueueEntry represents a merge queue entry. When the GraphQL field +// mergeQueueEntry is null (PR not queued), the pointer will be nil. +type MergeQueueEntry struct { + ID string `graphql:"id"` +} + // PullRequest represents a GitHub pull request. type PullRequest struct { - ID string `graphql:"id"` - Number int `graphql:"number"` - Title string `graphql:"title"` - State string `graphql:"state"` - URL string `graphql:"url"` - HeadRefName string `graphql:"headRefName"` - BaseRefName string `graphql:"baseRefName"` - IsDraft bool `graphql:"isDraft"` - Merged bool `graphql:"merged"` + ID string `graphql:"id"` + Number int `graphql:"number"` + Title string `graphql:"title"` + State string `graphql:"state"` + URL string `graphql:"url"` + HeadRefName string `graphql:"headRefName"` + BaseRefName string `graphql:"baseRefName"` + IsDraft bool `graphql:"isDraft"` + Merged bool `graphql:"merged"` + MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"` +} + +// IsQueued reports whether the pull request is currently in a merge queue. +func (pr *PullRequest) IsQueued() bool { + return pr != nil && pr.MergeQueueEntry != nil && pr.MergeQueueEntry.ID != "" } // Client wraps GitHub API operations. @@ -94,15 +106,16 @@ func (c *Client) FindPRForBranch(branch string) (*PullRequest, error) { n := nodes[0] return &PullRequest{ - ID: n.ID, - Number: n.Number, - Title: n.Title, - State: n.State, - URL: n.URL, - HeadRefName: n.HeadRefName, - BaseRefName: n.BaseRefName, - IsDraft: n.IsDraft, - Merged: n.Merged, + ID: n.ID, + Number: n.Number, + Title: n.Title, + State: n.State, + URL: n.URL, + HeadRefName: n.HeadRefName, + BaseRefName: n.BaseRefName, + IsDraft: n.IsDraft, + Merged: n.Merged, + MergeQueueEntry: n.MergeQueueEntry, }, nil } @@ -133,15 +146,16 @@ func (c *Client) FindAnyPRForBranch(branch string) (*PullRequest, error) { n := nodes[0] return &PullRequest{ - ID: n.ID, - Number: n.Number, - Title: n.Title, - State: n.State, - URL: n.URL, - HeadRefName: n.HeadRefName, - BaseRefName: n.BaseRefName, - IsDraft: n.IsDraft, - Merged: n.Merged, + ID: n.ID, + Number: n.Number, + Title: n.Title, + State: n.State, + URL: n.URL, + HeadRefName: n.HeadRefName, + BaseRefName: n.BaseRefName, + IsDraft: n.IsDraft, + Merged: n.Merged, + MergeQueueEntry: n.MergeQueueEntry, }, nil } @@ -227,6 +241,7 @@ type PRDetails struct { URL string IsDraft bool Merged bool + IsQueued bool CommentsCount int } @@ -237,16 +252,17 @@ func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { Repository struct { PullRequests struct { Nodes []struct { - ID string `graphql:"id"` - Number int `graphql:"number"` - Title string `graphql:"title"` - State string `graphql:"state"` - URL string `graphql:"url"` - HeadRefName string `graphql:"headRefName"` - BaseRefName string `graphql:"baseRefName"` - IsDraft bool `graphql:"isDraft"` - Merged bool `graphql:"merged"` - Comments struct { + ID string `graphql:"id"` + Number int `graphql:"number"` + Title string `graphql:"title"` + State string `graphql:"state"` + URL string `graphql:"url"` + HeadRefName string `graphql:"headRefName"` + BaseRefName string `graphql:"baseRefName"` + IsDraft bool `graphql:"isDraft"` + Merged bool `graphql:"merged"` + MergeQueueEntry *MergeQueueEntry `graphql:"mergeQueueEntry"` + Comments struct { TotalCount int `graphql:"totalCount"` } `graphql:"comments"` } @@ -277,6 +293,7 @@ func (c *Client) FindPRDetailsForBranch(branch string) (*PRDetails, error) { URL: n.URL, IsDraft: n.IsDraft, Merged: n.Merged, + IsQueued: n.MergeQueueEntry != nil && n.MergeQueueEntry.ID != "", CommentsCount: n.Comments.TotalCount, }, nil } diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 8ba93b8..4efee87 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -26,3 +26,23 @@ func TestPRURL(t *testing.T) { }) } } + +func TestPullRequest_IsQueued(t *testing.T) { + t.Run("not queued when MergeQueueEntry is nil", func(t *testing.T) { + pr := &PullRequest{Number: 1} + assert.False(t, pr.IsQueued()) + }) + + t.Run("queued when MergeQueueEntry has ID", func(t *testing.T) { + pr := &PullRequest{ + Number: 1, + MergeQueueEntry: &MergeQueueEntry{ID: "MQE_123"}, + } + assert.True(t, pr.IsQueued()) + }) + + t.Run("nil receiver is safe", func(t *testing.T) { + var pr *PullRequest + assert.False(t, pr.IsQueued()) + }) +} diff --git a/internal/stack/stack.go b/internal/stack/stack.go index 5405c7a..5dd89ea 100644 --- a/internal/stack/stack.go +++ b/internal/stack/stack.go @@ -33,6 +33,11 @@ type BranchRef struct { Head string `json:"head,omitempty"` Base string `json:"base,omitempty"` PullRequest *PullRequestRef `json:"pullRequest,omitempty"` + + // Queued is a transient (not persisted) flag indicating the branch's + // PR is currently in a merge queue. It is populated by syncStackPRs + // from the GitHub API on each command run. + Queued bool `json:"-"` } // Stack represents a single stack of branches. @@ -96,11 +101,23 @@ func (b *BranchRef) IsMerged() bool { return b.PullRequest != nil && b.PullRequest.Merged } -// ActiveBranches returns only non-merged branches, preserving order. +// IsQueued returns whether a branch's PR is currently in a merge queue. +// This is a transient state populated from the GitHub API on each run. +func (b *BranchRef) IsQueued() bool { + return b.Queued +} + +// IsSkipped returns whether a branch should be skipped during push/sync/submit. +// A branch is skipped if its PR has been merged or is currently queued. +func (b *BranchRef) IsSkipped() bool { + return b.IsMerged() || b.IsQueued() +} + +// ActiveBranches returns only branches that are pushable (not merged, not queued). func (s *Stack) ActiveBranches() []BranchRef { var active []BranchRef for _, b := range s.Branches { - if !b.IsMerged() { + if !b.IsSkipped() { active = append(active, b) } } @@ -118,21 +135,32 @@ func (s *Stack) MergedBranches() []BranchRef { return merged } -// FirstActiveBranchIndex returns the index of the first non-merged branch, or -1. +// QueuedBranches returns only queued branches, preserving order. +func (s *Stack) QueuedBranches() []BranchRef { + var queued []BranchRef + for _, b := range s.Branches { + if b.IsQueued() { + queued = append(queued, b) + } + } + return queued +} + +// FirstActiveBranchIndex returns the index of the first active (not merged, not queued) branch, or -1. func (s *Stack) FirstActiveBranchIndex() int { for i, b := range s.Branches { - if !b.IsMerged() { + if !b.IsSkipped() { return i } } return -1 } -// ActiveBranchIndices returns the indices of all non-merged branches. +// ActiveBranchIndices returns the indices of all active (not merged, not queued) branches. func (s *Stack) ActiveBranchIndices() []int { var indices []int for i, b := range s.Branches { - if !b.IsMerged() { + if !b.IsSkipped() { indices = append(indices, i) } } @@ -140,15 +168,15 @@ func (s *Stack) ActiveBranchIndices() []int { } // ActiveBaseBranch returns the effective parent for a branch, skipping merged -// ancestors. For the first active branch (or any branch whose downstack is all -// merged), this returns the trunk. +// and queued ancestors. For the first active branch (or any branch whose +// downstack is all merged/queued), this returns the trunk. func (s *Stack) ActiveBaseBranch(branch string) string { idx := s.IndexOf(branch) if idx <= 0 { return s.Trunk.Branch } for j := idx - 1; j >= 0; j-- { - if !s.Branches[j].IsMerged() { + if !s.Branches[j].IsSkipped() { return s.Branches[j].Branch } } diff --git a/internal/stack/stack_test.go b/internal/stack/stack_test.go index 3fa65de..9d78610 100644 --- a/internal/stack/stack_test.go +++ b/internal/stack/stack_test.go @@ -382,3 +382,226 @@ func TestRemoveStackForBranch(t *testing.T) { assert.Len(t, sf.Stacks, 1) }) } + +// --- Queued state: transient merge queue support --- + +func makeQueuedBranch(name string, prNum int) BranchRef { + return BranchRef{ + Branch: name, + PullRequest: &PullRequestRef{Number: prNum}, + Queued: true, + } +} + +func TestIsQueued(t *testing.T) { + t.Run("queued branch", func(t *testing.T) { + b := makeQueuedBranch("b1", 1) + assert.True(t, b.IsQueued()) + assert.False(t, b.IsMerged()) + assert.True(t, b.IsSkipped()) + }) + + t.Run("merged branch", func(t *testing.T) { + b := makeMergedBranch("b1", 1) + assert.False(t, b.IsQueued()) + assert.True(t, b.IsMerged()) + assert.True(t, b.IsSkipped()) + }) + + t.Run("active branch", func(t *testing.T) { + b := BranchRef{Branch: "b1"} + assert.False(t, b.IsQueued()) + assert.False(t, b.IsMerged()) + assert.False(t, b.IsSkipped()) + }) +} + +func TestQueuedBranches(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + {Branch: "b1"}, + makeQueuedBranch("b2", 2), + {Branch: "b3"}, + makeQueuedBranch("b4", 4), + }, + } + queued := s.QueuedBranches() + assert.Len(t, queued, 2) + assert.Equal(t, "b2", queued[0].Branch) + assert.Equal(t, "b4", queued[1].Branch) +} + +func TestActiveBranches_ExcludesQueued(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + {Branch: "b2"}, + makeMergedBranch("b3", 3), + {Branch: "b4"}, + }, + } + active := s.ActiveBranches() + assert.Len(t, active, 2) + assert.Equal(t, "b2", active[0].Branch) + assert.Equal(t, "b4", active[1].Branch) +} + +func TestFirstActiveBranchIndex_SkipsQueued(t *testing.T) { + t.Run("queued first, then active", func(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + {Branch: "b2"}, + }, + } + assert.Equal(t, 1, s.FirstActiveBranchIndex()) + }) + + t.Run("all queued", func(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + makeQueuedBranch("b2", 2), + }, + } + assert.Equal(t, -1, s.FirstActiveBranchIndex()) + }) + + t.Run("merged then queued then active", func(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeMergedBranch("b1", 1), + makeQueuedBranch("b2", 2), + {Branch: "b3"}, + }, + } + assert.Equal(t, 2, s.FirstActiveBranchIndex()) + }) +} + +func TestActiveBranchIndices_SkipsQueued(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + {Branch: "b2"}, + makeMergedBranch("b3", 3), + {Branch: "b4"}, + makeQueuedBranch("b5", 5), + }, + } + assert.Equal(t, []int{1, 3}, s.ActiveBranchIndices()) +} + +func TestActiveBaseBranch_SkipsQueued(t *testing.T) { + tests := []struct { + name string + stack Stack + branch string + expected string + }{ + { + name: "queued ancestor skipped to trunk", + stack: Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + {Branch: "b2"}, + }, + }, + branch: "b2", + expected: "main", + }, + { + name: "queued ancestor skipped to active sibling", + stack: Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + {Branch: "b1"}, + makeQueuedBranch("b2", 2), + {Branch: "b3"}, + }, + }, + branch: "b3", + expected: "b1", + }, + { + name: "mixed merged and queued ancestors skip to trunk", + stack: Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeMergedBranch("b1", 1), + makeQueuedBranch("b2", 2), + {Branch: "b3"}, + }, + }, + branch: "b3", + expected: "main", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.stack.ActiveBaseBranch(tt.branch)) + }) + } +} + +func TestQueuedState_NotPersisted(t *testing.T) { + dir := t.TempDir() + original := &StackFile{ + Repository: "owner/repo", + Stacks: []Stack{ + { + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + { + Branch: "b1", + PullRequest: &PullRequestRef{Number: 1}, + Queued: true, // set transient state + }, + }, + }, + }, + } + + require.NoError(t, Save(dir, original)) + + loaded, err := Load(dir) + require.NoError(t, err) + require.Len(t, loaded.Stacks, 1) + require.Len(t, loaded.Stacks[0].Branches, 1) + + // Queued state should NOT be persisted (json:"-") + assert.False(t, loaded.Stacks[0].Branches[0].Queued) + assert.False(t, loaded.Stacks[0].Branches[0].IsQueued()) +} + +func TestIsFullyMerged_NotAffectedByQueued(t *testing.T) { + t.Run("all queued is not fully merged", func(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeQueuedBranch("b1", 1), + makeQueuedBranch("b2", 2), + }, + } + assert.False(t, s.IsFullyMerged()) + }) + + t.Run("merged and queued is not fully merged", func(t *testing.T) { + s := Stack{ + Trunk: BranchRef{Branch: "main"}, + Branches: []BranchRef{ + makeMergedBranch("b1", 1), + makeQueuedBranch("b2", 2), + }, + } + assert.False(t, s.IsFullyMerged()) + }) +} diff --git a/internal/tui/stackview/model.go b/internal/tui/stackview/model.go index dae09c1..8f845f9 100644 --- a/internal/tui/stackview/model.go +++ b/internal/tui/stackview/model.go @@ -250,15 +250,20 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) { contentLine := (screenY - yOffset) + m.scrollOffset // Walk through rendered lines to find which node was clicked. - // Account for the merged separator line that may appear between nodes. + // Account for the merged/queued separator lines that may appear between nodes. line := 0 prevWasMerged := false + prevWasQueued := false for i := 0; i < len(m.nodes); i++ { isMerged := m.nodes[i].Ref.IsMerged() + isQueued := m.nodes[i].Ref.IsQueued() if isMerged && !prevWasMerged && i > 0 { line++ // separator line + } else if isQueued && !prevWasQueued && !prevWasMerged && i > 0 { + line++ // separator line } prevWasMerged = isMerged + prevWasQueued = isQueued nodeStart := line nodeLines := m.nodeLineCount(i) @@ -359,11 +364,9 @@ func (m Model) prLabelColumns(idx int) (int, int) { node := m.nodes[idx] // Layout: "├ " (2) + optional status icon + " " (2) + "#N..." col := 2 // bullet + space - if node.PR != nil && (node.PR.Merged || !node.IsLinear || node.PR.Number != 0) { - icon := m.statusIcon(node) - if icon != "" { - col += 2 // icon (1 visible char) + space - } + icon := m.statusIcon(node) + if icon != "" { + col += 2 // icon (1 visible char) + space } prLabel := fmt.Sprintf("#%d", node.PR.Number) return col, col + len(prLabel) @@ -378,19 +381,27 @@ func (m *Model) ensureVisible() { // Calculate the line range for the cursor node, accounting for separator lines startLine := 0 prevWasMerged := false + prevWasQueued := false for i := 0; i < m.cursor; i++ { isMerged := m.nodes[i].Ref.IsMerged() + isQueued := m.nodes[i].Ref.IsQueued() if isMerged && !prevWasMerged && i > 0 { startLine++ // separator line + } else if isQueued && !prevWasQueued && !prevWasMerged && i > 0 { + startLine++ // separator line } prevWasMerged = isMerged + prevWasQueued = isQueued startLine += m.nodeLineCount(i) } // Check if the cursor node itself is preceded by a separator if m.cursor < len(m.nodes) { isMerged := m.nodes[m.cursor].Ref.IsMerged() + isQueued := m.nodes[m.cursor].Ref.IsQueued() if isMerged && !prevWasMerged && m.cursor > 0 { startLine++ + } else if isQueued && !prevWasQueued && !prevWasMerged && m.cursor > 0 { + startLine++ } } endLine := startLine + m.nodeLineCount(m.cursor) @@ -423,12 +434,17 @@ func (m Model) showShortcuts() bool { func (m Model) totalContentLines() int { lines := 0 prevWasMerged := false + prevWasQueued := false for i := 0; i < len(m.nodes); i++ { isMerged := m.nodes[i].Ref.IsMerged() + isQueued := m.nodes[i].Ref.IsQueued() if isMerged && !prevWasMerged && i > 0 { lines++ // separator line + } else if isQueued && !prevWasQueued && !prevWasMerged && i > 0 { + lines++ // separator line } prevWasMerged = isMerged + prevWasQueued = isQueued lines += m.nodeLineCount(i) } lines++ // trunk line @@ -478,13 +494,18 @@ func (m Model) View() string { // Render nodes in order (index 0 = top of stack, displayed first) prevWasMerged := false + prevWasQueued := false for i := 0; i < len(m.nodes); i++ { isMerged := m.nodes[i].Ref.IsMerged() + isQueued := m.nodes[i].Ref.IsQueued() if isMerged && !prevWasMerged && i > 0 { b.WriteString(connectorStyle.Render("────") + dimStyle.Render(" merged ") + connectorStyle.Render("─────") + "\n") + } else if isQueued && !prevWasQueued && !prevWasMerged && i > 0 { + b.WriteString(connectorStyle.Render("────") + dimStyle.Render(" queued ") + connectorStyle.Render("─────") + "\n") } m.renderNode(&b, i) prevWasMerged = isMerged + prevWasQueued = isQueued } // Trunk @@ -535,10 +556,14 @@ func (m Model) renderHeader(b *strings.Builder) { // Build info lines (placed to the right of art on specific rows) mergedCount := 0 + queuedCount := 0 for _, n := range m.nodes { if n.Ref.IsMerged() { mergedCount++ } + if n.Ref.IsQueued() { + queuedCount++ + } } branchCount := len(m.nodes) branchInfo := fmt.Sprintf("%d branches", branchCount) @@ -548,6 +573,9 @@ func (m Model) renderHeader(b *strings.Builder) { if mergedCount > 0 { branchInfo += fmt.Sprintf(" (%d merged)", mergedCount) } + if queuedCount > 0 { + branchInfo += fmt.Sprintf(" (%d queued)", queuedCount) + } // Branch progress icon: ○ none merged, ◐ some merged, ● all merged branchIcon := "○" @@ -682,8 +710,9 @@ func (m Model) renderNode(b *strings.Builder, idx int) { // Determine connector character and style connector := "│" connStyle := connectorStyle - isMerged := node.PR != nil && node.PR.Merged - if !node.IsLinear && !isMerged { + isMerged := node.Ref.IsMerged() + isQueued := node.Ref.IsQueued() + if !node.IsLinear && !isMerged && !isQueued { connector = "┊" connStyle = connectorDashedStyle } @@ -693,6 +722,8 @@ func (m Model) renderNode(b *strings.Builder, idx int) { connStyle = connectorCurrentStyle } else if isMerged { connStyle = connectorMergedStyle + } else if isQueued { + connStyle = connectorQueuedStyle } else { connStyle = connectorFocusedStyle } @@ -745,6 +776,9 @@ func (m Model) renderPRHeader(b *strings.Builder, node BranchNode, isFocused boo case pr.Merged: stateLabel = " MERGED" style = prMergedStyle + case pr.IsQueued: + stateLabel = " QUEUED" + style = prQueuedStyle case pr.State == "CLOSED": stateLabel = " CLOSED" style = prClosedStyle @@ -767,8 +801,6 @@ func (m Model) renderBranchLine(b *strings.Builder, node BranchNode, connector s branchName := node.Ref.Branch if node.IsCurrent { b.WriteString(currentBranchStyle.Render(branchName + " (current)")) - } else if node.PR != nil && node.PR.Merged { - b.WriteString(normalBranchStyle.Render(branchName)) } else { b.WriteString(normalBranchStyle.Render(branchName)) } @@ -816,9 +848,12 @@ func (m Model) renderDiffStats(b *strings.Builder, node BranchNode) { // statusIcon returns the appropriate status icon for a branch. func (m Model) statusIcon(node BranchNode) string { - if node.PR != nil && node.PR.Merged { + if node.Ref.IsMerged() { return mergedIcon } + if node.Ref.IsQueued() { + return queuedIcon + } if !node.IsLinear { return warningIcon } diff --git a/internal/tui/stackview/model_test.go b/internal/tui/stackview/model_test.go index 60e44c5..96c72f0 100644 --- a/internal/tui/stackview/model_test.go +++ b/internal/tui/stackview/model_test.go @@ -247,6 +247,32 @@ func TestView_HeaderShowsMergedCount(t *testing.T) { assert.Contains(t, view, "3 branches (1 merged)") } +func TestView_HeaderShowsQueuedCount(t *testing.T) { + nodes := makeNodes("b1", "b2", "b3") + nodes[1].Ref.Queued = true + nodes[1].Ref.PullRequest = &stack.PullRequestRef{Number: 10} + m := New(nodes, testTrunk, "0.0.1") + + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 40}) + m = updated.(Model) + + view := m.View() + assert.Contains(t, view, "3 branches (1 queued)") +} + +func TestView_QueuedPRShowsQueuedLabel(t *testing.T) { + nodes := makeNodes("b1") + nodes[0].PR = &ghapi.PRDetails{Number: 99, IsQueued: true} + m := New(nodes, testTrunk, "0.0.1") + + updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 30}) + m = updated.(Model) + + view := m.View() + assert.Contains(t, view, "QUEUED") + assert.Contains(t, view, "#99") +} + func TestView_BranchProgressIcon(t *testing.T) { tests := []struct { name string diff --git a/internal/tui/stackview/styles.go b/internal/tui/stackview/styles.go index a93ce04..6f90e99 100644 --- a/internal/tui/stackview/styles.go +++ b/internal/tui/stackview/styles.go @@ -15,12 +15,14 @@ var ( mergedIcon = lipgloss.NewStyle().Foreground(lipgloss.Color("5")).Render("✓") // magenta warningIcon = lipgloss.NewStyle().Foreground(lipgloss.Color("3")).Render("⚠") // yellow openIcon = lipgloss.NewStyle().Foreground(lipgloss.Color("2")).Render("○") // green + queuedIcon = lipgloss.NewStyle().Foreground(lipgloss.Color("130")).Render("◎") // brown // PR status prOpenStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green prMergedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("5")) // magenta prClosedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("1")) // red prDraftStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) // gray + prQueuedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("130")) // brown // Diff stats additionsStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green @@ -37,6 +39,7 @@ var ( connectorFocusedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("15")) // white (focused) connectorCurrentStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("14")) // cyan (current branch focused) connectorMergedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("5")) // magenta (merged branch focused) + connectorQueuedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("130")) // brown (queued branch focused) // Dim text (separators, secondary labels) dimStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("240"))