Skip to content

Conversation

lox
Copy link

@lox lox commented Oct 8, 2025

Summary

Introduces automatic namespacing of worktrees by repository name to prevent collisions when multiple projects share a parent directory.

Problem

When multiple projects share a parent directory (e.g., /projects/myproject and /projects/another-project), their worktrees would collide in ../worktrees. This forced users to manually create project-specific directories like ../myproject-worktrees as a workaround.

Solution

Worktrees are now automatically namespaced by repository directory name:

Before:

../worktrees/
├── feature/auth      # Could be from any project! 
└── fix/bug-123       # Collision risk

After:

../worktrees/
├── myproject/
│   ├── feature/auth
│   └── fix/bug-123
└── another-project/
    └── feature/new-feature

Key Features

1. Auto-Detection for Backwards Compatibility

  • New projects: Automatically use namespaced layout
  • Existing projects: Auto-detected based on existing worktrees
  • Legacy projects: Continue working without changes

2. Migration Command

# Preview migration
wtp migrate-worktrees --dry-run

# Perform migration
wtp migrate-worktrees

# Consolidate from ../myproject-worktrees to ../worktrees
wtp migrate-worktrees --new-base-dir=../worktrees

3. Configuration

defaults:
  namespace_by_repo: true   # Use namespaced layout (default for new projects)
  namespace_by_repo: false  # Use legacy layout (opt-out)
  # Omit for auto-detection based on existing worktrees

4. Helpful Warnings

When legacy layout is detected, users see:

⚠️  Legacy worktree layout detected. Consider migrating to namespaced layout:
   wtp migrate-worktrees --dry-run  # Preview changes
   wtp migrate-worktrees            # Migrate worktrees

   Or to keep current layout, add to .wtp.yml:
   defaults:
     namespace_by_repo: false

Changes

  • Auto-namespace worktrees by repository directory name
  • Auto-detect existing layouts for backwards compatibility
  • Add migrate-worktrees command with --dry-run and --new-base-dir flags
  • Show migration warnings when legacy layout is auto-detected
  • Config option: namespace_by_repo (true/false/nil for auto-detect)
  • Comprehensive documentation in README and architecture docs
  • Tests for migration command and config logic

Benefits

✅ Multiple projects can safely share ../worktrees
✅ No manual workaround with project-specific directories
✅ Fully backwards compatible with existing setups
✅ Gradual migration path with helpful warnings
✅ Optional consolidation from custom base_dir to shared directory

Test Plan

  • Unit tests pass for config auto-detection
  • Unit tests pass for migrate command structure
  • Manual testing of migration with --dry-run and --new-base-dir
  • Documentation updated

Files Changed

  • cmd/wtp/migrate.go: Migration command implementation
  • cmd/wtp/migrate_test.go: Migration tests
  • internal/config/config.go: Auto-detection logic and namespacing support
  • internal/config/config_test.go: Updated tests for namespacing
  • cmd/wtp/list.go: Added migration warnings
  • cmd/wtp/cd.go: Added migration warnings
  • cmd/wtp/main.go: Registered migrate command
  • README.md: Documentation for namespacing and migration
  • docs/architecture.md: Architecture documentation

Summary by CodeRabbit

  • New Features

    • Namespaced worktree layout by repository enabled by default, with auto-detection of legacy layouts and an opt-out.
    • New migrate-worktrees CLI command with dry-run and optional new base-dir to relocate legacy worktrees.
    • Listing now surfaces a migration warning when legacy layout is detected.
  • Documentation

    • README and architecture docs updated with namespacing, migration/consolidation guidance, examples, and opt-out steps.
  • Tests

    • Added tests for namespaced path resolution and the migration command (including integration placeholders).

Introduces automatic namespacing of worktrees by repository name to prevent
collisions when multiple projects share a parent directory.

When multiple projects share a parent directory (e.g., /projects/myproject
and /projects/another-project), their worktrees would collide in ../worktrees.
This forced users to manually create project-specific directories like
../myproject-worktrees.

Now worktrees are automatically namespaced:
- Old: ../worktrees/feature/auth (collision risk)
- New: ../worktrees/myproject/feature/auth (isolated)

Changes:
- Auto-namespace worktrees by repository directory name
- Auto-detect existing layouts for backwards compatibility
- Add migrate-worktrees command with --dry-run and --new-base-dir flags
- Show migration warnings when legacy layout is auto-detected
- Config option: namespace_by_repo (true/false/nil for auto-detect)
- Comprehensive documentation in README and architecture docs

Benefits:
- Multiple projects can safely share ../worktrees
- No manual workaround with project-specific directories
- Fully backwards compatible with existing setups
- Gradual migration path with helpful warnings
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds namespaced-by-repo worktree layout (default), auto-detects legacy layouts, exposes a new migrate-worktrees CLI with dry-run and new-base-dir options, updates config APIs and path resolution to honor namespacing, surfaces migration warnings after listing, and updates docs/tests to reflect the change.

Changes

Cohort / File(s) Change Summary
Documentation: namespaced layout & migration
README.md, docs/architecture.md
Introduce namespace_by_repo defaults, update examples/diagrams to namespaced worktrees, document auto-detection, opt-out, migration and consolidation steps, and add migration commands/YAML examples.
CLI: migrate command & list UX
cmd/wtp/main.go, cmd/wtp/migrate.go, cmd/wtp/migrate_test.go, cmd/wtp/list.go
Add migrate-worktrees command (--dry-run, --new-base-dir), implement migration plan/move/save-config flow, add tests for command and discovery, and print migration warning after list when applicable.
Commands: cd/remove/list path handling
cmd/wtp/cd.go, cmd/wtp/remove.go, cmd/wtp/add_test.go, cmd/wtp/testhelpers_test.go
When cfg is nil, initialize default config with legacy-mode pointer for NamespaceByRepo; adjust relative path computation to account for repo-scoped namespace when namespacing is enabled; update tests to toggle legacy mode.
Core config: defaults, detection, helpers
internal/config/config.go, internal/config/config_test.go
Add Defaults.NamespaceByRepo *bool, Config.namespaceAutoDetected bool; auto-detect legacy layout on load/update NamespaceByRepo; add helpers ShouldNamespaceByRepo, UsesLegacyLayout, ShouldShowMigrationWarning, GetMigrationWarning; add hasLegacyWorktrees / containsGitFile and update ResolveWorktreePath.
Tests: config & migrate behavior
internal/config/config_test.go, cmd/wtp/migrate_test.go, other updated tests...*
Expand tests for ResolveWorktreePath with namespacing true/false/nil, add migrate command tests and integration test placeholders, and update callers to set NamespaceByRepo in test defaults.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant CLI as wtp CLI
  participant C as Config
  participant FS as Filesystem
  participant Git as git

  rect rgb(245,248,255)
  note over U,CLI: Migrate legacy worktrees to namespaced layout
  U->>CLI: wtp migrate-worktrees [--dry-run] [--new-base-dir]
  CLI->>C: LoadConfig()
  C-->>CLI: config (auto-detect legacy if needed)
  CLI->>FS: findLegacyWorktreesForMigration(config.BaseDir)
  FS-->>CLI: legacy worktree list
  loop For each legacy worktree
    CLI->>CLI: compute new namespaced path
    alt Dry-run
      CLI-->>U: print planned move source -> target
    else Perform migration
      CLI->>FS: mkdir -p <target parent>
      CLI->>Git: git worktree move <source> <target>
      Git-->>CLI: result
      CLI-->>U: print moved status
    end
  end
  alt new-base-dir or enabling namespacing
    CLI->>C: set Defaults.NamespaceByRepo=true (and BaseDir?)
    C-->>CLI: save config
  end
  CLI-->>U: summary (dry-run/performed)
  end
Loading
sequenceDiagram
  autonumber
  participant U as User
  participant CLI as wtp list
  participant C as Config

  U->>CLI: wtp list
  CLI->>C: LoadConfig()
  C-->>CLI: config (with auto-detect flags)
  CLI-->>U: display worktrees
  alt legacy auto-detected and warning enabled
    CLI-->>U: print migration warning (GetMigrationWarning)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

I hop through branches, tidy and bright,
Nest each worktree by repo at night.
With dry-run paws I plot the leap,
Then move the huts in a namespaced sweep.
Config set, I nose the way—migration done, I munch my hay. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the primary change of the pull request—adding automatic worktree namespacing by repository—which directly matches the objectives and main feature introduced in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd1c96b and 2315f16.

📒 Files selected for processing (5)
  • cmd/wtp/add_test.go (12 hunks)
  • cmd/wtp/cd.go (3 hunks)
  • cmd/wtp/list.go (2 hunks)
  • cmd/wtp/remove.go (3 hunks)
  • cmd/wtp/testhelpers_test.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Run gofmt and goimports; keep imports grouped and organized; local import prefix follows module path github.com/satococoa/wtp
Adhere to golangci-lint rules configured for the project (vet, staticcheck, gosec, mnd, lll=120, etc.)
Errors should be wrapped with context and must not be ignored
Use snake_case for Go filenames; document exported identifiers when non-trivial

Files:

  • cmd/wtp/cd.go
  • cmd/wtp/remove.go
  • cmd/wtp/list.go
  • cmd/wtp/testhelpers_test.go
  • cmd/wtp/add_test.go
cmd/wtp/**

📄 CodeRabbit inference engine (AGENTS.md)

cmd/wtp/**: CLI entrypoint and commands are implemented under cmd/wtp
Update CLI help text to reflect user-facing changes
Command behavior: wtp cd prints only the absolute worktree path with no side effects
Command behavior: wtp completion generates pure completion scripts via urfave/cli
Command behavior: wtp hook emits shell functions that intercept wtp cd; wtp shell-init combines completion and hook output

Files:

  • cmd/wtp/cd.go
  • cmd/wtp/remove.go
  • cmd/wtp/list.go
  • cmd/wtp/testhelpers_test.go
  • cmd/wtp/add_test.go
**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Unit tests live alongside packages and use *_test.go naming
Prefer table-driven tests and mock git operations in unit tests

Files:

  • cmd/wtp/testhelpers_test.go
  • cmd/wtp/add_test.go
🧠 Learnings (2)
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/completion.go : Use getWorktreeNameFromPath() to resolve worktree display names consistently across completion, errors, and parsing

Applied to files:

  • cmd/wtp/cd.go
  • cmd/wtp/remove.go
  • cmd/wtp/add_test.go
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp cd prints only the absolute worktree path with no side effects

Applied to files:

  • cmd/wtp/cd.go
🧬 Code graph analysis (5)
cmd/wtp/cd.go (1)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/remove.go (1)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/list.go (1)
internal/config/config.go (3)
  • GetMigrationWarning (281-291)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/testhelpers_test.go (1)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/add_test.go (1)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7025099 and 4376c1b.

📒 Files selected for processing (9)
  • README.md (5 hunks)
  • cmd/wtp/cd.go (1 hunks)
  • cmd/wtp/list.go (1 hunks)
  • cmd/wtp/main.go (1 hunks)
  • cmd/wtp/migrate.go (1 hunks)
  • cmd/wtp/migrate_test.go (1 hunks)
  • docs/architecture.md (2 hunks)
  • internal/config/config.go (4 hunks)
  • internal/config/config_test.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Run gofmt and goimports; keep imports grouped and organized; local import prefix follows module path github.com/satococoa/wtp
Adhere to golangci-lint rules configured for the project (vet, staticcheck, gosec, mnd, lll=120, etc.)
Errors should be wrapped with context and must not be ignored
Use snake_case for Go filenames; document exported identifiers when non-trivial

Files:

  • internal/config/config_test.go
  • cmd/wtp/list.go
  • cmd/wtp/migrate.go
  • cmd/wtp/migrate_test.go
  • cmd/wtp/cd.go
  • cmd/wtp/main.go
  • internal/config/config.go
internal/{git,config,hooks,command,errors,io}/**

📄 CodeRabbit inference engine (AGENTS.md)

Place internal packages within internal/{git,config,hooks,command,errors,io}

Files:

  • internal/config/config_test.go
  • internal/config/config.go
**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Unit tests live alongside packages and use *_test.go naming
Prefer table-driven tests and mock git operations in unit tests

Files:

  • internal/config/config_test.go
  • cmd/wtp/migrate_test.go
cmd/wtp/**

📄 CodeRabbit inference engine (AGENTS.md)

cmd/wtp/**: CLI entrypoint and commands are implemented under cmd/wtp
Update CLI help text to reflect user-facing changes
Command behavior: wtp cd prints only the absolute worktree path with no side effects
Command behavior: wtp completion generates pure completion scripts via urfave/cli
Command behavior: wtp hook emits shell functions that intercept wtp cd; wtp shell-init combines completion and hook output

Files:

  • cmd/wtp/list.go
  • cmd/wtp/migrate.go
  • cmd/wtp/migrate_test.go
  • cmd/wtp/cd.go
  • cmd/wtp/main.go
README.md

📄 CodeRabbit inference engine (AGENTS.md)

Update README when user-facing behavior changes

Files:

  • README.md
docs/**

📄 CodeRabbit inference engine (AGENTS.md)

Maintain docs such as docs/architecture.md and docs/testing-guidelines.md alongside changes

Files:

  • docs/architecture.md
🧠 Learnings (2)
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp cd prints only the absolute worktree path with no side effects

Applied to files:

  • cmd/wtp/cd.go
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/completion.go : Use getWorktreeNameFromPath() to resolve worktree display names consistently across completion, errors, and parsing

Applied to files:

  • README.md
🧬 Code graph analysis (6)
internal/config/config_test.go (1)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/list.go (1)
internal/config/config.go (1)
  • GetMigrationWarning (256-266)
cmd/wtp/migrate.go (4)
internal/errors/errors.go (3)
  • DirectoryAccessFailed (233-257)
  • NotInGitRepository (12-20)
  • GitCommandFailed (22-35)
internal/git/repository.go (2)
  • NewRepository (16-21)
  • Repository (12-14)
internal/config/config.go (4)
  • LoadConfig (52-101)
  • Defaults (22-25)
  • SaveConfig (147-164)
  • Config (12-19)
internal/git/worktree.go (1)
  • Worktree (14-19)
cmd/wtp/migrate_test.go (2)
cmd/wtp/migrate.go (1)
  • NewMigrateCommand (18-46)
internal/config/config.go (2)
  • Config (12-19)
  • Defaults (22-25)
cmd/wtp/cd.go (1)
internal/config/config.go (2)
  • LoadConfig (52-101)
  • GetMigrationWarning (256-266)
cmd/wtp/main.go (1)
cmd/wtp/migrate.go (1)
  • NewMigrateCommand (18-46)
🪛 markdownlint-cli2 (0.18.1)
README.md

260-260: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


380-380: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

1. Remove migration warning from wtp cd (violated command contract)
   - wtp cd must only output the path with no side effects
   - Warnings are still shown in wtp list

2. Fix legacy worktree detection for nested branches
   - Add recursive containsGitFile() helper (depth limit 3)
   - Now correctly detects feature/auth/.git patterns
   - Critical for auto-detection accuracy

3. Add language tags to markdown fenced code blocks
   - Fixes MD040 linting errors
   - All ASCII tree diagrams now use ```text
@lox
Copy link
Author

lox commented Oct 8, 2025

Thanks @coderabbitai for the thorough review! All issues addressed in dd1c96b:

Major fixes:

  1. ✅ Removed migration warning from wtp cd - you're absolutely right that this violated the command contract. The warning is still shown in wtp list where it belongs.

  2. ✅ Fixed legacy worktree detection to handle nested branches like feature/auth/.git. Added recursive containsGitFile() helper with depth limit of 3 to catch all patterns while avoiding infinite recursion.

Minor fixes:
3. ✅ Added text language tags to fenced code blocks (MD040 lint)

The auto-detection should now work correctly for all existing installations with slashed branch names.

Copy link

coderabbitai bot commented Oct 8, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the commit dd1c96b to see what changed
git show dd1c96b --stat

echo "---"
echo "Checking for migration warning removal in cd.go:"
git show dd1c96b -- cmd/wtp/cd.go | grep -A5 -B5 "migration"

echo "---"
echo "Checking for containsGitFile helper in config.go:"
git show dd1c96b -- internal/config/config.go | grep -A20 "containsGitFile"

echo "---"
echo "Checking for text language tags in README:"
git show dd1c96b -- README.md | grep -A3 -B3 '```text'

Length of output: 4307


@lox Thank you for the quick turnaround! I've verified commit dd1c96b and can confirm all three issues have been properly addressed:

Major fixes verified:

  1. Migration warning removed from wtp cd - The 7 lines containing the migration warning logic have been cleanly removed, preserving the command's contract to only output the path
  2. Recursive containsGitFile() helper - Excellent implementation with proper depth limiting (max 3 levels) to handle nested branch patterns like feature/auth/.git while preventing infinite recursion

Minor fix verified:
3. ✅ Markdown lint fixes - All ASCII tree diagrams now properly use ````text` language tags

The commit message is also well-structured and clearly explains the rationale for each change. The auto-detection logic should now correctly handle all existing installations with slashed branch names. Great work! 🎉


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

When namespace_by_repo is enabled, worktree names in list/cd/remove
commands now display without the repository prefix (e.g., "docs-cleanup"
instead of "zig-poker-eval/docs-cleanup"). This provides a cleaner UX
while the actual paths on disk remain properly namespaced.

Updated test configs to explicitly use legacy mode for backward
compatibility testing.
@satococoa
Copy link
Owner

Thank you so much for putting this PR together!

After reviewing the changes, it looks like setting base_dir to ../<your_project_name> in .wtp.yml already covers the layout we’re aiming for.
With that in mind, I’ll close this PR for now, but I genuinely appreciate the ideas and the level of detail you brought; this will absolutely help shape future improvements.

Please keep the feedback and suggestions coming. They’re really valuable to me.

@satococoa satococoa closed this Oct 16, 2025
@lox
Copy link
Author

lox commented Oct 16, 2025

It does, it seems kind of gross that you need to do that on every single project you use wtp for though doesn't it? Is that what you do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants