diff --git a/cmd/wtp/cd.go b/cmd/wtp/cd.go index 043ba63..e05a705 100644 --- a/cmd/wtp/cd.go +++ b/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 diff --git a/cmd/wtp/list.go b/cmd/wtp/list.go index 775c76b..816d467 100644 --- a/cmd/wtp/list.go +++ b/cmd/wtp/list.go @@ -162,45 +162,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 diff --git a/cmd/wtp/remove.go b/cmd/wtp/remove.go index 906a6b9..91ba3ba 100644 --- a/cmd/wtp/remove.go +++ b/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 @@ -133,7 +95,7 @@ func removeCommandWithCommandExecutor( _ *cli.Command, w io.Writer, executor command.Executor, - _ string, + cwd string, worktreeName string, force, withBranch, forceBranch bool, ) error { @@ -153,6 +115,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}) @@ -189,6 +165,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, diff --git a/cmd/wtp/remove_test.go b/cmd/wtp/remove_test.go index e735916..9b8af97 100644 --- a/cmd/wtp/remove_test.go +++ b/cmd/wtp/remove_test.go @@ -3,6 +3,7 @@ package main import ( "bytes" "context" + "fmt" "os" "path/filepath" "testing" @@ -389,6 +390,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{ diff --git a/cmd/wtp/worktree_managed.go b/cmd/wtp/worktree_managed.go new file mode 100644 index 0000000..9c30b47 --- /dev/null +++ b/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 +} diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 1b71df1..2a0bd6b 100644 --- a/internal/errors/errors.go +++ b/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 ' 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) diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index 6e68680..8a06054 100644 --- a/internal/errors/errors_test.go +++ b/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