-
Notifications
You must be signed in to change notification settings - Fork 10
feat(cd): default to main worktree sans args #63
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
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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. Comment |
There was a problem hiding this 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
🧹 Nitpick comments (1)
cmd/wtp/cd.go (1)
47-54: Defaulting to "@" is the right place, but consider covering it directly in tests.
The logic is simple and correct, but current tests mostly validate"@" → mainresolution, not the no-args → "@" selection itself.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/wtp/cd.go(1 hunks)cmd/wtp/cd_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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 pathgithub.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:
cmd/wtp/cd_test.gocmd/wtp/cd.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Unit tests must be placed alongside packages as*_test.gofiles 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:
cmd/wtp/cd_test.go
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
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-02T13:33:48.693Z
Learning: Shell integration must separate completion and hooks for clarity; `wtp cd` outputs absolute worktree path with no side effects
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
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-02T13:33:48.693Z
Learning: Toggle shell integration paths with `WTP_SHELL_INTEGRATION=1` when testing cd behavior
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to **/*_test.go : Target 70% code coverage with unit tests focusing on fast feedback and mocked git interactions using table-driven test cases
Applied to files:
cmd/wtp/cd_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Run commands from inside a worktree to mimic real usage (e.g., `go run ../cmd/wtp add feature/new-feature`)
Applied to files:
cmd/wtp/cd_test.gocmd/wtp/cd.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Shell integration must separate completion and hooks for clarity; `wtp cd` outputs absolute worktree path with no side effects
Applied to files:
cmd/wtp/cd_test.gocmd/wtp/cd.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Applies to test/e2e/**/*.go : End-to-end tests must be placed in `test/e2e` and exercise real git workflows using the built binary
Applied to files:
cmd/wtp/cd_test.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: CLI entrypoint is located at `cmd/wtp`; new command implementations must integrate with the main CLI structure
Applied to files:
cmd/wtp/cd_test.gocmd/wtp/cd.go
📚 Learning: 2025-12-02T13:33:48.693Z
Learnt from: CR
Repo: satococoa/wtp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T13:33:48.693Z
Learning: Toggle shell integration paths with `WTP_SHELL_INTEGRATION=1` when testing cd behavior
Applied to files:
cmd/wtp/cd_test.go
🧬 Code graph analysis (1)
cmd/wtp/cd.go (1)
internal/command/types.go (1)
Command(4-8)
🔇 Additional comments (1)
cmd/wtp/cd.go (1)
31-41: Help text and argument usage formatting are consistent with the actual behavior. TheArgsUsage: "[worktree-name]"correctly indicates an optional argument using square brackets (a documentation convention in urfave/cli), and the examples inDescriptionaccurately reflect the behavior when invoked with no arguments (Args().Len() == 0). Good alignment with the "@ is main worktree" convention.
cmd/wtp/cd_test.go
Outdated
| // Test that bare "wtp cd" navigates to main worktree (like cd goes to $HOME) | ||
| func TestCdCommand_NoArgsDefaultsToMainWorktree(t *testing.T) { | ||
| worktreeList := `worktree /Users/dev/project/main | ||
| HEAD abc123 | ||
| branch refs/heads/main | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| app := &cli.Command{ | ||
| Commands: []*cli.Command{ | ||
| NewCdCommand(), | ||
| }, | ||
| } | ||
| worktree /Users/dev/project/worktrees/feature/auth | ||
| HEAD def456 | ||
| branch refs/heads/feature/auth | ||
| var buf bytes.Buffer | ||
| app.Writer = &buf | ||
| ` | ||
|
|
||
| ctx := context.Background() | ||
| cmdArgs := []string{"wtp", "cd"} | ||
| cmdArgs = append(cmdArgs, tt.args...) | ||
| worktrees := parseWorktreesFromOutput(worktreeList) | ||
| mainPath := findMainWorktreePath(worktrees) | ||
|
|
||
| err := app.Run(ctx, cmdArgs) | ||
| assert.Error(t, err) | ||
| assert.Contains(t, err.Error(), tt.expectedError) | ||
| }) | ||
| } | ||
| // Empty worktree name should resolve to main worktree via "@" | ||
| resolvedPath := resolveCdWorktreePath("@", worktrees, mainPath) | ||
| assert.Equal(t, "/Users/dev/project/main", resolvedPath, | ||
| "bare wtp cd should resolve to main worktree") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test name/comment don’t match what’s actually asserted (it tests "@" resolution, not no-args).
Either rename + adjust the comment, or add a small helper to unit-test “no args ⇒ @”. (As per test guidelines, keeping this table-driven is optional but nice.) Based on coding guidelines.
Suggested minimal rename/comment fix:
-// Test that bare "wtp cd" navigates to main worktree (like cd goes to $HOME)
-func TestCdCommand_NoArgsDefaultsToMainWorktree(t *testing.T) {
+// Test that "@" resolves to the main worktree path.
+func TestCdCommand_AtResolvesToMainWorktree(t *testing.T) {
@@
- // Empty worktree name should resolve to main worktree via "@"
+ // cdToWorktree defaults to "@", so ensure "@" resolves to main.
resolvedPath := resolveCdWorktreePath("@", worktrees, mainPath)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Test that bare "wtp cd" navigates to main worktree (like cd goes to $HOME) | |
| func TestCdCommand_NoArgsDefaultsToMainWorktree(t *testing.T) { | |
| worktreeList := `worktree /Users/dev/project/main | |
| HEAD abc123 | |
| branch refs/heads/main | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| app := &cli.Command{ | |
| Commands: []*cli.Command{ | |
| NewCdCommand(), | |
| }, | |
| } | |
| worktree /Users/dev/project/worktrees/feature/auth | |
| HEAD def456 | |
| branch refs/heads/feature/auth | |
| var buf bytes.Buffer | |
| app.Writer = &buf | |
| ` | |
| ctx := context.Background() | |
| cmdArgs := []string{"wtp", "cd"} | |
| cmdArgs = append(cmdArgs, tt.args...) | |
| worktrees := parseWorktreesFromOutput(worktreeList) | |
| mainPath := findMainWorktreePath(worktrees) | |
| err := app.Run(ctx, cmdArgs) | |
| assert.Error(t, err) | |
| assert.Contains(t, err.Error(), tt.expectedError) | |
| }) | |
| } | |
| // Empty worktree name should resolve to main worktree via "@" | |
| resolvedPath := resolveCdWorktreePath("@", worktrees, mainPath) | |
| assert.Equal(t, "/Users/dev/project/main", resolvedPath, | |
| "bare wtp cd should resolve to main worktree") | |
| } | |
| // Test that "@" resolves to the main worktree path. | |
| func TestCdCommand_AtResolvesToMainWorktree(t *testing.T) { | |
| worktreeList := `worktree /Users/dev/project/main | |
| HEAD abc123 | |
| branch refs/heads/main | |
| worktree /Users/dev/project/worktrees/feature/auth | |
| HEAD def456 | |
| branch refs/heads/feature/auth | |
| ` | |
| worktrees := parseWorktreesFromOutput(worktreeList) | |
| mainPath := findMainWorktreePath(worktrees) | |
| // cdToWorktree defaults to "@", so ensure "@" resolves to main. | |
| resolvedPath := resolveCdWorktreePath("@", worktrees, mainPath) | |
| assert.Equal(t, "/Users/dev/project/main", resolvedPath, | |
| "bare wtp cd should resolve to main worktree") | |
| } |
🤖 Prompt for AI Agents
In cmd/wtp/cd_test.go around lines 125 to 144, the test name and comment claim
to verify that bare "wtp cd" (no args) defaults to the main worktree but the
assertion actually tests resolution of "@" to the main worktree; rename the test
and update the top-line comment to reflect that it verifies "@" resolution
(e.g., TestCdCommand_AtResolvesToMainWorktree) or alternatively add a tiny
helper/unit test that verifies empty string resolves via "@" (call
resolveCdWorktreePath("", worktrees, mainPath) and assert same path) and keep
the current test name focusing on "@" — choose one approach and update the
function name/comment or add the helper accordingly.
Like `cd` defaults to $HOME, `wtp cd` now defaults to the main worktree (@) when invoked without arguments. Assisted-by: Claude Opus 4.5 via Crush
6bd3019 to
ff70dda
Compare
satococoa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement, thanks! Merging. 👍
Like
cddefaults to$HOMEwithout args, I thought it would be nice ifwtp cdtook the user to the main worktree,@, when invoked without args instead of emitting an error.Assisted-by: Claude Opus 4.5 via Crush
Summary by CodeRabbit
New Features
cdcommand now defaults to the main worktree when no worktree is specified; usage and examples updated to show the worktree argument as optional.Tests
✏️ Tip: You can customize this high-level summary in your review settings.