Repository navigation
resolve_base_dir detects worktrees dir inside repo when it's actually not #12
Description
Activity
@coderabbitai do it
Reacted by coderabbitai- added a commit that references this issue
on Nov 17, 2025 @coderabbitai plan
Reacted by coderabbitaiImplementation Plan
Approach
Fix a bug where
resolve_base_dirincorrectly identifies a worktrees directory as being inside the repository when the configured path uses..(parent directory) notation. The issue occurs because bash pattern matching evaluates path strings before they're canonicalized, causing paths like../worktreesto incorrectly match the repository path pattern.Observations
This is a bash-based git worktree management tool with a modular structure. The core functionality lives in
lib/core.sh, which includes theresolve_base_dirfunction responsible for resolving and validating worktree directory locations. Path canonicalization is handled by thecanonicalize_pathfunction that usescd -Pto resolve symlinks and normalize paths. Configuration is managed throughlib/config.sh, and the codebase currently has no automated test infrastructure.Assumptions
Assumption 1: How to handle path normalization for non-existent directories?
Options Considered:
- Add a text-based path normalization function that manually processes
..and.components - Use external utilities like
realpath -morreadlink -mwith fallback logic - Canonicalize the parent directory and append the basename for non-existent paths
Chosen Option: Canonicalize the parent directory and append the basename for non-existent paths
Rationale: This approach reuses the existing
canonicalize_pathfunction, aligns with the codebase's bash-centric patterns, requires minimal code changes, and handles the common case where the parent directory exists even if the worktrees directory itself hasn't been created yet. Developers will find this approach intuitive since it builds on existing patterns rather than introducing new utilities or complex string manipulation.💡 User Tips
Regenerate the plan with different choices with
@coderabbitai <feedback>.Plan
Phase 1: Fix Path Canonicalization Timing
This phase addresses the root cause by ensuring paths are fully normalized before the containment check runs, regardless of whether the worktrees directory exists.
Task 1: Add Path Normalization for Non-Existent Directories
Enhance the path resolution logic in
lib/core.shwithin theresolve_base_dirfunction to handle non-existent paths.- Add logic after the tilde expansion block (after line 61) but before the existing canonicalization block (before line 71)
- When
base_diris set but doesn't exist as a directory, canonicalize its parent directory using the existingcanonicalize_pathfunction - Extract the parent path using
dirnameand the final component usingbasename - If the parent directory exists and can be canonicalized, reconstruct
base_diras the canonicalized parent plus the basename - This ensures paths like
/repo/../worktreesare normalized to/worktreeseven when the worktrees directory doesn't exist yet
Task 2: Verify Canonicalization Happens Before Pattern Matching
Ensure the containment check uses canonicalized paths.
- Confirm that by the time execution reaches the pattern matching check (currently line 91:
[[ "$base_dir" == "$canonical_repo_root"/* ]]), thebase_dirvariable has been normalized through either the existing canonicalization block (for existing directories) or the new normalization logic (for non-existent directories) - The existing conditional check
[ -d "$base_dir" ]before the pattern match remains appropriate since it prevents false warnings when the path is being normalized but hasn't been created yet
🤖 Prompt for AI agents
Fix the path canonicalization timing issue in `resolve_base_dir` to ensure paths are normalized before containment checks. In `lib/core.sh`, modify the `resolve_base_dir` function: - Add path normalization logic for non-existent directories after tilde expansion but before existing canonicalization - When `base_dir` is set but doesn't exist, use `dirname` to get the parent path and `basename` to get the final component - Canonicalize the parent directory using the existing `canonicalize_path` function - Reconstruct `base_dir` by combining the canonicalized parent with the basename - Ensure this normalization happens before the pattern matching check that validates worktrees aren't inside the repository - Verify the containment check receives canonicalized paths from either the existing block (for existing dirs) or new logic (for non-existent dirs)Phase 2: Validate Fix Correctness
This phase ensures the fix works correctly across different scenarios without breaking existing functionality.
Task 1: Add Inline Documentation
Document the path normalization logic and the bug fix in
lib/core.sh.- Add comments explaining why the parent directory canonicalization is necessary for non-existent paths
- Document the specific case being handled: relative paths with
..that would incorrectly match the repository pattern before normalization - Note that this handles the gap between configuration parsing and directory creation
Task 2: Manual Testing Verification
Since there's no automated test infrastructure, document the manual testing approach for developers.
- Update
CLAUDE.mdor relevant documentation with test scenarios to verify the fix - Test cases should include: setting
gtr.worktrees.dir = .., setting it to../worktrees, using both environment variable and git config, and testing with both existing and non-existing worktrees directories - Verify that the warning about worktrees being inside the repository is correctly suppressed when using
..paths that resolve outside the repo - Verify that legitimate warnings still appear when worktrees are actually configured inside the repository
🤖 Prompt for AI agents
Document the fix and provide manual testing guidance. In `lib/core.sh`: - Add inline comments explaining the parent directory canonicalization for non-existent paths - Document that this fixes relative paths with `..` incorrectly matching repository patterns before normalization - Note this handles the gap between config parsing and directory creation In `CLAUDE.md` or relevant documentation: - Add manual test scenarios including: `gtr.worktrees.dir = ..`, `../worktrees`, environment variable vs git config, existing vs non-existing directories - Document expected behavior: warnings suppressed for `..` paths resolving outside repo, warnings still shown for paths actually inside repo
🚀 Next Steps
🤖 All AI agent prompts combined
Task: 1 Fix the path canonicalization timing issue in `resolve_base_dir` to ensure paths are normalized before containment checks. In `lib/core.sh`, modify the `resolve_base_dir` function: - Add path normalization logic for non-existent directories after tilde expansion but before existing canonicalization - When `base_dir` is set but doesn't exist, use `dirname` to get the parent path and `basename` to get the final component - Canonicalize the parent directory using the existing `canonicalize_path` function - Reconstruct `base_dir` by combining the canonicalized parent with the basename - Ensure this normalization happens before the pattern matching check that validates worktrees aren't inside the repository - Verify the containment check receives canonicalized paths from either the existing block (for existing dirs) or new logic (for non-existent dirs) =============================================================================== Task: 2 Document the fix and provide manual testing guidance. In `lib/core.sh`: - Add inline comments explaining the parent directory canonicalization for non-existent paths - Document that this fixes relative paths with `..` incorrectly matching repository patterns before normalization - Note this handles the gap between config parsing and directory creation In `CLAUDE.md` or relevant documentation: - Add manual test scenarios including: `gtr.worktrees.dir = ..`, `../worktrees`, environment variable vs git config, existing vs non-existing directories - Document expected behavior: warnings suppressed for `..` paths resolving outside repo, warnings still shown for paths actually inside repo💡 Iterate on the plan with:
@coderabbitai <feedback>Example Feedback - @coderabbitai You can skip phase 3. Add a simple unit test case for phase 2. - @coderabbitai For assumption 1 go ahead with option 3 and replan.- Add a text-based path normalization function that manually processes
Implementation Plan
Approach
Fix a bug where
resolve_base_dirincorrectly identifies a worktrees directory as being inside the repository when the configured path uses..(parent directory) notation. The issue occurs because bash pattern matching evaluates path strings before they're canonicalized, causing paths like../worktreesto incorrectly match the repository path pattern.Observations
This is a bash-based git worktree management tool with a modular structure. The core functionality lives in
lib/core.sh, which includes theresolve_base_dirfunction responsible for resolving and validating worktree directory locations. Path canonicalization is handled by thecanonicalize_pathfunction that usescd -Pto resolve symlinks and normalize paths. Configuration is managed throughlib/config.sh, and the codebase currently has no automated test infrastructure.Assumptions
Assumption 1: How to handle path normalization for non-existent directories?
Options Considered:
- Add a text-based path normalization function that manually processes
..and.components - Use external utilities like
realpath -morreadlink -mwith fallback logic - Canonicalize the parent directory and append the basename for non-existent paths
Chosen Option: Canonicalize the parent directory and append the basename for non-existent paths
Rationale: This approach reuses the existing
canonicalize_pathfunction, aligns with the codebase's bash-centric patterns, requires minimal code changes, and handles the common case where the parent directory exists even if the worktrees directory itself hasn't been created yet. Developers will find this approach intuitive since it builds on existing patterns rather than introducing new utilities or complex string manipulation.💡 User Tips
Regenerate the plan with different choices with
@coderabbitai <feedback>.Plan
Phase 1: Fix Path Canonicalization Timing
This phase addresses the root cause by ensuring paths are fully normalized before the containment check runs, regardless of whether the worktrees directory exists.
Task 1: Add Path Normalization for Non-Existent Directories
Enhance the path resolution logic in
lib/core.shwithin theresolve_base_dirfunction to handle non-existent paths.- Add logic after the tilde expansion block (after line 61) but before the existing canonicalization block (before line 71)
- When
base_diris set but doesn't exist as a directory, canonicalize its parent directory using the existingcanonicalize_pathfunction - Extract the parent path using
dirnameand the final component usingbasename - If the parent directory exists and can be canonicalized, reconstruct
base_diras the canonicalized parent plus the basename - This ensures paths like
/repo/../worktreesare normalized to/worktreeseven when the worktrees directory doesn't exist yet
Task 2: Verify Canonicalization Happens Before Pattern Matching
Ensure the containment check uses canonicalized paths.
- Confirm that by the time execution reaches the pattern matching check (currently line 91:
[[ "$base_dir" == "$canonical_repo_root"/* ]]), thebase_dirvariable has been normalized through either the existing canonicalization block (for existing directories) or the new normalization logic (for non-existent directories) - The existing conditional check
[ -d "$base_dir" ]before the pattern match remains appropriate since it prevents false warnings when the path is being normalized but hasn't been created yet
🤖 Prompt for AI agents
Fix the path canonicalization timing issue in `resolve_base_dir` to ensure paths are normalized before containment checks. In `lib/core.sh`, modify the `resolve_base_dir` function: - Add path normalization logic for non-existent directories after tilde expansion but before existing canonicalization - When `base_dir` is set but doesn't exist, use `dirname` to get the parent path and `basename` to get the final component - Canonicalize the parent directory using the existing `canonicalize_path` function - Reconstruct `base_dir` by combining the canonicalized parent with the basename - Ensure this normalization happens before the pattern matching check that validates worktrees aren't inside the repository - Verify the containment check receives canonicalized paths from either the existing block (for existing dirs) or new logic (for non-existent dirs)Phase 2: Validate Fix Correctness
This phase ensures the fix works correctly across different scenarios without breaking existing functionality.
Task 1: Add Inline Documentation
Document the path normalization logic and the bug fix in
lib/core.sh.- Add comments explaining why the parent directory canonicalization is necessary for non-existent paths
- Document the specific case being handled: relative paths with
..that would incorrectly match the repository pattern before normalization - Note that this handles the gap between configuration parsing and directory creation
Task 2: Manual Testing Verification
Since there's no automated test infrastructure, document the manual testing approach for developers.
- Update
CLAUDE.mdor relevant documentation with test scenarios to verify the fix - Test cases should include: setting
gtr.worktrees.dir = .., setting it to../worktrees, using both environment variable and git config, and testing with both existing and non-existing worktrees directories - Verify that the warning about worktrees being inside the repository is correctly suppressed when using
..paths that resolve outside the repo - Verify that legitimate warnings still appear when worktrees are actually configured inside the repository
🤖 Prompt for AI agents
Document the fix and provide manual testing guidance. In `lib/core.sh`: - Add inline comments explaining the parent directory canonicalization for non-existent paths - Document that this fixes relative paths with `..` incorrectly matching repository patterns before normalization - Note this handles the gap between config parsing and directory creation In `CLAUDE.md` or relevant documentation: - Add manual test scenarios including: `gtr.worktrees.dir = ..`, `../worktrees`, environment variable vs git config, existing vs non-existing directories - Document expected behavior: warnings suppressed for `..` paths resolving outside repo, warnings still shown for paths actually inside repo
🚀 Next Steps
🤖 All AI agent prompts combined
Task: 1 Fix the path canonicalization timing issue in `resolve_base_dir` to ensure paths are normalized before containment checks. In `lib/core.sh`, modify the `resolve_base_dir` function: - Add path normalization logic for non-existent directories after tilde expansion but before existing canonicalization - When `base_dir` is set but doesn't exist, use `dirname` to get the parent path and `basename` to get the final component - Canonicalize the parent directory using the existing `canonicalize_path` function - Reconstruct `base_dir` by combining the canonicalized parent with the basename - Ensure this normalization happens before the pattern matching check that validates worktrees aren't inside the repository - Verify the containment check receives canonicalized paths from either the existing block (for existing dirs) or new logic (for non-existent dirs) =============================================================================== Task: 2 Document the fix and provide manual testing guidance. In `lib/core.sh`: - Add inline comments explaining the parent directory canonicalization for non-existent paths - Document that this fixes relative paths with `..` incorrectly matching repository patterns before normalization - Note this handles the gap between config parsing and directory creation In `CLAUDE.md` or relevant documentation: - Add manual test scenarios including: `gtr.worktrees.dir = ..`, `../worktrees`, environment variable vs git config, existing vs non-existing directories - Document expected behavior: warnings suppressed for `..` paths resolving outside repo, warnings still shown for paths actually inside repo💡 Iterate on the plan with:
@coderabbitai <feedback>Example Feedback - @coderabbitai You can skip phase 3. Add a simple unit test case for phase 2. - @coderabbitai For assumption 1 go ahead with option 3 and replan.- Add a text-based path normalization function that manually processes
If
gtr.worktrees.dirorGTR_WORKTREES_DIRare set to..,resolve_base_dirwill detect the worktrees dir "inside" the repo when it's actually not.The culprit seems to be this conditional:
where
/*includes directory entries.and..