From 1fdd772252143405f43b834b14426dfcade39cd0 Mon Sep 17 00:00:00 2001 From: Sameen Karim Date: Tue, 19 May 2026 10:05:52 -0400 Subject: [PATCH] Allow `gh stack add` to adopt existing branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, `gh stack add` rejected any branch name that already existed in git with a blanket "branch already exists" error. This was overly restrictive — users who create branches ahead of time (e.g. from the GitHub UI or via `git branch`) had no way to incorporate them into a stack without deleting and recreating them. Now, if the specified branch exists in git but is not part of any existing stack, `add` adopts it: it skips branch creation, checks out the existing branch, and appends it to the stack metadata. This mirrors the adopt-or-create pattern already used by `gh stack init`. Branches that belong to another stack are still rejected by the existing `ValidateNoDuplicateBranch` guard, so there is no risk of cross-stack conflicts. Behavioral summary: - Existing branch, not in any stack → adopted (checkout only, no create) - Existing branch, already in a stack → error (unchanged) - Non-existent branch → created (unchanged) - Staging/commit flags (-A, -u, -m) work with adopted branches Tests added: - TestAdd_AdoptsExistingBranch - TestAdd_RejectsExistingBranchInStack - TestAdd_AdoptsExistingBranchWithCommit --- cmd/add.go | 31 ++++++++----- cmd/add_test.go | 115 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 11 deletions(-) diff --git a/cmd/add.go b/cmd/add.go index 649e067..8dcdffc 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -180,10 +180,9 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error { return ErrInvalidArgs } - if git.BranchExists(branchName) { - cfg.Errorf("branch %q already exists", branchName) - return ErrInvalidArgs - } + // If the branch already exists in git but is not part of any stack, + // adopt it instead of erroring. This mirrors the init command's behavior. + adopted := git.BranchExists(branchName) // Stage changes before creating the branch so we can fail early if // there's nothing to commit (avoids leaving an empty orphan branch). @@ -193,10 +192,12 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error { } } - // Create the new branch from the current HEAD and check it out - if err := git.CreateBranch(branchName, currentBranch); err != nil { - cfg.Errorf("failed to create branch: %s", err) - return ErrSilent + if !adopted { + // Create the new branch from the current HEAD and check it out + if err := git.CreateBranch(branchName, currentBranch); err != nil { + cfg.Errorf("failed to create branch: %s", err) + return ErrSilent + } } if err := git.CheckoutBranch(branchName); err != nil { @@ -227,10 +228,18 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error { // Print summary position := len(s.Branches) - if commitSHA != "" { - cfg.Successf("Created branch %s (layer %d) with commit %s", cfg.ColorBold(branchName), position, commitSHA) + if adopted { + if commitSHA != "" { + cfg.Successf("Adopted branch %s (layer %d) with commit %s", cfg.ColorBold(branchName), position, commitSHA) + } else { + cfg.Successf("Adopted existing branch %q into the stack", branchName) + } } else { - cfg.Successf("Created and checked out branch %q", branchName) + if commitSHA != "" { + cfg.Successf("Created branch %s (layer %d) with commit %s", cfg.ColorBold(branchName), position, commitSHA) + } else { + cfg.Successf("Created and checked out branch %q", branchName) + } } return nil diff --git a/cmd/add_test.go b/cmd/add_test.go index 335153c..2b3f213 100644 --- a/cmd/add_test.go +++ b/cmd/add_test.go @@ -474,3 +474,118 @@ func TestAdd_FromTrunk(t *testing.T) { names := sf.Stacks[0].BranchNames() assert.Equal(t, "newbranch", names[len(names)-1], "new branch should be appended to stack") } + +func TestAdd_AdoptsExistingBranch(t *testing.T) { + gitDir := t.TempDir() + saveStack(t, gitDir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b1"}}, + }) + + createBranchCalled := false + var checkedOut string + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "b1", nil }, + BranchExistsFn: func(name string) bool { return name == "existing-branch" }, + CreateBranchFn: func(name, base string) error { + createBranchCalled = true + return nil + }, + CheckoutBranchFn: func(name string) error { + checkedOut = name + return nil + }, + RevParseFn: func(ref string) (string, error) { return "abc", nil }, + }) + defer restore() + + cfg, outR, errR := config.NewTestConfig() + err := runAdd(cfg, &addOptions{}, []string{"existing-branch"}) + output := collectOutput(cfg, outR, errR) + + require.NoError(t, err) + require.NotContains(t, output, "\u2717", "unexpected error") + assert.False(t, createBranchCalled, "CreateBranch should NOT be called for existing branch") + assert.Equal(t, "existing-branch", checkedOut, "should checkout the existing branch") + assert.Contains(t, output, "Adopted") + + sf, err := stack.Load(gitDir) + require.NoError(t, err) + names := sf.Stacks[0].BranchNames() + assert.Equal(t, "existing-branch", names[len(names)-1], "adopted branch appended to stack") +} + +func TestAdd_RejectsExistingBranchInStack(t *testing.T) { + gitDir := t.TempDir() + // Two stacks: the current one and another that owns "taken-branch" + sf := &stack.StackFile{ + SchemaVersion: 1, + Stacks: []stack.Stack{ + { + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b1"}}, + }, + { + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "taken-branch"}}, + }, + }, + } + require.NoError(t, stack.Save(gitDir, sf), "saving seed stacks") + + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "b1", nil }, + BranchExistsFn: func(name string) bool { return name == "taken-branch" }, + }) + defer restore() + + cfg, outR, errR := config.NewTestConfig() + runAdd(cfg, &addOptions{}, []string{"taken-branch"}) + output := collectOutput(cfg, outR, errR) + + assert.Contains(t, output, "already exists in the stack") +} + +func TestAdd_AdoptsExistingBranchWithCommit(t *testing.T) { + gitDir := t.TempDir() + saveStack(t, gitDir, stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{{Branch: "b1"}}, + }) + + createBranchCalled := false + commitCalled := false + restore := git.SetOps(&git.MockOps{ + GitDirFn: func() (string, error) { return gitDir, nil }, + CurrentBranchFn: func() (string, error) { return "b1", nil }, + BranchExistsFn: func(name string) bool { return name == "existing-branch" }, + RevParseMultiFn: func(refs []string) ([]string, error) { + return []string{"aaa", "bbb"}, nil // different SHAs = branch has commits + }, + CreateBranchFn: func(name, base string) error { + createBranchCalled = true + return nil + }, + CheckoutBranchFn: func(name string) error { return nil }, + RevParseFn: func(ref string) (string, error) { return "abc", nil }, + StageAllFn: func() error { return nil }, + HasStagedChangesFn: func() bool { return true }, + CommitFn: func(msg string) (string, error) { + commitCalled = true + return "def1234567890", nil + }, + }) + defer restore() + + cfg, outR, errR := config.NewTestConfig() + err := runAdd(cfg, &addOptions{stageAll: true, message: "new commit"}, []string{"existing-branch"}) + output := collectOutput(cfg, outR, errR) + + require.NoError(t, err) + require.NotContains(t, output, "\u2717", "unexpected error") + assert.False(t, createBranchCalled, "CreateBranch should NOT be called") + assert.True(t, commitCalled, "Commit should be called on the adopted branch") + assert.Contains(t, output, "Adopted") +}