Skip to content

Commit 754d190

Browse files
authored
Fork unmerged branches into new stack (#154)
* Fork unmerged branches into a new stack when the base stack is fully merged Once every PR that is officially part of a stack on GitHub has been merged -- especially after the merged branches are deleted upstream -- you can no longer add to that stack. A new PR on top would target the trunk directly instead of chaining onto the merged PRs, so the remote stack's "each PR's base ref is the previous PR's head ref" invariant no longer holds. On the next `gh stack submit`, the stack update was rejected and surfaced as a confusing, dead-end warning: Failed to update stack on GitHub: Pull requests must form a stack, where each PR's base ref is the previous PR's head ref `submit` had no handling for this: `syncStack` always sent the full PR list (including the merged-and-deleted ones), so the API rejected the broken chain even though the new PRs had already been created with correct bases. Fork the survivors into a fresh stack instead of failing. After syncing PR state and before pushing, `runSubmit` now calls `maybeForkFromMergedBase`: - It triggers only when every PR officially part of the tracked remote stack (`s.ID`) has merged. Membership is read from the stacks API, so open PRs that are not part of the remote stack do not count, and -- this is the key guard -- a normal partial, bottom-up merge (where the remote stack still lists an open PR) is left completely untouched. A cheap pre-check (the local stack must have at least one merged branch) avoids an extra ListStacks call on the common path. - The local branches are partitioned: those still in the merged remote stack stay behind; everything else (new branches, plus open PRs that were never part of that remote stack) is lifted into a brand-new stack rooted at the original trunk, with an empty remote ID. The bottom survivor is re-based onto the trunk. - `runSubmit` continues with the new stack, so the push loop, PR creation, and `syncStack` all operate on it; the empty ID routes `syncStack` through the adopt/create path and a fresh stack is created on GitHub. - The original, fully merged stack is left untouched on GitHub. Locally it is kept as a record only if at least one of its branches still exists in the working copy; otherwise it is dropped. No data is lost -- those PRs are already merged on GitHub. To restructure the stack file safely, add `StackFile.IndexOfStack`, which locates a stack by pointer identity so the fork can capture what it needs before `AddStack`/`RemoveStack` reallocate the underlying slice. Also soften the partial-merge case that does not fork: when an `UpdateStack` call fails with the "must form a stack" 422 and the stack still contains merged branches, report it as an informational note (the unmerged PRs were pushed and re-based onto the trunk) rather than a scary failure warning. Scope is limited to `submit`. `add` and `checkout` keep their existing "refuse and suggest `gh stack init`" behavior on fully merged stacks. Tests: - cmd/submit_test.go: TestSubmit_ForksWhenRemoteStackFullyMerged covers both disposition variants (the old stack is removed when its merged branches are gone locally, kept when they still exist) and asserts that only the new branches are pushed, the fork message is printed, a fresh stack is created, and the local stack file is split into two stacks. TestSubmit_NoForkWhenRemoteStackHasOpenPR verifies the everyday bottom-up merge is not forked and that the broken-chain 422 is reported calmly. TestUpdateStack_BrokenChainAfterMerge checks the calm-vs-warn branch. - internal/stack/stack_test.go: TestIndexOfStack covers identity lookup and the not-found case. Docs: README, the CLI reference, the stacked-PRs guide, the FAQ, and the agent SKILL.md note that submitting onto a fully merged stack starts a new stack rooted at the trunk. * Handle fully merged stacks gracefully in the view and modify TUIs Merged branches (and their PRs) are not selectable, so once an entire stack has landed there is nothing to act on -- yet the TUIs did not reflect that: - `gh stack view` still drew a highlighted cursor on the top branch even though it could not be selected. Navigation, checkout, and the per-branch toggles all silently did nothing, with no indication of why. - `gh stack modify` opened its full editor on a stack with nothing left to restructure, instead of short-circuiting like `gh stack submit` does when there is nothing to submit. Reflect the "nothing actionable" state in both TUIs. View (internal/tui/stackview/model.go): - Hide the cursor when every branch is merged. `New` now starts the cursor at -1 and only lands it on the current or first non-merged branch; when none exists the cursor stays hidden, so no row is rendered as focused. The existing `m.cursor >= 0` guards and merged-skipping `moveCursor` already make every cursor action a no-op in that state, and mouse-wheel scrolling still works for tall merged stacks. - Dim the shortcuts that depend on the cursor. `buildHeaderConfig` marks navigate, commits, files, open PR, and checkout as `Disabled` (rendered gray via the existing ShortcutEntry.Disabled styling) when all branches are merged, leaving only `q quit` active. Modify (cmd/modify.go): - Short-circuit before opening the TUI. After preconditions pass and PR state is synced, `runModify` now returns early when the stack is fully merged, printing "All branches in this stack have been merged" and pointing at `gh stack init`, exiting cleanly (exit 0) like submit's "nothing to submit" path. The linearity and merge-queue precondition checks already skip merged branches, so they do not fire spuriously. Tests: - internal/tui/stackview/model_test.go: the cursor is hidden (-1) when all branches are merged; up/down/enter do not move it or trigger a checkout; View renders without panicking on a hidden cursor; buildHeaderConfig disables every cursor-dependent shortcut (and only those) when all merged, and leaves them all enabled when active branches remain. - cmd/modify_test.go: runModify short-circuits on a fully merged stack, printing the message and returning no error without launching the TUI.
1 parent 9b30b7f commit 754d190

13 files changed

Lines changed: 606 additions & 10 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,8 @@ Creates a Stacked PR for every branch in the stack, pushing branches to the remo
372372

373373
After creating PRs, `submit` automatically creates a **Stack** on GitHub to link the PRs together. If the stack already exists on GitHub (e.g., from a previous submit), new PRs will be added to the top of the stack.
374374

375+
If every PR in the stack has already been merged, that stack is complete and can't be extended — a new PR on top would target the trunk directly rather than chaining onto the merged PRs. In that case `submit` automatically starts a **new** stack rooted at the trunk for your unmerged branches and creates it on GitHub, leaving the merged stack untouched.
376+
375377
In an interactive terminal, `submit` opens a full-screen, mouse- and keyboard-driven editor on a single screen. Every branch without a PR is included by default — deselect any you don't want on the left panel (<kbd>Ctrl</kbd>+<kbd>X</kbd>). Because each PR builds on the branch below it, deselecting a branch also deselects the ones stacked above it, and re-including a branch re-includes the ones below it. Draft each PR's title, description (with a markdown preview and `$EDITOR` escape), and choose ready-for-review or draft on the right, then submit them all at once with <kbd>Ctrl</kbd>+<kbd>S</kbd>. Pass `--auto` (or run in CI) to skip the editor and use auto-generated titles.
376378

377379
In the editor, new PRs default to ready for review; flip any PR to draft with the ready ↔ draft toggle. With `--auto`, new PRs are created as drafts unless you pass `--open`.

cmd/modify.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ func runModify(cfg *config.Config) error {
7474
s := result.Stack
7575
currentBranch := result.CurrentBranch
7676

77+
// A fully merged stack has nothing left to restructure. Short-circuit
78+
// before opening the TUI, mirroring submit's "nothing to submit" behavior.
79+
if s.IsFullyMerged() {
80+
cfg.Warningf("All branches in this stack have been merged")
81+
cfg.Printf("There's nothing to modify — start a new stack with `%s`", cfg.ColorCyan("gh stack init"))
82+
return nil
83+
}
84+
7785
// Load branch data for the TUI
7886
viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch, result.PRDetails)
7987

cmd/modify_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd
22

33
import (
44
"encoding/json"
5+
"io"
56
"os"
67
"path/filepath"
78
"testing"
@@ -636,6 +637,50 @@ func TestCheckModifyPreconditions_AllPass(t *testing.T) {
636637
assert.Equal(t, "b1", result.CurrentBranch)
637638
}
638639

640+
func TestRunModify_FullyMergedStack_ShortCircuits(t *testing.T) {
641+
s := stack.Stack{
642+
Trunk: stack.BranchRef{Branch: "main"},
643+
Branches: []stack.BranchRef{
644+
{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 1, Merged: true}},
645+
{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 2, Merged: true}},
646+
},
647+
}
648+
649+
tmpDir := t.TempDir()
650+
writeStackFile(t, tmpDir, s)
651+
652+
mock := &git.MockOps{
653+
GitDirFn: func() (string, error) { return tmpDir, nil },
654+
CurrentBranchFn: func() (string, error) { return "b1", nil },
655+
IsRebaseInProgressFn: func() bool { return false },
656+
HasUncommittedChangesFn: func() (bool, error) { return false, nil },
657+
BranchExistsFn: func(string) bool { return true },
658+
IsAncestorFn: func(a, d string) (bool, error) { return true, nil },
659+
LogMergesFn: func(base, head string) ([]git.CommitInfo, error) { return nil, nil },
660+
}
661+
restore := git.SetOps(mock)
662+
defer restore()
663+
664+
cfg, _, errR := config.NewTestConfig()
665+
cfg.ForceInteractive = true
666+
cfg.GitHubClientOverride = &github.MockClient{
667+
FindPRForBranchFn: func(string) (*github.PullRequest, error) { return nil, nil },
668+
}
669+
670+
// runModify must short-circuit (and never launch the TUI) on a fully
671+
// merged stack, returning cleanly like submit's "nothing to submit" path.
672+
err := runModify(cfg)
673+
674+
cfg.Out.Close()
675+
cfg.Err.Close()
676+
out, _ := io.ReadAll(errR)
677+
output := string(out)
678+
679+
assert.NoError(t, err)
680+
assert.Contains(t, output, "All branches in this stack have been merged")
681+
assert.Contains(t, output, "gh stack init")
682+
}
683+
639684
// ---------------------------------------------------------------------------
640685
// 5. State file path / exists edge cases
641686
// ---------------------------------------------------------------------------

cmd/submit.go

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ func runSubmit(cfg *config.Config, opts *submitOptions) error {
143143
// Sync PR state to detect merged/queued PRs before pushing.
144144
prDetails := syncStackPRs(cfg, s)
145145

146+
// If the active branches now sit on top of a fully-merged base, they can no
147+
// longer extend the existing remote stack. Fork them into a fresh stack
148+
// rooted at the trunk and continue the submit with that new stack.
149+
if stacksAvailable {
150+
s = maybeForkFromMergedBase(cfg, client, sf, s, gitDir)
151+
}
152+
146153
// Resolve remote for pushing
147154
remote, err := pickRemote(cfg, currentBranch, opts.remote)
148155
if err != nil {
@@ -477,6 +484,144 @@ func humanize(s string) string {
477484
}, s)
478485
}
479486

487+
// maybeForkFromMergedBase detects when every PR that is officially part of the
488+
// stack on GitHub has already been merged, and forks the remaining local
489+
// branches (new branches, or open PRs that were never part of that remote stack)
490+
// into a brand-new stack rooted at the trunk.
491+
//
492+
// Once all of a stack's remote PRs are merged — especially after their branches
493+
// are deleted upstream — you can no longer add to that stack: a new PR on top
494+
// would target the trunk, breaking the remote stack's "each PR's base ref ==
495+
// previous PR's head ref" chain. Rather than failing the stack update on GitHub,
496+
// we lift the survivors into a new local stack with no remote ID so the
497+
// subsequent submit creates a fresh stack on GitHub. The original (fully merged)
498+
// remote stack is left untouched on GitHub.
499+
//
500+
// It returns the stack submit should continue with: the new forked stack when a
501+
// fork happens, or the original stack otherwise.
502+
func maybeForkFromMergedBase(cfg *config.Config, client github.ClientOps, sf *stack.StackFile, s *stack.Stack, gitDir string) *stack.Stack {
503+
// Only meaningful when there is a tracked remote stack to evaluate. A fork
504+
// can only happen if every remote-stack PR is merged, which implies at least
505+
// one locally tracked branch is merged — checking that first avoids an extra
506+
// ListStacks call on the common path.
507+
if s.ID == "" || len(s.MergedBranches()) == 0 {
508+
return s
509+
}
510+
511+
remotePRs := remoteStackPRs(client, s.ID)
512+
if len(remotePRs) == 0 {
513+
return s
514+
}
515+
516+
// Every PR officially in the remote stack must be merged. Open PRs that are
517+
// not part of the remote stack do not count.
518+
merged := mergedPRNumbers(s)
519+
for _, n := range remotePRs {
520+
if !merged[n] {
521+
return s // a remote-stack PR is still open — not a fork situation
522+
}
523+
}
524+
525+
stackIdx := sf.IndexOfStack(s)
526+
if stackIdx < 0 {
527+
return s
528+
}
529+
530+
// Partition the local branches: those that are part of the merged remote
531+
// stack stay behind; everything else (new branches and open PRs that were
532+
// never part of the remote stack) is forked into a new stack.
533+
remoteSet := make(map[int]bool, len(remotePRs))
534+
for _, n := range remotePRs {
535+
remoteSet[n] = true
536+
}
537+
var keepBranches, forkBranches []stack.BranchRef
538+
for _, b := range s.Branches {
539+
if b.PullRequest != nil && remoteSet[b.PullRequest.Number] {
540+
keepBranches = append(keepBranches, b)
541+
} else {
542+
forkBranches = append(forkBranches, b)
543+
}
544+
}
545+
if len(forkBranches) == 0 {
546+
return s // nothing new to fork — the whole stack is merged and done
547+
}
548+
549+
// Capture trunk/prefix before mutating sf.Stacks (RemoveStack/AddStack can
550+
// reallocate the slice and invalidate the s pointer).
551+
trunk := s.Trunk
552+
prefix := s.Prefix
553+
numbered := s.Numbered
554+
555+
// The bottom surviving branch re-bases onto the trunk.
556+
if base, err := git.MergeBase(forkBranches[0].Branch, trunk.Branch); err == nil {
557+
forkBranches[0].Base = base
558+
}
559+
560+
cfg.Warningf("Every PR in this stack has already been merged on GitHub")
561+
cfg.Printf("Adding to a fully merged stack isn't supported — starting a new stack for your %d unmerged %s based on %s",
562+
len(forkBranches), plural(len(forkBranches), "branch", "branches"), cfg.ColorCyan(trunk.Branch))
563+
564+
// Decide the fate of the original (fully merged) stack: keep it as a record
565+
// only if at least one of its branches still exists locally; otherwise drop
566+
// it. The merged stack is left intact on GitHub either way.
567+
removeOld := true
568+
for _, b := range keepBranches {
569+
if git.BranchExists(b.Branch) {
570+
removeOld = false
571+
break
572+
}
573+
}
574+
575+
if removeOld {
576+
sf.RemoveStack(stackIdx)
577+
} else {
578+
sf.Stacks[stackIdx].Branches = keepBranches
579+
}
580+
581+
sf.AddStack(stack.Stack{
582+
Prefix: prefix,
583+
Numbered: numbered,
584+
Trunk: trunk,
585+
Branches: forkBranches,
586+
})
587+
588+
if err := stack.Save(gitDir, sf); err != nil {
589+
// Persisting the split failed, but the in-memory model is correct;
590+
// surface the error and continue so the PRs still get submitted.
591+
_ = handleSaveError(cfg, err)
592+
}
593+
594+
return &sf.Stacks[len(sf.Stacks)-1]
595+
}
596+
597+
// remoteStackPRs returns the PR numbers that are officially part of the remote
598+
// stack identified by stackID, or nil if it can't be determined.
599+
func remoteStackPRs(client github.ClientOps, stackID string) []int {
600+
stacks, err := client.ListStacks()
601+
if err != nil {
602+
return nil
603+
}
604+
for _, rs := range stacks {
605+
if strconv.Itoa(rs.ID) == stackID {
606+
return rs.PullRequests
607+
}
608+
}
609+
return nil
610+
}
611+
612+
// mergedPRNumbers returns the set of PR numbers whose local branch is marked
613+
// merged. Call after syncStackPRs so the merge flags reflect the remote state.
614+
func mergedPRNumbers(s *stack.Stack) map[int]bool {
615+
merged := make(map[int]bool)
616+
for i := range s.Branches {
617+
b := &s.Branches[i]
618+
if b.IsMerged() && b.PullRequest != nil {
619+
merged[b.PullRequest.Number] = true
620+
}
621+
}
622+
return merged
623+
}
624+
480625
// handlePendingModify handles the stack recreation after a modify operation.
481626
// It deletes the old remote stack and clears s.ID so syncStack creates a new
482627
// one. The state file is NOT cleared here — it is cleared after syncStack
@@ -665,6 +810,17 @@ func updateStack(cfg *config.Config, client github.ClientOps, s *stack.Stack, pr
665810
// immediately try to re-create it.
666811
s.ID = ""
667812
createNewStack(cfg, client, s, prNumbers)
813+
case 422:
814+
// A merged branch whose ref has been deleted upstream breaks the
815+
// stack's base→head chain, so the update is rejected. This is
816+
// expected once part of the stack has landed; the unmerged PRs
817+
// were still pushed and re-based, so explain it calmly rather
818+
// than alarming the user with a raw API error.
819+
if strings.Contains(httpErr.Message, "must form a stack") && len(s.MergedBranches()) > 0 {
820+
cfg.Infof("Merged PRs have left the stack on GitHub, so it wasn't updated — your unmerged PRs were pushed and re-based onto the trunk")
821+
return
822+
}
823+
cfg.Warningf("Failed to update stack on GitHub: %s", httpErr.Message)
668824
default:
669825
cfg.Warningf("Failed to update stack on GitHub: %s", httpErr.Message)
670826
}

0 commit comments

Comments
 (0)