Harden destructive worktree workflows

README.md

@@ -112,8 +112,12 @@ bonsai push feat/search
 bonsai push --pr
 bonsai push --web
 bonsai push --pr --remove
+bonsai push --pr --remove --yes
 ```
 
+`--remove` asks before deleting the worktree. Add `--yes` only when that
+removal has already been approved in an automated workflow.
+
 ### `bonsai clean`
 
 Open an interactive picker for merged, stale, or otherwise removable worktrees.

cmd/list.go

@@ -105,6 +105,7 @@ func runList(cmd *cobra.Command, args []string) error {
 					wt.PRURL = pr.URL
 					wt.PRNumber = pr.Number
 					wt.PRHeadOID = pr.HeadRefOID
+					reconcileMergedPRStatus(wt)
 				} else if errors.Is(err, github.ErrNoPR) {
 					wt.PRStatus = "none"
 				} else {
@@ -134,7 +135,7 @@ func runList(cmd *cobra.Command, args []string) error {
 	if filterNoPR {
 		var filtered []*git.Worktree
 		for _, wt := range worktrees {
-			if wt.IsMain || wt.PRStatus == "none" || wt.PRStatus == "unknown" {
+			if matchesNoPRFilter(wt) {
 				filtered = append(filtered, wt)
 			}
 		}
@@ -155,6 +156,21 @@ func runList(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+func matchesNoPRFilter(wt *git.Worktree) bool {
+	return wt.IsMain || wt.PRStatus == "none"
+}
+
+func reconcileMergedPRStatus(wt *git.Worktree) {
+	// Squash and rebase merges do not make the original branch commits
+	// ancestors of the base branch. If GitHub confirms that this exact HEAD was
+	// the merged PR head, it is not unpushed work and should not get a warning.
+	if wt.PRStatus == "merged" && wt.PRHeadOID != "" && wt.HEAD == wt.PRHeadOID {
+		wt.HasUnpushed = false
+		wt.UnpushedCommits = 0
+		wt.UnpushedKnown = true
+	}
+}
+
 // jsonWorktree is the JSON shape emitted by `bonsai list --json`.
 type jsonWorktree struct {
 	Number             int    `json:"number,omitempty"`

cmd/push.go

@@ -23,7 +23,8 @@ If a path or branch name is provided, bonsai will find the matching
 worktree. If omitted, the current working directory is used.
 
 After pushing, you can optionally open a GitHub PR via gh CLI,
-and then remove the worktree.`,
+and then remove the worktree. Removal requires confirmation unless --yes is
+provided together with --remove.`,
 	Args: cobra.MaximumNArgs(1),
 	RunE: runPush,
 }
@@ -33,6 +34,7 @@ func init() {
 	pushCmd.Flags().Bool("pr", false, "open a PR after pushing")
 	pushCmd.Flags().Bool("web", false, "open PR creation in browser (implies --pr)")
 	pushCmd.Flags().BoolP("remove", "r", false, "remove worktree after push/PR")
+	pushCmd.Flags().BoolP("yes", "y", false, "confirm worktree removal without prompting (requires --remove)")
 	pushCmd.Flags().Bool("dry-run", false, "show what would happen without doing it")
 }
 
@@ -45,7 +47,11 @@ func runPush(cmd *cobra.Command, args []string) error {
 	openPR, _ := cmd.Flags().GetBool("pr")
 	web, _ := cmd.Flags().GetBool("web")
 	remove, _ := cmd.Flags().GetBool("remove")
+	autoYes, _ := cmd.Flags().GetBool("yes")
 	dryRun, _ := cmd.Flags().GetBool("dry-run")
+	if autoYes && !remove {
+		return fmt.Errorf("--yes requires --remove")
+	}
 	if web {
 		openPR = true
 	}
@@ -126,6 +132,17 @@ func runPush(cmd *cobra.Command, args []string) error {
 
 	// Remove
 	if remove {
+		approved, quit := approvePushRemoval(autoYes, func() (bool, bool) {
+			return confirmOrQuit(fmt.Sprintf("  Remove worktree %s? [y/N/q] ", wt.Path))
+		})
+		if !approved {
+			if quit {
+				fmt.Println("  removal cancelled; worktree kept")
+			} else {
+				fmt.Println("  worktree kept")
+			}
+			return nil
+		}
 		if wt.HasUnpushed {
 			// After push, re-check — but we just pushed, so should be clear.
 			// Re-enrich to be safe.
@@ -141,6 +158,13 @@ func runPush(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+func approvePushRemoval(autoYes bool, prompt func() (yes bool, quit bool)) (yes bool, quit bool) {
+	if autoYes {
+		return true, false
+	}
+	return prompt()
+}
+
 // extractTicket returns the first ticket ID found in branch using the configured
 // regexp pattern. Returns "" if pattern is empty or no match.
 func extractTicket(branch, pattern string) string {

cmd/rm.go

@@ -54,9 +54,9 @@ func runRm(cmd *cobra.Command, args []string) error {
 	var targets []*git.Worktree
 	seen := map[int]bool{}
 	for _, a := range args {
-		n, err := strconv.Atoi(strings.TrimSpace(a))
-		if err != nil || n < 1 || n > len(added) {
-			return fmt.Errorf("invalid worktree number %q (valid range: 1–%d)", a, len(added))
+		n, err := parseWorktreeNumber(a, len(added))
+		if err != nil {
+			return err
 		}
 		if seen[n] {
 			continue
@@ -114,3 +114,11 @@ func runRm(cmd *cobra.Command, args []string) error {
 	}
 	return nil
 }
+
+func parseWorktreeNumber(value string, total int) (int, error) {
+	n, err := strconv.Atoi(strings.TrimSpace(value))
+	if err != nil || n < 1 || n > total {
+		return 0, fmt.Errorf("invalid worktree number %q (valid range: 1-%d)", value, total)
+	}
+	return n, nil
+}

cmd/safety_hardening_test.go

@@ -0,0 +1,94 @@
+package cmd
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/sauravpanda/bonsai/internal/git"
+)
+
+func TestMatchesNoPRFilterExcludesUnknownStatus(t *testing.T) {
+	tests := []struct {
+		name string
+		wt   *git.Worktree
+		want bool
+	}{
+		{name: "main", wt: &git.Worktree{IsMain: true, PRStatus: "unknown"}, want: true},
+		{name: "no PR", wt: &git.Worktree{PRStatus: "none"}, want: true},
+		{name: "unknown", wt: &git.Worktree{PRStatus: "unknown"}, want: false},
+		{name: "open", wt: &git.Worktree{PRStatus: "open"}, want: false},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := matchesNoPRFilter(test.wt); got != test.want {
+				t.Fatalf("matchesNoPRFilter() = %t, want %t", got, test.want)
+			}
+		})
+	}
+}
+
+func TestReconcileMergedPRStatusClearsFalseUnpushedWarning(t *testing.T) {
+	wt := &git.Worktree{
+		HEAD:            "abc123",
+		PRStatus:        "merged",
+		PRHeadOID:       "abc123",
+		HasUnpushed:     true,
+		UnpushedCommits: 3,
+	}
+	reconcileMergedPRStatus(wt)
+	if wt.HasUnpushed || wt.UnpushedCommits != 0 || !wt.UnpushedKnown {
+		t.Fatalf("matching merged PR head was not reconciled: %+v", wt)
+	}
+}
+
+func TestReconcileMergedPRStatusKeepsCommitsAfterPRHead(t *testing.T) {
+	wt := &git.Worktree{
+		HEAD:            "new-work",
+		PRStatus:        "merged",
+		PRHeadOID:       "merged-head",
+		HasUnpushed:     true,
+		UnpushedCommits: 1,
+	}
+	reconcileMergedPRStatus(wt)
+	if !wt.HasUnpushed || wt.UnpushedCommits != 1 {
+		t.Fatalf("post-merge work was incorrectly cleared: %+v", wt)
+	}
+}
+
+func TestParseWorktreeNumberUsesPortableRange(t *testing.T) {
+	if got, err := parseWorktreeNumber("2", 3); err != nil || got != 2 {
+		t.Fatalf("parseWorktreeNumber() = %d, %v", got, err)
+	}
+	_, err := parseWorktreeNumber("4", 3)
+	if err == nil || !strings.Contains(err.Error(), "1-3") {
+		t.Fatalf("expected ASCII range in error, got %v", err)
+	}
+	if strings.Contains(err.Error(), "–") {
+		t.Fatalf("error contains a non-ASCII en dash: %v", err)
+	}
+}
+
+func TestApprovePushRemoval(t *testing.T) {
+	prompted := false
+	yes, quit := approvePushRemoval(true, func() (bool, bool) {
+		prompted = true
+		return false, false
+	})
+	if !yes || quit || prompted {
+		t.Fatalf("--yes should approve without prompting: yes=%t quit=%t prompted=%t", yes, quit, prompted)
+	}
+
+	yes, quit = approvePushRemoval(false, func() (bool, bool) {
+		return false, true
+	})
+	if yes || !quit {
+		t.Fatalf("manual cancellation was not preserved: yes=%t quit=%t", yes, quit)
+	}
+}
+
+func TestAbortFailedSyncReportsAbortFailure(t *testing.T) {
+	err := abortFailedSync(t.TempDir(), false)
+	if err == nil || !strings.Contains(err.Error(), "git rebase --abort") {
+		t.Fatalf("expected actionable abort failure, got %v", err)
+	}
+}

cmd/sync.go

@@ -122,11 +122,10 @@ func runSync(cmd *cobra.Command, args []string) error {
 					fmt.Printf("    %s\n", line)
 				}
 			}
-			// Abort rebase on conflict so the worktree is not left in a broken state.
-			if !useMerge {
-				exec.Command("git", "-C", wt.Path, "rebase", "--abort").Run() //nolint:errcheck
-			} else {
-				exec.Command("git", "-C", wt.Path, "merge", "--abort").Run() //nolint:errcheck
+			// Abort on conflict so the worktree is not left in a broken state.
+			if abortErr := abortFailedSync(wt.Path, useMerge); abortErr != nil {
+				fmt.Fprintf(os.Stderr, "    %s\n",
+					warnStyle.Render("warning: could not abort cleanly: "+abortErr.Error()))
 			}
 			failed++
 		} else {
@@ -148,3 +147,19 @@ func runSync(cmd *cobra.Command, args []string) error {
 
 	return nil
 }
+
+func abortFailedSync(path string, useMerge bool) error {
+	operation := "rebase"
+	if useMerge {
+		operation = "merge"
+	}
+	out, err := exec.Command("git", "-C", path, operation, "--abort").CombinedOutput()
+	if err == nil {
+		return nil
+	}
+	detail := strings.TrimSpace(string(out))
+	if detail == "" {
+		return fmt.Errorf("git %s --abort: %w", operation, err)
+	}
+	return fmt.Errorf("git %s --abort: %w: %s", operation, err, detail)
+}