diff --git a/README.md b/README.md index 2827983..1b5e900 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ functionality with automated setup, branch tracking, and project-specific hooks. solution:** `wtp add feature/auth` wtp automatically generates sensible paths based on branch names. Your -`feature/auth` branch goes to `../worktrees/feature/auth` - no redundant typing, +`feature/auth` branch goes to `../worktrees//feature/auth` - no redundant typing, no path errors. ### 🧹 Clean Branch Management @@ -127,20 +127,20 @@ sudo mv wtp /usr/local/bin/ # or add to PATH ```bash # Create worktree from existing branch (local or remote) -# β†’ Creates worktree at ../worktrees/feature/auth +# β†’ Creates worktree at ../worktrees//feature/auth # Automatically tracks remote branch if not found locally wtp add feature/auth # Create worktree with new branch -# β†’ Creates worktree at ../worktrees/feature/new-feature +# β†’ Creates worktree at ../worktrees//feature/new-feature wtp add -b feature/new-feature # Create new branch from specific commit -# β†’ Creates worktree at ../worktrees/hotfix/urgent +# β†’ Creates worktree at ../worktrees//hotfix/urgent wtp add -b hotfix/urgent abc1234 # Create new branch tracking a different remote branch -# β†’ Creates worktree at ../worktrees/feature/test with branch tracking origin/main +# β†’ Creates worktree at ../worktrees//feature/test with branch tracking origin/main wtp add -b feature/test origin/main # Remote branch handling examples: @@ -188,7 +188,8 @@ wtp uses `.wtp.yml` for project-specific configuration: version: "1.0" defaults: # Base directory for worktrees (relative to project root) - base_dir: "../worktrees" + # ${WTP_REPO_BASENAME} expands to the repository directory name + base_dir: "../worktrees/${WTP_REPO_BASENAME}" hooks: post_create: @@ -213,6 +214,17 @@ hooks: work_dir: "." ``` +The `${WTP_REPO_BASENAME}` placeholder expands to the repository's directory +name when resolving paths, ensuring zero-config isolation between different +repositories. You can combine it with additional path segments as needed. + +> **Breaking change (vNEXT):** If you relied on the previous implicit default +> of `../worktrees` without a `.wtp.yml`, existing worktrees will now appear +> unmanaged because the new default expects +> `../worktrees/${WTP_REPO_BASENAME}`. Add a `.wtp.yml` with +> `base_dir: "../worktrees"` (or reorganize your worktrees) before upgrading +> to keep the legacy layout working. + ### Copy Hooks: Main Worktree Reference Copy hooks are designed to help you bootstrap new worktrees using files from @@ -326,7 +338,7 @@ evaluates `wtp shell-init ` once for your sessionβ€”tab completion and ## Worktree Structure -With the default configuration (`base_dir: "../worktrees"`): +With the default configuration (`base_dir: "../worktrees/${WTP_REPO_BASENAME}"`): ``` / @@ -335,12 +347,13 @@ With the default configuration (`base_dir: "../worktrees"`): └── src/ ../worktrees/ -β”œβ”€β”€ main/ -β”œβ”€β”€ feature/ -β”‚ β”œβ”€β”€ auth/ # wtp add feature/auth -β”‚ └── payment/ # wtp add feature/payment -└── hotfix/ - └── bug-123/ # wtp add hotfix/bug-123 +└── / + β”œβ”€β”€ main/ + β”œβ”€β”€ feature/ + β”‚ β”œβ”€β”€ auth/ # wtp add feature/auth + β”‚ └── payment/ # wtp add feature/payment + └── hotfix/ + └── bug-123/ # wtp add hotfix/bug-123 ``` Branch names with slashes are preserved as directory structure, automatically diff --git a/cmd/wtp/cd.go b/cmd/wtp/cd.go index e05a705..8cd2dfe 100644 --- a/cmd/wtp/cd.go +++ b/cmd/wtp/cd.go @@ -62,8 +62,10 @@ func cdToWorktree(_ context.Context, cmd *cli.Command) error { return errors.NotInGitRepository() } + rootCmd := cmd.Root() + // Get the writer from cli.Command - w := cmd.Root().Writer + w := rootCmd.Writer if w == nil { w = os.Stdout } @@ -74,7 +76,7 @@ func cdToWorktree(_ context.Context, cmd *cli.Command) error { } func cdCommandWithCommandExecutor( - _ *cli.Command, + cmd *cli.Command, w io.Writer, executor command.Executor, _ string, @@ -93,6 +95,25 @@ func cdCommandWithCommandExecutor( // Find the main worktree path mainWorktreePath := findMainWorktreePath(worktrees) + errWriter := io.Discard + if cmd != nil { + if root := cmd.Root(); root != nil && root.ErrWriter != nil { + errWriter = root.ErrWriter + } else if root != nil && root.ErrWriter == nil { + errWriter = os.Stderr + } + } + + if mainWorktreePath != "" { + cfgForWarning, cfgErr := config.LoadConfig(mainWorktreePath) + if cfgErr != nil { + cfgForWarning = &config.Config{ + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, + } + } + maybeWarnLegacyWorktreeLayout(errWriter, mainWorktreePath, cfgForWarning, worktrees) + } + // Find the worktree using multiple resolution strategies targetPath := resolveCdWorktreePath(worktreeName, worktrees, mainWorktreePath) @@ -240,10 +261,7 @@ func getWorktreeNameFromPathCd(worktreePath string, cfg *config.Config, mainRepo } // Get base_dir path - baseDir := cfg.Defaults.BaseDir - if !filepath.IsAbs(baseDir) { - baseDir = filepath.Join(mainRepoPath, baseDir) - } + baseDir := cfg.ResolveWorktreePath(mainRepoPath, "") // Calculate relative path from base_dir relPath, err := filepath.Rel(baseDir, worktreePath) diff --git a/cmd/wtp/cd_test.go b/cmd/wtp/cd_test.go index 1513d44..b3f804f 100644 --- a/cmd/wtp/cd_test.go +++ b/cmd/wtp/cd_test.go @@ -20,7 +20,7 @@ func TestCdCommand_AlwaysOutputsAbsolutePath(t *testing.T) { HEAD abc123 branch refs/heads/main -worktree /Users/dev/project/worktrees/feature/auth +worktree /Users/dev/project/worktrees/main/feature/auth HEAD def456 branch refs/heads/feature/auth @@ -41,13 +41,13 @@ branch refs/heads/feature/auth { name: "feature worktree by branch name", worktreeName: "feature/auth", - expectedPath: "/Users/dev/project/worktrees/feature/auth", + expectedPath: "/Users/dev/project/worktrees/main/feature/auth", shouldSucceed: true, }, { name: "feature worktree by directory name", worktreeName: "auth", - expectedPath: "/Users/dev/project/worktrees/feature/auth", + expectedPath: "/Users/dev/project/worktrees/main/feature/auth", shouldSucceed: true, // Directory-based resolution works as expected }, { diff --git a/cmd/wtp/init.go b/cmd/wtp/init.go index e314efa..78475a3 100644 --- a/cmd/wtp/init.go +++ b/cmd/wtp/init.go @@ -16,6 +16,7 @@ const configFileMode = 0o600 // Variable to allow mocking in tests var osGetwd = os.Getwd +var writeFile = os.WriteFile // NewInitCommand creates the init command definition func NewInitCommand() *cli.Command { @@ -54,7 +55,8 @@ version: "1.0" # Default settings for worktrees defaults: # Base directory for worktrees (relative to repository root) - base_dir: ../worktrees + # ${WTP_REPO_BASENAME} expands to the repository directory name + base_dir: ../worktrees/${WTP_REPO_BASENAME} # Hooks that run after creating a worktree hooks: @@ -87,7 +89,7 @@ hooks: ` // Write configuration file with comments - if err := os.WriteFile(configPath, []byte(configContent), configFileMode); err != nil { + if err := writeFile(configPath, []byte(configContent), configFileMode); err != nil { return errors.DirectoryAccessFailed("create configuration file", configPath, err) } diff --git a/cmd/wtp/init_test.go b/cmd/wtp/init_test.go index d53b0b7..48cf1d2 100644 --- a/cmd/wtp/init_test.go +++ b/cmd/wtp/init_test.go @@ -138,7 +138,7 @@ func TestInitCommand_Success(t *testing.T) { // Check for required sections assert.Contains(t, contentStr, "version: \"1.0\"") assert.Contains(t, contentStr, "defaults:") - assert.Contains(t, contentStr, "base_dir: ../worktrees") + assert.Contains(t, contentStr, "base_dir: ../worktrees/${WTP_REPO_BASENAME}") assert.Contains(t, contentStr, "hooks:") assert.Contains(t, contentStr, "post_create:") @@ -198,6 +198,12 @@ func TestInitCommand_WriteFileError(t *testing.T) { cmd := NewInitCommand() ctx := context.Background() + originalWriteFile := writeFile + writeFile = func(string, []byte, os.FileMode) error { + return assert.AnError + } + defer func() { writeFile = originalWriteFile }() + err = cmd.Action(ctx, &cli.Command{}) assert.Error(t, err) diff --git a/cmd/wtp/legacy_warning.go b/cmd/wtp/legacy_warning.go new file mode 100644 index 0000000..be4fa73 --- /dev/null +++ b/cmd/wtp/legacy_warning.go @@ -0,0 +1,141 @@ +package main + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/satococoa/wtp/internal/config" + "github.com/satococoa/wtp/internal/git" +) + +const legacyWarningExampleLimit = 3 + +type legacyWorktreeMigration struct { + currentRel string + suggestedRel string +} + +func maybeWarnLegacyWorktreeLayout( + w io.Writer, + mainRepoPath string, + cfg *config.Config, + worktrees []git.Worktree, +) { + if w == nil { + w = os.Stderr + } + + if cfg == nil || mainRepoPath == "" || len(worktrees) == 0 { + return + } + + if hasConfigFile(mainRepoPath) { + return + } + + migrations := detectLegacyWorktreeMigrations(mainRepoPath, cfg, worktrees) + if len(migrations) == 0 { + return + } + + repoBase := filepath.Base(mainRepoPath) + fmt.Fprintln(w, "⚠️ Legacy worktree layout detected.") + fmt.Fprintf(w, " wtp now expects worktrees under '../worktrees/%s/...'\n", repoBase) + fmt.Fprintln(w, " Move existing worktrees to the new layout (run from the repository root):") + + limit := len(migrations) + if limit > legacyWarningExampleLimit { + limit = legacyWarningExampleLimit + } + for i := 0; i < limit; i++ { + migration := migrations[i] + fmt.Fprintf(w, " git worktree move %s %s\n", migration.currentRel, migration.suggestedRel) + } + + if len(migrations) > legacyWarningExampleLimit { + fmt.Fprintf(w, " ... and %d more\n", len(migrations)-legacyWarningExampleLimit) + } + + fmt.Fprintln(w, " (Alternatively, run 'wtp init' and set defaults.base_dir to keep a custom layout.)") + fmt.Fprintln(w) +} + +func detectLegacyWorktreeMigrations( + mainRepoPath string, + cfg *config.Config, + worktrees []git.Worktree, +) []legacyWorktreeMigration { + if cfg == nil || mainRepoPath == "" { + return nil + } + + mainRepoPath = filepath.Clean(mainRepoPath) + + newBaseDir := filepath.Clean(cfg.ResolveWorktreePath(mainRepoPath, "")) + legacyBaseDir := filepath.Clean(filepath.Join(filepath.Dir(mainRepoPath), "worktrees")) + repoBase := filepath.Base(mainRepoPath) + + if legacyBaseDir == newBaseDir { + return nil + } + + var migrations []legacyWorktreeMigration + for _, wt := range worktrees { + if wt.IsMain { + continue + } + + worktreePath := filepath.Clean(wt.Path) + + if strings.HasPrefix(worktreePath, newBaseDir+string(os.PathSeparator)) || + worktreePath == newBaseDir { + continue + } + + if !strings.HasPrefix(worktreePath, legacyBaseDir+string(os.PathSeparator)) { + continue + } + + legacyRel, err := filepath.Rel(legacyBaseDir, worktreePath) + if err != nil || legacyRel == "." { + continue + } + + if strings.HasPrefix(legacyRel, repoBase+string(os.PathSeparator)) { + // Already under the new structure (worktrees//...) + continue + } + + suggestedPath := filepath.Join(legacyBaseDir, repoBase, legacyRel) + + currentRel := relativeToRepo(mainRepoPath, worktreePath) + suggestedRel := relativeToRepo(mainRepoPath, suggestedPath) + + migrations = append(migrations, legacyWorktreeMigration{ + currentRel: currentRel, + suggestedRel: suggestedRel, + }) + } + + return migrations +} + +func relativeToRepo(mainRepoPath, targetPath string) string { + rel, err := filepath.Rel(mainRepoPath, targetPath) + if err != nil { + return targetPath + } + if !strings.HasPrefix(rel, "..") { + rel = filepath.Join(".", rel) + } + return filepath.Clean(rel) +} + +func hasConfigFile(mainRepoPath string) bool { + configPath := filepath.Join(mainRepoPath, config.ConfigFileName) + _, err := os.Stat(configPath) + return err == nil +} diff --git a/cmd/wtp/legacy_warning_test.go b/cmd/wtp/legacy_warning_test.go new file mode 100644 index 0000000..e34973d --- /dev/null +++ b/cmd/wtp/legacy_warning_test.go @@ -0,0 +1,108 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/satococoa/wtp/internal/config" + "github.com/satococoa/wtp/internal/git" +) + +func TestDetectLegacyWorktreeMigrations(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + requireNoErr(t, os.MkdirAll(mainRepoPath, 0o755)) + + cfg := &config.Config{ + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, + } + + legacyWorktreePath := filepath.Join(filepath.Dir(mainRepoPath), "worktrees", "feature", "foo") + modernWorktreePath := filepath.Join( + filepath.Dir(mainRepoPath), + "worktrees", + filepath.Base(mainRepoPath), + "feature", + "bar", + ) + + worktrees := []git.Worktree{ + {Path: mainRepoPath, IsMain: true}, + {Path: legacyWorktreePath}, + {Path: modernWorktreePath}, + } + + migrations := detectLegacyWorktreeMigrations(mainRepoPath, cfg, worktrees) + + assert.Len(t, migrations, 1) + expectedCurrent := filepath.Join("..", "worktrees", "feature", "foo") + expectedSuggested := filepath.Join("..", "worktrees", filepath.Base(mainRepoPath), "feature", "foo") + + assert.Equal(t, filepath.Clean(expectedCurrent), migrations[0].currentRel) + assert.Equal(t, filepath.Clean(expectedSuggested), migrations[0].suggestedRel) +} + +func TestMaybeWarnLegacyWorktreeLayout_EmitsWarningWithoutConfig(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + requireNoErr(t, os.MkdirAll(mainRepoPath, 0o755)) + + cfg := &config.Config{ + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, + } + + legacyWorktree := filepath.Join(filepath.Dir(mainRepoPath), "worktrees", "feature", "foo") + worktrees := []git.Worktree{ + {Path: mainRepoPath, IsMain: true}, + {Path: legacyWorktree}, + } + + var buf bytes.Buffer + maybeWarnLegacyWorktreeLayout(&buf, mainRepoPath, cfg, worktrees) + + output := buf.String() + assert.NotEmpty(t, output) + assert.Contains(t, output, "Legacy worktree layout detected") + + moveCommand := strings.Join([]string{ + "git worktree move", + filepath.Join("..", "worktrees", "feature", "foo"), + filepath.Join("..", "worktrees", filepath.Base(mainRepoPath), "feature", "foo"), + }, " ") + assert.Contains(t, output, moveCommand) +} + +func TestMaybeWarnLegacyWorktreeLayout_SuppressedWhenConfigPresent(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + requireNoErr(t, os.MkdirAll(mainRepoPath, 0o755)) + + configPath := filepath.Join(mainRepoPath, config.ConfigFileName) + requireNoErr(t, os.WriteFile(configPath, []byte("version: 1.0\n"), 0o600)) + + cfg := &config.Config{ + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, + } + + worktrees := []git.Worktree{ + {Path: mainRepoPath, IsMain: true}, + {Path: filepath.Join(filepath.Dir(mainRepoPath), "worktrees", "feature", "foo")}, + } + + var buf bytes.Buffer + maybeWarnLegacyWorktreeLayout(&buf, mainRepoPath, cfg, worktrees) + + assert.Empty(t, buf.String()) +} + +func requireNoErr(t *testing.T, err error) { + t.Helper() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/cmd/wtp/list.go b/cmd/wtp/list.go index 5a73690..43653fa 100644 --- a/cmd/wtp/list.go +++ b/cmd/wtp/list.go @@ -103,8 +103,10 @@ func listCommand(_ context.Context, cmd *cli.Command) error { return errors.GitCommandFailed("get main worktree path", err.Error()) } + rootCmd := cmd.Root() + // Get the writer from cli.Command - w := cmd.Root().Writer + w := rootCmd.Writer if w == nil { w = os.Stdout } @@ -124,7 +126,7 @@ func listCommand(_ context.Context, cmd *cli.Command) error { } func listCommandWithCommandExecutor( - _ *cli.Command, w io.Writer, executor command.Executor, cfg *config.Config, mainRepoPath string, quiet bool, + cmd *cli.Command, w io.Writer, executor command.Executor, cfg *config.Config, mainRepoPath string, quiet bool, opts listDisplayOptions, ) error { // Get current working directory @@ -150,6 +152,16 @@ func listCommandWithCommandExecutor( return nil } + errWriter := io.Discard + if cmd != nil { + if root := cmd.Root(); root != nil && root.ErrWriter != nil { + errWriter = root.ErrWriter + } else if root != nil && root.ErrWriter == nil { + errWriter = os.Stderr + } + } + maybeWarnLegacyWorktreeLayout(errWriter, mainRepoPath, cfg, worktrees) + // Display worktrees if quiet { displayWorktreesQuiet(w, worktrees, cfg, mainRepoPath) diff --git a/cmd/wtp/list_test.go b/cmd/wtp/list_test.go index 62079f5..0c71333 100644 --- a/cmd/wtp/list_test.go +++ b/cmd/wtp/list_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "os" "strings" "testing" @@ -134,9 +135,10 @@ func TestListCommand_CommandConstruction(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -211,9 +213,10 @@ func TestListCommand_Output(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -264,9 +267,10 @@ func TestListCommand_ExecutionError(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -294,9 +298,10 @@ func TestListCommand_NoWorktrees(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -363,9 +368,10 @@ func TestListCommand_InternationalCharacters(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -439,9 +445,10 @@ func TestListCommand_LongPaths(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -497,9 +504,10 @@ branch refs/heads/feature/test var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -536,9 +544,10 @@ func TestListCommand_HeaderFormatting(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -647,9 +656,10 @@ branch refs/heads/feature/awesome var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -783,6 +793,7 @@ branch refs/heads/hoge var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard // Extract main repo path from mock output lines := strings.Split(tt.mockOutput, "\n") @@ -792,7 +803,7 @@ branch refs/heads/hoge } // Special handling for the base_dir test case - baseDir := "../worktrees" + baseDir := config.DefaultBaseDir if tt.name == "paths relative to main worktree when in subdirectory" { baseDir = ".worktrees" mainRepoPath = "/Users/satoshi/dev/src/github.com/satococoa/giselle" @@ -889,9 +900,10 @@ branch refs/heads/feature/long-branch-name-that-might-also-be-truncated var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ - Defaults: config.Defaults{BaseDir: "../worktrees"}, + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, } err := listCommandWithCommandExecutor( cmd, @@ -948,6 +960,7 @@ branch refs/heads/feature/long var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{Defaults: config.Defaults{BaseDir: ".worktrees"}} err := listCommandWithCommandExecutor( @@ -986,6 +999,7 @@ branch refs/heads/feature/test var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{Defaults: config.Defaults{BaseDir: ".worktrees"}} err := listCommandWithCommandExecutor( @@ -1024,6 +1038,7 @@ branch refs/heads/feature/test var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{Defaults: config.Defaults{BaseDir: ".worktrees"}} opts := defaultListDisplayOptionsForTests() @@ -1065,6 +1080,7 @@ branch refs/heads/feature/test var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{Defaults: config.Defaults{BaseDir: ".worktrees"}} opts := defaultListDisplayOptionsForTests() @@ -1107,6 +1123,7 @@ branch refs/heads/feature/long var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{Defaults: config.Defaults{BaseDir: ".worktrees"}} opts := defaultListDisplayOptionsForTests() @@ -1141,6 +1158,7 @@ func TestListCommand_QuietMode_SingleWorktree(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "../worktrees"}, @@ -1189,6 +1207,7 @@ branch refs/heads/feature/another var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ Defaults: config.Defaults{BaseDir: ".worktrees"}, @@ -1229,6 +1248,7 @@ func TestListCommand_QuietMode_NoWorktrees(t *testing.T) { var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "../worktrees"}, @@ -1269,6 +1289,7 @@ detached var buf bytes.Buffer cmd := &cli.Command{} + cmd.ErrWriter = io.Discard cfg := &config.Config{ Defaults: config.Defaults{BaseDir: ".worktrees"}, diff --git a/cmd/wtp/remove.go b/cmd/wtp/remove.go index b86e1cf..0447d37 100644 --- a/cmd/wtp/remove.go +++ b/cmd/wtp/remove.go @@ -59,8 +59,10 @@ func NewRemoveCommand() *cli.Command { } func removeCommand(_ context.Context, cmd *cli.Command) error { + rootCmd := cmd.Root() + // Get the writer from cli.Command - w := cmd.Root().Writer + w := rootCmd.Writer if w == nil { w = os.Stdout } @@ -93,7 +95,7 @@ func removeCommand(_ context.Context, cmd *cli.Command) error { } func removeCommandWithCommandExecutor( - _ *cli.Command, + cmd *cli.Command, w io.Writer, executor command.Executor, cwd string, @@ -110,15 +112,71 @@ func removeCommandWithCommandExecutor( // Parse worktrees from command output worktrees := parseWorktreesFromOutput(result.Results[0].Output) - // Find target worktree - targetWorktree, err := findTargetWorktreeFromList(worktrees, worktreeName) + notifyLegacyLayout(cmd, worktrees) + + targetWorktree, err := locateTargetWorktree(worktrees, worktreeName) if err != nil { return err } - absTargetPath, err := filepath.Abs(targetWorktree.Path) + if err := guardAgainstRemovingCurrentWorktree(targetWorktree.Path, cwd, worktreeName); err != nil { + return err + } + + if err := executeWorktreeRemoval(executor, targetWorktree.Path, force); err != nil { + return err + } + + fmt.Fprintf(w, "Removed worktree '%s' at %s\n", worktreeName, targetWorktree.Path) + + if withBranch && targetWorktree.Branch != "" { + if err := removeBranchWithCommandExecutor(w, executor, targetWorktree.Branch, forceBranch); err != nil { + return err + } + } + + return nil +} + +func notifyLegacyLayout(cmd *cli.Command, worktrees []git.Worktree) { + errWriter := io.Discard + if cmd != nil { + if root := cmd.Root(); root != nil && root.ErrWriter != nil { + errWriter = root.ErrWriter + } else if root != nil && root.ErrWriter == nil { + errWriter = os.Stderr + } + } + + mainRepoPath := findMainWorktreePath(worktrees) + if mainRepoPath == "" { + return + } + + cfgForWarning, cfgErr := config.LoadConfig(mainRepoPath) + if cfgErr != nil { + cfgForWarning = &config.Config{ + Defaults: config.Defaults{BaseDir: config.DefaultBaseDir}, + } + } + maybeWarnLegacyWorktreeLayout(errWriter, mainRepoPath, cfgForWarning, worktrees) +} + +func locateTargetWorktree(worktrees []git.Worktree, worktreeName string) (*git.Worktree, error) { + targetWorktree, err := findTargetWorktreeFromList(worktrees, worktreeName) + if err != nil { + return nil, err + } + if targetWorktree == nil { + return nil, errors.WorktreeNotFound(worktreeName, nil) + } + return targetWorktree, nil +} + +func guardAgainstRemovingCurrentWorktree(targetPath, cwd, worktreeName string) error { + absTargetPath, err := filepath.Abs(targetPath) if err != nil { - return errors.WorktreeRemovalFailed(targetWorktree.Path, err) + return errors.WorktreeRemovalFailed(targetPath, err) } absCwd, err := filepath.Abs(cwd) @@ -129,30 +187,23 @@ func removeCommandWithCommandExecutor( if isPathWithin(absTargetPath, absCwd) { return errors.CannotRemoveCurrentWorktree(worktreeName, absTargetPath) } + return nil +} - // Remove worktree using CommandExecutor - removeCmd := command.GitWorktreeRemove(targetWorktree.Path, force) - result, err = executor.Execute([]command.Command{removeCmd}) +func executeWorktreeRemoval(executor command.Executor, worktreePath string, force bool) error { + removeCmd := command.GitWorktreeRemove(worktreePath, force) + result, err := executor.Execute([]command.Command{removeCmd}) if err != nil { - return errors.WorktreeRemovalFailed(targetWorktree.Path, err) + return errors.WorktreeRemovalFailed(worktreePath, err) } if len(result.Results) > 0 && result.Results[0].Error != nil { gitOutput := result.Results[0].Output if gitOutput != "" { combinedError := fmt.Errorf("%v: %s", result.Results[0].Error, gitOutput) - return errors.WorktreeRemovalFailed(targetWorktree.Path, combinedError) - } - return errors.WorktreeRemovalFailed(targetWorktree.Path, result.Results[0].Error) - } - fmt.Fprintf(w, "Removed worktree '%s' at %s\n", worktreeName, targetWorktree.Path) - - // Remove branch if requested - if withBranch && targetWorktree.Branch != "" { - if err := removeBranchWithCommandExecutor(w, executor, targetWorktree.Branch, forceBranch); err != nil { - return err + return errors.WorktreeRemovalFailed(worktreePath, combinedError) } + return errors.WorktreeRemovalFailed(worktreePath, result.Results[0].Error) } - return nil } @@ -282,10 +333,7 @@ func getWorktreeNameFromPath(worktreePath string, cfg *config.Config, mainRepoPa } // Get base_dir path - baseDir := cfg.Defaults.BaseDir - if !filepath.IsAbs(baseDir) { - baseDir = filepath.Join(mainRepoPath, baseDir) - } + baseDir := cfg.ResolveWorktreePath(mainRepoPath, "") // Calculate relative path from base_dir relPath, err := filepath.Rel(baseDir, worktreePath) diff --git a/cmd/wtp/remove_test.go b/cmd/wtp/remove_test.go index 91e04bc..509da35 100644 --- a/cmd/wtp/remove_test.go +++ b/cmd/wtp/remove_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "os" "path/filepath" "testing" @@ -54,15 +55,15 @@ func TestRemoveCommand_WorktreeResolution(t *testing.T) { name: "find by exact name", worktreeName: "feature-branch", worktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", - expectedPath: "/path/to/worktrees/feature-branch", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + expectedPath: "/path/to/worktrees/main/feature-branch", shouldFind: true, }, { name: "not found", worktreeName: "nonexistent", worktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/other\nHEAD def456\nbranch refs/heads/other\n\n", + "worktree /path/to/worktrees/main/other\nHEAD def456\nbranch refs/heads/other\n\n", expectedPath: "", shouldFind: false, }, @@ -142,7 +143,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { flags: map[string]any{}, worktreeName: "feature-branch", mockWorktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", expectedCommands: []command.Command{ { Name: "git", @@ -150,7 +151,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { }, { Name: "git", - Args: []string{"worktree", "remove", "/path/to/worktrees/feature-branch"}, + Args: []string{"worktree", "remove", "/path/to/worktrees/main/feature-branch"}, }, }, }, @@ -159,7 +160,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { flags: map[string]any{"force": true}, worktreeName: "feature-branch", mockWorktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", expectedCommands: []command.Command{ { Name: "git", @@ -167,7 +168,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { }, { Name: "git", - Args: []string{"worktree", "remove", "--force", "/path/to/worktrees/feature-branch"}, + Args: []string{"worktree", "remove", "--force", "/path/to/worktrees/main/feature-branch"}, }, }, }, @@ -176,7 +177,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { flags: map[string]any{"branch": true}, worktreeName: "feature-branch", mockWorktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", expectedCommands: []command.Command{ { Name: "git", @@ -184,7 +185,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { }, { Name: "git", - Args: []string{"worktree", "remove", "/path/to/worktrees/feature-branch"}, + Args: []string{"worktree", "remove", "/path/to/worktrees/main/feature-branch"}, }, { Name: "git", @@ -263,7 +264,7 @@ func TestRemoveCommand_SuccessMessage(t *testing.T) { results: []command.Result{ { Output: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", Error: nil, }, { @@ -374,7 +375,7 @@ func TestRemoveCommand_WorktreeNotFound_ShowsConsistentNames(t *testing.T) { results: []command.Result{ { Output: "worktree /repo\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /repo/.worktrees/feat/hogehoge\nHEAD def456\nbranch refs/heads/feat/hogehoge\n\n", + "worktree /somewhere/else/feat/hogehoge\nHEAD def456\nbranch refs/heads/feat/hogehoge\n\n", Error: nil, }, }, @@ -392,7 +393,7 @@ func TestRemoveCommand_WorktreeNotFound_ShowsConsistentNames(t *testing.T) { } func TestRemoveCommand_FailsWhenRemovingCurrentWorktree(t *testing.T) { - targetPath := "/worktrees/feature/foo" + targetPath := "/worktrees/repo/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", @@ -446,7 +447,7 @@ func TestRemoveCommand_ExecutionError(t *testing.T) { results: []command.Result{ { Output: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", + "worktree /path/to/worktrees/main/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", Error: nil, }, }, @@ -474,7 +475,7 @@ func TestRemoveCommand_DirtyWorktree(t *testing.T) { { name: "remove dirty worktree without force fails", forceFlag: false, - gitError: "fatal: '/path/to/worktrees/dirty-feature' " + + gitError: "fatal: '/path/to/worktrees/main/dirty-feature' " + "contains modified or untracked files, use --force to delete it", shouldSucceed: false, expectedMsg: []string{ @@ -501,7 +502,7 @@ func TestRemoveCommand_DirtyWorktree(t *testing.T) { // First result is always the worktree list mockResults = append(mockResults, command.Result{ Output: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/dirty-feature\nHEAD def456\nbranch refs/heads/dirty-feature\n\n", + "worktree /path/to/worktrees/main/dirty-feature\nHEAD def456\nbranch refs/heads/dirty-feature\n\n", Error: nil, }) @@ -590,7 +591,7 @@ func TestRemoveCommand_BranchRemovalWithUnmergedCommits(t *testing.T) { mockResults = append(mockResults, command.Result{ Output: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + - "worktree /path/to/worktrees/feature-unmerged\nHEAD def456\nbranch refs/heads/feature-unmerged\n\n", + "worktree /path/to/worktrees/main/feature-unmerged\nHEAD def456\nbranch refs/heads/feature-unmerged\n\n", Error: nil, }, command.Result{ @@ -658,17 +659,17 @@ func TestRemoveCommand_InternationalCharacters(t *testing.T) { { name: "Japanese characters", branchName: "ζ©Ÿθƒ½/ログむン", - worktreePath: "/path/to/worktrees/ζ©Ÿθƒ½/ログむン", + worktreePath: "/path/to/worktrees/main/ζ©Ÿθƒ½/ログむン", }, { name: "Spanish accents", branchName: "funciΓ³n/aΓ±adir", - worktreePath: "/path/to/worktrees/funciΓ³n/aΓ±adir", + worktreePath: "/path/to/worktrees/main/funciΓ³n/aΓ±adir", }, { name: "Emoji characters", branchName: "feature/πŸš€-rocket", - worktreePath: "/path/to/worktrees/feature/πŸš€-rocket", + worktreePath: "/path/to/worktrees/main/feature/πŸš€-rocket", }, } @@ -704,7 +705,7 @@ func TestRemoveCommand_InternationalCharacters(t *testing.T) { } func TestRemoveCommand_PathWithSpaces(t *testing.T) { - worktreePath := "/path/to/main/../worktrees/feature branch" + worktreePath := "/path/to/main/../worktrees/main/feature branch" mockOutput := "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + "worktree " + worktreePath + "\nHEAD def456\nbranch refs/heads/feature-branch\n\n" @@ -738,15 +739,15 @@ func TestRemoveCommand_MultipleMatchingWorktrees(t *testing.T) { HEAD abc123 branch refs/heads/main -worktree /path/to/worktrees/feature-test +worktree /path/to/worktrees/main/feature-test HEAD def456 branch refs/heads/feature-test -worktree /path/to/worktrees/feature-test-2 +worktree /path/to/worktrees/main/feature-test-2 HEAD ghi789 branch refs/heads/feature-test-2 -worktree /path/to/worktrees/test-feature +worktree /path/to/worktrees/main/test-feature HEAD jkl012 branch refs/heads/test-feature @@ -756,9 +757,9 @@ branch refs/heads/test-feature input string expectedPath string }{ - {"feature-test", "/path/to/worktrees/feature-test"}, - {"feature-test-2", "/path/to/worktrees/feature-test-2"}, - {"test-feature", "/path/to/worktrees/test-feature"}, + {"feature-test", "/path/to/worktrees/main/feature-test"}, + {"feature-test-2", "/path/to/worktrees/main/feature-test-2"}, + {"test-feature", "/path/to/worktrees/main/test-feature"}, } for _, tt := range tests { @@ -810,6 +811,8 @@ func createRemoveTestCLICommand(flags map[string]any, args []string) *cli.Comman }, } + app.ErrWriter = io.Discard + cmdArgs := []string{"test", "remove"} for key, value := range flags { switch v := value.(type) { diff --git a/cmd/wtp/worktree_managed.go b/cmd/wtp/worktree_managed.go index 9c30b47..fea4d61 100644 --- a/cmd/wtp/worktree_managed.go +++ b/cmd/wtp/worktree_managed.go @@ -26,14 +26,23 @@ func isWorktreeManagedCommon(worktreePath string, cfg *config.Config, mainRepoPa baseDir := cfg.ResolveWorktreePath(mainRepoPath, "") baseDir = strings.TrimSuffix(baseDir, string(filepath.Separator)) - absWorktreePath, err := filepath.Abs(worktreePath) - if err != nil { - return false + // Convert to absolute paths only if they're not already absolute + absWorktreePath := worktreePath + if !filepath.IsAbs(worktreePath) { + var err error + absWorktreePath, err = filepath.Abs(worktreePath) + if err != nil { + return false + } } - absBaseDir, err := filepath.Abs(baseDir) - if err != nil { - return false + absBaseDir := baseDir + if !filepath.IsAbs(baseDir) { + var err error + absBaseDir, err = filepath.Abs(baseDir) + if err != nil { + return false + } } relPath, err := filepath.Rel(absBaseDir, absWorktreePath) diff --git a/cmd/wtp/worktree_managed_test.go b/cmd/wtp/worktree_managed_test.go new file mode 100644 index 0000000..771218a --- /dev/null +++ b/cmd/wtp/worktree_managed_test.go @@ -0,0 +1,85 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/satococoa/wtp/internal/config" +) + +func TestIsWorktreeManagedCommon_ReturnsTrueForManagedWorktree(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + worktreePath := filepath.Join(tempDir, "worktrees", "repo", "feature", "foo") + + require.NoError(t, os.MkdirAll(mainRepoPath, 0o755)) + + cfg := &config.Config{ + Defaults: config.Defaults{ + BaseDir: config.DefaultBaseDir, + }, + } + + assert.True(t, isWorktreeManagedCommon(worktreePath, cfg, mainRepoPath, false)) +} + +func TestIsWorktreeManagedCommon_ReturnsFalseOutsideBaseDir(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + worktreePath := filepath.Join(tempDir, "external", "feature", "foo") + + require.NoError(t, os.MkdirAll(mainRepoPath, 0o755)) + + cfg := &config.Config{ + Defaults: config.Defaults{ + BaseDir: config.DefaultBaseDir, + }, + } + + assert.False(t, isWorktreeManagedCommon(worktreePath, cfg, mainRepoPath, false)) +} + +func TestIsWorktreeManagedCommon_HandlesRelativeWorktreePaths(t *testing.T) { + tempDir := t.TempDir() + mainRepoPath := filepath.Join(tempDir, "repo") + cfg := &config.Config{ + Defaults: config.Defaults{ + BaseDir: config.DefaultBaseDir, + }, + } + + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, os.Chdir(cwd)) + }) + + require.NoError(t, os.MkdirAll(mainRepoPath, 0o755)) + + canonicalMainRepoPath, err := filepath.EvalSymlinks(mainRepoPath) + require.NoError(t, err) + + require.NoError(t, os.Chdir(canonicalMainRepoPath)) + + relativePath := filepath.Join("..", "worktrees", "repo", "feature", "bar") + + baseDir := cfg.ResolveWorktreePath(canonicalMainRepoPath, "") + baseDir = strings.TrimSuffix(baseDir, string(filepath.Separator)) + baseDir = filepath.Clean(baseDir) + + absWorktreePath, err := filepath.Abs(relativePath) + require.NoError(t, err) + + relPath, err := filepath.Rel(baseDir, absWorktreePath) + require.NoError(t, err) + + require.NotEqual(t, "..", relPath) + require.False(t, strings.HasPrefix(relPath, ".."+string(filepath.Separator))) + + assert.True(t, isWorktreeManagedCommon(relativePath, cfg, canonicalMainRepoPath, false)) +} diff --git a/docs/architecture.md b/docs/architecture.md index 7559ee6..9304ff8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -102,7 +102,7 @@ We chose YAML for configuration because: ```yaml defaults: - base_dir: "../worktrees" + base_dir: "../worktrees/${WTP_REPO_BASENAME}" cd: true hooks: @@ -168,10 +168,10 @@ wtp cd feature/auth WTP_SHELL_INTEGRATION=1 wtp cd feature/auth # Go command returns path: -/path/to/worktrees/feature/auth +/path/to/worktrees//feature/auth # Shell function performs: -cd /path/to/worktrees/feature/auth +cd /path/to/worktrees//feature/auth ``` ### Key Design Decisions @@ -212,7 +212,7 @@ This ensures all team members use the same tool versions defined in go.mod. ## Path Handling -Branch names with slashes become directory structure (e.g., `feature/auth` β†’ `../worktrees/feature/auth/`) +Branch names with slashes become directory structure (e.g., `feature/auth` β†’ `../worktrees//feature/auth/`) ## Error Handling diff --git a/internal/config/config.go b/internal/config/config.go index c6d976d..b19160f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "go.yaml.in/yaml/v3" ) @@ -38,7 +39,7 @@ type Hook struct { const ( ConfigFileName = ".wtp.yml" CurrentVersion = "1.0" - DefaultBaseDir = "../worktrees" + DefaultBaseDir = "../worktrees/${WTP_REPO_BASENAME}" HookTypeCopy = "copy" HookTypeCommand = "command" configFilePermissions = 0o600 @@ -53,7 +54,7 @@ func LoadConfig(repoRoot string) (*Config, error) { return &Config{ Version: CurrentVersion, Defaults: Defaults{ - BaseDir: "../worktrees", + BaseDir: DefaultBaseDir, }, Hooks: Hooks{}, }, nil @@ -149,9 +150,20 @@ func (c *Config) HasHooks() bool { // ResolveWorktreePath resolves the full path for a worktree given a name func (c *Config) ResolveWorktreePath(repoRoot, worktreeName string) string { - baseDir := c.Defaults.BaseDir + baseDir := expandBaseDirVariables(c.Defaults.BaseDir, repoRoot) if !filepath.IsAbs(baseDir) { baseDir = filepath.Join(repoRoot, baseDir) } return filepath.Join(baseDir, worktreeName) } + +func expandBaseDirVariables(baseDir, repoRoot string) string { + if baseDir == "" { + return baseDir + } + + repoBaseName := filepath.Base(repoRoot) + expanded := strings.ReplaceAll(baseDir, "${WTP_REPO_BASENAME}", repoBaseName) + + return expanded +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index fdac0cb..f8dc108 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -18,8 +18,55 @@ func TestLoadConfig_NonExistentFile(t *testing.T) { t.Errorf("Expected version %s, got %s", CurrentVersion, config.Version) } - if config.Defaults.BaseDir != "../worktrees" { - t.Errorf("Expected default base_dir '../worktrees', got %s", config.Defaults.BaseDir) + if config.Defaults.BaseDir != DefaultBaseDir { + t.Errorf("Expected default base_dir '%s', got %s", DefaultBaseDir, config.Defaults.BaseDir) + } +} + +func TestExpandBaseDirVariables(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + baseDir string + repoRoot string + expected string + }{ + { + name: "single placeholder", + baseDir: "../worktrees/${WTP_REPO_BASENAME}", + repoRoot: "/home/user/project", + expected: "../worktrees/project", + }, + { + name: "multiple placeholders", + baseDir: "${WTP_REPO_BASENAME}/${WTP_REPO_BASENAME}", + repoRoot: "/home/user/project", + expected: "project/project", + }, + { + name: "no placeholder", + baseDir: "../custom", + repoRoot: "/home/user/project", + expected: "../custom", + }, + { + name: "trailing slash in repo root", + baseDir: "../worktrees/${WTP_REPO_BASENAME}", + repoRoot: "/home/user/project/", + expected: "../worktrees/project", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + result := expandBaseDirVariables(tt.baseDir, tt.repoRoot) + if result != tt.expected { + t.Fatalf("expected %q, got %q", tt.expected, result) + } + }) } } @@ -151,7 +198,7 @@ func TestConfigValidate(t *testing.T) { config: &Config{ Version: "1.0", Defaults: Defaults{ - BaseDir: "../worktrees", + BaseDir: DefaultBaseDir, }, Hooks: Hooks{ PostCreate: []Hook{ @@ -169,7 +216,7 @@ func TestConfigValidate(t *testing.T) { name: "empty version gets default", config: &Config{ Defaults: Defaults{ - BaseDir: "../worktrees", + BaseDir: DefaultBaseDir, }, }, expectError: false, @@ -333,15 +380,26 @@ func TestResolveWorktreePath(t *testing.T) { expected string }{ { - name: "relative base_dir", + name: "default base_dir expands repo basename", + config: &Config{ + Defaults: Defaults{ + BaseDir: DefaultBaseDir, + }, + }, + repoRoot: "/home/user/project", + worktreeName: "feature/auth", + expected: "/home/user/worktrees/project/feature/auth", + }, + { + name: "relative base_dir without placeholders", config: &Config{ Defaults: Defaults{ - BaseDir: "../worktrees", + BaseDir: "../custom-worktrees", }, }, repoRoot: "/home/user/project", worktreeName: "feature/auth", - expected: "/home/user/worktrees/feature/auth", + expected: "/home/user/custom-worktrees/feature/auth", }, { name: "absolute base_dir", @@ -355,15 +413,15 @@ func TestResolveWorktreePath(t *testing.T) { expected: "/tmp/worktrees/feature/auth", }, { - name: "simple worktree name", + name: "empty worktree name returns base dir", config: &Config{ Defaults: Defaults{ - BaseDir: "../worktrees", + BaseDir: DefaultBaseDir, }, }, repoRoot: "/home/user/project", - worktreeName: "main", - expected: "/home/user/worktrees/main", + worktreeName: "", + expected: "/home/user/worktrees/project", }, } diff --git a/internal/hooks/executor_test.go b/internal/hooks/executor_test.go index 33eef6c..534a382 100644 --- a/internal/hooks/executor_test.go +++ b/internal/hooks/executor_test.go @@ -857,8 +857,12 @@ func TestExecutor_copyDir_DestinationCreateError(t *testing.T) { executor := NewExecutor(nil, "/test/repo") - // Try to create directory where parent doesn't exist and we can't create it - invalidDest := "/root/nonexistent/dest" // Should fail on most systems + // Create a file to block directory creation at the target path + blockingFile := filepath.Join(tempDir, "dest-blocker") + err = os.WriteFile(blockingFile, []byte("blocker"), 0o644) + require.NoError(t, err) + + invalidDest := filepath.Join(blockingFile, "nested") err = executor.copyDir(srcDir, invalidDest) assert.Error(t, err) @@ -874,18 +878,15 @@ func TestExecutor_copyDir_ReadDirectoryError(t *testing.T) { srcDir := filepath.Join(tempDir, "source") dstDir := filepath.Join(tempDir, "dest") - // Create source directory + // Create source directory and then replace it with a file to force ReadDir to fail err := os.MkdirAll(srcDir, directoryPermissions) require.NoError(t, err) - // Remove read permission from source directory - err = os.Chmod(srcDir, 0200) // write-only + err = os.Remove(srcDir) require.NoError(t, err) - // Restore permissions after test - defer func() { - _ = os.Chmod(srcDir, directoryPermissions) - }() + err = os.WriteFile(srcDir, []byte("not a directory"), 0o644) + require.NoError(t, err) executor := NewExecutor(nil, "/test/repo") @@ -947,19 +948,16 @@ func TestExecutor_copyDir_FailsWhenNestedFileCannotBeCopied(t *testing.T) { err := os.MkdirAll(nestedDir, directoryPermissions) require.NoError(t, err) - // Create a file that we'll make unreadable + // Create a file that we'll replace with a broken symlink to force a copy error unreadableFile := filepath.Join(nestedDir, "unreadable.txt") - err = os.WriteFile(unreadableFile, []byte("content"), 0644) + err = os.WriteFile(unreadableFile, []byte("content"), 0o644) require.NoError(t, err) - // Remove read permission from the file - err = os.Chmod(unreadableFile, 0000) + err = os.Remove(unreadableFile) require.NoError(t, err) - // Restore permissions after test - defer func() { - _ = os.Chmod(unreadableFile, 0644) - }() + err = os.Symlink(filepath.Join(nestedDir, "missing-target"), unreadableFile) + require.NoError(t, err) executor := NewExecutor(nil, "/test/repo") diff --git a/test/e2e/hook_streaming_test.go b/test/e2e/hook_streaming_test.go index 13763bc..3c3e3fa 100644 --- a/test/e2e/hook_streaming_test.go +++ b/test/e2e/hook_streaming_test.go @@ -46,7 +46,7 @@ func TestHookOutputStreaming(t *testing.T) { framework.AssertOutputContains(t, output, "Completed!") framework.AssertOutputContains(t, output, "βœ“ All hooks executed successfully") - // Verify worktree was created (default location is ../worktrees/branch-name) - worktreePath := filepath.Join(repo.Path(), "..", "worktrees", "test-branch") + // Verify worktree was created (default location is ../worktrees//branch-name) + worktreePath := filepath.Join(repo.Path(), "..", "worktrees", filepath.Base(repo.Path()), "test-branch") framework.AssertWorktreeExists(t, repo, worktreePath) } diff --git a/test/e2e/worktree_test.go b/test/e2e/worktree_test.go index 76793f6..303f2c9 100644 --- a/test/e2e/worktree_test.go +++ b/test/e2e/worktree_test.go @@ -2,6 +2,7 @@ package e2e import ( "os" + "path/filepath" "strings" "testing" @@ -23,6 +24,25 @@ func TestWorktreeCreation(t *testing.T) { worktrees := repo.ListWorktrees() framework.AssertEqual(t, 2, len(worktrees)) framework.AssertWorktreeExists(t, repo, "feature/test-branch") + + expectedBaseDir := filepath.Join(filepath.Dir(repo.Path()), "worktrees", filepath.Base(repo.Path())) + expectedPath := filepath.Clean(filepath.Join(expectedBaseDir, "feature", "test-branch")) + resolvedExpectedPath, err := filepath.EvalSymlinks(expectedPath) + framework.AssertNoError(t, err) + + found := false + for _, wt := range worktrees { + if filepath.Clean(wt) == resolvedExpectedPath { + found = true + break + } + } + + framework.AssertTrue( + t, + found, + "expected worktrees to include "+resolvedExpectedPath+"; got: "+strings.Join(worktrees, ", "), + ) }) t.Run("NonexistentBranch", func(t *testing.T) {