Merge pull request #33 from satococoa/codex/design-wtp-remove-behavior-for-current-wotktree

fix: prevent removing current worktree

Author: satococoa

cmd/wtp/cd.go

@@ -20,45 +20,7 @@ import (
 
 // isWorktreeManagedCd determines if a worktree is managed by wtp (for cd command)
 func isWorktreeManagedCd(worktreePath string, cfg *config.Config, mainRepoPath string, isMain bool) bool {
-	// Main worktree is always managed
-	if isMain {
-		return true
-	}
-
-	// Get base directory - use default config if config is not available
-	if cfg == nil {
-		// Create default config when none is available
-		defaultCfg := &config.Config{
-			Defaults: config.Defaults{
-				BaseDir: "../worktrees",
-			},
-		}
-		cfg = defaultCfg
-	}
-
-	baseDir := cfg.ResolveWorktreePath(mainRepoPath, "")
-	// Remove trailing slash if it exists
-	baseDir = strings.TrimSuffix(baseDir, "/")
-
-	// Check if worktree path is under base directory
-	absWorktreePath, err := filepath.Abs(worktreePath)
-	if err != nil {
-		return false
-	}
-
-	absBaseDir, err := filepath.Abs(baseDir)
-	if err != nil {
-		return false
-	}
-
-	// Check if worktree is within base directory
-	relPath, err := filepath.Rel(absBaseDir, absWorktreePath)
-	if err != nil {
-		return false
-	}
-
-	// If relative path starts with "..", it's outside base directory
-	return !strings.HasPrefix(relPath, "..")
+	return isWorktreeManagedCommon(worktreePath, cfg, mainRepoPath, isMain)
 }
 
 // NewCdCommand creates the cd command definition

cmd/wtp/list.go

@@ -163,45 +163,7 @@ func parseWorktreesFromOutput(output string) []git.Worktree {
 
 // isWorktreeManagedList determines if a worktree is managed by wtp (for list command)
 func isWorktreeManagedList(worktreePath string, cfg *config.Config, mainRepoPath string, isMain bool) bool {
-	// Main worktree is always managed
-	if isMain {
-		return true
-	}
-
-	// Get base directory - use default config if config is not available
-	if cfg == nil {
-		// Create default config when none is available
-		defaultCfg := &config.Config{
-			Defaults: config.Defaults{
-				BaseDir: "../worktrees",
-			},
-		}
-		cfg = defaultCfg
-	}
-
-	baseDir := cfg.ResolveWorktreePath(mainRepoPath, "")
-	// Remove trailing slash if it exists
-	baseDir = strings.TrimSuffix(baseDir, "/")
-
-	// Check if worktree path is under base directory
-	absWorktreePath, err := filepath.Abs(worktreePath)
-	if err != nil {
-		return false
-	}
-
-	absBaseDir, err := filepath.Abs(baseDir)
-	if err != nil {
-		return false
-	}
-
-	// Check if worktree is within base directory
-	relPath, err := filepath.Rel(absBaseDir, absWorktreePath)
-	if err != nil {
-		return false
-	}
-
-	// If relative path starts with "..", it's outside base directory
-	return !strings.HasPrefix(relPath, "..")
+	return isWorktreeManagedCommon(worktreePath, cfg, mainRepoPath, isMain)
 }
 
 // formatBranchDisplay formats branch name for display, following Git conventions

cmd/wtp/remove.go

@@ -23,45 +23,7 @@ var removeGetwd = os.Getwd
 
 // isWorktreeManaged determines if a worktree is managed by wtp
 func isWorktreeManaged(worktreePath string, cfg *config.Config, mainRepoPath string, isMain bool) bool {
-	// Main worktree is always managed
-	if isMain {
-		return true
-	}
-
-	// Get base directory - use default config if config is not available
-	if cfg == nil {
-		// Create default config when none is available
-		defaultCfg := &config.Config{
-			Defaults: config.Defaults{
-				BaseDir: "../worktrees",
-			},
-		}
-		cfg = defaultCfg
-	}
-
-	baseDir := cfg.ResolveWorktreePath(mainRepoPath, "")
-	// Remove trailing slash if it exists
-	baseDir = strings.TrimSuffix(baseDir, "/")
-
-	// Check if worktree path is under base directory
-	absWorktreePath, err := filepath.Abs(worktreePath)
-	if err != nil {
-		return false
-	}
-
-	absBaseDir, err := filepath.Abs(baseDir)
-	if err != nil {
-		return false
-	}
-
-	// Check if worktree is within base directory
-	relPath, err := filepath.Rel(absBaseDir, absWorktreePath)
-	if err != nil {
-		return false
-	}
-
-	// If relative path starts with "..", it's outside base directory
-	return !strings.HasPrefix(relPath, "..")
+	return isWorktreeManagedCommon(worktreePath, cfg, mainRepoPath, isMain)
 }
 
 // NewRemoveCommand creates the remove command definition
@@ -134,7 +96,7 @@ func removeCommandWithCommandExecutor(
 	_ *cli.Command,
 	w io.Writer,
 	executor command.Executor,
-	_ string,
+	cwd string,
 	worktreeName string,
 	force, withBranch, forceBranch bool,
 ) error {
@@ -154,6 +116,20 @@ func removeCommandWithCommandExecutor(
 		return err
 	}
 
+	absTargetPath, err := filepath.Abs(targetWorktree.Path)
+	if err != nil {
+		return errors.WorktreeRemovalFailed(targetWorktree.Path, err)
+	}
+
+	absCwd, err := filepath.Abs(cwd)
+	if err != nil {
+		return errors.DirectoryAccessFailed("access current", cwd, err)
+	}
+
+	if isPathWithin(absTargetPath, absCwd) {
+		return errors.CannotRemoveCurrentWorktree(worktreeName, absTargetPath)
+	}
+
 	// Remove worktree using CommandExecutor
 	removeCmd := command.GitWorktreeRemove(targetWorktree.Path, force)
 	result, err = executor.Execute([]command.Command{removeCmd})
@@ -190,6 +166,23 @@ func validateRemoveInput(worktreeName string, withBranch, forceBranch bool) erro
 	return nil
 }
 
+func isPathWithin(basePath, targetPath string) bool {
+	rel, err := filepath.Rel(basePath, targetPath)
+	if err != nil {
+		return false
+	}
+
+	if rel == "." || rel == "" {
+		return true
+	}
+
+	if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+		return false
+	}
+
+	return true
+}
+
 func removeBranchWithCommandExecutor(
 	w io.Writer,
 	executor command.Executor,

cmd/wtp/remove_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -390,6 +391,56 @@ func TestRemoveCommand_WorktreeNotFound_ShowsConsistentNames(t *testing.T) {
 	assert.Contains(t, err.Error(), "No worktrees found")
 }
 
+func TestRemoveCommand_FailsWhenRemovingCurrentWorktree(t *testing.T) {
+	targetPath := "/worktrees/feature/foo"
+	mockWorktreeList := fmt.Sprintf(
+		"worktree /repo\nHEAD abc123\nbranch refs/heads/main\n\n"+
+			"worktree %s\nHEAD def456\nbranch refs/heads/feature/foo\n\n",
+		targetPath,
+	)
+
+	tests := []struct {
+		name string
+		cwd  string
+	}{
+		{
+			name: "exact worktree path",
+			cwd:  targetPath,
+		},
+		{
+			name: "nested directory inside worktree",
+			cwd:  filepath.Join(targetPath, "nested"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockExec := &mockRemoveCommandExecutor{
+				results: []command.Result{
+					{
+						Output: mockWorktreeList,
+						Error:  nil,
+					},
+				},
+			}
+
+			cmd := createRemoveTestCLICommand(map[string]any{}, []string{"feature/foo"})
+			var buf bytes.Buffer
+
+			err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, tt.cwd, "feature/foo", false, false, false)
+
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "cannot remove worktree 'feature/foo'")
+			assert.Equal(t, []command.Command{
+				{
+					Name: "git",
+					Args: []string{"worktree", "list", "--porcelain"},
+				},
+			}, mockExec.executedCommands)
+		})
+	}
+}
+
 func TestRemoveCommand_ExecutionError(t *testing.T) {
 	mockExec := &mockRemoveCommandExecutor{
 		results: []command.Result{

cmd/wtp/worktree_managed.go

@@ -0,0 +1,53 @@
+package main
+
+import (
+	"path/filepath"
+	"strings"
+
+	"github.com/satococoa/wtp/internal/config"
+)
+
+// isWorktreeManagedCommon determines whether a worktree path is considered managed by wtp.
+// The logic is shared across multiple commands so that we consistently classify worktrees.
+func isWorktreeManagedCommon(worktreePath string, cfg *config.Config, mainRepoPath string, isMain bool) bool {
+	if isMain {
+		return true
+	}
+
+	// Fallback to default configuration if none is provided
+	if cfg == nil {
+		cfg = &config.Config{
+			Defaults: config.Defaults{
+				BaseDir: config.DefaultBaseDir,
+			},
+		}
+	}
+
+	baseDir := cfg.ResolveWorktreePath(mainRepoPath, "")
+	baseDir = strings.TrimSuffix(baseDir, string(filepath.Separator))
+
+	absWorktreePath, err := filepath.Abs(worktreePath)
+	if err != nil {
+		return false
+	}
+
+	absBaseDir, err := filepath.Abs(baseDir)
+	if err != nil {
+		return false
+	}
+
+	relPath, err := filepath.Rel(absBaseDir, absWorktreePath)
+	if err != nil {
+		return false
+	}
+
+	if relPath == "." || relPath == "" {
+		return true
+	}
+
+	if relPath == ".." || strings.HasPrefix(relPath, ".."+string(filepath.Separator)) {
+		return false
+	}
+
+	return true
+}

internal/errors/errors.go

@@ -165,6 +165,13 @@ func WorktreeRemovalFailed(path string, gitError error) error {
 	return errors.New(msg)
 }
 
+func CannotRemoveCurrentWorktree(worktreeName, path string) error {
+	msg := fmt.Sprintf("cannot remove worktree '%s' while you are currently inside it", worktreeName)
+	msg += fmt.Sprintf("\n\nCurrent location: %s", path)
+	msg += "\n\nTip: Run 'wtp cd @' or 'wtp cd <another-worktree>' to switch before removing."
+	return errors.New(msg)
+}
+
 func BranchRemovalFailed(branchName string, gitError error, isForced bool) error {
 	msg := fmt.Sprintf("failed to remove branch '%s'", branchName)
 

internal/errors/errors_test.go

@@ -211,6 +211,15 @@ func TestWorktreeRemovalFailed(t *testing.T) {
 	assert.Contains(t, err.Error(), "Original error:")
 }
 
+func TestCannotRemoveCurrentWorktree(t *testing.T) {
+	err := CannotRemoveCurrentWorktree("feature/foo", "/repo/.worktrees/feature/foo")
+
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "cannot remove worktree 'feature/foo'")
+	assert.Contains(t, err.Error(), "Current location: /repo/.worktrees/feature/foo")
+	assert.Contains(t, err.Error(), "wtp cd @")
+}
+
 func TestBranchRemovalFailed(t *testing.T) {
 	tests := []struct {
 		name       string