feat: default worktrees under .git/wtp/worktrees with legacy warning #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

satococoa wants to merge 3 commits into main

from feature/default-worktree-dir

Open

feat: default worktrees under .git/wtp/worktrees with legacy warning #56

satococoa wants to merge 3 commits into main from feature/default-worktree-dir

Conversation

@satococoa

Copy link

Owner

@satococoa satococoa commented Dec 4, 2025 •

edited by coderabbitai bot

Loading

Summary

change default worktree base_dir to .git/wtp/worktrees
emit a warning when legacy ../worktrees exists and no config is present
update docs and tests for the new default

Testing

go test ./...

Summary by CodeRabbit

New Features
- Automatic warnings now notify users if legacy worktree locations are present and suggest remediation.
Changes
- Default worktree storage moved to .git/wtp/worktrees (replaces previous ../worktrees).
Documentation
- Docs, examples, and architecture guides updated to reflect the new default worktree path.
Tests
- Test expectations updated to align with the new worktree path and warning behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

satococoa added 2 commits

December 4, 2025 10:45

@satococoa


 feat: change default worktree base dir to .git/wtp/worktrees

@satococoa


 feat: warn when legacy worktrees directory is present

a0f56d9

Copilot AI review requested due to automatic review settings

December 4, 2025 14:17

Copilot started reviewing on behalf of satococoa

December 4, 2025 14:18

View session

Copilot finished reviewing on behalf of satococoa

December 4, 2025 14:20

Copilot AI reviewed

Dec 4, 2025

View reviewed changes

Copy link

Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes the default worktree directory structure by moving from ../worktrees to .git/wtp/worktrees, keeping all worktree-related files within the repository's .git directory. A backward-compatible warning system alerts users with existing legacy worktree directories.

Key Changes:

Default worktree base directory changed from ../worktrees to .git/wtp/worktrees
Added legacy directory detection with helpful migration warning
Comprehensive test suite and documentation updates across the codebase

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/config/config.go	Updates default base directory constant and adds ConfigFileExists helper function
internal/config/config_test.go	Updates tests to use new default path constant and adds ConfigFileExists test coverage
cmd/wtp/legacy_warning.go	Implements warning logic to detect legacy `../worktrees` directories when no config exists
cmd/wtp/legacy_warning_test.go	Comprehensive test coverage for legacy warning scenarios
cmd/wtp/add.go	Integrates legacy warning into the add command workflow
cmd/wtp/cd.go	Integrates legacy warning into the cd command workflow
cmd/wtp/list.go	Integrates legacy warning into the list command workflow
cmd/wtp/remove.go	Integrates legacy warning into the remove command workflow
cmd/wtp/init.go	Updates template configuration to use new default base directory
cmd/wtp/init_test.go	Verifies init command generates config with new default path
test/e2e/worktree_test.go	Updates end-to-end tests to use new default worktree paths
test/e2e/hook_streaming_test.go	Updates hook streaming tests with new default path and comment
cmd/wtp/remove_test.go	Updates all test fixtures with new default worktree path structure
cmd/wtp/cd_test.go	Updates test fixtures to reflect new default path
cmd/wtp/list_test.go	Updates all test cases to use new default base directory constant
internal/command/executor_test.go	Updates command executor tests with new default paths
docs/architecture.md	Updates architecture documentation with new default directory structure
README.md	Updates user-facing documentation with new default paths and directory structure examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai

Copy link

coderabbitai bot commented Dec 4, 2025 •

edited

Loading

Walkthrough

Default worktree base directory changed from ../worktrees to .git/wtp/worktrees across code, docs, and tests. A new private helper emits a legacy-base-dir warning when old worktrees are present and no config exists; commands call this warning and error-writer handling was added.

Changes

Cohort / File(s)	Summary
Configuration & Constants `internal/config/config.go`, `internal/config/config_test.go`	`DefaultBaseDir` updated to `.git/wtp/worktrees`. Added public `FileExists(repoRoot string) bool` to check for config presence (absolute path normalization). Tests updated to use `DefaultBaseDir` and cover `FileExists`.
Legacy Warning Feature `cmd/wtp/legacy_warning.go`, `cmd/wtp/legacy_warning_test.go`	New `warnLegacyBaseDir(errWriter io.Writer, repoRoot string)` helper that checks for legacy worktrees, config absence, and writes a two-line warning to the provided writer. Tests cover warning/no-warning scenarios.
Command Implementations `cmd/wtp/add.go`, `cmd/wtp/cd.go`, `cmd/wtp/remove.go`, `cmd/wtp/list.go`	Commands now obtain an error writer from `cmd.Root().ErrWriter` (fallback `os.Stderr`), determine main repository path, and call `warnLegacyBaseDir()` prior to execution. Minor repo initialization adjustments to capture repo/mainRepoPath.
Init Command `cmd/wtp/init.go`, `cmd/wtp/init_test.go`	Default generated `.wtp.yml` template now sets `base_dir: .git/wtp/worktrees`. Tests updated to expect the new template content.
Command Tests `cmd/wtp/cd_test.go`, `cmd/wtp/list_test.go`, `cmd/wtp/remove_test.go`, other `cmd/wtp/*_test.go`	Numerous tests updated to use `config.DefaultBaseDir` or the new `.git/wtp/worktrees` path for expected worktree locations and command argument assertions. Some tests adjust special-case relative-base handling.
Command Executor Tests `internal/command/executor_test.go`	Test fixtures and expected command args updated to reference `.git/wtp/worktrees/...` instead of `../worktrees/...`.
Documentation `README.md`, `docs/architecture.md`	Examples, diagrams, and narrative updated to show `.git/wtp/worktrees` as the default base_dir and to use the new path in example outputs.
End-to-End Tests `test/e2e/worktree_test.go`, `test/e2e/hook_streaming_test.go`	E2E assertions and test setup updated to validate worktrees under `repo.Path() + "/.git/wtp/worktrees/..."` and adjusted hook-related checks accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Files needing extra attention:
- cmd/wtp/legacy_warning.go — correctness of filesystem checks and config-path construction
- internal/config/config.go — FileExists absolute-path behavior and interaction with LoadConfig defaults
- Command changes in cmd/wtp/* — consistent error-writer retrieval and mainRepoPath fallback handling
- Wide test updates — ensure all path string substitutions are consistent and no lingering hard-coded paths remain

Possibly related PRs

Simplify add command interface for better usability #9 — touches cmd/wtp/add.go, related to add command flow adjustments and error-writer handling.
feat: refactor shell integration architecture #19 — modifies CLI command files (cd.go, others) overlapping with this PR's command changes.
fix: prevent removing current worktree #33 — updates remove.go and worktree path/main-repo handling similar to changes here.

Suggested labels

codex

Poem

🐰 A nook beneath .git where small worktrees hide,
I hop and I whisper when old paths still bide.
A gentle two-line warning I gladly bestow,
So new bases flourish where quiet roots grow. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.45% 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 summarizes the main changes: defaulting worktrees to .git/wtp/worktrees and adding a legacy warning, matching the primary objectives of the PR.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/default-worktree-dir

📜 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 a0f56d9 and ec732f8.

📒 Files selected for processing (4)

cmd/wtp/legacy_warning.go (1 hunks)
internal/config/config.go (3 hunks)
internal/config/config_test.go (6 hunks)
test/e2e/worktree_test.go (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

test/e2e/worktree_test.go
cmd/wtp/legacy_warning.go
internal/config/config.go
internal/config/config_test.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Test (macos-latest, 1.24)

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.}

coderabbitai[bot]

coderabbitai bot reviewed

Dec 4, 2025

View reviewed changes

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

internal/config/config.go (1)
45-189: Rename ConfigFileExists to satisfy lint and avoid stuttering package name

The new default DefaultBaseDir = ".git/wtp/worktrees" and the use of this constant in LoadConfig look consistent and correct.

The main issue is the exported helper:
// ConfigFileExists reports whether the config file exists in the given repository root.
func ConfigFileExists(repoRoot string) bool {
 ...
}
Static analysis correctly notes that config.ConfigFileExists stutters the package name; this will keep the lint job red.

Consider renaming it to avoid the stutter, e.g.:
-// ConfigFileExists reports whether the config file exists in the given repository root.
-func ConfigFileExists(repoRoot string) bool {
+// FileExists reports whether the config file exists in the given repository root.
+func FileExists(repoRoot string) bool {
and update call sites accordingly. Behavior (path cleaning, os.Stat, and conservative false on errors) is otherwise fine.
test/e2e/worktree_test.go (1)
145-160: Aligns e2e paths with new base_dir; split long git command line for linter

The e2e updates correctly move expectations from legacy ../worktrees to .git/wtp/worktrees, both for the remove‐outside‐base_dir case and for post‐create hooks.

There is one linter issue on the long env.RunInDir line in the RemoveOnlyWorksWithinBaseDir test; you can fix it by splitting arguments:
-		env.RunInDir(repo.Path(), "git", "worktree", "add", ".git/wtp/worktrees/feature/remove-test", "-b", "feature/remove-test")
+		env.RunInDir(
+			repo.Path(),
+			"git",
+			"worktree",
+			"add",
+			".git/wtp/worktrees/feature/remove-test",
+			"-b",
+			"feature/remove-test",
+		)
This keeps behavior identical while satisfying the lll (max line length) rule.

Also applies to: 275-292

🧹 Nitpick comments (1)

cmd/wtp/init.go (1)

65-102: Config template now matches new default base_dir

The generated .wtp.yml now uses base_dir: .git/wtp/worktrees, matching config.DefaultBaseDir and the rest of the PR. Looks good.

If you ever find this drifting again, you might consider building this string with config.DefaultBaseDir instead of a hard-coded literal, but that’s strictly optional.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7387d and a0f56d9.

📒 Files selected for processing (18)

README.md (5 hunks)
cmd/wtp/add.go (1 hunks)
cmd/wtp/cd.go (1 hunks)
cmd/wtp/cd_test.go (2 hunks)
cmd/wtp/init.go (1 hunks)
cmd/wtp/init_test.go (1 hunks)
cmd/wtp/legacy_warning.go (1 hunks)
cmd/wtp/legacy_warning_test.go (1 hunks)
cmd/wtp/list.go (1 hunks)
cmd/wtp/list_test.go (13 hunks)
cmd/wtp/remove.go (1 hunks)
cmd/wtp/remove_test.go (15 hunks)
docs/architecture.md (3 hunks)
internal/command/executor_test.go (4 hunks)
internal/config/config.go (3 hunks)
internal/config/config_test.go (6 hunks)
test/e2e/hook_streaming_test.go (1 hunks)
test/e2e/worktree_test.go (4 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Follow standard Go style with tabs and gofmt; package names must be short and lowercase
Keep import groups tidy; use goimports for organization with local prefix following module path github.com/satococoa/wtp/v2
Adhere to linting rules in .golangci.yml (vet, staticcheck, gosec, mnd, lll=120)
Wrap errors with context; avoid ignoring errors in Go code
Use snake_case for Go filenames (e.g., remove.go, shell_integration.go)
Document exported items in Go when non-trivial; follow godoc conventions

Files:

test/e2e/hook_streaming_test.go
cmd/wtp/add.go
cmd/wtp/cd_test.go
cmd/wtp/list.go
test/e2e/worktree_test.go
cmd/wtp/legacy_warning.go
internal/config/config.go
cmd/wtp/init_test.go
cmd/wtp/remove.go
cmd/wtp/legacy_warning_test.go
internal/command/executor_test.go
cmd/wtp/init.go
internal/config/config_test.go
cmd/wtp/list_test.go
cmd/wtp/remove_test.go
cmd/wtp/cd.go

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*_test.go: Unit tests must be placed alongside packages as *_test.go files in the same directory
Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases

Files:

test/e2e/hook_streaming_test.go
cmd/wtp/cd_test.go
test/e2e/worktree_test.go
cmd/wtp/init_test.go
cmd/wtp/legacy_warning_test.go
internal/command/executor_test.go
internal/config/config_test.go
cmd/wtp/list_test.go
cmd/wtp/remove_test.go

test/e2e/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

End-to-end tests must be placed in test/e2e and exercise real git workflows using the built binary

Files:

test/e2e/hook_streaming_test.go
test/e2e/worktree_test.go

🧠 Learnings (14)

📓 Common learnings

Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年12月02日T13:33:48.673Z
Learning: Worktree naming convention: main worktree is rendered as `@`; non-main worktrees display path relative to `base_dir` (e.g., `.worktrees/feat/hoge` → `feat/hoge`)

Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年12月02日T13:33:48.673Z
Learning: Run commands from inside a worktree to mimic real usage (e.g., `go run ../cmd/wtp add feature/new-feature`)

Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025年12月02日T13:33:48.673Z
Learning: Shell integration must separate completion and hooks for clarity; `wtp cd` outputs absolute worktree path with no side effects