diff --git a/README.md b/README.md index a025aab..4746005 100644 --- a/README.md +++ b/README.md @@ -109,10 +109,10 @@ merging the PRs on GitHub. gh stack can only add branches at the top of a stack, so `git create` can't start one in the middle either; create it on top and `git reorder` it into place. Reordering a stack, moving -branches onto another stack and adopting branches you made by hand are `git stack reorder`, -`git stack move` and `git stack init`. The one thing left that needs gh stack directly (and then -`git restack`) is folding a branch into another, or dropping one from the middle of a stack: -`gh stack modify`. +branches onto another stack, folding a branch into the one below it and adopting branches you made +by hand are `git stack reorder`, `git stack move`, `git stack fold` and `git stack init`. The one +thing left that needs gh stack directly (and then `git restack`) is dropping a branch from the +middle of a stack: `gh stack modify`. ## Install @@ -146,7 +146,7 @@ Claude Code and Codex if you have them installed**. Each of those is on by defau change before it happens, and `--uninstall` removes what it recorded installing and nothing else (that record lives in `git config --global stack.managed*`). -- **Aliases** (`--aliases`): `git create|modify|absorb|reorder|move|track|restack|continue|abort|submit|sync|up|down|top|bottom` +- **Aliases** (`--aliases`): `git create|modify|absorb|reorder|move|fold|track|restack|continue|abort|submit|sync|up|down|top|bottom` and the short ones `git c|m|ab|rs|ss|u|d|t|b|co`, written with `git config --global`. Anything that clashes with a git builtin or an installed command is skipped, and an alias you already have is only replaced if you pass `--force` or say yes when asked. @@ -387,6 +387,33 @@ Flags: - `-s` / `--source ` move this branch instead of the current one - `--only` move just the branch and leave the ones above it behind +### `git stack fold` + +> Alias: `git fold` + +Merges the current branch into the branch below it, for when two PRs should really have been one. +The parent keeps its name and PR and ends up with both sets of commits; the current branch is +deleted and you're left on the parent, with any uncommitted changes carried along. With `--keep` +it's the other way round: the current branch keeps its name and PR and takes the parent's place in +the stack, and the parent is deleted. + +Nothing is rebased, since the branches above already have every commit involved and only their +parent changes, so the branch has to be sitting on its parent first (`git restack` if it isn't). +It won't fold the bottom branch of a stack into trunk, and it stops before anything changes when +the parent is checked out in another worktree. The PR of the branch that went is closed at the next +`git ss`, with a comment pointing at the one it went into, and its branch on GitHub is deleted once +nothing is based on it any more. + +``` +# on billing-webhook-alerts +❯ git fold +✔ Folded billing-webhook-alerts into billing-webhook-schema + #418 (billing-webhook-alerts) will be closed at the next submit +``` + +Flags: +- `--keep` keep this branch's name and pull request instead of the parent's + ### `git stack init [branch...]` > Alias: `git track`; `git stack track` works too diff --git a/cmd/fold.go b/cmd/fold.go new file mode 100644 index 0000000..3a0a54a --- /dev/null +++ b/cmd/fold.go @@ -0,0 +1,52 @@ +package cmd + +import ( + "github.com/spf13/cobra" + + "github.com/DomBlack/git-stack/pkg/app" +) + +func newFoldCmd(c *cli) *cobra.Command { + var keep bool + cmd := &cobra.Command{ + Use: "fold", + Short: "Fold the current branch into the branch below it", + Long: `Merges the current branch into its parent, so the two become one branch with +one pull request and both sets of commits. The parent keeps its name and pull +request and you're left on it; with --keep it's the other way round, the +current branch takes the parent's place and the parent is deleted. + +The current branch has to sit on its parent (run "git stack restack" first if +it doesn't). Nothing is rebased: the branches above already have every commit +involved, only their parent changes. Local changes come along. The pull request +of the branch that went is closed at the next submit, with a comment saying +where it went.`, + Example: ` # fold this branch into the one below it + git fold + + # keep this branch's name and pull request instead + git fold --keep`, + Args: cobra.NoArgs, + ValidArgsFunction: completeNothing, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + a, repo, err := c.app(ctx) + if err != nil { + return err + } + rep := c.report() + res, err := a.Fold(ctx, repo, app.FoldOptions{Keep: keep}) + renderNotices(rep, res.Notices) + if err != nil { + return err + } + rep.Success("Folded %s into %s", res.Folded, res.Into) + if res.Closing != 0 { + rep.Info("#%d (%s) will be closed at the next submit", res.Closing, res.Folded) + } + return nil + }, + } + cmd.Flags().BoolVar(&keep, "keep", false, "keep this branch's name and pull request instead of the parent's") + return cmd +} diff --git a/cmd/fold_test.go b/cmd/fold_test.go new file mode 100644 index 0000000..5662947 --- /dev/null +++ b/cmd/fold_test.go @@ -0,0 +1,26 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/git/gittest" +) + +func TestFoldCommand(t *testing.T) { + dir, run := stackOf3(t) + out, errOut, err := run("fold") + if err != nil || !strings.Contains(out, "ok: Folded c into b") { + t.Errorf("fold: %q %q %v", out, errOut, err) + } + if cur := gittest.Run(t, dir, "branch", "--show-current"); cur != "b" { + t.Errorf("current = %s", cur) + } + out, errOut, err = run("fold", "--keep") + if err != nil || !strings.Contains(out, "ok: Folded a into b") { + t.Errorf("fold --keep: %q %q %v", out, errOut, err) + } + if _, _, err = run("fold"); err == nil || !strings.Contains(err.Error(), "bottom of its stack") { + t.Errorf("bottom: %v", err) + } +} diff --git a/cmd/root.go b/cmd/root.go index 921447c..76af1c0 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -324,6 +324,7 @@ func newRootCmd(c *cli) *cobra.Command { newAbsorbCmd(c), newReorderCmd(c), newMoveCmd(c), + newFoldCmd(c), newInitCmd(c), newRestackCmd(c), newContinueCmd(c), diff --git a/cmd/submit.go b/cmd/submit.go index f53339a..fac82b5 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/forge" ) func newSubmitCmd(c *cli) *cobra.Command { @@ -116,6 +117,13 @@ really do want them gone.`, rep.Info("%s", rep.Branch(pr.Branch)) } } + for _, c := range res.Closed { + gone := ", and deleted its branch" + if !c.BranchDeleted { + gone = "" + } + rep.Info("%s %s closed%s", rep.Branch(c.Branch), rep.Ref(c.Number, "", forge.StateClosed), gone) + } for _, n := range res.Notices { rep.Warn("%s", n) } diff --git a/docs/architecture.md b/docs/architecture.md index b8345f8..08bbb8c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -445,6 +445,19 @@ already gone counts as done, and a failure to dissolve keeps the entry and says Keeping the two apart also means the reshape itself works offline. PRs that are queued for merge or have auto merge on stay stacked on GitHub's side, so creating the new stack fails with gh stack's usual warning until they land. +**Fold is a reshape without a restack.** Folding C into its parent P only works when C already +sits on P, so after it every branch above still has every commit involved and only its recorded +parent changes. The reshape step does the metadata (P's head becomes C's tip and C is dropped, or +with `--keep` P is dropped and C takes its base) and the dissolve as usual, with no segments to +run. Then P's ref is fast forwarded to C's tip and switched to, which leaves the working tree alone +because they're the same commit, and C is deleted. P can't be checked out anywhere else, since its +ref moves (or is deleted) under that checkout. A failure before the switch puts the ref and the +snapshot back. The PR of the branch that went goes on a second list in the same queue file, closed +at the next submit with a comment saying where it went. That happens after the backend submit, +because deleting a branch on GitHub closes every open PR based on it, and it's the backend submit +that moves the PR above onto the surviving branch; a remote branch something is still based on is +kept and its delete stays queued. + **Merged branches are deleted by default.** `stack.sync.prune` decides: `always` (default) deletes branches whose PRs merged or closed, `ask` prompts on a terminal and keeps with a notice otherwise, `never` keeps. `-d` and `-f` delete regardless. A branch checked out in diff --git a/docs/mcp.md b/docs/mcp.md index 9f45efe..74db1ed 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -25,6 +25,7 @@ path is resolved with `git rev-parse`; anything outside a repository returns `no | `stack_absorb` | Fold each staged hunk into the commit of the current stack that last changed those lines, rewrite it and everything above, then restack what sits above the current branch; the result's `plan` lists every hunk's destination and what stays staged | `staging` = `all`/`update`/`none`, `dry_run` | destructive | | `stack_reorder` | Change the order of the current stack's branches and restack them, checking nothing out; the next `stack_submit` dissolves the stale stack on GitHub and creates it again in the new order; a branch checked out with changes in another worktree refuses the call with nothing changed | `order` (required: every branch of the stack once, bottom to top) | destructive | | `stack_move` | Move a branch, and unless `only` is set everything above it, onto the top of another stack or onto a trunk as a new stack, then restack; moves within one stack are `stack_reorder`'s job | `onto` (required: a trunk or another stack's top branch), `source` (default current), `only` | destructive | +| `stack_fold` | Merge the current branch into the branch below it, one branch and one PR; the parent survives and is checked out, or with `keep` the current branch takes the parent's place; nothing is rebased, and the next `stack_submit` closes the PR of the branch that went | `keep` | destructive | | `stack_init` | Track branches that already exist as a new stack on a trunk; only the metadata changes, and `needsRestack` lists the branches that aren't on their parent | `branches` (bottom to top, default current), `base` | destructive (it writes the metadata, though no branch moves) | | `stack_continue` | Finish an interrupted restack, modify, reorder or move once the conflicts are resolved and git added | `stage_all` | destructive | | `stack_abort` | Give up an interrupted restack and put every moved branch back (and, after a reorder or move, the old stack order) | | destructive | diff --git a/pkg/app/fold.go b/pkg/app/fold.go new file mode 100644 index 0000000..5f11550 --- /dev/null +++ b/pkg/app/fold.go @@ -0,0 +1,215 @@ +package app + +import ( + "context" + "errors" + "fmt" + "slices" + + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// FoldOptions is one fold of the current branch into its parent. +type FoldOptions struct { + // Keep keeps the current branch's name and pull request instead of + // the parent's. + Keep bool +} + +// FoldResult reports what happened. +type FoldResult struct { + // Into is the branch that survives, Folded the one that's gone. + Into string `json:"into"` + Folded string `json:"folded"` + // Closing is the pull request queued to close at the next submit, 0 if + // none. + Closing int `json:"closing,omitempty"` + Notices []string `json:"notices,omitempty"` +} + +const foldCommand = "git stack fold" + +// Fold merges the current branch and its parent into one branch, the +// parent's (or with Keep the current one's), and queues the other's pull +// request to be closed at the next submit. The current branch must sit on +// its parent, so nothing needs restacking: the branches above already have +// every commit involved and only their recorded parent changes. +func (a *App) Fold(ctx context.Context, repo git.Repo, o FoldOptions) (FoldResult, error) { + if err := a.noRebaseActive(ctx, repo); err != nil { + return FoldResult{}, err + } + current, err := a.d.Git.CurrentBranch(ctx, repo) + if err != nil { + if errors.Is(err, git.ErrDetached) { + return FoldResult{}, stack.New(stack.KindInvalidArgs, "HEAD is detached; check out a stacked branch first") + } + return FoldResult{}, err + } + graph, err := a.d.Meta.Load(ctx, repo) + if err != nil { + return FoldResult{}, err + } + s, idx, ok := graph.StackOf(current) + if !ok { + if graph.IsTrunk(current) { + return FoldResult{}, stack.Newf(stack.KindInvalidArgs, "%s is a trunk; fold works on stacked branches", current) + } + return FoldResult{}, stack.Newf(stack.KindNotInStack, "%s is not in a stack", current). + WithSteps("git stack init " + current + " to start tracking it") + } + if idx == 0 { + return FoldResult{}, stack.Newf(stack.KindInvalidArgs, "%s is the bottom of its stack; there's nothing to fold it into", current) + } + if err := noMergedBranches(s); err != nil { + return FoldResult{}, err + } + parent := s.Branches[idx-1] + local, err := a.localBranches(ctx, repo) + if err != nil { + return FoldResult{}, err + } + pl, ok := local[parent.Name] + if !ok { + return FoldResult{}, stack.Newf(stack.KindInvalidArgs, "%s, the branch below %s, no longer exists", parent.Name, current). + WithSteps("run git stack sync first") + } + cTip, pTip := local[current].Head, pl.Head + if !a.isAncestor(ctx, repo, pTip, cTip) || a.baseAbove(ctx, repo, s.Branches[idx].Base, pTip, cTip) { + return FoldResult{}, stack.Newf(stack.KindInvalidArgs, "%s isn't on %s yet", current, parent.Name). + WithSteps("run git stack restack first") + } + // P's ref moves (or with Keep is deleted), so it can't be checked out + // anywhere; C is checked out here, which the switch takes care of. + if pl.Worktree != "" { + return FoldResult{}, stack.Newf(stack.KindInvalidArgs, "%s is checked out in %s", parent.Name, shortPath(pl.Worktree)). + WithSteps("switch that checkout to another branch first") + } + res := FoldResult{Into: parent.Name, Folded: current} + if o.Keep { + res.Into, res.Folded = current, parent.Name + } + // With Keep, C takes P's base. P's own commits start there; without a + // recorded one, at the merge base with P's parent. + keepBase := parent.Base + if o.Keep && keepBase == "" { + below := s.Trunk + if idx > 1 { + below = s.Branches[idx-2].Name + } + if lb, ok := local[below]; ok { + if keepBase, err = a.d.Git.MergeBase(ctx, repo, pTip, lb.Head); err != nil { + return FoldResult{}, err + } + } + } + + var queued []int + if c := a.foldClose(ctx, repo, graph, res, local[res.Folded].Head); c != nil { + if err := addPendingClose(repo, []pendingClose{*c}); err != nil { + return res, err + } + queued = []int{c.Number} + } + bottom := s.Bottom() + snap := snapshotOf(graph, []string{bottom}) + e := reshapeEdit{ + touched: []string{bottom}, + relink: []string{bottom}, + edit: func(g *stack.Graph) error { + s, i, ok := g.StackOf(current) + if !ok || i == 0 || s.Branches[i-1].Name != parent.Name { + return stack.New(stack.KindInvalidArgs, "the stack changed while it was being folded; try again") + } + if o.Keep { + s.Branches[i].Base = keepBase + s.Branches = slices.Delete(s.Branches, i-1, i) + return nil + } + s.Branches[i-1].Head = cTip + s.Branches = slices.Delete(s.Branches, i, i+1) + return nil + }, + } + // undo puts the shape back and forgets the close, for a failure before + // the fold is visible. + undo := func(err error) error { + if derr := dropPendingClose(repo, queued); derr != nil { + err = errors.Join(err, derr) + } + if rerr := a.restoreReshape(ctx, repo, snap); rerr != nil { + return asStackError(err).WithSteps(fmt.Sprintf("the stack's new shape was kept because putting it back failed (%v)", rerr)) + } + return asStackError(err).WithSteps("nothing was changed") + } + if _, err := a.reshape(ctx, repo, foldCommand, "", e); err != nil { + if derr := dropPendingClose(repo, queued); derr != nil { + err = errors.Join(err, derr) + } + return res, err + } + if o.Keep { + if err := a.d.Git.DeleteBranch(ctx, repo, parent.Name); err != nil { + return res, undo(err) + } + } else { + ref := "refs/heads/" + parent.Name + if err := a.d.Git.UpdateRefs(ctx, repo, []git.RefUpdate{{Ref: ref, New: cTip, Old: pTip}}); err != nil { + return res, undo(err) + } + // P and C are the same commit now, so the switch leaves the working + // tree alone and carries any local changes along. + if err := a.d.Git.Switch(ctx, repo, parent.Name); err != nil { + if rerr := a.d.Git.UpdateRefs(ctx, repo, []git.RefUpdate{{Ref: ref, New: pTip, Old: cTip}}); rerr != nil { + return res, asStackError(errors.Join(err, rerr)).WithSteps(fmt.Sprintf("%s was moved to %s's tip and could not be put back", parent.Name, current)) + } + return res, undo(err) + } + if err := a.d.Git.DeleteBranch(ctx, repo, current); err != nil { + res.Notices = append(res.Notices, fmt.Sprintf("%s could not be deleted (%v); git branch -D %s removes it", current, err, current)) + } + } + if len(queued) > 0 { + res.Closing = queued[0] + } + return res, nil +} + +// foldClose is the close to queue for the folded branch's open pull +// request, or nil when it has none. tip is the folded branch's local tip. +func (a *App) foldClose(ctx context.Context, repo git.Repo, graph *stack.Graph, res FoldResult, tip string) *pendingClose { + prs, _ := a.loadPRs(ctx, repo, PRsCached) + byHead := PRsFor(prs) + open := func(name string) int { + if pr, ok := byHead[name]; ok { + if pr.State == forge.StateOpen || pr.State == forge.StateDraft { + return pr.Number + } + return 0 + } + // Not in the cache yet (submitted since the last refresh); gh + // stack's own record has it. + if s, i, ok := graph.StackOf(name); ok { + if pr := s.Branches[i].PR; pr != nil && !pr.Merged { + return pr.Number + } + } + return 0 + } + n := open(res.Folded) + if n == 0 { + return nil + } + comment := "Folded into " + res.Into + if into := open(res.Into); into != 0 { + comment = fmt.Sprintf("Folded into #%d (%s)", into, res.Into) + } + // The lease for the remote delete is the tip last pushed, as far as the + // tracking ref knows; else the local tip. + remote := a.d.Git.RemoteFor(ctx, repo, res.Folded) + if pushed, err := a.d.Git.RevParse(ctx, repo, "refs/remotes/"+remote+"/"+res.Folded); err == nil { + tip = pushed + } + return &pendingClose{Number: n, Branch: res.Folded, Comment: comment, Tip: tip} +} diff --git a/pkg/app/fold_test.go b/pkg/app/fold_test.go new file mode 100644 index 0000000..e037651 --- /dev/null +++ b/pkg/app/fold_test.go @@ -0,0 +1,242 @@ +package app_test + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/cache" + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/git/gittest" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// withPRs gives the fixture a PR cache holding prs. +func withPRs(t *testing.T, f *restackFixture, prs ...forge.PullRequest) { + t.Helper() + c := cache.New(f.repo) + if err := cache.Write(c, "prs", app.PRSnapshot{PRs: prs}); err != nil { + t.Fatal(err) + } + f.app = app.New(app.Deps{Git: f.fb.git, Meta: f.fb, Tracker: f.fb, Cache: c}) +} + +func pendingQueue(t *testing.T, f *restackFixture) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(f.repo.CommonDir, "git-stack", "remote-stacks.json")) + if os.IsNotExist(err) { + return "" + } + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func TestFoldTopBranchIntoItsParent(t *testing.T) { + f := newRestackFixture(t) // main -> a -> b -> c, HEAD on c + tip := f.rev(t, "c") + res, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}) + if err != nil { + t.Fatal(err) + } + if res.Into != "b" || res.Folded != "c" || res.Closing != 0 { + t.Errorf("result = %+v", res) + } + if f.rev(t, "b") != tip || branchExists(t, f.dir, "c") { + t.Error("b should be at c's old tip and c gone") + } + if got := stackNames(f); !slices.EqualFunc(got, [][]string{{"a", "b"}}, slices.Equal) { + t.Errorf("stacks = %v", got) + } + if cur := gittest.Run(t, f.dir, "branch", "--show-current"); cur != "b" { + t.Errorf("current = %s", cur) + } + if st := gittest.Run(t, f.dir, "status", "--porcelain"); st != "" { + t.Errorf("working tree should be clean: %q", st) + } + if !f.has(t, "b", "c.txt") { + t.Error("b should have c's file") + } + if f.fb.graph.Stacks[0].Branches[1].Head != tip { + t.Errorf("b's recorded head = %s", f.fb.graph.Stacks[0].Branches[1].Head) + } + if q := pendingQueue(t, f); q != "" { + t.Errorf("nothing should be queued without a pull request: %s", q) + } +} + +// Folding a middle branch leaves the one above where it is: it already has +// every commit, only its parent changes. +func TestFoldMiddleBranchKeepsTheBranchAbove(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "b") + bTip, cTip, cBase := f.rev(t, "b"), f.rev(t, "c"), f.base("c") + res, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}) + if err != nil { + t.Fatal(err) + } + if res.Into != "a" || res.Folded != "b" { + t.Errorf("result = %+v", res) + } + if got := stackNames(f); !slices.EqualFunc(got, [][]string{{"a", "c"}}, slices.Equal) { + t.Errorf("stacks = %v", got) + } + if f.rev(t, "a") != bTip || f.rev(t, "c") != cTip || f.base("c") != cBase { + t.Error("a should be at b's tip, c untouched with its base") + } + if cur := gittest.Run(t, f.dir, "branch", "--show-current"); cur != "a" { + t.Errorf("current = %s", cur) + } + for _, file := range []string{"a.txt", "b.txt", "c.txt"} { + if !f.has(t, "c", file) { + t.Errorf("c lost %s", file) + } + } +} + +func TestFoldKeepTakesTheParentsPlace(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "b") + bTip, aBase := f.rev(t, "b"), f.base("a") + res, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{Keep: true}) + if err != nil { + t.Fatal(err) + } + if res.Into != "b" || res.Folded != "a" { + t.Errorf("result = %+v", res) + } + if got := stackNames(f); !slices.EqualFunc(got, [][]string{{"b", "c"}}, slices.Equal) { + t.Errorf("stacks = %v", got) + } + if branchExists(t, f.dir, "a") || f.rev(t, "b") != bTip { + t.Error("a should be deleted and b unmoved") + } + if f.base("b") != aBase || aBase != f.rev(t, "main") { + t.Errorf("b's base = %s, want a's old base %s", f.base("b"), aBase) + } + if cur := gittest.Run(t, f.dir, "branch", "--show-current"); cur != "b" { + t.Errorf("current = %s", cur) + } +} + +func TestFoldCarriesUncommittedChanges(t *testing.T) { + f := newRestackFixture(t) + gittest.WriteFile(t, f.dir, "shared.txt", "edited") + if _, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(filepath.Join(f.dir, "shared.txt")) + if err != nil || string(data) != "edited" { + t.Errorf("shared.txt = %q %v", data, err) + } +} + +func TestFoldRefuses(t *testing.T) { + for _, tc := range []struct { + name string + setup func(t *testing.T, f *restackFixture) + keep bool + want string + }{ + {"bottom", func(t *testing.T, f *restackFixture) { gittest.Run(t, f.dir, "switch", "-q", "a") }, false, "a is the bottom of its stack"}, + {"trunk", func(t *testing.T, f *restackFixture) { gittest.Run(t, f.dir, "switch", "-q", "main") }, false, "main is a trunk"}, + {"not on parent", func(t *testing.T, f *restackFixture) { + f.amend(t, "b", "b2.txt") + }, false, "c isn't on b yet"}, + {"parent elsewhere", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "worktree", "add", "-q", filepath.Join(t.TempDir(), "wt"), "b") + }, false, "b is checked out in"}, + {"deleted elsewhere", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "worktree", "add", "-q", filepath.Join(t.TempDir(), "wt"), "b") + }, true, "b is checked out in"}, + {"merged", func(t *testing.T, f *restackFixture) { + f.fb.graph.Stacks[0].Branches[0].PR = &stack.PRRef{Number: 1, Merged: true} + }, false, "a has merged"}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newRestackFixture(t) + tc.setup(t, f) + before := stackNames(f) + heads := gittest.Run(t, f.dir, "for-each-ref", "refs/heads/") + _, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{Keep: tc.keep}) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want %q", err, tc.want) + } + if kindOf(err) != stack.KindInvalidArgs { + t.Errorf("kind = %v", kindOf(err)) + } + if got := stackNames(f); !slices.EqualFunc(got, before, slices.Equal) { + t.Errorf("stacks changed: %v", got) + } + if gittest.Run(t, f.dir, "for-each-ref", "refs/heads/") != heads { + t.Error("refs changed") + } + }) + } +} + +func TestFoldUntrackedBranchIsNotInStack(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "-c", "loose") + _, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}) + if kindOf(err) != stack.KindNotInStack { + t.Errorf("err = %v", err) + } +} + +func TestFoldQueuesThePullRequestToClose(t *testing.T) { + f := newRestackFixture(t) + withPRs(t, f, + forge.PullRequest{Number: 11, Head: "b", Base: "a", State: forge.StateOpen}, + forge.PullRequest{Number: 12, Head: "c", Base: "b", State: forge.StateOpen}, + ) + tip := f.rev(t, "c") + res, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}) + if err != nil { + t.Fatal(err) + } + if res.Closing != 12 { + t.Errorf("closing = %d", res.Closing) + } + q := pendingQueue(t, f) + for _, want := range []string{`"number":12`, `"branch":"c"`, `"comment":"Folded into #11 (b)"`, `"tip":"` + tip + `"`} { + if !strings.Contains(q, want) { + t.Errorf("queue %s lacks %s", q, want) + } + } +} + +func TestFoldKeepQueuesTheParentsPullRequest(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "b") + withPRs(t, f, forge.PullRequest{Number: 10, Head: "a", Base: "main", State: forge.StateOpen}) + res, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{Keep: true}) + if err != nil { + t.Fatal(err) + } + if res.Closing != 10 { + t.Errorf("closing = %d", res.Closing) + } + if q := pendingQueue(t, f); !strings.Contains(q, `"number":10`) || !strings.Contains(q, `"comment":"Folded into b"`) { + t.Errorf("queue = %s", q) + } +} + +func TestFoldDissolvesTheRemoteStackOnce(t *testing.T) { + f := newRestackFixture(t) + f.fb.graph.Stacks[0].ID, f.fb.graph.Stacks[0].Number = "S1", 1 + if _, err := f.app.Fold(context.Background(), f.repo, app.FoldOptions{}); err != nil { + t.Fatal(err) + } + if f.fb.graph.Stacks[0].ID != "" || f.fb.graph.Stacks[0].Number != 0 { + t.Errorf("id = %q %d", f.fb.graph.Stacks[0].ID, f.fb.graph.Stacks[0].Number) + } + if q := pendingQueue(t, f); strings.Count(q, `"id":"S1"`) != 1 { + t.Errorf("queue = %s", q) + } +} diff --git a/pkg/app/prs_test.go b/pkg/app/prs_test.go index 3c1b6f5..3605040 100644 --- a/pkg/app/prs_test.go +++ b/pkg/app/prs_test.go @@ -39,6 +39,7 @@ func (f *fakeForge) Checks(context.Context, git.Repo, []int) (map[int]forge.Chec func (f *fakeForge) CreateStack(context.Context, git.Repo, []int) (forge.StackRef, error) { return forge.StackRef{}, nil } +func (f *fakeForge) ClosePR(context.Context, git.Repo, int, string) error { return nil } func TestPRsForPrefersOpenThenMergedThenRecent(t *testing.T) { t1 := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) diff --git a/pkg/app/remote_stacks.go b/pkg/app/remote_stacks.go index 66e21eb..4354aa8 100644 --- a/pkg/app/remote_stacks.go +++ b/pkg/app/remote_stacks.go @@ -20,11 +20,28 @@ type remoteStack struct { Stack string `json:"stack"` } -// pendingRemote lists the stacks on GitHub that a reshape made stale: their -// order or members changed and the stacks API can only create or append, -// so the next submit dissolves them before gh stack creates them again. +// pendingClose is a pull request a pop or fold made redundant: the next +// submit closes it with Comment, then deletes its remote branch. +type pendingClose struct { + Number int `json:"number"` + Branch string `json:"branch"` + Comment string `json:"comment"` + // Tip is the branch's tip when it was queued, the lease for deleting + // the remote branch; empty deletes it without one. + Tip string `json:"tip,omitempty"` + // Closed is set once the pull request is closed and only the delete is + // left. + Closed bool `json:"closed,omitempty"` +} + +// pendingRemote is what a reshape left for the next submit to do on +// GitHub. Dissolve lists stacks made stale: their order or members changed +// and the stacks API can only create or append, so they are dissolved +// before gh stack creates them again. Close lists pull requests whose +// branch a pop or fold took away. type pendingRemote struct { - Dissolve []remoteStack `json:"dissolve"` + Dissolve []remoteStack `json:"dissolve"` + Close []pendingClose `json:"close,omitempty"` } // remoteStacksPath is /git-stack/remote-stacks.json; it is @@ -53,7 +70,7 @@ func loadPendingRemote(repo git.Repo) (*pendingRemote, error) { // nothing is left. func savePendingRemote(repo git.Repo, p *pendingRemote) error { path := remoteStacksPath(repo) - if p == nil || len(p.Dissolve) == 0 { + if p == nil || (len(p.Dissolve) == 0 && len(p.Close) == 0) { err := os.Remove(path) if errors.Is(err, fs.ErrNotExist) { return nil @@ -100,3 +117,30 @@ func dropPendingRemote(repo git.Repo, ids []string) error { p.Dissolve = slices.DeleteFunc(p.Dissolve, func(r remoteStack) bool { return slices.Contains(ids, r.ID) }) return savePendingRemote(repo, p) } + +// addPendingClose records pull requests to close, once each by number. +func addPendingClose(repo git.Repo, closes []pendingClose) error { + p, err := loadPendingRemote(repo) + if err != nil { + return err + } + if p == nil { + p = &pendingRemote{} + } + for _, c := range closes { + if !slices.ContainsFunc(p.Close, func(q pendingClose) bool { return q.Number == c.Number }) { + p.Close = append(p.Close, c) + } + } + return savePendingRemote(repo, p) +} + +// dropPendingClose forgets the pull requests with the given numbers. +func dropPendingClose(repo git.Repo, numbers []int) error { + p, err := loadPendingRemote(repo) + if err != nil || p == nil { + return err + } + p.Close = slices.DeleteFunc(p.Close, func(c pendingClose) bool { return slices.Contains(numbers, c.Number) }) + return savePendingRemote(repo, p) +} diff --git a/pkg/app/remote_stacks_test.go b/pkg/app/remote_stacks_test.go index 10241af..c0ae797 100644 --- a/pkg/app/remote_stacks_test.go +++ b/pkg/app/remote_stacks_test.go @@ -36,3 +36,34 @@ func TestPendingRemoteStacksRoundTrip(t *testing.T) { t.Error("an empty list should remove the file") } } + +func TestPendingClosesRoundTrip(t *testing.T) { + repo := git.Repo{CommonDir: t.TempDir()} + if err := addPendingClose(repo, []pendingClose{{Number: 12, Branch: "c", Comment: "Folded into #11 (b)", Tip: "abc"}}); err != nil { + t.Fatal(err) + } + if err := addPendingClose(repo, []pendingClose{{Number: 13, Branch: "d"}, {Number: 12, Branch: "c"}}); err != nil { + t.Fatal(err) + } + p, err := loadPendingRemote(repo) + if err != nil || p == nil || len(p.Close) != 2 || p.Close[0].Comment != "Folded into #11 (b)" || p.Close[0].Tip != "abc" { + t.Fatalf("closes = %+v, %v", p, err) + } + // A stack to dissolve keeps the file when the closes are gone, and the + // file goes once both lists are empty. + if err := addPendingRemote(repo, []remoteStack{{ID: "S1", Number: 7, Stack: "a"}}); err != nil { + t.Fatal(err) + } + if err := dropPendingClose(repo, []int{12, 13}); err != nil { + t.Fatal(err) + } + if p, err := loadPendingRemote(repo); err != nil || p == nil || len(p.Close) != 0 || len(p.Dissolve) != 1 { + t.Fatalf("after dropping the closes = %+v, %v", p, err) + } + if err := dropPendingRemote(repo, []string{"S1"}); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(remoteStacksPath(repo)); !os.IsNotExist(err) { + t.Errorf("the file should be gone: %v", err) + } +} diff --git a/pkg/app/submit.go b/pkg/app/submit.go index fc918a1..c39d82f 100644 --- a/pkg/app/submit.go +++ b/pkg/app/submit.go @@ -70,8 +70,20 @@ type SubmitResult struct { Draft bool `json:"draft"` DryRun bool `json:"dryRun"` // Output is the backend's own output (captured runs only). - Output string `json:"output,omitempty"` - Notices []string `json:"notices,omitempty"` + Output string `json:"output,omitempty"` + // Closed are the pull requests a pop or fold left behind that this + // submit closed. + Closed []ClosedPR `json:"closed,omitempty"` + Notices []string `json:"notices,omitempty"` +} + +// ClosedPR is a queued pull request a submit closed. +type ClosedPR struct { + Number int `json:"number"` + Branch string `json:"branch"` + // BranchDeleted is false when the remote branch had to stay because an + // open pull request is still based on it. + BranchDeleted bool `json:"branchDeleted"` } // Submit pushes the current stack and creates or updates its pull requests. @@ -259,6 +271,7 @@ func (a *App) Submit(ctx context.Context, repo git.Repo, o SubmitOptions) (Submi } } a.recreateDissolved(ctx, repo, s, dissolved, after, &res) + a.closePending(ctx, repo, afterPRs, &res) if (len(texts) > 0 || fixed) && a.d.Forge != nil { // Titles or bases changed; refresh the cache so views show them. if _, err := a.RefreshPRs(ctx, repo); err != nil { @@ -596,3 +609,55 @@ func dissolvedNotice(graph *stack.Graph, s *stack.Stack, r remoteStack, hasForge } return fmt.Sprintf("dissolved stack #%d on GitHub; its pull requests now belong to the stack they were moved to", r.Number) } + +// closePending works through the pull requests a pop or fold queued: each +// is closed with its comment and then its remote branch deleted. It runs +// after the backend submit because that is what moves the pull requests +// that were based on the removed branch onto its replacement, and GitHub +// closes every open pull request based on a branch that is deleted; so a +// branch something is still based on is kept, and its delete stays queued. +func (a *App) closePending(ctx context.Context, repo git.Repo, prs []forge.PullRequest, res *SubmitResult) { + p, err := loadPendingRemote(repo) + if err != nil { + res.Notices = append(res.Notices, fmt.Sprintf("could not read %s (%v); delete it to forget the pull requests waiting to be closed", remoteStacksPath(repo), err)) + return + } + if p == nil || len(p.Close) == 0 { + return + } + if a.d.Forge == nil { + res.Notices = append(res.Notices, fmt.Sprintf("no forge configured, so %s left behind by a pop or fold stayed open", pluralise(len(p.Close), "a pull request", "pull requests"))) + return + } + var keep []pendingClose + for _, c := range p.Close { + if !c.Closed { + if err := a.d.Forge.ClosePR(ctx, repo, c.Number, c.Comment); err != nil { + keep = append(keep, c) + res.Notices = append(res.Notices, fmt.Sprintf("could not close #%d (%s): %v; the next submit tries again", c.Number, c.Branch, err)) + continue + } + c.Closed = true + } + closed := ClosedPR{Number: c.Number, Branch: c.Branch} + if i := slices.IndexFunc(prs, func(pr forge.PullRequest) bool { + return pr.Base == c.Branch && pr.Number != c.Number && (pr.State == forge.StateOpen || pr.State == forge.StateDraft) + }); i >= 0 { + keep = append(keep, c) + res.Notices = append(res.Notices, fmt.Sprintf("kept %s on GitHub because #%d is still based on it; the next submit deletes it once that has moved", c.Branch, prs[i].Number)) + } else if err := a.d.Git.DeleteRemoteBranch(ctx, repo, a.d.Git.RemoteFor(ctx, repo, c.Branch), c.Branch, c.Tip); err != nil { + // Gone already is fine; anything else (it moved on the remote, + // say) is left for whoever owns it. + if !strings.Contains(err.Error(), "remote ref does not exist") { + res.Notices = append(res.Notices, fmt.Sprintf("closed #%d but could not delete %s on GitHub: %v", c.Number, c.Branch, err)) + } + } else { + closed.BranchDeleted = true + } + res.Closed = append(res.Closed, closed) + } + p.Close = keep + if err := savePendingRemote(repo, p); err != nil { + res.Notices = append(res.Notices, fmt.Sprintf("could not update the list of pull requests to close: %v", err)) + } +} diff --git a/pkg/app/submit_close_test.go b/pkg/app/submit_close_test.go new file mode 100644 index 0000000..5f1432b --- /dev/null +++ b/pkg/app/submit_close_test.go @@ -0,0 +1,126 @@ +package app_test + +import ( + "context" + "errors" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/git/gittest" +) + +// closeFixture is submitFixture with an origin remote that has branch c +// (a pull request a fold or pop left behind, #12) queued to be closed. +func closeFixture(t *testing.T) (*app.App, *recordingForge, git.Repo, string, string) { + t.Helper() + deps, _, fg, repo, dir := submitFixture(t) + origin := t.TempDir() + gittest.Run(t, origin, "init", "-q", "--bare") + gittest.Run(t, dir, "remote", "add", "origin", origin) + gittest.Run(t, dir, "branch", "c", "b") + gittest.Run(t, dir, "push", "-q", "origin", "c") + tip := gittest.Run(t, dir, "rev-parse", "c") + gittest.Run(t, dir, "branch", "-D", "c") + fg.prs = append(fg.prs, forge.PullRequest{Number: 12, Head: "c", Base: "b", State: forge.StateOpen, URL: "u/c"}) + path := filepath.Join(repo.CommonDir, "git-stack", "remote-stacks.json") + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + queue := `{"dissolve":[],"close":[{"number":12,"branch":"c","comment":"Folded into #7 (a)","tip":"` + tip + `"}]}` + if err := os.WriteFile(path, []byte(queue), 0o644); err != nil { + t.Fatal(err) + } + return app.New(deps), fg, repo, path, origin +} + +func remoteHas(t *testing.T, origin, branch string) bool { + t.Helper() + return strings.Contains(gittest.Run(t, origin, "branch", "--list", branch), branch) +} + +func TestSubmitClosesQueuedPullRequestsAndDeletesTheirBranch(t *testing.T) { + a, fg, repo, path, origin := closeFixture(t) + ctx := context.Background() + + if _, err := a.Submit(ctx, repo, app.SubmitOptions{DryRun: true}); err != nil { + t.Fatal(err) + } + if len(fg.closed) != 0 || !remoteHas(t, origin, "c") { + t.Fatal("a dry run must not close or delete anything") + } + + res, err := a.Submit(ctx, repo, app.SubmitOptions{Publish: true}) + if err != nil { + t.Fatal(err) + } + if len(fg.closed) != 1 || fg.closed[0].number != 12 || fg.closed[0].comment != "Folded into #7 (a)" { + t.Errorf("closed = %+v", fg.closed) + } + if remoteHas(t, origin, "c") { + t.Error("the remote branch should be deleted") + } + if len(res.Closed) != 1 || res.Closed[0].Number != 12 || res.Closed[0].Branch != "c" || !res.Closed[0].BranchDeleted { + t.Errorf("result closed = %+v", res.Closed) + } + if _, err := os.Stat(path); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("nothing should be left queued: %v", err) + } +} + +// Deleting a branch on GitHub closes every pull request based on it, so +// the branch is kept while one still is. +func TestSubmitKeepsTheBranchOfAClosedPullRequestSomethingIsBasedOn(t *testing.T) { + a, fg, repo, path, origin := closeFixture(t) + fg.prs = append(fg.prs, forge.PullRequest{Number: 13, Head: "d", Base: "c", State: forge.StateOpen, URL: "u/d"}) + res, err := a.Submit(context.Background(), repo, app.SubmitOptions{Publish: true}) + if err != nil { + t.Fatal(err) + } + if len(fg.closed) != 1 || !remoteHas(t, origin, "c") { + t.Errorf("closed = %+v; the branch should stay", fg.closed) + } + if n := strings.Join(res.Notices, "\n"); !strings.Contains(n, "kept c on GitHub because #13 is still based on it") { + t.Errorf("notices = %v", res.Notices) + } + data, err := os.ReadFile(path) + if err != nil || !strings.Contains(string(data), `"closed":true`) { + t.Errorf("the delete should stay queued: %q %v", data, err) + } + + // Next time, with #13 retargeted, only the delete is left to do. + for i := range fg.prs { + if fg.prs[i].Number == 13 { + fg.prs[i].Base = "b" + } + } + if _, err := a.Submit(context.Background(), repo, app.SubmitOptions{Publish: true}); err != nil { + t.Fatal(err) + } + if len(fg.closed) != 1 || remoteHas(t, origin, "c") { + t.Errorf("closed again (%+v) or branch still there", fg.closed) + } +} + +func TestSubmitKeepsAQueuedCloseThatFailed(t *testing.T) { + a, fg, repo, path, origin := closeFixture(t) + fg.closeErr = errors.New("boom") + res, err := a.Submit(context.Background(), repo, app.SubmitOptions{Publish: true}) + if err != nil { + t.Fatal(err) + } + if n := strings.Join(res.Notices, "\n"); !strings.Contains(n, "could not close #12 (c)") { + t.Errorf("notices = %v", res.Notices) + } + if !remoteHas(t, origin, "c") { + t.Error("the branch must stay while its pull request is open") + } + if data, err := os.ReadFile(path); err != nil || !strings.Contains(string(data), `"number":12`) || strings.Contains(string(data), `"closed":true`) { + t.Errorf("the close should stay queued: %q %v", data, err) + } +} diff --git a/pkg/app/submit_test.go b/pkg/app/submit_test.go index f192a84..4189c69 100644 --- a/pkg/app/submit_test.go +++ b/pkg/app/submit_test.go @@ -99,6 +99,27 @@ type recordingForge struct { stacks [][]int createRef forge.StackRef createErr error + // closed records every ClosePR call; closeErr is what it returns. + closed []closeCall + closeErr error +} + +type closeCall struct { + number int + comment string +} + +func (f *recordingForge) ClosePR(_ context.Context, _ git.Repo, n int, comment string) error { + if f.closeErr != nil { + return f.closeErr + } + f.closed = append(f.closed, closeCall{n, comment}) + for i := range f.prs { + if f.prs[i].Number == n { + f.prs[i].State = forge.StateClosed + } + } + return nil } func (f *recordingForge) CreateStack(_ context.Context, _ git.Repo, numbers []int) (forge.StackRef, error) { diff --git a/pkg/forge/forge.go b/pkg/forge/forge.go index 112b671..72015f0 100644 --- a/pkg/forge/forge.go +++ b/pkg/forge/forge.go @@ -128,6 +128,10 @@ type Forge interface { // each based on the head of the one before. A refusal comes back as a // *stack.Error of kind api_failure carrying the forge's message. CreateStack(ctx context.Context, repo git.Repo, numbers []int) (StackRef, error) + // ClosePR closes pull request number without merging it, leaving + // comment on it first. One that is already closed or merged is not an + // error. + ClosePR(ctx context.Context, repo git.Repo, number int, comment string) error // PullRequestURL is the web address of pull request number, worked out // from local configuration only (never the network), so output can // link a PR the forge hasn't been asked about. "" when it can't tell. diff --git a/pkg/forge/github/github.go b/pkg/forge/github/github.go index bdc1c31..c9ad165 100644 --- a/pkg/forge/github/github.go +++ b/pkg/forge/github/github.go @@ -203,6 +203,20 @@ func (f *Forge) view(ctx context.Context, repo git.Repo, selector string) (forge return p.toForge(), nil } +// reAlreadyClosed is gh pr close's refusal for a pull request that is +// already closed or merged. +var reAlreadyClosed = regexp.MustCompile(`(?i)already closed|can't be closed because it was already merged|is already merged`) + +// ClosePR closes a pull request with a comment, through gh pr close. gh +// refuses one that is already closed or merged, which counts as done. +func (f *Forge) ClosePR(ctx context.Context, repo git.Repo, number int, comment string) error { + _, err := f.gh(ctx, repo, "", "pr", "close", strconv.Itoa(number), "--comment", comment) + if ee, ok := errors.AsType[*exec.ExitError](err); ok && reAlreadyClosed.MatchString(ee.Result.Err()) { + return nil + } + return err +} + // UpdatePR edits title, body and base, and can mark a draft ready. func (f *Forge) UpdatePR(ctx context.Context, repo git.Repo, number int, in forge.UpdatePR) error { num := strconv.Itoa(number) diff --git a/pkg/forge/github/github_test.go b/pkg/forge/github/github_test.go index 1863e13..cc37693 100644 --- a/pkg/forge/github/github_test.go +++ b/pkg/forge/github/github_test.go @@ -133,3 +133,26 @@ func TestListPRsCarriesMergeAndHeadCommits(t *testing.T) { } } } + +func TestClosePR(t *testing.T) { + f := exectest.New() + f.On("gh", "pr", "close").Reply("") + if err := New(f).ClosePR(context.Background(), git.Repo{TopLevel: "/repo"}, 12, "Folded into #11 (b)"); err != nil { + t.Fatal(err) + } + if got := strings.Join(f.Calls()[0].Args, " "); got != "pr close 12 --comment Folded into #11 (b)" { + t.Errorf("args = %q", got) + } + + // Already closed or merged counts as done; anything else is an error. + f = exectest.New() + f.On("gh", "pr", "close").Fail(1, "X Pull request #12 (old) can't be closed because it was already merged") + if err := New(f).ClosePR(context.Background(), git.Repo{}, 12, "x"); err != nil { + t.Errorf("already merged: %v", err) + } + f = exectest.New() + f.On("gh", "pr", "close").Fail(1, "GraphQL: Could not resolve to a PullRequest") + if err := New(f).ClosePR(context.Background(), git.Repo{}, 12, "x"); err == nil { + t.Error("a real failure should be an error") + } +} diff --git a/pkg/git/remote.go b/pkg/git/remote.go index 0e4c96c..c49c629 100644 --- a/pkg/git/remote.go +++ b/pkg/git/remote.go @@ -38,6 +38,26 @@ type PushLease struct { Expect string } +// DeleteRemoteBranch deletes branch on remote. With expect set, the delete +// is leased against it, so a branch that has moved on the remote since is +// refused rather than deleted; with it empty the branch is deleted whatever +// its tip. +func (c *Client) DeleteRemoteBranch(ctx context.Context, repo Repo, remote, branch, expect string) error { + ref := "refs/heads/" + branch + args := []string{"push", "--quiet"} + if expect != "" { + args = append(args, "--force-with-lease="+ref+":"+expect) + } + args = append(args, remote, ":"+ref) + if _, err := c.gitIn(ctx, repo, args...); err != nil { + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + return fmt.Errorf("delete %s on %s: %s", branch, remote, ee.Result.Err()) + } + return err + } + return nil +} + // Push force pushes each branch to the same name on remote, all or nothing. // Each one is leased against its Expect, so a remote that has moved since // the caller looked is refused rather than overwritten. diff --git a/pkg/gitconfig/aliases.go b/pkg/gitconfig/aliases.go index 7bf0ff8..f213206 100644 --- a/pkg/gitconfig/aliases.go +++ b/pkg/gitconfig/aliases.go @@ -31,6 +31,7 @@ var DefaultAliases = []Alias{ {"absorb", "stack absorb"}, {"reorder", "stack reorder"}, {"move", "stack move"}, + {"fold", "stack fold"}, {"track", "stack init"}, {"restack", "stack restack"}, {"submit", "stack submit"}, diff --git a/pkg/install/install_test.go b/pkg/install/install_test.go index e38b285..8661386 100644 --- a/pkg/install/install_test.go +++ b/pkg/install/install_test.go @@ -192,7 +192,7 @@ func TestInstallAndUninstall(t *testing.T) { } func gitconfigDefaults() []string { - return []string{"create", "modify", "absorb", "reorder", "move", "track", "restack", "submit", "sync", "up", "down", "top", "bottom", "continue", "abort", "c", "m", "ab", "rs", "ss", "u", "d", "t", "b", "co"} + return []string{"create", "modify", "absorb", "reorder", "move", "fold", "track", "restack", "submit", "sync", "up", "down", "top", "bottom", "continue", "abort", "c", "m", "ab", "rs", "ss", "u", "d", "t", "b", "co"} } func TestSkillOptInIsAsked(t *testing.T) { diff --git a/pkg/install/skill/SKILL.md b/pkg/install/skill/SKILL.md index 60e9f2f..14416b7 100644 --- a/pkg/install/skill/SKILL.md +++ b/pkg/install/skill/SKILL.md @@ -23,6 +23,7 @@ that touches a stack: | Rebase the stack after edits lower down | `stack_restack` | | Change the order of a stack's branches | `stack_reorder` (`order` bottom to top) | | Move a branch and what's above it onto another stack or onto trunk | `stack_move` (`only: true` to leave the rest behind) | +| Merge the current branch into the one below it (one branch, one PR) | `stack_fold` (`keep: true` keeps the current branch's name and PR) | | Track branches made by hand as a stack | `stack_init` | | Move around | `stack_navigate` (`up`/`down`/`top`/`bottom` or `branch`) | | Push and open/update PRs | `stack_submit`; you write `pull_requests` `{branch: {title, body}}`; ready for review unless `draft: true` | @@ -54,7 +55,7 @@ the one calling. ## CLI equivalents `git stack create -m "…"`, `git stack modify -a`, `git stack absorb`, `git stack reorder a b c`, -`git stack move --onto `, `git stack init a b`, `git stack restack`, +`git stack move --onto `, `git stack fold`, `git stack init a b`, `git stack restack`, `git stack up/down/top/bottom`, `git stack checkout`, `git stack submit --no-edit`, -`git stack sync -f`. Aliases: `git c`, `git m`, `git ab`, `git reorder`, `git move`, `git track`, +`git stack sync -f`. Aliases: `git c`, `git m`, `git ab`, `git reorder`, `git move`, `git fold`, `git track`, `git rs`, `git u`, `git d`, `git t`, `git b`, `git co`, `git ss`. diff --git a/pkg/mcp/instructions.go b/pkg/mcp/instructions.go index 0640f48..70229b2 100644 --- a/pkg/mcp/instructions.go +++ b/pkg/mcp/instructions.go @@ -2,7 +2,7 @@ package mcp // Instructions is sent to clients at initialisation. Codex weighs the start // of it most, so the key guidance comes first. -const Instructions = `git-stack manages stacked branches and stacked pull requests on top of GitHub's gh stack. Prefer these tools over raw git whenever the user works in a stack: stack_view shows every stack (branches, parents, PR state, needs_restack); stack_create starts a new branch on top of the current one (you write the commit message); stack_modify amends the current branch (or adds a commit) and restacks everything above it; stack_absorb folds each staged hunk into the commit of the stack that last changed those lines and rewrites that commit and everything above it (dry_run shows the plan first); use it to fix something lower down without navigating there; stack_reorder changes the order of the current stack's branches (order, bottom to top) and restacks them; stack_move puts a branch and what's above it on top of another stack or on a trunk as a new stack (only: true leaves the branches above behind); stack_init tracks branches you already have as a new stack, without rebasing. stack_reorder and stack_move leave the stack's pull requests to be regrouped on GitHub: the next stack_submit dissolves every stale GitHub stack and creates the ones it did not submit again, its notices naming each (a stack with fewer than two pull requests has nothing to group; a branch without an open pull request, or a failed create, gets a notice saying to submit from that stack); stack_restack rebases the stack locally without checking anything out, bottom branch onto the local trunk included; on a conflict it leaves one git rebase in progress for the files to be resolved; stack_continue finishes an interrupted restack once the conflicts are resolved and git added; stack_abort gives it up and puts every moved branch back; stack_navigate moves up/down/top/bottom or checks out a branch; stack_submit pushes the stack (or with down only the current branch and the ones below it) and creates or updates PRs (you write titles and bodies; ready for review unless draft is true); stack_sync fetches, fast forwards trunk, deletes branches whose pull requests merged or closed, fast forwards branches that moved on the remote and restacks every stack without checking anything out; it never pushes. stack_merge merges the stack's pull requests up to a branch into trunk in one all or nothing operation (drafts must be published first) and then syncs; it refuses while any of those pull requests has a check that failed (code checks_failing) or is still running (code checks_pending), listing them in the error's checks field. Fix the failures and push with stack_submit, or wait for running checks and call it again; only pass force: true when the user has said to merge anyway. +const Instructions = `git-stack manages stacked branches and stacked pull requests on top of GitHub's gh stack. Prefer these tools over raw git whenever the user works in a stack: stack_view shows every stack (branches, parents, PR state, needs_restack); stack_create starts a new branch on top of the current one (you write the commit message); stack_modify amends the current branch (or adds a commit) and restacks everything above it; stack_absorb folds each staged hunk into the commit of the stack that last changed those lines and rewrites that commit and everything above it (dry_run shows the plan first); use it to fix something lower down without navigating there; stack_reorder changes the order of the current stack's branches (order, bottom to top) and restacks them; stack_move puts a branch and what's above it on top of another stack or on a trunk as a new stack (only: true leaves the branches above behind); stack_fold merges the current branch into the branch below it, one branch and one pull request (keep: true keeps the current branch's name and pull request instead of the parent's), and the next stack_submit closes the pull request of the branch that went; stack_init tracks branches you already have as a new stack, without rebasing. stack_reorder and stack_move leave the stack's pull requests to be regrouped on GitHub: the next stack_submit dissolves every stale GitHub stack and creates the ones it did not submit again, its notices naming each (a stack with fewer than two pull requests has nothing to group; a branch without an open pull request, or a failed create, gets a notice saying to submit from that stack); stack_restack rebases the stack locally without checking anything out, bottom branch onto the local trunk included; on a conflict it leaves one git rebase in progress for the files to be resolved; stack_continue finishes an interrupted restack once the conflicts are resolved and git added; stack_abort gives it up and puts every moved branch back; stack_navigate moves up/down/top/bottom or checks out a branch; stack_submit pushes the stack (or with down only the current branch and the ones below it) and creates or updates PRs (you write titles and bodies; ready for review unless draft is true); stack_sync fetches, fast forwards trunk, deletes branches whose pull requests merged or closed, fast forwards branches that moved on the remote and restacks every stack without checking anything out; it never pushes. stack_merge merges the stack's pull requests up to a branch into trunk in one all or nothing operation (drafts must be published first) and then syncs; it refuses while any of those pull requests has a check that failed (code checks_failing) or is still running (code checks_pending), listing them in the error's checks field. Fix the failures and push with stack_submit, or wait for running checks and call it again; only pass force: true when the user has said to merge anyway. Never run git rebase or git commit --amend by hand inside a stack: descendants would not be restacked. Use stack_modify / stack_restack instead. diff --git a/pkg/mcp/server.go b/pkg/mcp/server.go index 3a9aab7..d7690e3 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -218,6 +218,7 @@ var cliToTool = []struct{ from, to string }{ {"git stack absorb", "stack_absorb"}, {"git stack reorder", "stack_reorder"}, {"git stack move", "stack_move"}, + {"git stack fold", "stack_fold"}, {"git stack init", "stack_init"}, {"git stack track", "stack_init"}, {"git stack continue", "call stack_continue"}, diff --git a/pkg/mcp/server_test.go b/pkg/mcp/server_test.go index 9328931..ca8a7eb 100644 --- a/pkg/mcp/server_test.go +++ b/pkg/mcp/server_test.go @@ -45,6 +45,7 @@ func (f *fakeForge) MergeStack(_ context.Context, _ git.Repo, n int, _ forge.Mer func (f *fakeForge) ListPRs(context.Context, git.Repo) ([]forge.PullRequest, error) { return slices.Clone(f.prs), nil } +func (f *fakeForge) ClosePR(context.Context, git.Repo, int, string) error { return nil } func (f *fakeForge) CreateStack(context.Context, git.Repo, []int) (forge.StackRef, error) { return forge.StackRef{}, nil } @@ -222,7 +223,7 @@ func TestToolsAreListedWithAnnotationsAndSchemas(t *testing.T) { for _, tool := range res.Tools { got[tool.Name] = tool } - for _, name := range []string{"stack_view", "stack_create", "stack_modify", "stack_absorb", "stack_reorder", "stack_move", "stack_init", "stack_restack", "stack_navigate", "stack_submit", "stack_sync"} { + for _, name := range []string{"stack_view", "stack_create", "stack_modify", "stack_absorb", "stack_reorder", "stack_move", "stack_fold", "stack_init", "stack_restack", "stack_navigate", "stack_submit", "stack_sync"} { if got[name] == nil { t.Errorf("missing tool %s", name) } @@ -795,3 +796,22 @@ func TestReorderMoveInit(t *testing.T) { t.Errorf("no onto = %+v", te) } } + +func TestFold(t *testing.T) { + h := newHarness(t) + defer h.assertNoStdout() + var res app.FoldResult + h.call("stack_fold", map[string]any{"keep": true}, &res) + if res.Into != "b" || res.Folded != "a" { + t.Fatalf("fold = %+v", res) + } + if s, _, ok := h.backend.Graph().StackOf("b"); !ok || len(s.Branches) != 1 { + t.Errorf("b's stack = %+v", s) + } + if h.current() != "b" { + t.Errorf("current = %s", h.current()) + } + if te := h.toolErr("stack_fold", nil); te.Code != "invalid_args" || !strings.Contains(te.Message, "bottom of its stack") { + t.Errorf("bottom = %+v", te) + } +} diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index 8148e45..e0c5bb3 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -265,6 +265,23 @@ func (s *Server) move(ctx context.Context, req *mcp.CallToolRequest, in moveInpu return nil, res, nil } +type foldInput struct { + repoArg + Keep bool `json:"keep,omitempty" jsonschema:"keep the current branch's name and pull request instead of the parent's"` +} + +func (s *Server) fold(ctx context.Context, req *mcp.CallToolRequest, in foldInput) (*mcp.CallToolResult, app.FoldResult, error) { + repo, a, err := s.appFor(ctx, req, in.RepoPath) + if err != nil { + return nil, app.FoldResult{}, wrapErr(err) + } + res, err := a.Fold(ctx, repo, app.FoldOptions{Keep: in.Keep}) + if err != nil { + return nil, app.FoldResult{}, wrapErr(err) + } + return nil, res, nil +} + type initInput struct { repoArg Branches []string `json:"branches,omitempty" jsonschema:"existing branches to track, bottom to top (default: the current branch)"` @@ -564,6 +581,12 @@ func (s *Server) registerTools() { Description: "Move a branch (and, unless only is true, everything above it) onto the top of another stack, or onto a trunk as a new stack, then restack. A branch can't be moved within its own stack (use stack_reorder) or under another stack's branch. Conflicts and undo work as for stack_reorder.", Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(false)}, }, s.move) + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "stack_fold", + Title: "Fold the current branch into its parent", + Description: "Merge the current branch into the branch below it: one branch, one pull request, both sets of commits. The parent survives and is checked out (local changes come along); with keep: true the current branch survives and takes the parent's place. Nothing is rebased or pushed. The pull request of the branch that went is closed at the next stack_submit (closing in the result). Refuses with invalid_args on a stack's bottom branch, when the branch isn't on its parent yet (stack_restack first) or when the parent is checked out in another worktree.", + Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(false)}, + }, s.fold) mcp.AddTool(s.mcp, &mcp.Tool{ Name: "stack_init", Title: "Track existing branches as a stack",