From 6786613ac49bb52e5063fbc026271f18083b6d66 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Fri, 16 Oct 2026 13:29:39 +0100 Subject: [PATCH 1/3] Let the git client point HEAD at a branch without checking it out Pop needs to leave a branch's changes staged on its parent, which is what you get when HEAD moves to the parent but the index and working tree stay as they are. A switch or reset would rewrite one or the other. So the git client gets PointHEAD, a git symbolic-ref onto the branch. git will point HEAD at a branch that doesn't exist without complaint, so it checks the branch is there first. --- pkg/git/refs.go | 22 ++++++++++++++++++++++ pkg/git/refs_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/pkg/git/refs.go b/pkg/git/refs.go index 2cf4f69..50f0ac9 100644 --- a/pkg/git/refs.go +++ b/pkg/git/refs.go @@ -75,6 +75,28 @@ func (c *Client) DeleteBranch(ctx context.Context, repo Repo, name string) error return nil } +// PointHEAD makes branch the current branch without touching the index or +// the working tree, so whatever differs from its tip shows as changes. git +// would happily point HEAD at a branch that doesn't exist, so that is +// refused first. +func (c *Client) PointHEAD(ctx context.Context, repo Repo, branch string) error { + _, ok, err := c.Tip(ctx, repo, "refs/heads/"+branch) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("point HEAD at %q: no such branch", branch) + } + _, err = c.gitIn(ctx, repo, "symbolic-ref", "HEAD", "refs/heads/"+branch) + if err != nil { + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + return fmt.Errorf("point HEAD at %q: %s", branch, ee.Result.Err()) + } + return err + } + return nil +} + // MergeBase returns the best common ancestor of a and b. func (c *Client) MergeBase(ctx context.Context, repo Repo, a, b string) (string, error) { res, err := c.gitIn(ctx, repo, "merge-base", a, b) diff --git a/pkg/git/refs_test.go b/pkg/git/refs_test.go index 8a9a5c3..398c6b6 100644 --- a/pkg/git/refs_test.go +++ b/pkg/git/refs_test.go @@ -273,3 +273,34 @@ func TestPush(t *testing.T) { t.Errorf("b was pushed despite the refusal: %s", got) } } + +func TestPointHEADKeepsTheIndexAndWorkingTree(t *testing.T) { + gittest.Isolate(t) + dir := gittest.InitRepo(t) + c := git.New(exec.New()) + ctx := context.Background() + repo, err := c.Discover(ctx, dir) + if err != nil { + t.Fatal(err) + } + gittest.Run(t, dir, "switch", "-q", "-c", "f") + gittest.Commit(t, dir, "f.txt", "f", "feat: f") + gittest.WriteFile(t, dir, "untracked.txt", "u") + if err := c.PointHEAD(ctx, repo, "main"); err != nil { + t.Fatal(err) + } + if cur, _ := c.CurrentBranch(ctx, repo); cur != "main" { + t.Errorf("current = %q", cur) + } + if got := gittest.Run(t, dir, "diff", "--cached", "--name-only"); got != "f.txt" { + t.Errorf("staged = %q", got) + } + for _, f := range []string{"f.txt", "untracked.txt"} { + if _, err := os.Stat(filepath.Join(dir, f)); err != nil { + t.Errorf("%s: %v", f, err) + } + } + if err := c.PointHEAD(ctx, repo, "nope"); err == nil { + t.Error("PointHEAD must refuse a branch that does not exist") + } +} From 3beb9129e2a7be90e595eaf1cf8d889f28613aa1 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Fri, 16 Oct 2026 13:36:18 +0100 Subject: [PATCH 2/3] Add the pop use case Taking a branch back out of a stack (to redo it, or because it shouldn't be its own PR yet) still meant going to gh stack modify. Pop does it here: the current branch goes away and its changes end up staged on its parent, much like git reset --soft onto the parent would leave them. It's a reshape like reorder and move. The branch is dropped from its stack and the branches above are restacked onto the parent, so they no longer carry its commits. Only once that's done does HEAD move to the parent (without touching the index or working tree) and the branch get deleted. That order means the restack never has to deal with the checked out branch, and if it stops on a conflict the saved state remembers the finish so git stack continue does it at the end; git stack abort puts the branch back in its stack, since nothing was deleted yet. It refuses up front when the tree has changes to tracked files (they'd be mixed in with the popped ones and you couldn't tell them apart), when the branch isn't sitting on its parent, and when the parent is checked out in another worktree. The branch's open PR is queued to close at the next submit and the stack queued for dissolving, like the other reshapes. --- pkg/app/pop.go | 178 +++++++++++++++++++++ pkg/app/pop_test.go | 330 +++++++++++++++++++++++++++++++++++++++ pkg/app/reshape.go | 94 ++++++++++- pkg/app/restack.go | 26 ++- pkg/app/restack_state.go | 10 ++ 5 files changed, 622 insertions(+), 16 deletions(-) create mode 100644 pkg/app/pop.go create mode 100644 pkg/app/pop_test.go diff --git a/pkg/app/pop.go b/pkg/app/pop.go new file mode 100644 index 0000000..171d738 --- /dev/null +++ b/pkg/app/pop.go @@ -0,0 +1,178 @@ +package app + +import ( + "context" + "errors" + "fmt" + + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// PopResult reports what a pop did. +type PopResult struct { + Branch string `json:"branch"` + Parent string `json:"parent"` + // StackGone is true when the popped branch was the only one. + StackGone bool `json:"stackGone,omitempty"` + Restacked []string `json:"restacked,omitempty"` + // Closing is the popped branch's pull request, queued to close at the + // next submit. + Closing int `json:"closing,omitempty"` + Notices []string `json:"notices,omitempty"` +} + +const popCommand = "git stack pop" + +// popComment is left on the popped branch's pull request when it closes. +const popComment = "Closed by git stack pop; its changes went back into the working tree." + +// Pop deletes the current branch and takes it out of its stack, leaving +// its changes staged on its parent. The branches above are restacked onto +// the parent first, while the popped branch is still checked out, and its +// pull request is queued to close at the next submit. +func (a *App) Pop(ctx context.Context, repo git.Repo) (PopResult, error) { + if err := a.noRebaseActive(ctx, repo); err != nil { + return PopResult{}, err + } + current, err := a.d.Git.CurrentBranch(ctx, repo) + if err != nil { + if errors.Is(err, git.ErrDetached) { + return PopResult{}, stack.New(stack.KindInvalidArgs, "HEAD is detached; check out the branch to pop first") + } + return PopResult{}, err + } + graph, err := a.d.Meta.Load(ctx, repo) + if err != nil { + return PopResult{}, err + } + s, idx, ok := graph.StackOf(current) + if !ok { + if graph.IsTrunk(current) { + return PopResult{}, stack.Newf(stack.KindInvalidArgs, "%s is a trunk; check out the branch to pop", current) + } + return PopResult{}, stack.Newf(stack.KindNotInStack, "%s is not in a stack", current). + WithSteps("git stack init " + current + " to start tracking it") + } + if err := noMergedBranches(s); err != nil { + return PopResult{}, err + } + parent, _ := graph.Parent(current) + res := PopResult{Branch: current, Parent: parent, StackGone: len(s.Branches) == 1} + local, err := a.localBranches(ctx, repo) + if err != nil { + return res, err + } + tip, parentTip := local[current].Head, local[parent].Head + if !a.isAncestor(ctx, repo, parentTip, tip) || a.baseAbove(ctx, repo, s.Branches[idx].Base, parentTip, tip) { + return res, stack.Newf(stack.KindInvalidArgs, "%s isn't on %s yet", current, parent). + WithSteps("run git stack restack first") + } + changes, err := a.d.Git.LocalChanges(ctx, repo) + if err != nil { + return res, err + } + if len(changes) > 0 { + return res, stack.Newf(stack.KindInvalidArgs, "pop needs a clean working tree; you have changes in %s", someNames(changes, 3)). + WithSteps("commit or stash them first") + } + if wt := local[parent].Worktree; wt != "" && wt != repo.TopLevel { + return res, stack.Newf(stack.KindInvalidArgs, "%s is checked out in %s, and popping %s needs to check it out here", parent, shortPath(wt), current). + WithSteps("check out another branch there, then run " + popCommand + " again") + } + above := s.Names()[idx+1:] + // The branch above keeps only its own commits: when it sits on the + // popped branch, they start at the popped branch's tip. + nextBase := "" + if len(above) > 0 && a.isAncestor(ctx, repo, tip, local[above[0]].Head) { + nextBase = tip + } + e := reshapeEdit{ + touched: []string{s.Bottom()}, + relink: []string{s.Bottom()}, + dropped: []string{current}, + finish: &reshapeFinish{Pop: current, Parent: parent}, + edit: func(g *stack.Graph) error { + s, i, ok := g.StackOf(current) + if !ok { + return stack.Newf(stack.KindInvalidArgs, "%s left its stack while it was being popped", current) + } + if i+1 < len(s.Branches) && nextBase != "" { + s.Branches[i+1].Base = nextBase + } + s.Branches = append(s.Branches[:i:i], s.Branches[i+1:]...) + return nil + }, + } + if len(above) > 0 { + e.segments = []restackSegment{{Parent: parent, Branches: above}} + } + if n := a.openPR(ctx, repo, s.Branches[idx]); n > 0 { + e.closes = []pendingClose{{Number: n, Branch: current, Comment: popComment, Tip: tip}} + res.Closing = n + } + headline := fmt.Sprintf("Popping %s; restacking %d %s onto %s", current, len(above), pluralise(len(above), "branch", "branches"), parent) + rr, err := a.reshape(ctx, repo, popCommand, headline, e) + res.Restacked = movedNames(rr) + res.Notices = rr.Notices + if err != nil { + res.Closing = 0 + } + return res, err +} + +// openPR is the number of b's open or draft pull request, from the cached +// pull requests or else the stack metadata, or 0 when it has none. +func (a *App) openPR(ctx context.Context, repo git.Repo, b stack.Branch) int { + prs, _ := a.loadPRs(ctx, repo, PRsCached) + if pr, ok := PRsFor(prs)[b.Name]; ok { + if pr.State == forge.StateOpen || pr.State == forge.StateDraft { + return pr.Number + } + return 0 + } + if b.PR != nil && !b.PR.Merged { + return b.PR.Number + } + return 0 +} + +// finishPop points HEAD at the parent without touching the index or the +// working tree, so the popped branch's changes show as staged, then deletes +// the branch. Run again after it stopped part way, it only does what is +// left. +func (a *App) finishPop(ctx context.Context, repo git.Repo, f *reshapeFinish) error { + cur, err := a.d.Git.CurrentBranch(ctx, repo) + if err != nil && !errors.Is(err, git.ErrDetached) { + return err + } + switch cur { + case f.Parent: + case f.Pop: + if err := a.d.Git.PointHEAD(ctx, repo, f.Parent); err != nil { + return err + } + default: + return stack.Newf(stack.KindInvalidArgs, "%s is no longer checked out, so its changes can't be staged on %s", f.Pop, f.Parent). + WithSteps("git switch " + f.Pop) + } + exists, err := a.d.Git.BranchExists(ctx, repo, f.Pop) + if err != nil || !exists { + return err + } + return a.d.Git.DeleteBranch(ctx, repo, f.Pop) +} + +// someNames lists up to n names ("x, y and z"), then how many more there +// are. +func someNames(names []string, n int) string { + switch { + case len(names) == 1: + return names[0] + case len(names) <= n: + return joinNames(names[:len(names)-1]) + " and " + names[len(names)-1] + default: + return fmt.Sprintf("%s and %d more", joinNames(names[:n]), len(names)-n) + } +} diff --git a/pkg/app/pop_test.go b/pkg/app/pop_test.go new file mode 100644 index 0000000..6336cd8 --- /dev/null +++ b/pkg/app/pop_test.go @@ -0,0 +1,330 @@ +package app_test + +import ( + "context" + "errors" + "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" +) + +func (f *restackFixture) current(t *testing.T) string { + t.Helper() + return gittest.Run(t, f.dir, "branch", "--show-current") +} + +func (f *restackFixture) staged(t *testing.T) []string { + t.Helper() + out := gittest.Run(t, f.dir, "diff", "--cached", "--name-only") + if out == "" { + return nil + } + return strings.Split(out, "\n") +} + +func (f *restackFixture) exists(t *testing.T, branch string) bool { + t.Helper() + _, err := gittest.RunErr(t, f.dir, "rev-parse", "--verify", "--quiet", "refs/heads/"+branch) + return err == nil +} + +func (f *restackFixture) pop(t *testing.T) (app.PopResult, error) { + t.Helper() + return f.app.Pop(context.Background(), f.repo) +} + +func TestPopTheTopBranch(t *testing.T) { + f := newRestackFixture(t) // main -> a -> b -> c, HEAD on c + res, err := f.pop(t) + if err != nil { + t.Fatal(err) + } + if res.Branch != "c" || res.Parent != "b" || res.StackGone || len(res.Restacked) != 0 { + t.Errorf("result = %+v", res) + } + if cur := f.current(t); cur != "b" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); !slices.Equal(got, []string{"c.txt"}) { + t.Errorf("staged = %v", got) + } + if f.exists(t, "c") { + t.Error("c should be deleted") + } + if !slices.Equal(f.names(), []string{"a", "b"}) { + t.Errorf("stack = %v", f.names()) + } +} + +func TestPopAMiddleBranchRestacksTheOnesAbove(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "b") + res, err := f.pop(t) + if err != nil { + t.Fatal(err) + } + if res.Branch != "b" || res.Parent != "a" || !slices.Equal(res.Restacked, []string{"c"}) { + t.Errorf("result = %+v", res) + } + if cur := f.current(t); cur != "a" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); !slices.Equal(got, []string{"b.txt"}) { + t.Errorf("staged = %v", got) + } + if f.exists(t, "b") { + t.Error("b should be deleted") + } + if !slices.Equal(f.names(), []string{"a", "c"}) { + t.Errorf("stack = %v", f.names()) + } + if f.has(t, "c", "b.txt") || !f.has(t, "c", "c.txt") || !f.has(t, "c", "a.txt") { + t.Error("c should sit on a without b's file") + } + if f.base("c") != f.rev(t, "a") { + t.Errorf("c's base = %s, want a's tip", f.base("c")) + } +} + +func TestPopTheBottomBranchLandsOnTrunk(t *testing.T) { + f := newRestackFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "a") + res, err := f.pop(t) + if err != nil { + t.Fatal(err) + } + if res.Parent != "main" || !slices.Equal(res.Restacked, []string{"b", "c"}) { + t.Errorf("result = %+v", res) + } + if cur := f.current(t); cur != "main" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); !slices.Equal(got, []string{"a.txt"}) { + t.Errorf("staged = %v", got) + } + if !slices.Equal(f.names(), []string{"b", "c"}) { + t.Errorf("stack = %v", f.names()) + } + if f.has(t, "b", "a.txt") || f.has(t, "c", "a.txt") || !f.has(t, "c", "b.txt") { + t.Error("b and c should sit on main without a's file") + } +} + +func TestPopTheOnlyBranchRemovesTheStack(t *testing.T) { + f := newLinearFixture(t, []fileCommit{{"a", "a.txt", "a"}}) + res, err := f.pop(t) + if err != nil { + t.Fatal(err) + } + if !res.StackGone || res.Parent != "main" { + t.Errorf("result = %+v", res) + } + if len(f.fb.graph.Stacks) != 0 { + t.Errorf("stacks = %v", f.fb.graph.Stacks) + } + if cur := f.current(t); cur != "main" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); !slices.Equal(got, []string{"a.txt"}) { + t.Errorf("staged = %v", got) + } +} + +func TestPopKeepsUntrackedFiles(t *testing.T) { + f := newRestackFixture(t) + gittest.WriteFile(t, f.dir, "notes.txt", "mine") + if _, err := f.pop(t); err != nil { + t.Fatal(err) + } + if st := gittest.Run(t, f.dir, "status", "--porcelain", "--", "notes.txt"); st != "?? notes.txt" { + t.Errorf("status of notes.txt = %q", st) + } +} + +func TestPopRefusals(t *testing.T) { + for _, tc := range []struct { + name string + prep func(t *testing.T, f *restackFixture) + kind stack.Kind + want string + }{ + {"dirty tree", func(t *testing.T, f *restackFixture) { + gittest.WriteFile(t, f.dir, "a.txt", "changed") + }, stack.KindInvalidArgs, "a.txt"}, + {"parent checked out elsewhere", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "worktree", "add", "-q", filepath.Join(t.TempDir(), "wt"), "b") + }, stack.KindInvalidArgs, "checked out in"}, + {"not on its parent", func(t *testing.T, f *restackFixture) { + f.amend(t, "b", "b2.txt") + }, stack.KindInvalidArgs, "restack"}, + {"merged branch", func(t *testing.T, f *restackFixture) { + f.fb.graph.Stacks[0].Branches[0].PR = &stack.PRRef{Number: 1, Merged: true} + }, stack.KindInvalidArgs, "sync"}, + {"detached", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "switch", "-q", "--detach", "c") + }, stack.KindInvalidArgs, "detached"}, + {"trunk", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "switch", "-q", "main") + }, stack.KindInvalidArgs, "trunk"}, + {"untracked branch", func(t *testing.T, f *restackFixture) { + gittest.Run(t, f.dir, "switch", "-q", "-c", "loose") + }, stack.KindNotInStack, "init"}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newRestackFixture(t) + tc.prep(t, f) + tips := map[string]string{"a": f.rev(t, "a"), "b": f.rev(t, "b"), "c": f.rev(t, "c")} + _, err := f.pop(t) + var se *stack.Error + if !errors.As(err, &se) || se.Kind != tc.kind || !strings.Contains(se.Error()+strings.Join(se.NextSteps, " "), tc.want) { + t.Fatalf("want %s mentioning %q, got %v", tc.kind, tc.want, err) + } + if !slices.Equal(f.names(), []string{"a", "b", "c"}) { + t.Errorf("stack = %v", f.names()) + } + for n, tip := range tips { + if f.rev(t, n) != tip { + t.Errorf("%s moved", n) + } + } + }) + } +} + +// popConflictFixture: main -> a (a.txt) -> b (shared.txt "b") -> c +// (shared.txt "c"), HEAD on b. Popping b replays c's edit of b's line onto +// a, which conflicts. +func popConflictFixture(t *testing.T) *restackFixture { + f := newLinearFixture(t, []fileCommit{{"a", "a.txt", "a"}, {"b", "shared.txt", "b"}, {"c", "shared.txt", "c"}}) + gittest.Run(t, f.dir, "switch", "-q", "b") + return f +} + +func TestPopConflictThenContinueFinishesThePop(t *testing.T) { + f := popConflictFixture(t) + ctx := context.Background() + _, err := f.pop(t) + var se *stack.Error + if !errors.As(err, &se) || se.Kind != stack.KindConflict || se.Branch != "c" { + t.Fatalf("want a conflict on c, got %v", err) + } + if !f.exists(t, "b") { + t.Fatal("b must survive until the pop finishes") + } + gittest.WriteFile(t, f.dir, "shared.txt", "c") + gittest.Run(t, f.dir, "add", "shared.txt") + res, err := f.app.Restack(ctx, f.repo, app.RestackOptions{Continue: true}) + if err != nil { + t.Fatal(err) + } + if res.Popped != "b" { + t.Errorf("popped = %q", res.Popped) + } + if cur := f.current(t); cur != "a" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); !slices.Equal(got, []string{"shared.txt"}) { + t.Errorf("staged = %v", got) + } + if got := gittest.Run(t, f.dir, "show", ":shared.txt"); got != "b" { + t.Errorf("staged shared.txt = %q, want b's version", got) + } + if f.exists(t, "b") { + t.Error("b should be deleted") + } + if !slices.Equal(f.names(), []string{"a", "c"}) || f.stateExists() { + t.Errorf("stack = %v, state left = %v", f.names(), f.stateExists()) + } +} + +func TestPopConflictThenAbortPutsEverythingBack(t *testing.T) { + f := popConflictFixture(t) + ctx := context.Background() + tips := map[string]string{"a": f.rev(t, "a"), "b": f.rev(t, "b"), "c": f.rev(t, "c")} + bases := map[string]string{"a": f.base("a"), "b": f.base("b"), "c": f.base("c")} + if _, err := f.pop(t); !errors.Is(err, &stack.Error{Kind: stack.KindConflict}) { + t.Fatalf("want a conflict, got %v", err) + } + res, err := f.app.Restack(ctx, f.repo, app.RestackOptions{Abort: true}) + if err != nil || !res.OrderRestored { + t.Fatalf("abort: %+v %v", res, err) + } + if !slices.Equal(f.names(), []string{"a", "b", "c"}) { + t.Errorf("stack = %v", f.names()) + } + for n, tip := range tips { + if f.rev(t, n) != tip || f.base(n) != bases[n] { + t.Errorf("%s: tip %s base %s, want %s %s", n, f.rev(t, n), f.base(n), tip, bases[n]) + } + } + if cur := f.current(t); cur != "b" { + t.Errorf("current = %s", cur) + } + if got := f.staged(t); len(got) != 0 { + t.Errorf("staged = %v", got) + } +} + +func TestPopQueuesThePullRequestAndTheStack(t *testing.T) { + f := newRestackFixture(t) + f.fb.graph.Stacks[0].ID, f.fb.graph.Stacks[0].Number = "S1", 1 + store := cache.New(f.repo) + prs := app.PRSnapshot{PRs: []forge.PullRequest{{Number: 12, Head: "b", State: forge.StateOpen}}} + if err := cache.Write(store, "prs", prs); err != nil { + t.Fatal(err) + } + f.app = app.New(app.Deps{Git: f.fb.git, Meta: f.fb, Tracker: f.fb, Cache: store}) + gittest.Run(t, f.dir, "switch", "-q", "b") + tip := f.rev(t, "b") + res, err := f.pop(t) + if err != nil { + t.Fatal(err) + } + if res.Closing != 12 { + t.Errorf("closing = %d", res.Closing) + } + data, err := os.ReadFile(filepath.Join(f.repo.CommonDir, "git-stack", "remote-stacks.json")) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{`"id":"S1"`, `"number":12`, `"branch":"b"`, `"tip":"` + tip + `"`, "Closed by git stack pop"} { + if !strings.Contains(string(data), want) { + t.Errorf("queue %s lacks %s", data, want) + } + } + if f.fb.graph.Stacks[0].ID != "" { + t.Error("the stack's remote id should be cleared") + } +} + +func TestPopAbortForgetsTheQueuedClose(t *testing.T) { + f := popConflictFixture(t) + store := cache.New(f.repo) + prs := app.PRSnapshot{PRs: []forge.PullRequest{{Number: 12, Head: "b", State: forge.StateOpen}}} + if err := cache.Write(store, "prs", prs); err != nil { + t.Fatal(err) + } + f.app = app.New(app.Deps{Git: f.fb.git, Meta: f.fb, Tracker: f.fb, Cache: store}) + ctx := context.Background() + if _, err := f.pop(t); !errors.Is(err, &stack.Error{Kind: stack.KindConflict}) { + t.Fatalf("want a conflict, got %v", err) + } + path := filepath.Join(f.repo.CommonDir, "git-stack", "remote-stacks.json") + if data, err := os.ReadFile(path); err != nil || !strings.Contains(string(data), `"number":12`) { + t.Fatalf("the close should be queued while the pop is stopped: %q %v", data, err) + } + if _, err := f.app.Restack(ctx, f.repo, app.RestackOptions{Abort: true}); err != nil { + t.Fatal(err) + } + if data, err := os.ReadFile(path); err == nil && strings.Contains(string(data), `"number":12`) { + t.Errorf("abort should forget the close: %s", data) + } +} diff --git a/pkg/app/reshape.go b/pkg/app/reshape.go index cf49d56..f2d0808 100644 --- a/pkg/app/reshape.go +++ b/pkg/app/reshape.go @@ -19,6 +19,12 @@ type reshapeSnapshot struct { // Created names the bottom branch of each stack the reshape added; // restoring empties them, which removes them. Created []string `json:"created,omitempty"` + // Dropped names the branches the reshape took out of their stack; + // restoring puts back each one that still exists. + Dropped []string `json:"dropped,omitempty"` + // Closes are the pull requests the reshape queued to close; restoring + // forgets them again. + Closes []int `json:"closes,omitempty"` } type snapshotStack struct { @@ -75,6 +81,16 @@ func stackIndex(g *stack.Graph, s *stack.Stack) int { // Stacks the reshape created are emptied, which removes them; any other // branch the snapshot does not name stays on the stack it is on now. func (a *App) restoreReshape(ctx context.Context, repo git.Repo, snap *reshapeSnapshot) error { + dropped := map[string]stack.Branch{} + for _, name := range snap.Dropped { + tip, ok, err := a.d.Git.Tip(ctx, repo, "refs/heads/"+name) + if err != nil { + return err + } + if ok { + dropped[name] = stack.Branch{Name: name, Head: tip} + } + } var ids []string err := a.d.Meta.Update(ctx, repo, func(g *stack.Graph) error { ids = nil @@ -115,6 +131,11 @@ func (a *App) restoreReshape(ctx context.Context, repo git.Repo, snap *reshapeSn } g.Stacks[i].Branches = nil } + for name, b := range dropped { + if _, _, tracked := g.StackOf(name); !tracked { + pool[name] = b + } + } // Each stack goes home to the first free slot, in index order, that // still carries its remote id; else that has its bottom branch at // the bottom on the same trunk; else that is on the same trunk; @@ -181,6 +202,9 @@ func (a *App) restoreReshape(ctx context.Context, repo git.Repo, snap *reshapeSn if err != nil { return err } + if err := dropPendingClose(repo, snap.Closes); err != nil { + return err + } return dropPendingRemote(repo, ids) } @@ -200,6 +224,13 @@ type reshapeEdit struct { // membership changed; its remote id is cleared and the stack queued // for dissolving on the next submit. relink []string + // dropped names the branches edit takes out of their stack, and closes + // their pull requests to close at the next submit. + dropped []string + closes []pendingClose + // finish runs once the segments are restacked, here or at the end of + // git stack continue. + finish *reshapeFinish } // reshape runs an edit: snapshot, metadata update with bases fixed, remote @@ -214,7 +245,10 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri return RestackResult{}, err } snap := snapshotOf(graph, e.touched) - snap.Created = e.created + snap.Created, snap.Dropped = e.created, e.dropped + for _, c := range e.closes { + snap.Closes = append(snap.Closes, c.Number) + } local, err := a.localBranches(ctx, repo) if err != nil { return RestackResult{}, err @@ -224,7 +258,7 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri return RestackResult{}, err } var pending []remoteStack - queued := false + queued, closing := false, false err = a.d.Meta.Update(ctx, repo, func(g *stack.Graph) error { before := parentsOf(g) // The stacks to relink, and the names GitHub knows them by, are @@ -232,7 +266,7 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri // its bottom branch elsewhere, and the edit only appends stacks and // edits branches, so indexes hold. var relink []int - pending, queued = nil, false + pending, queued, closing = nil, false, false for _, name := range e.relink { if s, _, ok := g.StackOf(name); ok && s.ID != "" { if i := stackIndex(g, s); !slices.Contains(relink, i) { @@ -268,6 +302,12 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri for _, i := range relink { g.Stacks[i].ID, g.Stacks[i].Number = "", 0 } + if len(e.closes) > 0 { + if err := addPendingClose(repo, e.closes); err != nil { + return err + } + closing = true + } // Queue the dissolves before the cleared ids are written, so a // failed write never leaves an id gone with nothing queued. if len(pending) > 0 { @@ -279,6 +319,11 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri return nil }) if err != nil { + if closing { + if derr := dropPendingClose(repo, snap.Closes); derr != nil { + err = errors.Join(err, derr) + } + } if queued { // The ids were not cleared after all; forget the dissolves again. ids := make([]string, len(pending)) @@ -291,17 +336,29 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri } return RestackResult{}, err } + run := &restackRun{repo: repo, command: command, originalBranch: current, reshape: snap, finish: e.finish} + res := RestackResult{Scope: stack.ScopeUpstack} if len(e.segments) == 0 { - return RestackResult{}, nil + if err := a.finishReshape(ctx, run, &res); err != nil { + return res, a.saveStopped(run, err) + } + return res, nil } - run := &restackRun{repo: repo, command: command, originalBranch: current, reshape: snap} - res := RestackResult{Scope: stack.ScopeUpstack} + restacked := false err = a.progress(ctx, PhaseRestack, headline, func(ctx context.Context) error { - return a.restackSegments(ctx, run, e.segments, &res) + if err := a.restackSegments(ctx, run, e.segments, &res); err != nil { + return err + } + restacked = true + return a.finishReshape(ctx, run, &res) }) if err == nil { return res, nil } + if restacked { + // Only the finish is left; continue does it again. + return res, a.saveStopped(run, err) + } // A saved state means the engine stopped somewhere continue and abort // can pick up (a conflict, usually); leave it to them. Without one, // put the shape back if no ref moved, else save what was done so the @@ -318,6 +375,27 @@ func (a *App) reshape(ctx context.Context, repo git.Repo, command, headline stri return res, a.saveStopped(run, err) } +// finishReshape runs the reshape's finish step, if it has one, once every +// segment is restacked and the user is back on the branch they started +// from. +func (a *App) finishReshape(ctx context.Context, run *restackRun, res *RestackResult) error { + if run.finish == nil { + return nil + } + if run.finish.Pop != "" { + if err := a.finishPop(ctx, run.repo, run.finish); err != nil { + // HEAD may already be on the parent with the changes staged; a + // retry must not switch back to the popped branch. + if cur, _ := a.d.Git.CurrentBranch(ctx, run.repo); cur == run.finish.Parent { + run.originalBranch = cur + } + return err + } + res.Popped = run.finish.Pop + } + return nil +} + // saveStopped saves a reshape that stopped without a conflict after some // refs moved: what moved, the segments still to do (the one that failed // first) and the snapshot, so git stack continue can finish it once the @@ -326,7 +404,7 @@ func (a *App) saveStopped(run *restackRun, err error) error { e := asStackError(err) st := &restackState{ Command: run.command, OriginalBranch: run.originalBranch, Trunk: run.trunk, - Moved: run.moved, Then: run.then, Reshape: run.reshape, + Moved: run.moved, Then: run.then, Reshape: run.reshape, Finish: run.finish, } if serr := saveRestackState(run.repo, st); serr != nil { return e.WithSteps(fmt.Sprintf("the new shape was kept but could not be saved for git stack continue or abort (%v); git stack restack finishes it", serr)) diff --git a/pkg/app/restack.go b/pkg/app/restack.go index a7b9f8c..477da59 100644 --- a/pkg/app/restack.go +++ b/pkg/app/restack.go @@ -40,8 +40,10 @@ type RestackResult struct { Restored []string `json:"restored,omitempty"` // OrderRestored is true when an abort also put a reorder or move's // stack shape back. - OrderRestored bool `json:"order_restored,omitempty"` - Notices []string `json:"notices,omitempty"` + OrderRestored bool `json:"order_restored,omitempty"` + // Popped is the branch a continue finished popping. + Popped string `json:"popped,omitempty"` + Notices []string `json:"notices,omitempty"` } const restackCommand = "git stack restack" @@ -66,10 +68,12 @@ type restackRun struct { command, trunk, stackName, originalBranch string moved []movedRef state *restackState - // then are the segments queued behind the one in progress, and - // reshape the snapshot abort puts back; both are saved on a conflict. + // then are the segments queued behind the one in progress, reshape + // the snapshot abort puts back and finish the reshape's last step; all + // are saved on a conflict. then []restackSegment reshape *reshapeSnapshot + finish *reshapeFinish } // restack is the shared body of Restack and modify's restack. command @@ -234,7 +238,7 @@ func (a *App) startConflictRebase(ctx context.Context, run *restackRun, p *resta Conflict: c.Branch, Onto: c.Onto, NewBase: p.conflictNewBase, ConflictTip: local[c.Branch].Head, ConflictOldBase: oldBase, Remaining: remaining, Moved: run.moved, - Then: run.then, Reshape: run.reshape, + Then: run.then, Reshape: run.reshape, Finish: run.finish, } if err := saveRestackState(run.repo, run.state); err != nil { return err @@ -524,7 +528,7 @@ func (a *App) restackContinue(ctx context.Context, repo git.Repo, stageAll bool) WithSteps("git stack abort to put the moved branches back, then " + st.Command + " again") } earlier := slices.Clone(st.Moved) - run := &restackRun{repo: repo, command: st.Command, trunk: st.Trunk, originalBranch: st.OriginalBranch, moved: st.Moved, state: st, then: st.Then, reshape: st.Reshape} + run := &restackRun{repo: repo, command: st.Command, trunk: st.Trunk, originalBranch: st.OriginalBranch, moved: st.Moved, state: st, then: st.Then, reshape: st.Reshape, finish: st.Finish} if g, err := a.d.Meta.Load(ctx, repo); err == nil { if s, _, ok := g.StackOf(st.Conflict); ok { run.stackName = s.Bottom() @@ -539,7 +543,10 @@ func (a *App) restackContinue(ctx context.Context, repo git.Repo, stageAll bool) } // Every queued segment may have been empty, which never reaches // finishRun on its own. - return a.finishRun(ctx, run) + if err := a.finishRun(ctx, run); err != nil { + return err + } + return a.finishReshape(ctx, run, &res) } if err := a.recordRebased(ctx, run, &res); err != nil { return err @@ -555,7 +562,10 @@ func (a *App) restackContinue(ctx context.Context, repo git.Repo, stageAll bool) } else if err := a.finishRun(ctx, run); err != nil { return err } - return a.restackSegments(ctx, run, st.Then, &res) + if err := a.restackSegments(ctx, run, st.Then, &res); err != nil { + return err + } + return a.finishReshape(ctx, run, &res) }) if err == nil { res.Moved = withEarlierMoves(earlier, res.Moved, run.stackName) diff --git a/pkg/app/restack_state.go b/pkg/app/restack_state.go index 4b5c3bf..cdff340 100644 --- a/pkg/app/restack_state.go +++ b/pkg/app/restack_state.go @@ -45,6 +45,9 @@ type restackState struct { // Reshape is the metadata as it was before the reorder or move that // started this run, for abort to put back. Reshape *reshapeSnapshot `json:"reshape,omitempty"` + // Finish is what the reshape still has to do once every branch is + // restacked: a pop's move of HEAD and delete of its branch. + Finish *reshapeFinish `json:"finish,omitempty"` } // branches are the branches the state still has to restack, the @@ -63,6 +66,13 @@ type restackSegment struct { Branches []string `json:"branches"` } +// reshapeFinish is work a reshape does once its restack is done. +type reshapeFinish struct { + // Pop is the branch to delete after pointing HEAD at Parent. + Pop string `json:"pop"` + Parent string `json:"parent"` +} + // movedRef is one branch move, with what abort needs to undo it. type movedRef struct { Name string `json:"name"` From 92b9be8b48196fbbf1502ab8c4ff1cfb88171f95 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Fri, 16 Oct 2026 13:41:28 +0100 Subject: [PATCH 3/3] Add git stack pop This puts the pop use case behind a command, so taking a branch back out of a stack no longer needs gh stack modify. git stack pop (or git pop) works on the branch you're on, says which parent its changes are now staged on, what it restacked, and which PR the next submit will close. A pop that stops on a conflict is finished by git stack continue like any other reshape, and continue now says when it also finished a pop, since that's the moment HEAD moves and the branch goes away. Agents get the same thing as stack_pop, and the README, MCP docs, skill and architecture notes cover it. The README's list of things that still need gh stack is down to folding a branch into another. --- README.md | 38 ++++++++++++++++++++----- cmd/pop.go | 55 +++++++++++++++++++++++++++++++++++++ cmd/pop_test.go | 39 ++++++++++++++++++++++++++ cmd/restack.go | 4 +++ cmd/restack_render_test.go | 9 ++++++ cmd/root.go | 1 + docs/architecture.md | 12 ++++++++ docs/mcp.md | 7 ++++- pkg/app/pop.go | 3 +- pkg/app/pop_test.go | 4 +-- pkg/app/reshape.go | 2 +- pkg/app/restack.go | 8 ++++-- pkg/gitconfig/aliases.go | 1 + pkg/install/install_test.go | 2 +- pkg/install/skill/SKILL.md | 5 ++-- pkg/mcp/instructions.go | 2 +- pkg/mcp/server.go | 1 + pkg/mcp/server_test.go | 19 ++++++++++++- pkg/mcp/tools.go | 29 ++++++++++++++++++- 19 files changed, 220 insertions(+), 21 deletions(-) create mode 100644 cmd/pop.go create mode 100644 cmd/pop_test.go diff --git a/README.md b/README.md index 4746005..d2a352e 100644 --- a/README.md +++ b/README.md @@ -109,10 +109,9 @@ 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, 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`. +branches onto another stack, folding a branch into the one below it, dropping one from a stack and +adopting branches you made by hand are `git stack reorder`, `git stack move`, `git stack fold`, +`git stack pop` and `git stack init`, so nothing needs gh stack directly any more. ## Install @@ -146,7 +145,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|fold|track|restack|continue|abort|submit|sync|up|down|top|bottom` +- **Aliases** (`--aliases`): `git create|modify|absorb|reorder|move|fold|track|pop|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. @@ -439,6 +438,30 @@ made with `git create`; this only adopts ones that exist. Flags: - `-b` / `--base ` the trunk to stack on +### `git stack pop` + +> Alias: `git pop` + +Deletes the branch you're on and takes it out of its stack, leaving its changes staged on its +parent, much like `git reset --soft` onto the parent would. Handy when you want to redo a branch +from scratch, or when it shouldn't be its own PR yet. The branches above it are restacked onto the +parent first, so they no longer carry its commits. + +It wants a clean working tree (untracked files are fine), so the popped changes don't get mixed up +with anything else, and the branch has to be sitting on its parent; `git restack` puts it there. +It also won't run while the parent is checked out in another worktree, since the parent is about +to be checked out here. A conflict in the restack stops like any other: fix it, `git add` and +`git continue`, which then finishes the pop, or `git abort` to leave everything as it was. If the +branch has an open PR, the next `git ss` closes it with a comment saying why. + +``` +# on billing-webhook-alerts +❯ git pop +✔ Popped billing-webhook-alerts; its changes are staged on billing-webhook-schema +✔ Restacked billing-webhook-retries + #412 will be closed at the next submit +``` + ### `git stack submit` > Aliases: `git submit`, `git ss` @@ -564,8 +587,9 @@ Flags: > Aliases: `git continue`, `git stack cont` -Once you've fixed and `git add`ed the conflicts, finishes the rebase a restack, modify, reorder or -move stopped on and then restacks whatever was left above it. +Once you've fixed and `git add`ed the conflicts, finishes the rebase a restack, modify, reorder, +move or pop stopped on and then restacks whatever was left above it (and, for a pop, then finishes +the pop). Flags: - `-a` / `--all` stage every change first diff --git a/cmd/pop.go b/cmd/pop.go new file mode 100644 index 0000000..a4baeea --- /dev/null +++ b/cmd/pop.go @@ -0,0 +1,55 @@ +package cmd + +import ( + "strings" + + "github.com/spf13/cobra" +) + +func newPopCmd(c *cli) *cobra.Command { + cmd := &cobra.Command{ + Use: "pop", + Short: "Take the current branch out of its stack, keeping its changes staged", + Long: `Deletes the current branch and takes it out of its stack, leaving its changes +staged on its parent, as if you had run git reset --soft onto the parent. The +branches above it are restacked onto the parent first, so they no longer carry +its commits. + +The working tree must have no changes to tracked files (untracked files are +fine), the branch must sit on its parent (git stack restack puts it there), and +the parent can't be checked out in another worktree. A conflict stops like a +restack does: fix the files, git add them and run "git stack continue", which +then finishes the pop, or "git stack abort" to leave everything as it was. If +the branch has an open pull request, the next submit closes it.`, + Example: ` # redo the current branch from scratch + git pop`, + 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.Pop(ctx, repo) + renderNotices(rep, res.Notices) + if err != nil { + return err + } + gone := "" + if res.StackGone { + gone = " (the stack is gone)" + } + rep.Success("Popped %s; its changes are staged on %s%s", res.Branch, res.Parent, gone) + if len(res.Restacked) > 0 { + rep.Success("Restacked %s", strings.Join(res.Restacked, ", ")) + } + if res.Closing > 0 { + rep.Info("#%d will be closed at the next submit", res.Closing) + } + return nil + }, + } + return cmd +} diff --git a/cmd/pop_test.go b/cmd/pop_test.go new file mode 100644 index 0000000..3e1ea16 --- /dev/null +++ b/cmd/pop_test.go @@ -0,0 +1,39 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/git/gittest" +) + +func TestPopCommand(t *testing.T) { + dir, run := stackOf3(t) + gittest.Run(t, dir, "switch", "-q", "b") + out, errOut, err := run("pop") + if err != nil || !strings.Contains(out, "ok: Popped b; its changes are staged on a") || !strings.Contains(out, "Restacked c") { + t.Errorf("pop: %q %q %v", out, errOut, err) + } + if got := gittest.Run(t, dir, "diff", "--cached", "--name-only"); got != "b.txt" { + t.Errorf("staged = %q", got) + } + gittest.Run(t, dir, "commit", "-q", "-m", "b again") + gittest.Run(t, dir, "switch", "-q", "c") + if _, _, err = run("pop"); err == nil || !strings.Contains(err.Error(), "c isn't on a yet") { + t.Errorf("c is no longer on a: %v", err) + } +} + +func TestPopCommandRemovesTheLastBranch(t *testing.T) { + gittest.Isolate(t) + dir := gittest.InitRepo(t) + f := fakeGh(t, dir) + gittest.WriteFile(t, dir, "a.txt", "a") + if _, _, err := runWith(t, f, "--cwd", dir, "create", "-a", "-m", "feat: a", "a"); err != nil { + t.Fatal(err) + } + out, errOut, err := runWith(t, f, "--cwd", dir, "pop") + if err != nil || !strings.Contains(out, "ok: Popped a; its changes are staged on main (the stack is gone)") { + t.Errorf("pop: %q %q %v", out, errOut, err) + } +} diff --git a/cmd/restack.go b/cmd/restack.go index a631458..5e687ab 100644 --- a/cmd/restack.go +++ b/cmd/restack.go @@ -96,6 +96,10 @@ func renderRestack(rep *ui.Reporter, res app.RestackResult, aborted bool) { } return } + if res.Popped != "" { + // A continue that finished a pop says so after what it restacked. + defer rep.Success("Popped %s; its changes are staged on %s", res.Popped, res.PoppedOnto) + } if len(res.Moved) == 0 { if len(res.InPlace) == 0 { rep.Success("Nothing to restack") diff --git a/cmd/restack_render_test.go b/cmd/restack_render_test.go index 165ba54..acb8191 100644 --- a/cmd/restack_render_test.go +++ b/cmd/restack_render_test.go @@ -39,3 +39,12 @@ func TestRenderRestackAborted(t *testing.T) { }) } } + +func TestRenderRestackFinishedAPop(t *testing.T) { + var out, errOut bytes.Buffer + rep := ui.NewReporter(nil, &out, &errOut, ui.ReporterOptions{}) + renderRestack(rep, app.RestackResult{Moved: []app.BranchMove{{Name: "c"}}, Popped: "b", PoppedOnto: "a"}, false) + if want := "ok: Restacked c\nok: Popped b; its changes are staged on a\n"; out.String() != want { + t.Errorf("out = %q, want %q", out.String(), want) + } +} diff --git a/cmd/root.go b/cmd/root.go index 76af1c0..44d8f8b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -326,6 +326,7 @@ func newRootCmd(c *cli) *cobra.Command { newMoveCmd(c), newFoldCmd(c), newInitCmd(c), + newPopCmd(c), newRestackCmd(c), newContinueCmd(c), newAbortCmd(c), diff --git a/docs/architecture.md b/docs/architecture.md index 08bbb8c..d2c569f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -423,6 +423,18 @@ conflict on one checked out elsewhere, refuses the whole reshape up front with n A failure before any ref has moved rolls the metadata back from the snapshot and the error says nothing was changed. +**Pop is a reshape with a finish step.** It drops the current branch from its stack, points the +branch above at the popped branch's tip as its base (so the restack replays only its own commits) +and restacks the branches above onto the parent, all while the popped branch stays checked out +and untouched. Only once that's done does it point HEAD at the parent with `git symbolic-ref`, +which leaves the index and working tree alone so the popped changes show as staged, and delete the +branch. Doing it in that order means the restack never moves or rebases the branch that's checked +out here, and a conflict leaves nothing half done: the saved restack state carries the finish step +so `continue` runs it at the end, and `abort` only has to put the dropped branch back in the +metadata (the snapshot lists it), since it was never deleted and HEAD never moved. Its open PR +goes on the close queue next to the dissolves, so the next submit closes it once gh stack has +moved the PR above it onto the new parent. + **GitHub's stack is dissolved and made again, not edited.** The stacks API can create a stack from a list of PRs and add PRs to the top of one, and that's all. Left alone, `gh stack submit` after a reorder pushes every branch and fixes each PR's base, then sees the stack on GitHub is diff --git a/docs/mcp.md b/docs/mcp.md index 74db1ed..ad5421b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -27,7 +27,8 @@ path is resolved with `git rev-parse`; anything outside a repository returns `no | `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_pop` | Delete the current branch and take it out of its stack, leaving its changes staged on its parent; the branches above are restacked onto the parent first, and its open PR is closed by the next `stack_submit`; refuses with nothing changed when tracked files have changes, the branch isn't on its parent or the parent is checked out in another worktree | | destructive | +| `stack_continue` | Finish an interrupted restack, modify, reorder, move or pop 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 | | `stack_restack` | Local rebase of the stack onto its parents, bottom branch onto the local trunk included; only checks out a branch to resolve a conflict through one git rebase | `scope` = `all`/`upstack`/`downstack`/`only`, `branch`, `continue`, `abort`, `stage_all` | destructive | | `stack_navigate` | `up`/`down`/`top`/`bottom` with `steps`, or `branch` | `direction`, `steps`, `branch` | idempotent | @@ -95,6 +96,10 @@ create (" has no open pull request, so 's stack was not created from it once it has one"); and a failed create says to submit from it to try again. With no forge configured the dissolve notice itself says to submit from that stack. If dissolving fails, a notice says so and the next submit tries again. +`conflict` from `stack_pop` means the branch is already out of the stack and the restack of the +branches above it stopped; the branch itself still exists and is still checked out. +`stack_continue` finishes the restack and then the pop (its result's `popped` names the branch), +and `stack_abort` puts the branch back in its stack with nothing staged. `checks_failing` and `checks_pending` come from `stack_merge` when a pull request it would land has a check that failed, or (with none failed) one still queued or running, on its head commit. Nothing is merged. The error's `checks` lists each held up pull request: diff --git a/pkg/app/pop.go b/pkg/app/pop.go index 171d738..81869ce 100644 --- a/pkg/app/pop.go +++ b/pkg/app/pop.go @@ -116,7 +116,8 @@ func (a *App) Pop(ctx context.Context, repo git.Repo) (PopResult, error) { rr, err := a.reshape(ctx, repo, popCommand, headline, e) res.Restacked = movedNames(rr) res.Notices = rr.Notices - if err != nil { + if st, _ := loadRestackState(repo); err != nil && st == nil { + // Nothing was left for continue, so nothing stays queued either. res.Closing = 0 } return res, err diff --git a/pkg/app/pop_test.go b/pkg/app/pop_test.go index 6336cd8..1057e34 100644 --- a/pkg/app/pop_test.go +++ b/pkg/app/pop_test.go @@ -225,8 +225,8 @@ func TestPopConflictThenContinueFinishesThePop(t *testing.T) { if err != nil { t.Fatal(err) } - if res.Popped != "b" { - t.Errorf("popped = %q", res.Popped) + if res.Popped != "b" || res.PoppedOnto != "a" { + t.Errorf("popped %q onto %q", res.Popped, res.PoppedOnto) } if cur := f.current(t); cur != "a" { t.Errorf("current = %s", cur) diff --git a/pkg/app/reshape.go b/pkg/app/reshape.go index f2d0808..45e95e6 100644 --- a/pkg/app/reshape.go +++ b/pkg/app/reshape.go @@ -391,7 +391,7 @@ func (a *App) finishReshape(ctx context.Context, run *restackRun, res *RestackRe } return err } - res.Popped = run.finish.Pop + res.Popped, res.PoppedOnto = run.finish.Pop, run.finish.Parent } return nil } diff --git a/pkg/app/restack.go b/pkg/app/restack.go index 477da59..dbe3ef5 100644 --- a/pkg/app/restack.go +++ b/pkg/app/restack.go @@ -41,9 +41,11 @@ type RestackResult struct { // OrderRestored is true when an abort also put a reorder or move's // stack shape back. OrderRestored bool `json:"order_restored,omitempty"` - // Popped is the branch a continue finished popping. - Popped string `json:"popped,omitempty"` - Notices []string `json:"notices,omitempty"` + // Popped is the branch a continue finished popping, and PoppedOnto + // the parent its changes are now staged on. + Popped string `json:"popped,omitempty"` + PoppedOnto string `json:"popped_onto,omitempty"` + Notices []string `json:"notices,omitempty"` } const restackCommand = "git stack restack" diff --git a/pkg/gitconfig/aliases.go b/pkg/gitconfig/aliases.go index f213206..a86eeb5 100644 --- a/pkg/gitconfig/aliases.go +++ b/pkg/gitconfig/aliases.go @@ -33,6 +33,7 @@ var DefaultAliases = []Alias{ {"move", "stack move"}, {"fold", "stack fold"}, {"track", "stack init"}, + {"pop", "stack pop"}, {"restack", "stack restack"}, {"submit", "stack submit"}, {"sync", "stack sync"}, diff --git a/pkg/install/install_test.go b/pkg/install/install_test.go index 8661386..6fb4312 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", "fold", "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", "pop", "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 14416b7..a8e1b43 100644 --- a/pkg/install/skill/SKILL.md +++ b/pkg/install/skill/SKILL.md @@ -25,6 +25,7 @@ that touches a stack: | 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` | +| Take the current branch out of its stack, keeping its changes staged on its parent | `stack_pop` | | 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` | | Pull trunk, restack, prune merged branches | `stack_sync` | @@ -55,7 +56,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 fold`, `git stack init a b`, `git stack restack`, +`git stack move --onto `, `git stack fold`, `git stack init a b`, `git stack pop`, `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 fold`, `git track`, +`git stack sync -f`. Aliases: `git c`, `git m`, `git ab`, `git reorder`, `git move`, `git fold`, `git track`, `git pop`, `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 70229b2..425005f 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_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. +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_pop deletes the current branch and takes it out of its stack, leaving its changes staged on its parent, and restacks the branches above (its pull request is closed by the next stack_submit). 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 d7690e3..3cb286d 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -221,6 +221,7 @@ var cliToTool = []struct{ from, to string }{ {"git stack fold", "stack_fold"}, {"git stack init", "stack_init"}, {"git stack track", "stack_init"}, + {"git stack pop", "stack_pop"}, {"git stack continue", "call stack_continue"}, {"git stack abort", "call stack_abort"}, {"git stack top", "stack_navigate {direction: \"top\"}"}, diff --git a/pkg/mcp/server_test.go b/pkg/mcp/server_test.go index ca8a7eb..ca4a4c0 100644 --- a/pkg/mcp/server_test.go +++ b/pkg/mcp/server_test.go @@ -223,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_fold", "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_pop", "stack_restack", "stack_navigate", "stack_submit", "stack_sync"} { if got[name] == nil { t.Errorf("missing tool %s", name) } @@ -815,3 +815,20 @@ func TestFold(t *testing.T) { t.Errorf("bottom = %+v", te) } } + +func TestPop(t *testing.T) { + h := newHarness(t) + defer h.assertNoStdout() + var res app.PopResult + h.call("stack_pop", nil, &res) + if res.Branch != "b" || res.Parent != "a" || res.StackGone { + t.Fatalf("pop = %+v", res) + } + if got := gittest.Run(t, h.dir, "diff", "--cached", "--name-only"); got != "b.txt" { + t.Errorf("staged = %q", got) + } + gittest.WriteFile(t, h.dir, "a.txt", "dirty") + if te := h.toolErr("stack_pop", nil); te.Code != "invalid_args" || len(te.NextSteps) == 0 { + t.Errorf("dirty tree = %+v", te) + } +} diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index e0c5bb3..823e395 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -300,6 +300,27 @@ func (s *Server) initStack(ctx context.Context, req *mcp.CallToolRequest, in ini return nil, res, nil } +// --- stack_pop --------------------------------------------------------- + +type popInput struct{ repoArg } + +func (s *Server) pop(ctx context.Context, req *mcp.CallToolRequest, in popInput) (*mcp.CallToolResult, app.PopResult, error) { + repo, a, err := s.appFor(ctx, req, in.RepoPath) + if err != nil { + return nil, app.PopResult{}, wrapErr(err) + } + res, err := a.Pop(ctx, repo) + if err != nil && errors.Is(err, &stack.Error{Kind: stack.KindConflict}) { + // The branch is already out of the stack and the restack above it + // hit a conflict: keep both, as stack_reorder does. + return &mcp.CallToolResult{IsError: true, Content: []mcp.Content{&mcp.TextContent{Text: wrapErr(err).Error()}}}, res, nil + } + if err != nil { + return nil, app.PopResult{}, wrapErr(err) + } + return nil, res, nil +} + // --- stack_continue / stack_abort --------------------------------------- type continueInput struct { @@ -593,6 +614,12 @@ func (s *Server) registerTools() { Description: "Track existing branches as a new stack on a trunk, bottom to top; nothing is committed, rebased or checked out. The result's needsRestack lists branches that aren't on their parent; stack_restack fixes them.", Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(false)}, }, s.initStack) + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "stack_pop", + Title: "Pop the current branch out of its stack", + Description: "Delete the current branch and take it out of its stack, leaving its changes staged on its parent (like git reset --soft onto the parent); the branches above are restacked onto the parent first. Refuses with invalid_args when tracked files have changes (untracked files are fine), when the branch isn't on its parent (stack_restack first) or when the parent is checked out in another worktree. A conflict comes back as code conflict with the result alongside; stack_continue finishes the restack and then the pop, stack_abort leaves everything as it was. The branch's open pull request is closed by the next stack_submit.", + Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(false)}, + }, s.pop) mcp.AddTool(s.mcp, &mcp.Tool{ Name: "stack_restack", Title: "Restack", @@ -602,7 +629,7 @@ func (s *Server) registerTools() { mcp.AddTool(s.mcp, &mcp.Tool{ Name: "stack_continue", Title: "Continue an interrupted restack", - Description: "After a conflict from stack_restack or stack_modify: once the files are resolved and git added (or with stage_all), finish that branch's rebase and restack whatever was left above it. Returns the branches moved.", + Description: "After a conflict from stack_restack or stack_modify: once the files are resolved and git added (or with stage_all), finish that branch's rebase and restack whatever was left above it. Returns the branches moved; after stack_pop, popped names the branch it then finished popping.", Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(true), OpenWorldHint: boolp(false)}, }, s.cont) mcp.AddTool(s.mcp, &mcp.Tool{