From 5a288236b2839ceefd286d15083770e8cf78df28 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 10 Jul 2026 17:20:47 -0700 Subject: [PATCH 1/2] feat(sync): add -y/--yes flag This adds a `-y/--yes` flag to `gh stack sync` which accepts the default behavior when a branch has been merged into the trunk (which is to delete the branch). --- README.md | 1 + cmd/sync.go | 13 ++++--- cmd/sync_internal_test.go | 73 +++++++++++++++++++++++++++++++++++++++ e2e/sync_test.go | 11 ++++++ 4 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 cmd/sync_internal_test.go diff --git a/README.md b/README.md index a604bce..f136ded 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ This is the command to run when upstream changes have occurred (e.g., a PR in yo | `-D, --dry-run` | Show what would be done | | `-w, --worktrees` | Rebase branches checked out in linked worktrees in-place | | `--no-update-refs` | Do not pass `--update-refs` to git (preserves untracked bookmark branches) | +| `-y, --yes` | Accept the default action (delete merged branches) without prompting | ### undo diff --git a/cmd/sync.go b/cmd/sync.go index 9389a6b..bc737cf 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -29,6 +29,7 @@ var ( syncDryRunFlag bool syncWorktreesFlag bool syncNoUpdateRefsFlag bool + syncYesFlag bool ) func init() { @@ -36,6 +37,7 @@ func init() { syncCmd.Flags().BoolVarP(&syncDryRunFlag, "dry-run", "D", false, "show what would be done") syncCmd.Flags().BoolVarP(&syncWorktreesFlag, "worktrees", "w", false, "rebase branches checked out in linked worktrees in-place") syncCmd.Flags().BoolVar(&syncNoUpdateRefsFlag, "no-update-refs", false, "do not pass --update-refs to git (preserves untracked bookmark branches pointing into the stack)") + syncCmd.Flags().BoolVarP(&syncYesFlag, "yes", "y", false, "accept the default action (delete merged branches) without prompting") rootCmd.AddCommand(syncCmd) } @@ -290,7 +292,7 @@ func runSync(cmd *cobra.Command, args []string) error { if syncDryRunFlag { fmt.Printf("%s Would handle merged branch %s\n", s.Muted("dry-run:"), s.Branch(branch)) } else { - action := handleMergedBranch(g, cfg, branch, trunk, ¤tBranch, s) + action := handleMergedBranch(g, cfg, branch, trunk, ¤tBranch, syncYesFlag, s) if action == mergedActionSkip { // User chose to skip - don't collect fork points or retarget children continue @@ -438,12 +440,13 @@ const ( // handleMergedBranch prompts the user for how to handle a merged branch and executes the choice. // Returns the action taken. If the user is on the merged branch, it will checkout trunk first. -// The currentBranch pointer is updated if a checkout occurs. -func handleMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string, s *style.Style) mergedAction { +// The currentBranch pointer is updated if a checkout occurs. If yes is true, the default action +// (delete) is taken without prompting. +func handleMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string, yes bool, s *style.Style) mergedAction { fmt.Printf("\nBranch %s appears to be %s into %s.\n", s.Branch(branch), s.Merged("merged"), s.Branch(trunk)) - // Default to delete in non-interactive mode - if !prompt.IsInteractive() { + // Default to delete when --yes is set or in non-interactive mode + if yes || !prompt.IsInteractive() { return deleteMergedBranch(g, cfg, branch, trunk, currentBranch, s) } diff --git a/cmd/sync_internal_test.go b/cmd/sync_internal_test.go new file mode 100644 index 0000000..8ab3234 --- /dev/null +++ b/cmd/sync_internal_test.go @@ -0,0 +1,73 @@ +// cmd/sync_internal_test.go +// +// Internal tests (package cmd) for unexported sync helpers. +package cmd + +import ( + "os/exec" + "strings" + "testing" + + "github.com/boneskull/gh-stack/internal/git" + "github.com/boneskull/gh-stack/internal/style" +) + +// TestHandleMergedBranchYesDeletes verifies that passing yes=true causes +// handleMergedBranch to delete the branch and clear stack config without +// showing an interactive prompt, even when stdin/stdout are a TTY. +func TestHandleMergedBranchYesDeletes(t *testing.T) { + cfg, dir := setupTestRepoWithDir(t) + g := git.New(dir) + s := style.New() + + // Determine trunk from the current HEAD branch. + out, err := exec.Command("git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output() + if err != nil { + t.Fatalf("could not determine trunk branch: %v", err) + } + trunk := strings.TrimSpace(string(out)) + + if err := cfg.SetTrunk(trunk); err != nil { + t.Fatalf("SetTrunk failed: %v", err) + } + + // Create feature-a with a commit so git can delete it. + if err := exec.Command("git", "-C", dir, "checkout", "-b", "feature-a").Run(); err != nil { + t.Fatalf("create branch failed: %v", err) + } + if err := exec.Command("git", "-C", dir, "commit", "--allow-empty", "-m", "feat").Run(); err != nil { + t.Fatalf("commit failed: %v", err) + } + if err := exec.Command("git", "-C", dir, "checkout", trunk).Run(); err != nil { + t.Fatalf("checkout trunk failed: %v", err) + } + + // Register feature-a in the stack. + if err := cfg.SetParent("feature-a", trunk); err != nil { + t.Fatalf("SetParent failed: %v", err) + } + if err := cfg.SetPR("feature-a", 42); err != nil { + t.Fatalf("SetPR failed: %v", err) + } + + currentBranch := trunk + action := handleMergedBranch(g, cfg, "feature-a", trunk, ¤tBranch, true, s) + + if action != mergedActionDelete { + t.Errorf("expected mergedActionDelete, got %v", action) + } + + // Stack config should be cleared. + if _, err := cfg.GetParent("feature-a"); err == nil { + t.Error("expected stackParent to be removed after --yes delete") + } + if _, err := cfg.GetPR("feature-a"); err == nil { + t.Error("expected stackPR to be removed after --yes delete") + } + + // Git branch should be gone. + branchOut, _ := exec.Command("git", "-C", dir, "branch", "--list", "feature-a").Output() + if len(strings.TrimSpace(string(branchOut))) != 0 { + t.Errorf("expected feature-a to be deleted, but git branch still shows: %q", string(branchOut)) + } +} diff --git a/e2e/sync_test.go b/e2e/sync_test.go index 821bfc9..f046807 100644 --- a/e2e/sync_test.go +++ b/e2e/sync_test.go @@ -28,3 +28,14 @@ func TestSyncAcceptsDryRunFlag(t *testing.T) { t.Errorf("expected sync --help to show --dry-run flag, got:\n%s", result.Stdout) } } + +func TestSyncAcceptsYesFlag(t *testing.T) { + // Verify --yes / -y flag is recognized by sync + env := NewTestEnv(t) + env.MustRun("init") + + result := env.Run("sync", "--help") + if !result.ContainsStdout("--yes") { + t.Errorf("expected sync --help to show --yes flag, got:\n%s", result.Stdout) + } +} From f157268d3a8732ad3f14be55e4d0408bc65a1004 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Sun, 12 Jul 2026 15:40:01 -0700 Subject: [PATCH 2/2] test(sync): inject isInteractive to reliably validate --yes flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior test could not distinguish between the `yes=true` path and the existing non-interactive default: under `go test` (no TTY), `prompt.IsInteractive()` is always false, so the delete path was taken regardless of `yes`. Fix by injecting `isInteractive func() bool` into `handleMergedBranch` and passing `prompt.IsInteractive` at the call site in `runSync`. Tests now force `isInteractive=true`, proving `yes=true` overrides the interactive branch. A companion test covers the non-interactive default (`yes=false` + forced non-interactive → delete). Also replaced the ignored `git branch --list` error check with `g.BranchExists` for a cleaner and reliable assertion. --- cmd/sync.go | 9 ++-- cmd/sync_internal_test.go | 97 +++++++++++++++++++++++++++++---------- 2 files changed, 77 insertions(+), 29 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index bc737cf..67bf434 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -292,7 +292,7 @@ func runSync(cmd *cobra.Command, args []string) error { if syncDryRunFlag { fmt.Printf("%s Would handle merged branch %s\n", s.Muted("dry-run:"), s.Branch(branch)) } else { - action := handleMergedBranch(g, cfg, branch, trunk, ¤tBranch, syncYesFlag, s) + action := handleMergedBranch(g, cfg, branch, trunk, ¤tBranch, syncYesFlag, prompt.IsInteractive, s) if action == mergedActionSkip { // User chose to skip - don't collect fork points or retarget children continue @@ -441,12 +441,13 @@ const ( // handleMergedBranch prompts the user for how to handle a merged branch and executes the choice. // Returns the action taken. If the user is on the merged branch, it will checkout trunk first. // The currentBranch pointer is updated if a checkout occurs. If yes is true, the default action -// (delete) is taken without prompting. -func handleMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string, yes bool, s *style.Style) mergedAction { +// (delete) is taken without prompting. isInteractive is injected so tests can force the +// interactive code path independently of terminal detection. +func handleMergedBranch(g *git.Git, cfg *config.Config, branch, trunk string, currentBranch *string, yes bool, isInteractive func() bool, s *style.Style) mergedAction { fmt.Printf("\nBranch %s appears to be %s into %s.\n", s.Branch(branch), s.Merged("merged"), s.Branch(trunk)) // Default to delete when --yes is set or in non-interactive mode - if yes || !prompt.IsInteractive() { + if yes || !isInteractive() { return deleteMergedBranch(g, cfg, branch, trunk, currentBranch, s) } diff --git a/cmd/sync_internal_test.go b/cmd/sync_internal_test.go index 8ab3234..d236438 100644 --- a/cmd/sync_internal_test.go +++ b/cmd/sync_internal_test.go @@ -12,15 +12,67 @@ import ( "github.com/boneskull/gh-stack/internal/style" ) -// TestHandleMergedBranchYesDeletes verifies that passing yes=true causes -// handleMergedBranch to delete the branch and clear stack config without -// showing an interactive prompt, even when stdin/stdout are a TTY. -func TestHandleMergedBranchYesDeletes(t *testing.T) { +// forceInteractive returns an isInteractive func that always returns true, +// allowing tests to exercise the interactive branch of handleMergedBranch +// regardless of whether stdin/stdout are a TTY. +func forceInteractive() func() bool { return func() bool { return true } } + +// forceNonInteractive returns an isInteractive func that always returns false. +func forceNonInteractive() func() bool { return func() bool { return false } } + +// TestHandleMergedBranchYesDeletesInInteractiveMode verifies that yes=true +// causes handleMergedBranch to delete without prompting even when the terminal +// is reported as interactive. Without the yes flag this would reach the prompt, +// so forcing isInteractive=true proves yes overrides the interactive branch. +func TestHandleMergedBranchYesDeletesInInteractiveMode(t *testing.T) { + cfg, dir := setupTestRepoWithDir(t) + g := git.New(dir) + s := style.New() + + trunk := setupBranches(t, dir, cfg) + + currentBranch := trunk + // isInteractive forced to true: without yes=true the function would reach + // the interactive prompt path and hang waiting for stdin input. + action := handleMergedBranch(g, cfg, "feature-a", trunk, ¤tBranch, true, forceInteractive(), s) + + if action != mergedActionDelete { + t.Errorf("expected mergedActionDelete with yes=true, got %v", action) + } + + assertBranchDeleted(t, g, dir, "feature-a", cfg) +} + +// TestHandleMergedBranchNoYesNonInteractiveDeletes verifies the existing +// non-interactive default: yes=false with a non-interactive terminal still +// deletes (no prompt). +func TestHandleMergedBranchNoYesNonInteractiveDeletes(t *testing.T) { cfg, dir := setupTestRepoWithDir(t) g := git.New(dir) s := style.New() - // Determine trunk from the current HEAD branch. + trunk := setupBranches(t, dir, cfg) + + currentBranch := trunk + action := handleMergedBranch(g, cfg, "feature-a", trunk, ¤tBranch, false, forceNonInteractive(), s) + + if action != mergedActionDelete { + t.Errorf("expected mergedActionDelete for non-interactive+yes=false, got %v", action) + } + + assertBranchDeleted(t, g, dir, "feature-a", cfg) +} + +// setupBranches creates a trunk+feature-a repo for handleMergedBranch tests and +// returns the trunk branch name. feature-a is registered in the stack with a +// commit so git will allow force-deletion. +func setupBranches(t *testing.T, dir string, cfg interface { + SetTrunk(string) error + SetParent(string, string) error + SetPR(string, int) error +}) string { + t.Helper() + out, err := exec.Command("git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output() if err != nil { t.Fatalf("could not determine trunk branch: %v", err) @@ -30,8 +82,6 @@ func TestHandleMergedBranchYesDeletes(t *testing.T) { if err := cfg.SetTrunk(trunk); err != nil { t.Fatalf("SetTrunk failed: %v", err) } - - // Create feature-a with a commit so git can delete it. if err := exec.Command("git", "-C", dir, "checkout", "-b", "feature-a").Run(); err != nil { t.Fatalf("create branch failed: %v", err) } @@ -41,33 +91,30 @@ func TestHandleMergedBranchYesDeletes(t *testing.T) { if err := exec.Command("git", "-C", dir, "checkout", trunk).Run(); err != nil { t.Fatalf("checkout trunk failed: %v", err) } - - // Register feature-a in the stack. if err := cfg.SetParent("feature-a", trunk); err != nil { t.Fatalf("SetParent failed: %v", err) } if err := cfg.SetPR("feature-a", 42); err != nil { t.Fatalf("SetPR failed: %v", err) } + return trunk +} - currentBranch := trunk - action := handleMergedBranch(g, cfg, "feature-a", trunk, ¤tBranch, true, s) +// assertBranchDeleted checks that the git branch no longer exists and that +// stackParent / stackPR have been cleared from config. +func assertBranchDeleted(t *testing.T, g *git.Git, dir string, branch string, cfg interface { + GetParent(string) (string, error) + GetPR(string) (int, error) +}) { + t.Helper() - if action != mergedActionDelete { - t.Errorf("expected mergedActionDelete, got %v", action) + if g.BranchExists(branch) { + t.Errorf("expected branch %q to be deleted, but it still exists", branch) } - - // Stack config should be cleared. - if _, err := cfg.GetParent("feature-a"); err == nil { - t.Error("expected stackParent to be removed after --yes delete") - } - if _, err := cfg.GetPR("feature-a"); err == nil { - t.Error("expected stackPR to be removed after --yes delete") + if _, err := cfg.GetParent(branch); err == nil { + t.Errorf("expected stackParent to be removed after delete, but it is still set for %q", branch) } - - // Git branch should be gone. - branchOut, _ := exec.Command("git", "-C", dir, "branch", "--list", "feature-a").Output() - if len(strings.TrimSpace(string(branchOut))) != 0 { - t.Errorf("expected feature-a to be deleted, but git branch still shows: %q", string(branchOut)) + if _, err := cfg.GetPR(branch); err == nil { + t.Errorf("expected stackPR to be removed after delete, but it is still set for %q", branch) } }