Skip to content

Commit f4afa45

Browse files
authored
fix(github): omit local-only branches from PR stack comments (#37)
* fix(github): omit local-only branches from PR stack comments Branches without PRs that don't exist on the remote are now filtered out of the rendered stack comment and the `[!WARNING]` header. Their children are promoted up a level so the tree stays connected. - Add `ListRemoteBranches()` to `git.Git` (reads cached tracking refs, no network call) - `GenerateStackComment` / `renderTree` accept an optional `remoteBranches` set; `nil` disables filtering - `findEffectiveParent()` walks ancestors to find the nearest visible parent for the warning header - Thread the set through `submit` and `sync` command paths * fix(github): address PR review comments for remote branch filtering - Add defensive `!isCurrent` guard to `skipNode` in `renderTree` - Assert WARNING header absent when effective parent is trunk after filtering - Assert WARNING header references correct non-trunk remote parent - Add test for multiple consecutive local-only branches being filtered
1 parent 07f807b commit f4afa45

File tree

5 files changed

+305
-60
lines changed

5 files changed

+305
-60
lines changed

cmd/submit.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,18 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
209209
}
210210
}
211211

212+
// Build remote branches set for stack comment filtering.
213+
// This uses locally-cached tracking refs which are up-to-date after Phase 2 push.
214+
var remoteBranches map[string]bool
215+
if !dryRun {
216+
var rbErr error
217+
remoteBranches, rbErr = g.ListRemoteBranches()
218+
if rbErr != nil {
219+
// Non-fatal: we can still render without filtering
220+
fmt.Printf("%s could not list remote branches, stack comments may reference local-only branches: %v\n", s.WarningIcon(), rbErr)
221+
}
222+
}
223+
212224
// Collect PR URLs for --web flag
213225
var prURLs []string
214226

@@ -236,7 +248,7 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
236248
}
237249
}
238250
// Update stack comment
239-
if err := ghClient.GenerateAndPostStackComment(root, b.Name, trunk, existingPR); err != nil {
251+
if err := ghClient.GenerateAndPostStackComment(root, b.Name, trunk, existingPR, remoteBranches); err != nil {
240252
fmt.Printf("%s failed to update stack comment for PR #%d: %v\n", s.WarningIcon(), existingPR, err)
241253
}
242254

@@ -248,7 +260,7 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
248260
if dryRun {
249261
fmt.Printf("%s Would create PR for %s (base: %s)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(parent))
250262
} else {
251-
prNum, adopted, err := createPRForBranch(g, ghClient, cfg, root, b.Name, parent, trunk, s)
263+
prNum, adopted, err := createPRForBranch(g, ghClient, cfg, root, b.Name, parent, trunk, remoteBranches, s)
252264
if err != nil {
253265
fmt.Printf("%s failed to create PR for %s: %v\n", s.WarningIcon(), s.Branch(b.Name), err)
254266
} else if adopted {
@@ -284,7 +296,7 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
284296
// createPRForBranch creates a PR for the given branch and stores the PR number.
285297
// If a PR already exists for the branch, it adopts the existing PR instead.
286298
// Returns (prNumber, adopted, error) where adopted is true if we adopted an existing PR.
287-
func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string, s *style.Style) (int, bool, error) {
299+
func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string, remoteBranches map[string]bool, s *style.Style) (int, bool, error) {
288300
// Determine if draft (not targeting trunk = middle of stack)
289301
draft := base != trunk
290302

@@ -309,7 +321,7 @@ func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config,
309321
if err != nil {
310322
// Check if PR already exists - if so, adopt it
311323
if strings.Contains(err.Error(), "pull request already exists") {
312-
prNum, adoptErr := adoptExistingPR(ghClient, cfg, root, branch, base, trunk, s)
324+
prNum, adoptErr := adoptExistingPR(ghClient, cfg, root, branch, base, trunk, remoteBranches, s)
313325
return prNum, true, adoptErr
314326
}
315327
return 0, false, err
@@ -326,7 +338,7 @@ func createPRForBranch(g *git.Git, ghClient *github.Client, cfg *config.Config,
326338
}
327339

328340
// Add stack navigation comment
329-
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, pr.Number); err != nil {
341+
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, pr.Number, remoteBranches); err != nil {
330342
fmt.Printf("%s failed to add stack comment to PR #%d: %v\n", s.WarningIcon(), pr.Number, err)
331343
}
332344

@@ -435,7 +447,7 @@ func promptForPRDetails(branch, defaultTitle, defaultBody string, s *style.Style
435447
}
436448

437449
// adoptExistingPR finds an existing PR for the branch and adopts it into the stack.
438-
func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string, s *style.Style) (int, error) {
450+
func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Node, branch, base, trunk string, remoteBranches map[string]bool, s *style.Style) (int, error) {
439451
existingPR, err := ghClient.FindPRByHead(branch)
440452
if err != nil {
441453
return 0, fmt.Errorf("failed to find existing PR: %w", err)
@@ -462,7 +474,7 @@ func adoptExistingPR(ghClient *github.Client, cfg *config.Config, root *tree.Nod
462474
}
463475

464476
// Add/update stack navigation comment
465-
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, existingPR.Number); err != nil {
477+
if err := ghClient.GenerateAndPostStackComment(root, branch, trunk, existingPR.Number, remoteBranches); err != nil {
466478
fmt.Printf("%s failed to update stack comment: %v\n", s.WarningIcon(), err)
467479
}
468480

cmd/sync.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func init() {
3434
}
3535

3636
// updateStackComments updates the navigation comment on all PRs in the stack.
37-
func updateStackComments(cfg *config.Config, gh *github.Client, s *style.Style) error {
37+
func updateStackComments(cfg *config.Config, g *git.Git, gh *github.Client, s *style.Style) error {
3838
trunk, err := cfg.GetTrunk()
3939
if err != nil {
4040
return err
@@ -53,13 +53,20 @@ func updateStackComments(cfg *config.Config, gh *github.Client, s *style.Style)
5353
prInfo = make(map[int]github.PRInfo)
5454
}
5555

56+
// Build remote branches set for filtering local-only branches
57+
remoteBranches, rbErr := g.ListRemoteBranches()
58+
if rbErr != nil {
59+
// Non-fatal: render without filtering
60+
fmt.Printf("%s could not list remote branches: %v\n", s.WarningIcon(), rbErr)
61+
}
62+
5663
// Walk tree and update each PR's comment
57-
return walkTreeAndUpdateComments(root, root, trunk, gh, prInfo, s)
64+
return walkTreeAndUpdateComments(root, root, trunk, gh, prInfo, remoteBranches, s)
5865
}
5966

60-
func walkTreeAndUpdateComments(node, root *tree.Node, trunk string, gh *github.Client, prInfo map[int]github.PRInfo, s *style.Style) error {
67+
func walkTreeAndUpdateComments(node, root *tree.Node, trunk string, gh *github.Client, prInfo map[int]github.PRInfo, remoteBranches map[string]bool, s *style.Style) error {
6168
if node.PR > 0 {
62-
comment := github.GenerateStackComment(root, node.Name, trunk, gh.RepoURL(), prInfo)
69+
comment := github.GenerateStackComment(root, node.Name, trunk, gh.RepoURL(), prInfo, remoteBranches)
6370
if comment != "" {
6471
if err := gh.CreateOrUpdateStackComment(node.PR, comment); err != nil {
6572
fmt.Printf("%s failed to update comment on PR #%d: %v\n", s.WarningIcon(), node.PR, err)
@@ -69,7 +76,7 @@ func walkTreeAndUpdateComments(node, root *tree.Node, trunk string, gh *github.C
6976
}
7077

7178
for _, child := range node.Children {
72-
if err := walkTreeAndUpdateComments(child, root, trunk, gh, prInfo, s); err != nil {
79+
if err := walkTreeAndUpdateComments(child, root, trunk, gh, prInfo, remoteBranches, s); err != nil {
7380
return err
7481
}
7582
}
@@ -355,7 +362,7 @@ func runSync(cmd *cobra.Command, args []string) error {
355362
// Update stack comments on all PRs
356363
if !syncDryRunFlag {
357364
fmt.Println("\nUpdating stack comments...")
358-
if err := updateStackComments(cfg, gh, s); err != nil {
365+
if err := updateStackComments(cfg, g, gh, s); err != nil {
359366
fmt.Printf("%s failed to update some comments: %v\n", s.WarningIcon(), err)
360367
}
361368
}

internal/git/git.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,24 @@ func (g *Git) RemoteBranchExists(branch string) bool {
225225
return err == nil
226226
}
227227

228+
// ListRemoteBranches returns the set of branch names that exist on origin,
229+
// based on locally-cached remote-tracking refs (no network call).
230+
// This is accurate as long as a fetch or push has been done recently.
231+
func (g *Git) ListRemoteBranches() (map[string]bool, error) {
232+
out, err := g.run("for-each-ref", "--format=%(refname:strip=3)", "refs/remotes/origin/")
233+
if err != nil {
234+
return nil, err
235+
}
236+
branches := make(map[string]bool)
237+
for _, line := range strings.Split(out, "\n") {
238+
line = strings.TrimSpace(line)
239+
if line != "" && line != "HEAD" {
240+
branches[line] = true
241+
}
242+
}
243+
return branches, nil
244+
}
245+
228246
// RebaseOnto rebases a branch onto a new base, replaying only commits after oldBase.
229247
// Checks out the branch first, then runs: git rebase --onto <newBase> <oldBase>
230248
// Useful when a parent branch was squash-merged and we need to replay only

internal/github/comments.go

Lines changed: 71 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ const StackCommentMarker = "<!-- gh-stack:nav -->"
1818
//
1919
// Only the current stack is rendered: the path from root to the current branch,
2020
// plus all descendants of the current branch. Sibling stacks are excluded.
21-
func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string, prInfo map[int]PRInfo) string {
21+
//
22+
// If remoteBranches is non-nil, branches without PRs that don't exist on the
23+
// remote are omitted (their children are promoted up a level). Pass nil to
24+
// disable this filtering.
25+
func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string, prInfo map[int]PRInfo, remoteBranches map[string]bool) string {
2226
var sb strings.Builder
2327

2428
// Find the current node
@@ -40,13 +44,18 @@ func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string,
4044
sb.WriteString(StackCommentMarker)
4145
sb.WriteString("\n")
4246

47+
// Find the effective parent: the nearest ancestor that would be visible
48+
// in the rendered tree. A node is visible if it has a PR, exists on the
49+
// remote, or remote filtering is disabled.
50+
effectiveParent := findEffectiveParent(currentNode, remoteBranches)
51+
4352
// Add warning if not targeting trunk
44-
if currentNode.Parent != nil && currentNode.Parent.Name != trunk {
53+
if effectiveParent != nil && effectiveParent.Name != trunk {
4554
sb.WriteString("> [!WARNING]\n")
46-
sb.WriteString(fmt.Sprintf("> This PR is part of a stack and targets branch `%s`, not `%s`.\n", currentNode.Parent.Name, trunk))
55+
sb.WriteString(fmt.Sprintf("> This PR is part of a stack and targets branch `%s`, not `%s`.\n", effectiveParent.Name, trunk))
4756

4857
// Build parent PR reference if available
49-
parentPR := currentNode.Parent.PR
58+
parentPR := effectiveParent.PR
5059
if parentPR > 0 {
5160
parentURL := fmt.Sprintf("%s/pull/%d", repoURL, parentPR)
5261
linkText := fmt.Sprintf("#%d", parentPR)
@@ -63,7 +72,7 @@ func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string,
6372
sb.WriteString("### :books: Pull Request Stack\n\n")
6473

6574
// Render tree from root as nested markdown list, filtered to current stack
66-
renderTree(&sb, root, currentBranch, repoURL, prInfo, 0, ancestorPath)
75+
renderTree(&sb, root, currentBranch, repoURL, prInfo, 0, ancestorPath, remoteBranches)
6776

6877
sb.WriteString("\n---\n")
6978
sb.WriteString("*Managed by [gh-stack](https://github.com/boneskull/gh-stack)*\n")
@@ -87,44 +96,70 @@ func collectPRNumbersRecursive(node *tree.Node, numbers *[]int) {
8796
}
8897
}
8998

99+
// findEffectiveParent walks up from a node's parent to find the nearest ancestor
100+
// that would be rendered in the comment. A node is "visible" if it has a PR or
101+
// exists on the remote (or if remote filtering is disabled). Returns nil if the
102+
// node has no parent (i.e., it's the root).
103+
func findEffectiveParent(node *tree.Node, remoteBranches map[string]bool) *tree.Node {
104+
for p := node.Parent; p != nil; p = p.Parent {
105+
if p.PR > 0 || remoteBranches == nil || remoteBranches[p.Name] {
106+
return p
107+
}
108+
}
109+
return nil
110+
}
111+
90112
// renderTree recursively renders the tree structure as nested markdown lists.
91113
//
92114
// The ancestorPath set controls which children are rendered at each level:
93115
// - For nodes that are ancestors of the current branch (on the path but not
94116
// the current branch itself), only the child on the ancestor path is shown.
95117
// - For the current branch and all its descendants, all children are shown.
96118
//
97-
// This ensures only the current stack is rendered, not sibling stacks.
98-
func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL string, prInfo map[int]PRInfo, depth int, ancestorPath map[string]bool) {
99-
// Build prefix based on depth (2 spaces per level for markdown nested lists)
100-
prefix := strings.Repeat(" ", depth) + "- "
101-
119+
// If remoteBranches is non-nil, nodes without PRs that aren't on the remote
120+
// are collapsed: their children are rendered at the skipped node's depth
121+
// instead of being indented further. This prevents local-only branches from
122+
// appearing in the GitHub comment.
123+
func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL string, prInfo map[int]PRInfo, depth int, ancestorPath map[string]bool, remoteBranches map[string]bool) {
102124
isCurrent := node.Name == currentBranch
103125

104-
// Format: "[Title #N](url) - branch: `name`" or "branch: `name`" if no PR
105-
if node.PR > 0 {
106-
prURL := fmt.Sprintf("%s/pull/%d", repoURL, node.PR)
107-
linkText := fmt.Sprintf("#%d", node.PR)
108-
if info, ok := prInfo[node.PR]; ok && info.Title != "" {
109-
linkText = fmt.Sprintf("%s #%d", info.Title, node.PR)
110-
}
126+
// Skip branches that have no PR and don't exist on the remote.
127+
// The current branch is never skipped (defensive: it should always have
128+
// a PR since we're generating a comment for it, but just in case).
129+
// When remoteBranches is nil, no filtering is applied.
130+
skipNode := node.PR == 0 && !isCurrent && remoteBranches != nil && !remoteBranches[node.Name]
131+
132+
nextDepth := depth
133+
if !skipNode {
134+
// Build prefix based on depth (2 spaces per level for markdown nested lists)
135+
prefix := strings.Repeat(" ", depth) + "- "
136+
137+
// Format: "[Title #N](url) - branch: `name`" or "branch: `name`" if no PR
138+
if node.PR > 0 {
139+
prURL := fmt.Sprintf("%s/pull/%d", repoURL, node.PR)
140+
linkText := fmt.Sprintf("#%d", node.PR)
141+
if info, ok := prInfo[node.PR]; ok && info.Title != "" {
142+
linkText = fmt.Sprintf("%s #%d", info.Title, node.PR)
143+
}
111144

112-
if isCurrent {
113-
// Bold the link for current PR
114-
fmt.Fprintf(sb, "%s**[%s](%s)** - branch: `%s` *(this PR)*", prefix, linkText, prURL, node.Name)
115-
} else {
116-
fmt.Fprintf(sb, "%s[%s](%s) - branch: `%s`", prefix, linkText, prURL, node.Name)
117-
}
118-
} else {
119-
// No PR - show "branch: `name`"
120-
if isCurrent {
121-
fmt.Fprintf(sb, "%s**branch: `%s`** *(this PR)*", prefix, node.Name)
145+
if isCurrent {
146+
// Bold the link for current PR
147+
fmt.Fprintf(sb, "%s**[%s](%s)** - branch: `%s` *(this PR)*", prefix, linkText, prURL, node.Name)
148+
} else {
149+
fmt.Fprintf(sb, "%s[%s](%s) - branch: `%s`", prefix, linkText, prURL, node.Name)
150+
}
122151
} else {
123-
fmt.Fprintf(sb, "%sbranch: `%s`", prefix, node.Name)
152+
// No PR - show "branch: `name`"
153+
if isCurrent {
154+
fmt.Fprintf(sb, "%s**branch: `%s`** *(this PR)*", prefix, node.Name)
155+
} else {
156+
fmt.Fprintf(sb, "%sbranch: `%s`", prefix, node.Name)
157+
}
124158
}
125-
}
126159

127-
sb.WriteString("\n")
160+
sb.WriteString("\n")
161+
nextDepth = depth + 1
162+
}
128163

129164
// For ancestor nodes (above the current branch), only render the child
130165
// that leads to the current branch. For the current branch and its
@@ -134,7 +169,7 @@ func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL str
134169
if isAncestorAboveCurrent && !ancestorPath[child.Name] {
135170
continue
136171
}
137-
renderTree(sb, child, currentBranch, repoURL, prInfo, depth+1, ancestorPath)
172+
renderTree(sb, child, currentBranch, repoURL, prInfo, nextDepth, ancestorPath, remoteBranches)
138173
}
139174
}
140175

@@ -177,7 +212,10 @@ func (c *Client) CreateOrUpdateStackComment(prNumber int, body string) error {
177212

178213
// GenerateAndPostStackComment generates and posts/updates a stack comment for a PR.
179214
// It fetches PR titles via GraphQL and renders the full comment.
180-
func (c *Client) GenerateAndPostStackComment(root *tree.Node, branch, trunk string, prNumber int) error {
215+
//
216+
// If remoteBranches is non-nil, branches without PRs that don't exist on the
217+
// remote are omitted from the rendered comment. Pass nil to disable filtering.
218+
func (c *Client) GenerateAndPostStackComment(root *tree.Node, branch, trunk string, prNumber int, remoteBranches map[string]bool) error {
181219
// Collect all PR numbers from the tree
182220
prNumbers := CollectPRNumbers(root)
183221

@@ -188,7 +226,7 @@ func (c *Client) GenerateAndPostStackComment(root *tree.Node, branch, trunk stri
188226
prInfo = make(map[int]PRInfo)
189227
}
190228

191-
comment := GenerateStackComment(root, branch, trunk, c.RepoURL(), prInfo)
229+
comment := GenerateStackComment(root, branch, trunk, c.RepoURL(), prInfo, remoteBranches)
192230
if comment == "" {
193231
return nil
194232
}

0 commit comments

Comments
 (0)