Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a git worktree synchronization system: two declarative configs (symlink and copy), a new wt-sync.sh to apply them across worktrees, and updates wt-add.sh to invoke the synchronizer and use explicit repository-root paths. Also tightens .gitignore patterns. Changes
Sequence DiagramsequenceDiagram
participant User
participant wt-add.sh
participant Git
participant wt-sync.sh
participant Configs
participant Worktree
User->>wt-add.sh: run wt-add <name>
wt-add.sh->>wt-add.sh: resolve SCRIPT_DIR, MAIN_REPO, WORKTREE_PATH
wt-add.sh->>Git: git -C MAIN_REPO worktree add <WORKTREE_PATH>
Git->>Worktree: create worktree
wt-add.sh->>wt-sync.sh: wt-sync.sh --worktree <WORKTREE_PATH>
wt-sync.sh->>Configs: read sync.conf and copy.conf
wt-sync.sh->>Worktree: create relative symlinks for shared files
wt-sync.sh->>Worktree: copy per-worktree files
wt-sync.sh->>wt-add.sh: return status
wt-add.sh->>Worktree: write VS Code settings, install hooks, run npm install
wt-add.sh->>User: report completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/worktree/wt-add.sh (1)
46-46: Invalid JSON comment syntax.Line 46 has
/ --- Formatting ---but JSON comments in VS Code settings use//. This will cause a JSON parse error when VS Code loads the settings.🐛 Proposed fix
- / --- Formatting --- + // --- Formatting ---
🤖 Fix all issues with AI agents
In `@scripts/worktree/wt-sync.sh`:
- Around line 120-131: The relative symlink logic in wt-sync.sh currently builds
relative_source as "../$MAIN_REPO_NAME/$rel_path", which fails when worktrees
aren’t direct siblings; replace that naive construction with a robust
relative-path computation (e.g., use realpath --relative-to "$target_dir"
"$source" and fall back to a short Python snippet when realpath --relative-to is
unavailable) to set relative_source, and when checking existing symlink targets
(readlink "$target") normalize/resolve both sides before comparison so
comparisons handle absolute vs relative links correctly; update the block that
computes target_dir/relative_source and the readlink comparison to use the new
computed relative path and normalization.
🧹 Nitpick comments (2)
scripts/worktree/wt-sync.sh (2)
121-121: Declare and assign variables separately to avoid masking return values.ShellCheck SC2155: When you use
local var=$(command), a failing command's exit status is masked bylocal's success. Separate declaration from assignment to preserve error handling withset -e.♻️ Proposed fixes
# Calculate relative path from target's parent to source - local target_dir=$(dirname "$target") + local target_dir + target_dir=$(dirname "$target") local relative_source="../$MAIN_REPO_NAME/$rel_path" # Already a correct symlink? if [[ -L "$target" ]]; then - local current=$(readlink "$target") + local current + current=$(readlink "$target") if [[ "$current" == "$relative_source" || "$current" == "$source" ]]; thenAt line 190:
# Create parent directory if needed - local target_dir=$(dirname "$target") + local target_dir + target_dir=$(dirname "$target") if [[ ! -d "$target_dir" ]]; thenAt line 211:
sync_worktree() { local worktree_path="$1" - local worktree_name=$(basename "$worktree_path") + local worktree_name + worktree_name=$(basename "$worktree_path")Also applies to: 126-126, 190-190, 211-211
100-102: Consider safer whitespace trimming.Using
xargsfor whitespace trimming can misbehave with paths containing quotes, backslashes, or special characters. A pure-bash approach is safer.♻️ Suggested safer whitespace trimming
# Trim whitespace - line=$(echo "$line" | xargs) + line="${line#"${line%%[![:space:]]*}"}" # trim leading + line="${line%"${line##*[![:space:]]}"}" # trim trailing [[ -n "$line" ]] && echo "$line"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignorescripts/worktree/copy.confscripts/worktree/sync.confscripts/worktree/wt-add.shscripts/worktree/wt-sync.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/worktree/wt-sync.sh
[warning] 121-121: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 126-126: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 190-190: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 211-211: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (8)
.gitignore (1)
97-97: LGTM! Broadening ignore patterns for AI assistant files is appropriate.The change adds general patterns (
.cursor,.claude,.codex,.gemini) that ignore both files and directories with these names, consistent with how other editor-related entries are handled (.ideaand.vscodeon lines 68-69). No tracked files match these patterns, so there are no unintended consequences.scripts/worktree/copy.conf (1)
1-10: LGTM!The configuration file is well-documented with clear format instructions. Using COPY mode for
.env.developmentis appropriate since each worktree may need independent environment configurations.scripts/worktree/wt-sync.sh (2)
80-87: LGTM!The worktree discovery logic correctly filters out the main repo and handles the porcelain output format properly.
249-294: LGTM!The main function properly handles both single worktree targeting and batch synchronization. The worktree path resolution aligns with how
wt-add.shcreates worktrees as siblings of the main repo.scripts/worktree/wt-add.sh (3)
4-6: LGTM!Good use of robust path resolution to establish
SCRIPT_DIR,MAIN_REPO, andPARENT_DIR. This ensures the script works correctly regardless of the current working directory.
29-31: LGTM!Clean delegation to
wt-sync.shfor file synchronization, keeping concerns separated between worktree creation and file syncing.
118-134: LGTM!The pre-commit hook setup correctly uses worktree-specific configuration with
extensions.worktreeConfigand--worktreeflag, ensuring the hook configuration doesn't affect other worktrees.scripts/worktree/sync.conf (1)
14-19: These entries do not yet exist in the repository, so the directory format cannot be verified.The sync.conf format documentation (lines 5–7) correctly specifies that directories require a trailing slash. However, all entries listed in lines 14–19 (.coderabbit.yml, .claude, CLAUDE.md, .codex, AGENTS.md, .gemini) are not found in the repository. When these entries are added, ensure that any directories include a trailing slash per the documented format.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
2 participants
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.