Undo the branch when create's commit fails

docs/architecture.md

@@ -188,7 +188,11 @@ rather than a confusing git error.
 **`create` from trunk runs `gh stack init`, from the top runs `gh stack add`, and we
 always commit natively.** This sidesteps both `add` quirks above. From the middle of a
 stack you get a `not_at_top` error with next steps; from an untracked branch you get told
-the branch isn't in a stack.
+the branch isn't in a stack. The branch is created, checked out and registered before the
+commit, so when the commit fails (a signing key that isn't unlocked, a hook saying no) create
+undoes all of that, forgetting the branch through `Update`, switching back and deleting it,
+and the staged changes are still staged; the error says to run it again. If the undo itself
+fails the error says what was left behind and that `modify -c` commits on it.
 
 **`modify` amends natively, then runs the native restack upstack.** Conflicts come back as
 a `conflict` error listing the files and the `git stack continue` / `git stack abort`

pkg/app/create.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
+	"time"
 
 	"github.com/DomBlack/git-stack/pkg/ai"
 	"github.com/DomBlack/git-stack/pkg/git"
@@ -36,6 +38,10 @@ type CreateResult struct {
 // recentSubjectsCount is how many trunk subjects feed the AI prompt.
 const recentSubjectsCount = 10
 
+// undoTimeout bounds the undo of a failed create, which runs even when the
+// command's own context was cancelled.
+const undoTimeout = 30 * time.Second
+
 // Create stages, creates a branch on top of the current one, commits and
 // registers the branch with the backend.
 func (a *App) Create(ctx context.Context, repo git.Repo, o CreateOptions) (CreateResult, error) {
@@ -96,13 +102,65 @@ func (a *App) Create(ctx context.Context, repo git.Repo, o CreateOptions) (Creat
 	}
 	sha, err := a.d.Git.Commit(ctx, repo, git.CommitOptions{Message: message, NoVerify: o.NoVerify})
 	if err != nil {
-		return res, commitError(err, "pass -m <message> (the branch was created; commit with git commit)")
+		return CreateResult{}, a.undoCreate(ctx, repo, name, current, commitError(err, "pass -m <message>"))
 	}
 	info := a.commitInfo(ctx, repo, sha)
 	res.Commit = &info
 	return res, nil
 }
 
+// undoCreate makes a failed create all or nothing: the branch was already
+// created, checked out and registered before the commit failed (a locked
+// signing key, a hook saying no), so forget it, go back to the parent and
+// delete it. The staged changes survive, both branches point at the same
+// commit. The error says to run create again, or, when undoing failed too,
+// what was left behind and how to commit on it instead.
+func (a *App) undoCreate(ctx context.Context, repo git.Repo, name, parent string, cause error) error {
+	err := asStackError(cause)
+	// The commit may have failed because ctx was cancelled; the undo still
+	// has to run, so give it its own bounded context.
+	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), undoTimeout)
+	defer cancel()
+	// Commit can fail after git has actually committed (reading HEAD back
+	// under a cancelled context, say). Then the branch holds the commit
+	// and must stay: deleting it would orphan the work.
+	if tip, ok, terr := a.d.Git.Tip(ctx, repo, name); terr == nil && ok {
+		if parentTip, _, perr := a.d.Git.Tip(ctx, repo, parent); perr == nil && tip != parentTip {
+			return err.WithSteps(fmt.Sprintf("%s has a commit after all, so it was kept and is tracked; check `git stack log`", name))
+		}
+	}
+	undo := func() error {
+		if err := a.d.Meta.Update(ctx, repo, func(g *stack.Graph) error {
+			for i := range g.Stacks {
+				g.Stacks[i].Branches = slices.DeleteFunc(g.Stacks[i].Branches, func(b stack.Branch) bool { return b.Name == name })
+			}
+			return nil
+		}); err != nil {
+			return fmt.Errorf("forget %s: %w", name, err)
+		}
+		if err := a.d.Git.Switch(ctx, repo, parent); err != nil {
+			return err
+		}
+		return a.d.Git.DeleteBranch(ctx, repo, name)
+	}
+	if undoErr := undo(); undoErr != nil {
+		a.d.Log.Debug("undo create", "branch", name, "err", undoErr)
+		return err.WithSteps(
+			fmt.Sprintf("%s was left behind with no commit of its own (undoing it failed: %v)", name, undoErr),
+			"check `git stack log`; if it is still tracked, commit on it with `git stack modify -c -m <message>`, otherwise delete it")
+	}
+	return err.WithSteps("nothing was created; run `git stack create` again once that is sorted")
+}
+
+// asStackError returns err as a *stack.Error, wrapping anything else as
+// KindUnknown so next steps can be attached.
+func asStackError(err error) *stack.Error {
+	if se, ok := errors.AsType[*stack.Error](err); ok {
+		return se
+	}
+	return stack.New(stack.KindUnknown, err.Error()).WithCause(err)
+}
+
 // isDefaultBranch reports whether name is the repository's default branch
 // while no stack has declared a trunk yet (the first stack starts here).
 func (a *App) isDefaultBranch(ctx context.Context, repo git.Repo, graph *stack.Graph, name string) bool {

pkg/app/modify.go

@@ -104,7 +104,11 @@ func (a *App) Modify(ctx context.Context, repo git.Repo, o ModifyOptions) (Modif
 	}
 	sha, err := a.d.Git.Commit(ctx, repo, co)
 	if err != nil {
-		return ModifyResult{}, commitError(err, "pass -m <message> or --no-edit")
+		err = commitError(err, "pass -m <message> or --no-edit")
+		if errors.Is(err, &stack.Error{Kind: stack.KindSigningFailed}) {
+			err = asStackError(err).WithSteps("then run `git stack modify` again; the branch is untouched")
+		}
+		return ModifyResult{}, err
 	}
 	res.Commit = a.commitInfo(ctx, repo, sha)
 

pkg/app/mutate_test.go

@@ -3,9 +3,12 @@ package app_test
 import (
 	"context"
 	"errors"
+	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/DomBlack/git-stack/pkg/ai"
 	"github.com/DomBlack/git-stack/pkg/app"
@@ -21,11 +24,16 @@ import (
 type fakeBackend struct {
 	git   *git.Client
 	graph *stack.Graph
+	// updateErr makes Update fail, for the paths that undo a change.
+	updateErr error
 }
 
 func (f *fakeBackend) Load(context.Context, git.Repo) (*stack.Graph, error) { return f.graph, nil }
 
 func (f *fakeBackend) Update(_ context.Context, _ git.Repo, fn func(*stack.Graph) error) error {
+	if f.updateErr != nil {
+		return f.updateErr
+	}
 	if err := fn(f.graph); err != nil {
 		return err
 	}
@@ -383,3 +391,117 @@ func TestCreateExplainsSigningFailure(t *testing.T) {
 		})
 	}
 }
+
+// rejectCommits installs a pre-commit hook that fails every commit.
+func rejectCommits(t *testing.T, dir string) {
+	t.Helper()
+	hooks := filepath.Join(dir, ".git", "hooks")
+	if err := os.MkdirAll(hooks, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(hooks, "pre-commit"), []byte("#!/bin/sh\necho nope >&2\nexit 1\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestCreateUndoesTheBranchWhenTheCommitFails(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("on top of a stack", func(t *testing.T) {
+		a, fb, repo, dir, _ := mutFixture(t)
+		rejectCommits(t, dir)
+		gittest.WriteFile(t, dir, "b.txt", "b")
+		gittest.Run(t, dir, "add", "b.txt")
+
+		_, err := a.Create(ctx, repo, app.CreateOptions{Name: "feat/b", Message: []string{"Add b"}})
+		se, ok := errors.AsType[*stack.Error](err)
+		if !ok || !strings.Contains(se.Detail+se.Msg, "nope") {
+			t.Fatalf("err = %v, want the hook's output", err)
+		}
+		if !slices.ContainsFunc(se.NextSteps, func(s string) bool { return strings.Contains(s, "git stack create") }) {
+			t.Errorf("next steps %q should say to run create again", se.NextSteps)
+		}
+		if cur := gittest.Run(t, dir, "branch", "--show-current"); cur != "a" {
+			t.Errorf("on %s, want back on a", cur)
+		}
+		if exists, _ := fb.git.BranchExists(ctx, repo, "feat/b"); exists {
+			t.Error("feat/b should have been deleted")
+		}
+		if names := fb.graph.Stacks[0].Names(); !slices.Equal(names, []string{"a"}) {
+			t.Errorf("stack = %v, want feat/b forgotten", names)
+		}
+		if out := gittest.Run(t, dir, "diff", "--cached", "--name-only"); out != "b.txt" {
+			t.Errorf("staged = %q, want b.txt still staged", out)
+		}
+	})
+
+	t.Run("new stack from trunk", func(t *testing.T) {
+		a, fb, repo, dir, _ := mutFixture(t)
+		gittest.Run(t, dir, "switch", "-q", "main")
+		fb.graph = stack.NewGraph(nil)
+		rejectCommits(t, dir)
+		gittest.WriteFile(t, dir, "b.txt", "b")
+		gittest.Run(t, dir, "add", "b.txt")
+
+		if _, err := a.Create(ctx, repo, app.CreateOptions{Name: "feat/b", Message: []string{"Add b"}}); err == nil {
+			t.Fatal("expected the commit to fail")
+		}
+		if cur := gittest.Run(t, dir, "branch", "--show-current"); cur != "main" {
+			t.Errorf("on %s, want back on main", cur)
+		}
+		if len(fb.graph.Stacks) != 0 {
+			t.Errorf("stacks = %+v, want the empty stack gone", fb.graph.Stacks)
+		}
+	})
+
+	t.Run("undo fails", func(t *testing.T) {
+		a, fb, repo, dir, _ := mutFixture(t)
+		rejectCommits(t, dir)
+		fb.updateErr = errors.New("metadata locked")
+		gittest.WriteFile(t, dir, "b.txt", "b")
+		gittest.Run(t, dir, "add", "b.txt")
+
+		_, err := a.Create(ctx, repo, app.CreateOptions{Name: "feat/b", Message: []string{"Add b"}})
+		se, ok := errors.AsType[*stack.Error](err)
+		if !ok {
+			t.Fatalf("err = %v, want *stack.Error", err)
+		}
+		if steps := strings.Join(se.NextSteps, "\n"); !strings.Contains(steps, "feat/b was left behind") || !strings.Contains(steps, "git stack modify") {
+			t.Errorf("next steps %q should say feat/b is left behind and how to commit on it", se.NextSteps)
+		}
+		if cur := gittest.Run(t, dir, "branch", "--show-current"); cur != "feat/b" {
+			t.Errorf("on %s, want still on feat/b", cur)
+		}
+	})
+}
+
+func TestCreateUndoesTheBranchWhenTheCommitIsCancelled(t *testing.T) {
+	a, fb, repo, dir, _ := mutFixture(t)
+	// A hook that outlives the deadline: the commit fails with the
+	// context's error, and the undo must still run on a context of its own.
+	hooks := filepath.Join(dir, ".git", "hooks")
+	if err := os.MkdirAll(hooks, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(hooks, "pre-commit"), []byte("#!/bin/sh\nsleep 5\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	gittest.WriteFile(t, dir, "b.txt", "b")
+	gittest.Run(t, dir, "add", "b.txt")
+
+	ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
+	defer cancel()
+	_, err := a.Create(ctx, repo, app.CreateOptions{Name: "feat/b", Message: []string{"Add b"}})
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("err = %v, want the deadline", err)
+	}
+	if cur := gittest.Run(t, dir, "branch", "--show-current"); cur != "a" {
+		t.Errorf("on %s, want back on a", cur)
+	}
+	if exists, _ := fb.git.BranchExists(context.Background(), repo, "feat/b"); exists {
+		t.Error("feat/b should have been deleted")
+	}
+	if names := fb.graph.Stacks[0].Names(); !slices.Equal(names, []string{"a"}) {
+		t.Errorf("stack = %v, want feat/b forgotten", names)
+	}
+}

pkg/app/staging.go

@@ -139,6 +139,6 @@ func signingError(se *git.SigningError) error {
 		e = stack.New(stack.KindSigningFailed, "commit signing failed: gpg could not sign the commit (its agent may need a terminal to ask for the passphrase)").
 			WithSteps("check that gpg can sign without a prompt: echo test | gpg --batch --clearsign")
 	}
-	return e.WithSteps("or turn signing off for this repository: git config commit.gpgsign false", "then run the command again").
+	return e.WithSteps("or turn signing off for this repository: git config commit.gpgsign false").
 		WithDetail(se.Detail).WithCause(se)
 }

pkg/mcp/tools.go

@@ -435,7 +435,7 @@ func (s *Server) registerTools() {
 	mcp.AddTool(s.mcp, &mcp.Tool{
 		Name:        "stack_create",
 		Title:       "Create a stacked branch",
-		Description: "Create a new branch on top of the current branch, commit staged changes with the given message, and register it in the stack. From the trunk this starts a new stack. Fails with not_at_top when the current branch is not the top of its stack.",
+		Description: "Create a new branch on top of the current branch, commit staged changes with the given message, and register it in the stack. From the trunk this starts a new stack. Fails with not_at_top when the current branch is not the top of its stack. If the commit fails (signing_failed, a hook) the branch is removed again and the changes stay staged, so fix the cause and call it again; should removing it fail too, next_steps says what was left behind and how to commit on it.",
 		Annotations: &mcp.ToolAnnotations{DestructiveHint: boolp(false), OpenWorldHint: boolp(false)},
 	}, s.create)
 	mcp.AddTool(s.mcp, &mcp.Tool{