prune merged branches#94
Conversation
a309b6a to
d985618
Compare
There was a problem hiding this comment.
Pull request overview
Adds an optional “prune merged branches” flow to gh stack sync, including a new --prune flag and an interactive confirmation prompt (interactive only). Also updates TUI behavior to prevent interacting with merged branches and fixes stack API syncing to include merged PRs so partially-merged stacks don’t get rejected.
Changes:
- Add
gh stack sync --pruneto delete local merged branches and their remote-tracking refs, with interactive confirmation when the flag isn’t provided. - Add
ConfirmFntest hook and a new git op (DeleteTrackingRef) to support pruning behavior and testing. - Harden TUI navigation/selection against merged nodes and include merged PRs in stack API update payloads.
Show a summary per file
| File | Description |
|---|---|
| skills/gh-stack/SKILL.md | Documents gh stack sync --prune usage in the skill guide. |
| README.md | Adds --prune flag docs and mentions interactive pruning behavior. |
| docs/src/content/docs/reference/cli.md | Updates CLI reference docs for the new prune step/flag. |
| docs/src/content/docs/guides/workflows.md | Notes pruning as an additional sync step (with --prune). |
| docs/src/content/docs/guides/stacked-prs.md | Mentions optional pruning as part of sync behavior. |
| cmd/sync.go | Implements --prune, interactive confirm prompt, branch switching, and pruning operations. |
| cmd/sync_test.go | Adds extensive coverage for prune flows (flagged, interactive confirm, switching behavior, failures). |
| internal/config/config.go | Adds ConfirmFn hook for tests to override confirm prompting. |
| internal/git/gitops.go | Extends git ops interface + default implementation with DeleteTrackingRef. |
| internal/git/git.go | Exposes DeleteTrackingRef via package-level wrapper. |
| internal/git/mock_ops.go | Extends git mock with DeleteTrackingRefFn + method implementation. |
| internal/tui/stackview/model.go | Prevents cursor/checkout/mouse selection of merged branches; cursor skips merged nodes. |
| internal/tui/stackview/model_test.go | Adds tests verifying cursor initialization/navigation skips merged nodes and enter guard. |
| internal/tui/modifyview/model.go | Makes modify TUI cursor movement skip merged branches; blocks mouse selection of merged nodes. |
| internal/tui/modifyview/model_test.go | Adds cursor navigation test for merged-node skipping in modify view. |
| cmd/submit.go | Includes merged PRs in stack API PUT payload to avoid “Stack contents have changed” rejections. |
| cmd/submit_test.go | Updates test expectation to require merged PRs included in payload. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
internal/tui/stackview/model.go:242
- The merged-branch guard returns early before handling
result.OpenURL(and toggles), which prevents users from clicking PR/commit/file links on merged branches. If the intent is only to prevent selecting/checkout, consider still honoringOpenURL(and possibly expand/collapse) while skipping cursor movement/selection for merged nodes.
// Don't allow selecting merged branches.
if m.nodes[result.NodeIndex].Ref.IsMerged() {
return m, nil
}
m.cursor = result.NodeIndex
if result.OpenURL != "" {
shared.OpenBrowserInBackground(result.OpenURL)
}
internal/tui/modifyview/model.go:882
- The merged-branch guard returns early before processing
result.OpenURL(and toggles), which prevents clicking PR/commit/file links on merged branches. If merged branches should only be non-selectable, consider still honoringOpenURL(and optionally toggles) while skipping cursor selection.
// Don't allow selecting merged branches.
if m.nodes[result.NodeIndex].Ref.IsMerged() {
return m, nil
}
m.cursor = result.NodeIndex
if result.OpenURL != "" {
shared.OpenBrowserInBackground(result.OpenURL)
}
- Files reviewed: 17/17 changed files
- Comments generated: 3
ktravers
left a comment
There was a problem hiding this comment.
The skip/include merged branches changes seem like they could be their own separate PR, but I might be misunderstanding the connection between those and pruned branches 🤷♀️
Otherwise looks good, appreciate the solid test coverage and documentation as always ✨ 🚀
Prune merged branches during sync
Add a
--pruneflag togh stack syncthat deletes local branches (and their remote-tracking refs) for merged PRs. In interactive terminals, users are prompted if they would like to prune merged branches. Non-interactive sessions (agents) require the explicit flag.Changes
cmd/sync.go): New--pruneflag deletes local branches for merged PRs. Deletes both the local branch (git branch -D) and the remote-tracking ref (git branch -dr) sogit checkoutcan't resurrect the branch. If the user is on a pruned branch, checkout moves to the nearest active branch or trunk.cmd/sync.go): When--pruneis not set and the terminal is interactive, prompts"Prune N merged branches?"with default yes. Skipped entirely in non-interactive sessions.internal/config/config.go): New test hook following the existingSelectFnpattern — allows tests to inject mock confirm responses.internal/git): New git operation to delete local remote-tracking refs (git branch -dr origin/<branch>).internal/tui/stackview,internal/tui/modifyview): Merged branches can no longer be selected via keyboard navigation, mouse click, or Enter/checkout in either the view or modify TUI. Cursor skips over merged nodes.cmd/submit.go): Include merged PRs in the PUT payload to the stacks API — omitting them caused "Stack contents have changed" rejections on partially-merged stacks.Stack created with GitHub Stacks CLI • Give Feedback 💬