Files
github__gh-stack/cmd/modify.go
T
Sameen Karim 754d190490 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.
2026-06-29 20:11:09 -04:00

379 lines
10 KiB
Go

package cmd
import (
"errors"
"fmt"
"strings"
tea "github.com/charmbracelet/bubbletea"
"github.com/github/gh-stack/internal/config"
"github.com/github/gh-stack/internal/git"
"github.com/github/gh-stack/internal/modify"
"github.com/github/gh-stack/internal/tui/modifyview"
"github.com/github/gh-stack/internal/tui/stackview"
"github.com/spf13/cobra"
)
type modifyOptions struct {
abort bool
cont bool
}
func ModifyCmd(cfg *config.Config) *cobra.Command {
opts := &modifyOptions{}
cmd := &cobra.Command{
Use: "modify",
Short: "Interactively restructure a stack",
Long: `Open an interactive TUI to restructure the current stack.
Operations available:
• Drop branches from the stack
• Fold branches into adjacent branches
• Insert new branches into the stack
• Reorder branches
• Rename branches
All changes are staged in the TUI and applied together when you press Ctrl+S.
If your changes affect branches with pull requests, run 'gh stack submit'
afterward to push changes, update PRs, and recreate the stack on GitHub.`,
Example: ` # Open the interactive TUI to restructure the stack
$ gh stack modify
# Abort a modify session and restore the stack
$ gh stack modify --abort
# Continue after resolving conflicts from a modify
$ gh stack modify --continue`,
RunE: func(cmd *cobra.Command, args []string) error {
if opts.abort {
return runModifyAbort(cfg)
}
if opts.cont {
return runModifyContinue(cfg)
}
return runModify(cfg)
},
}
cmd.Flags().BoolVar(&opts.abort, "abort", false, "Abort the modify session and restore the stack to its pre-modify state")
cmd.Flags().BoolVar(&opts.cont, "continue", false, "Continue after resolving conflicts")
return cmd
}
func runModify(cfg *config.Config) error {
// Run all precondition checks
result, err := checkModifyPreconditions(cfg)
if err != nil {
return err
}
gitDir := result.GitDir
sf := result.StackFile
s := result.Stack
currentBranch := result.CurrentBranch
// A fully merged stack has nothing left to restructure. Short-circuit
// before opening the TUI, mirroring submit's "nothing to submit" behavior.
if s.IsFullyMerged() {
cfg.Warningf("All branches in this stack have been merged")
cfg.Printf("There's nothing to modify — start a new stack with `%s`", cfg.ColorCyan("gh stack init"))
return nil
}
// Load branch data for the TUI
viewNodes := stackview.LoadBranchNodes(cfg, s, currentBranch, result.PRDetails)
// Reverse so index 0 = top of stack (matching visual order)
reversed := make([]stackview.BranchNode, len(viewNodes))
for i, n := range viewNodes {
reversed[len(viewNodes)-1-i] = n
}
// Convert to ModifyBranchNodes
modifyNodes := make([]modifyview.ModifyBranchNode, len(reversed))
for i, n := range reversed {
modifyNodes[i] = modifyview.ModifyBranchNode{
BranchNode: n,
OriginalPosition: i,
}
}
// Run the TUI
model := modifyview.New(modifyNodes, s.Trunk, Version)
p := tea.NewProgram(
model,
tea.WithAltScreen(),
tea.WithMouseAllMotion(),
)
finalModel, err := p.Run()
if err != nil {
return fmt.Errorf("running TUI: %w", err)
}
m, ok := finalModel.(modifyview.Model)
if !ok {
return fmt.Errorf("unexpected model type")
}
// Handle TUI result
if m.Cancelled() {
return nil
}
if !m.ApplyRequested() {
return nil
}
// Apply the staged changes
// Re-reverse nodes back to stack order (bottom to top) for the apply engine
applyNodes := m.Nodes()
reordered := make([]modifyview.ModifyBranchNode, len(applyNodes))
for i, n := range applyNodes {
reordered[len(applyNodes)-1-i] = n
}
applyResult, conflict, applyErr := modify.ApplyPlan(cfg, gitDir, s, sf, reordered, currentBranch, updateBaseSHAs)
if conflict != nil {
isCherryPick := applyErr != nil && strings.Contains(applyErr.Error(), "cherry-pick")
if isCherryPick {
cfg.Warningf("Cherry-pick conflict folding %s", conflict.Branch)
} else {
cfg.Warningf("Rebasing %s — conflict", conflict.Branch)
}
printConflictDetailsWithContinue(cfg, conflict.Branch, "gh stack modify --continue")
cfg.Printf("")
cfg.Printf("Or restore the stack to its pre-modify state with `%s`",
cfg.ColorCyan("gh stack modify --abort"))
return ErrConflict
}
if applyErr != nil {
cfg.Errorf("failed to apply modifications: %s", applyErr)
return ErrSilent
}
// Print success summary
printModifySuccess(cfg, applyResult)
return nil
}
// printModifySuccess prints a summary of what was applied.
func printModifySuccess(cfg *config.Config, result *modifyview.ApplyResult) {
if result == nil {
return
}
cfg.Printf("")
cfg.Successf("Stack modified successfully")
for _, r := range result.RenamedBranches {
cfg.Printf(" Renamed: %s → %s", r.OldName, r.NewName)
}
for _, name := range result.InsertedBranches {
cfg.Printf(" Inserted: %s", name)
}
for _, d := range result.DroppedPRs {
cfg.Printf(" Dropped: %s (PR #%d remains open — close with `%s`)",
d.Branch, d.PRNumber, cfg.ColorCyan(fmt.Sprintf("gh pr close %d", d.PRNumber)))
}
if result.MovedBranches > 0 {
cfg.Printf(" Rebased %d %s", result.MovedBranches,
plural(result.MovedBranches, "branch", "branches"))
}
cfg.Printf("")
if result.NeedsSubmit {
cfg.Printf("Run `%s` to push your changes and update the stack of PRs on GitHub",
cfg.ColorCyan("gh stack submit"))
}
}
// runModifyAbort handles recovery to a pre-modify state.
func runModifyAbort(cfg *config.Config) error {
gitDir, err := git.GitDir()
if err != nil {
cfg.Errorf("not a git repository")
return ErrNotInStack
}
state, err := modify.LoadState(gitDir)
if err != nil {
cfg.Errorf("failed to read modify state: %s", err)
return ErrSilent
}
if state == nil {
cfg.Printf("No modify session to abort")
return nil
}
switch state.Phase {
case modify.PhaseApplying:
cfg.Printf("A modify session was interrupted during the apply phase")
cfg.Printf("Restoring stack to pre-modify state...")
if err := modify.UnwindFromStateFile(cfg, gitDir); err != nil {
cfg.Errorf("recovery failed: %s", err)
cfg.Printf("The stack may be in an inconsistent state.")
cfg.Printf("Try `%s` to fix, or `%s` + `%s` to recreate.",
cfg.ColorCyan("gh stack rebase"), cfg.ColorCyan("gh stack unstack --local"),
cfg.ColorCyan("gh stack init --adopt"))
return ErrSilent
}
cfg.Successf("Stack restored successfully")
return nil
case modify.PhasePendingSubmit:
cfg.Printf("A modify completed but the stack has not been submitted")
cfg.Printf("Run `%s` to push changes and recreate the stack on GitHub",
cfg.ColorCyan("gh stack submit"))
return nil
default:
cfg.Errorf("unexpected modify state phase: %s", state.Phase)
cfg.Printf("Clearing invalid state file...")
modify.ClearState(gitDir)
return nil
}
}
// runModifyContinue continues applying after the user resolves a rebase conflict.
func runModifyContinue(cfg *config.Config) error {
gitDir, err := git.GitDir()
if err != nil {
cfg.Errorf("not a git repository")
return ErrNotInStack
}
if err := modify.ContinueApply(cfg, gitDir, updateBaseSHAs); err != nil {
cfg.Errorf("%s", err)
return ErrConflict
}
return nil
}
// ---------------------------------------------------------------------------
// Preconditions
// ---------------------------------------------------------------------------
// checkModifyPreconditions runs all precondition checks for the modify command.
func checkModifyPreconditions(cfg *config.Config) (*loadStackResult, error) {
if !cfg.IsInteractive() {
cfg.Errorf("modify requires an interactive terminal")
return nil, ErrSilent
}
result, err := loadStack(cfg, "")
if err != nil {
return nil, ErrNotInStack
}
gitDir := result.GitDir
s := result.Stack
// No existing modify state file
if err := checkNoModifyInProgress(cfg, gitDir); err != nil {
return nil, err
}
// No rebase in progress
if git.IsRebaseInProgress() {
cfg.Errorf("a rebase is currently in progress")
cfg.Printf("Complete the rebase with `%s` or abort with `%s`",
cfg.ColorCyan("gh stack rebase --continue"),
cfg.ColorCyan("gh stack rebase --abort"))
return nil, ErrRebaseActive
}
// Clean working tree
if dirty, err := git.HasUncommittedChanges(); err != nil {
cfg.Errorf("failed to check working tree status: %s", err)
return nil, ErrSilent
} else if dirty {
cfg.Errorf("uncommitted changes in working tree")
cfg.Printf("Commit or stash your changes before running modify")
return nil, ErrSilent
}
// Ensure trunk branch exists locally (it may be absent if the user
// renamed their initial branch before starting the stack).
if !git.BranchExists(s.Trunk.Branch) {
remote, err := pickRemote(cfg, result.CurrentBranch, "")
if err != nil {
if !errors.Is(err, errInterrupt) {
cfg.Errorf("failed to resolve remote: %s", err)
}
return nil, ErrSilent
}
if err := ensureLocalTrunk(cfg, s.Trunk.Branch, remote); err != nil {
cfg.Errorf("%s", err)
return nil, ErrSilent
}
}
// Show loading indicator while syncing PRs
fmt.Fprintf(cfg.Err, "Loading stack...")
// Sync PR state and check merge queue
prDetails := syncStackPRs(cfg, s)
result.PRDetails = prDetails
fmt.Fprintf(cfg.Err, "\r\033[2K")
if err := modify.CheckNoMergeQueuePRs(cfg, s); err != nil {
return nil, ErrSilent
}
// Stack linearity check
if err := modify.CheckStackLinearity(cfg, s); err != nil {
return nil, ErrSilent
}
return result, nil
}
// checkNoModifyInProgress checks if a modify state file already exists.
func checkNoModifyInProgress(cfg *config.Config, gitDir string) error {
state, err := modify.LoadState(gitDir)
if err != nil {
cfg.Warningf("failed to read modify state: %v", err)
return nil
}
if state == nil {
return nil
}
switch state.Phase {
case modify.PhaseApplying:
cfg.Errorf("a previous modify session was interrupted")
cfg.Printf("Run `%s` to restore your stack",
cfg.ColorCyan("gh stack modify --abort"))
return ErrModifyRecovery
case modify.PhaseConflict:
cfg.Errorf("a modify has unresolved conflicts")
cfg.Printf("Run `%s` to continue, or `%s` to restore your stack",
cfg.ColorCyan("gh stack modify --continue"),
cfg.ColorCyan("gh stack modify --abort"))
return ErrSilent
case modify.PhasePendingSubmit:
cfg.Errorf("a modify was completed but the stack has not been submitted yet")
cfg.Printf("Run `%s` to push changes and recreate the stack on GitHub",
cfg.ColorCyan("gh stack submit"))
return ErrSilent
default:
cfg.Errorf("unexpected modify state phase: %s", state.Phase)
return ErrSilent
}
}