From 479748fb4d7ddac18478137a2243d385d1747786 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 13 Feb 2026 15:09:23 -0800 Subject: [PATCH] fix(github): scope PR stack comment to current stack only - Filter rendered tree to only show the path from trunk to the current branch and its descendants; sibling stacks are excluded. - Branches without PRs now display `branch: `name`` instead of bare ``name`` for consistency with the PR line format. --- internal/github/comments.go | 43 ++++++++++++++---- internal/github/comments_test.go | 75 +++++++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 15 deletions(-) diff --git a/internal/github/comments.go b/internal/github/comments.go index dac9fa8..fc730d9 100644 --- a/internal/github/comments.go +++ b/internal/github/comments.go @@ -15,6 +15,9 @@ const StackCommentMarker = "" // It includes a warning if the PR targets a non-trunk branch. // The repoURL should be the base repository URL (e.g., "https://github.com/owner/repo"). // The prInfo map provides titles for PRs (keyed by PR number). +// +// Only the current stack is rendered: the path from root to the current branch, +// plus all descendants of the current branch. Sibling stacks are excluded. func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string, prInfo map[int]PRInfo) string { var sb strings.Builder @@ -24,6 +27,15 @@ func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string, return "" } + // Build ancestor path: the set of branch names from root down to (and + // including) the current branch. When rendering, only children on this + // path are shown for ancestor nodes; descendants of the current branch + // are always shown in full. + ancestorPath := make(map[string]bool) + for n := currentNode; n != nil; n = n.Parent { + ancestorPath[n.Name] = true + } + // Start with marker sb.WriteString(StackCommentMarker) sb.WriteString("\n") @@ -50,8 +62,8 @@ func GenerateStackComment(root *tree.Node, currentBranch, trunk, repoURL string, // Stack header sb.WriteString("### :books: Pull Request Stack\n\n") - // Render tree from root as nested markdown list - renderTree(&sb, root, currentBranch, repoURL, prInfo, 0) + // Render tree from root as nested markdown list, filtered to current stack + renderTree(&sb, root, currentBranch, repoURL, prInfo, 0, ancestorPath) sb.WriteString("\n---\n") sb.WriteString("*Managed by [gh-stack](https://github.com/boneskull/gh-stack)*\n") @@ -76,13 +88,20 @@ func collectPRNumbersRecursive(node *tree.Node, numbers *[]int) { } // renderTree recursively renders the tree structure as nested markdown lists. -func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL string, prInfo map[int]PRInfo, depth int) { +// +// The ancestorPath set controls which children are rendered at each level: +// - For nodes that are ancestors of the current branch (on the path but not +// the current branch itself), only the child on the ancestor path is shown. +// - For the current branch and all its descendants, all children are shown. +// +// This ensures only the current stack is rendered, not sibling stacks. +func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL string, prInfo map[int]PRInfo, depth int, ancestorPath map[string]bool) { // Build prefix based on depth (2 spaces per level for markdown nested lists) prefix := strings.Repeat(" ", depth) + "- " isCurrent := node.Name == currentBranch - // Format: "[Title #N](url) - branch: `name`" or just branch name if no PR + // Format: "[Title #N](url) - branch: `name`" or "branch: `name`" if no PR if node.PR > 0 { prURL := fmt.Sprintf("%s/pull/%d", repoURL, node.PR) linkText := fmt.Sprintf("#%d", node.PR) @@ -97,19 +116,25 @@ func renderTree(sb *strings.Builder, node *tree.Node, currentBranch, repoURL str fmt.Fprintf(sb, "%s[%s](%s) - branch: `%s`", prefix, linkText, prURL, node.Name) } } else { - // No PR - just show branch name (e.g., trunk) + // No PR - show "branch: `name`" if isCurrent { - fmt.Fprintf(sb, "%s**`%s`**", prefix, node.Name) + fmt.Fprintf(sb, "%s**branch: `%s`** *(this PR)*", prefix, node.Name) } else { - fmt.Fprintf(sb, "%s`%s`", prefix, node.Name) + fmt.Fprintf(sb, "%sbranch: `%s`", prefix, node.Name) } } sb.WriteString("\n") - // Render children + // For ancestor nodes (above the current branch), only render the child + // that leads to the current branch. For the current branch and its + // descendants, render all children. + isAncestorAboveCurrent := ancestorPath[node.Name] && !isCurrent for _, child := range node.Children { - renderTree(sb, child, currentBranch, repoURL, prInfo, depth+1) + if isAncestorAboveCurrent && !ancestorPath[child.Name] { + continue + } + renderTree(sb, child, currentBranch, repoURL, prInfo, depth+1, ancestorPath) } } diff --git a/internal/github/comments_test.go b/internal/github/comments_test.go index 509d7d7..6651b43 100644 --- a/internal/github/comments_test.go +++ b/internal/github/comments_test.go @@ -126,9 +126,9 @@ func TestGenerateStackComment(t *testing.T) { if strings.Contains(comment, "```") { t.Error("should not use code blocks") } - // Trunk has no PR, shown as backticked branch name - if !strings.Contains(comment, "- `main`") { - t.Error("should use markdown list format with trunk in backticks") + // Trunk has no PR, shown with "branch:" prefix + if !strings.Contains(comment, "- branch: `main`") { + t.Error("should use markdown list format with 'branch:' prefix for trunk") } }) @@ -141,6 +141,30 @@ func TestGenerateStackComment(t *testing.T) { t.Error("should fallback to '#N' when title not available") } }) + + t.Run("no-PR branches show branch prefix", func(t *testing.T) { + // Branch without a PR in the middle of a stack + noPRRoot := &tree.Node{Name: "main"} + noPRMiddle := &tree.Node{Name: "wip-branch", PR: 0, Parent: noPRRoot} + noPRChild := &tree.Node{Name: "child-branch", PR: 5, Parent: noPRMiddle} + + noPRRoot.Children = []*tree.Node{noPRMiddle} + noPRMiddle.Children = []*tree.Node{noPRChild} + + childPRInfo := makePRInfo(struct { + num int + title string + }{5, "Child feature"}) + comment := GenerateStackComment(noPRRoot, "child-branch", "main", testRepoURL, childPRInfo) + + // Trunk and middle branch both have no PR; both should show "branch:" prefix + if !strings.Contains(comment, "- branch: `main`") { + t.Error("trunk without PR should show 'branch:' prefix") + } + if !strings.Contains(comment, "- branch: `wip-branch`") { + t.Error("branch without PR should show 'branch:' prefix") + } + }) } func TestGenerateStackComment_EdgeCases(t *testing.T) { @@ -192,7 +216,7 @@ func TestGenerateStackComment_EdgeCases(t *testing.T) { } }) - t.Run("branch with siblings", func(t *testing.T) { + t.Run("branch with siblings only shows current stack", func(t *testing.T) { root := &tree.Node{Name: "main"} a := &tree.Node{Name: "feature-a", PR: 1, Parent: root} b := &tree.Node{Name: "feature-b", PR: 2, Parent: root} @@ -213,8 +237,47 @@ func TestGenerateStackComment_EdgeCases(t *testing.T) { if !strings.Contains(comment, "feature-a") { t.Error("should contain current branch") } - if !strings.Contains(comment, "feature-b") { - t.Error("should contain sibling branch") + if strings.Contains(comment, "feature-b") { + t.Error("should NOT contain sibling branch from a different stack") + } + }) + + t.Run("multiple stacks only shows current one", func(t *testing.T) { + // Tree: main -> {stack1-base (#1) -> stack1-child (#2), unrelated (#3)} + root := &tree.Node{Name: "main"} + stack1Base := &tree.Node{Name: "stack1-base", PR: 1, Parent: root} + stack1Child := &tree.Node{Name: "stack1-child", PR: 2, Parent: stack1Base} + unrelated := &tree.Node{Name: "unrelated", PR: 3, Parent: root} + + root.Children = []*tree.Node{stack1Base, unrelated} + stack1Base.Children = []*tree.Node{stack1Child} + + prInfo := makePRInfo( + struct { + num int + title string + }{1, "Stack 1 base"}, + struct { + num int + title string + }{2, "Stack 1 child"}, + struct { + num int + title string + }{3, "Unrelated feature"}, + ) + + // Viewing the child PR - should see stack1-base and stack1-child, but NOT unrelated + comment := GenerateStackComment(root, "stack1-child", "main", testRepoURL, prInfo) + + if !strings.Contains(comment, "stack1-base") { + t.Error("should contain ancestor branch in the current stack") + } + if !strings.Contains(comment, "stack1-child") { + t.Error("should contain current branch") + } + if strings.Contains(comment, "unrelated") { + t.Error("should NOT contain branch from a different stack") } })