Fix shell hook behavior for "wtp cd" with no args #64
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds no-argument support for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Shell as Shell (Bash/Zsh/Fish)
participant Hook as Shell Hook Script
participant CLI as wtp CLI
User->>Shell: run "cd" with hook (no args)
Shell->>Hook: invoke hook logic for wtp cd
activate Hook
Hook->>CLI: call "wtp cd" (no args)
activate CLI
CLI-->>Hook: prints path (main worktree) or returns non-zero
deactivate CLI
alt CLI returned valid path
Hook->>Shell: execute `cd "<target_dir>"`
Shell->>User: CWD changed
else no valid path
Hook->>CLI: call "wtp cd <arg>" or show usage (fallback)
Shell->>User: fallback behavior
end
deactivate Hook
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*_test.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
📚 Learning: 2025-12-02T13:33:48.693ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignore(1 hunks)README.md(3 hunks)cmd/wtp/hook.go(3 hunks)cmd/wtp/hook_test.go(3 hunks)docs/architecture.md(3 hunks)internal/errors/errors.go(0 hunks)internal/errors/errors_test.go(2 hunks)test/e2e/shell_test.go(2 hunks)
💤 Files with no reviewable changes (1)
- internal/errors/errors.go
🧰 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 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:
internal/errors/errors_test.gotest/e2e/shell_test.gocmd/wtp/hook.gocmd/wtp/hook_test.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:
internal/errors/errors_test.gotest/e2e/shell_test.gocmd/wtp/hook_test.go
test/e2e/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
End-to-end tests must be placed in
test/e2eand exercise real git workflows using the built binary
Files:
test/e2e/shell_test.go
🧠 Learnings (17)
📓 Common learnings
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: Toggle shell integration paths with `WTP_SHELL_INTEGRATION=1` when testing cd behavior
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: 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: Applies to .wtp.yml : Project hooks must be defined in `.wtp.yml`; keep commands deterministic and safe, avoiding destructive steps by default
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
📚 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:
internal/errors/errors_test.go.gitignoretest/e2e/shell_test.gocmd/wtp/hook_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: 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:
internal/errors/errors_test.gotest/e2e/shell_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:
internal/errors/errors_test.gotest/e2e/shell_test.goREADME.mddocs/architecture.mdcmd/wtp/hook.gocmd/wtp/hook_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: Applies to **/*.go : Document exported items in Go when non-trivial; follow godoc conventions
Applied to files:
.gitignore
📚 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 **/*.go : Keep import groups tidy; use goimports for organization with local prefix following module path `github.com/satococoa/wtp/v2`
Applied to files:
.gitignorecmd/wtp/hook.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 **/*.go : Follow standard Go style with tabs and gofmt; package names must be short and lowercase
Applied to files:
.gitignore
📚 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:
test/e2e/shell_test.goREADME.mddocs/architecture.mdcmd/wtp/hook.gocmd/wtp/hook_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: Shell integration must separate completion and hooks for clarity; `wtp cd` outputs absolute worktree path with no side effects
Applied to files:
test/e2e/shell_test.goREADME.mddocs/architecture.mdcmd/wtp/hook.gocmd/wtp/hook_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:
test/e2e/shell_test.goREADME.mddocs/architecture.mdcmd/wtp/hook.gocmd/wtp/hook_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: Applies to **/*_test.go : Unit tests must be placed alongside packages as `*_test.go` files in the same directory
Applied to files:
test/e2e/shell_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: Document new workflows with realistic scenarios in `test/e2e` directory
Applied to files:
test/e2e/shell_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: Use `go run ./cmd/wtp <args>` for rapid feedback during development instead of building binaries
Applied to files:
test/e2e/shell_test.godocs/architecture.mdcmd/wtp/hook.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: The root module is `github.com/satococoa/wtp/v2` running on Go 1.24; maintain module path consistency
Applied to files:
test/e2e/shell_test.gocmd/wtp/hook.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: Worktree naming convention: main worktree is rendered as `@`; non-main worktrees display path relative to `base_dir` (e.g., `.worktrees/feat/hoge` → `feat/hoge`)
Applied to files:
README.mddocs/architecture.mdcmd/wtp/hook.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: Update README and CLI help text when user-facing behavior changes
Applied to files:
README.md
📚 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 .wtp.yml : Project hooks must be defined in `.wtp.yml`; keep commands deterministic and safe, avoiding destructive steps by default
Applied to files:
docs/architecture.mdcmd/wtp/hook.gocmd/wtp/hook_test.go
🧬 Code graph analysis (2)
internal/errors/errors_test.go (1)
internal/errors/errors.go (1)
WorktreeNameRequiredForRemove(51-63)
test/e2e/shell_test.go (1)
test/e2e/framework/assertions.go (2)
AssertNoError(75-78)AssertTrue(162-165)
🪛 GitHub Check: Lint
cmd/wtp/hook_test.go
[failure] 115-115:
File is not properly formatted (gofmt)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: Test (macos-latest, 1.24)
🔇 Additional comments (15)
.gitignore (1)
39-44: LGTM!The added ignore entries for local caches and temporary directories are appropriate for development and CI workflows. The comment clearly explains their purpose.
cmd/wtp/hook.go (3)
70-101: LGTM! Bash hook correctly handles no-arg cd.The implementation properly:
- Captures the target directory from
wtp cdwith stderr suppression- Conditionally invokes with or without the worktree argument
- Falls back to showing errors when the command fails
- Handles spaces correctly with proper quoting
103-134: LGTM! Zsh hook correctly handles no-arg cd.The implementation mirrors the bash hook with proper zsh syntax, maintaining consistency across shells.
136-166: LGTM! Fish hook correctly handles no-arg cd.The implementation properly adapts the bash/zsh pattern to Fish syntax:
- Uses
set -lfor local variables- Uses
$argv[2]for argument access- Properly quotes
"$target_dir"for paths with spacescmd/wtp/hook_test.go (1)
104-160: LGTM! Test coverage for no-arg cd behavior is solid.The tests properly verify:
- No-arg branch detection for both bash and fish
- Default behavior when no worktree is specified
- Explicit worktree handling
- Absence of legacy usage messages
- Proper quoting for paths with spaces (fish)
Note: Zsh is not explicitly tested in edge cases, but since its logic is identical to bash, the coverage is acceptable.
README.md (3)
61-61: LGTM! Clear documentation of the no-arg shorthand.The parenthetical note effectively communicates that
wtp cdwithout arguments works as a shorthand to the main worktree.
287-302: LGTM! Examples clearly demonstrate both forms.The documentation now shows:
- The no-arg form:
cd "$(wtp cd)"- The explicit form:
cd "$(wtp cd @)"This helps users understand they have options while emphasizing the convenience of the no-arg form.
312-325: LGTM! Hook usage examples are clear and well-structured.The examples effectively demonstrate the simplified syntax with hooks:
- No-arg form shown first as the primary pattern
- Explicit
@form shown as an alternative- Comments clarify the equivalence
test/e2e/shell_test.go (1)
15-30: LGTM! E2E test correctly verifies no-arg cd behavior.The test properly:
- Invokes
wtp cdwithout arguments- Verifies the command succeeds
- Compares the output path to the main worktree
- Uses
filepath.Cleanfor robust path comparisonThe assumption that the first worktree from
ListWorktrees()is the main worktree is reasonable and aligns with git's worktree list behavior.internal/errors/errors_test.go (3)
82-90: LGTM! Test correctly renamed and updated.The test function properly validates the
WorktreeNameRequiredForRemoveerror constructor, ensuring it contains:
- Clear error message
- Remove command examples
- Helpful hint to run
wtp list
415-453: LGTM! Test case properly updated for remove command context.The test case now correctly validates that
WorktreeNameRequiredForRemovecontains examples relevant to the remove command rather than cd command, which aligns with the PR's goal of makingwtp cdwith no args valid.
455-481: LGTM! Error format test updated consistently.The test array now includes
WorktreeNameRequiredForRemove, maintaining format validation coverage for all error constructors.docs/architecture.md (3)
154-160: LGTM! Architecture docs accurately describe the no-arg behavior.The documentation clearly states:
- The
[worktree]argument is optional- Defaults to main worktree when omitted
- Maintains the pure path output contract
162-176: LGTM! Shell integration flow accurately documented.The flow correctly shows direct command invocation without environment variable triggers, matching the actual implementation.
178-183: LGTM! Key design decisions are clearly articulated.The documentation effectively emphasizes:
- Pure Path Output: Clarifies that
wtp cdhas no side effects- Shell Function Wrapper: Explains the technical necessity
These points help future contributors understand the design rationale.
There was a problem hiding this comment.
Pull request overview
This PR enables wtp cd to work without arguments, defaulting to the main worktree (similar to how cd without arguments goes to $HOME). The implementation removes the previous shell integration environment variable requirement and updates all shell hooks (bash/zsh/fish) to support this new behavior.
Key Changes:
- Modified shell hooks to handle
wtp cdwith no arguments, calling the command with default behavior instead of showing a usage error - Added e2e test coverage for the no-argument case
- Removed the unused
WorktreeNameRequired()error helper function
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cmd/wtp/hook.go |
Updated bash/zsh/fish shell hooks to support no-arg wtp cd by conditionally calling the command with or without arguments, and added proper quoting for paths with spaces |
cmd/wtp/hook_test.go |
Updated tests to verify hooks support no-arg wtp cd and don't show usage errors, plus added assertions for proper space handling |
test/e2e/shell_test.go |
Added test for wtp cd with no arguments returning main worktree path; removed obsolete WTP_SHELL_INTEGRATION environment variable usage |
internal/errors/errors.go |
Removed unused WorktreeNameRequired() error helper function |
internal/errors/errors_test.go |
Updated tests to remove references to deleted WorktreeNameRequired() function |
docs/architecture.md |
Updated documentation to reflect optional worktree argument and removal of environment variable check |
README.md |
Added examples showing wtp cd with no arguments as equivalent to wtp cd @ |
.gitignore |
Added local cache directories for development environments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This follows up on #63 (
wtp cdwithout arguments defaults to the main worktree).Changes:
wtp cdwith no args (cd to main worktree).wtp cdreturning the main worktree path.Tests:
GOCACHE=$PWD/.gocache go test ./...Summary by CodeRabbit
New Features
cdwith no arguments now defaults to the main worktree.Documentation
cd.Bug Fixes
cdmore reliably and avoids spurious usage messages.Tests
cdacross shells.Chores
✏️ Tip: You can customize this high-level summary in your review settings.