fix: add install script with platform detection #63
Conversation
WalkthroughAdds a platform-aware Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant I as install.sh
participant FS as Filesystem
participant SH as Shell / PATH
participant G as git (git-gtr)
U->>I: run `./install.sh`
I->>I: detect OS & choose install_dir
I->>FS: verify local `git-gtr` binary exists
alt binary missing
I->>U: error & exit
else
I->>FS: ensure install_dir exists (mkdir or sudo)
I->>FS: inspect `install_dir/git-gtr`
alt existing symlink -> same target
I->>U: report already installed
else existing symlink -> different target
I->>U: warn, replace symlink
I->>FS: update symlink
else non-symlink file present
I->>U: error & exit
end
I->>FS: create symlink (may use sudo)
I->>SH: check install_dir in PATH
I->>G: run `git gtr version`
alt verification success
I->>U: success message
else
I->>U: warning + PATH remediation steps
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
🧹 Nitpick comments (2)
install.sh (2)
60-82: Remove unusedplatformparameter.The
platformparameter is declared but never used in the function body. The installation directory selection logic doesn't vary by platform, so this parameter is unnecessary.🔎 Proposed fix
# Find the best installation directory find_install_dir() { - local platform="$1" - # Option 1: Homebrew bin (if Homebrew is installed) if command -v brew >/dev/null 2>&1; thenAlso update the caller at line 109:
- install_dir="$(find_install_dir "$platform")" + install_dir="$(find_install_dir)"
156-165: Consider detecting the user's shell for targeted guidance.The script assumes zsh or bash by suggesting
~/.zshrcor~/.bashrc. While these cover the majority of users, you could detect the current shell via$SHELLand provide shell-specific instructions (e.g.,~/.config/fish/config.fishfor Fish users).🔎 Example enhancement
if ! in_path "$install_dir"; then log_warn "$install_dir is not in your PATH" echo local shell_config case "$SHELL" in */zsh) shell_config="~/.zshrc" ;; */bash) shell_config="~/.bashrc" ;; */fish) shell_config="~/.config/fish/config.fish" echo "Add this to $shell_config:" echo " set -gx PATH \"$install_dir\" \$PATH" return ;; *) shell_config="your shell configuration file (~/.zshrc or ~/.bashrc)" ;; esac echo "Add this to $shell_config:" echo echo " export PATH=\"$install_dir:\$PATH\"" echo echo "Then restart your terminal or run: source $shell_config" echo fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.mdinstall.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for all shell scripts
Use snake_case naming for all functions and local variables in Bash scripts
Use UPPER_CASE naming for constants and environment variables in Bash scripts
Use 2-space indentation (no tabs) in all Bash scripts
Always quote variables and paths in Bash scripts (e.g., use"$var"not$var)
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Maintain POSIX-compatible Bash patterns targeting Bash 3.2+ compatibility
Git worktree operations must work cross-platform on macOS, Linux, and Windows Git Bash
All code must only use git and basic POSIX shell utilities - no external dependencies
Useset -efor error handling in Bash scripts where appropriate
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (7)
install.sh (4)
1-15: LGTM! Clean setup following shell script guidelines.The shebang, error handling with
set -e, and UPPER_CASE constants all follow the coding guidelines correctly.
85-88: LGTM! Clean permission check.The function correctly uses POSIX-compatible
[ ]tests and proper variable quoting.
115-135: LGTM! Robust symlink handling.The code correctly handles existing symlinks, uses
|| trueto prevent premature exit withset -e, and appropriately uses sudo when needed. The distinction between symlinks and regular files is properly managed.
170-197: LGTM! Thorough installation verification with helpful guidance.The function properly checks both PATH availability and functional operation of
git gtr, provides clear success/warning messages, and offers actionable next steps. Variable usage and quoting follow guidelines correctly.README.md (3)
24-57: LGTM! Clear installation instructions with proper shell practices.The documentation correctly references the new
install.shscript and provides platform-specific manual installation options. All command examples use proper quoting ("$(pwd)/bin/git-gtr") and the PATH export correctly escapes$PATH. The collapsible manual options section is a nice UX touch.
500-504: LGTM! Clear hook timing documentation.The updated table clearly explains when each hook executes and typical use cases. The formatting is clean and easy to read.
723-747: LGTM! Comprehensive troubleshooting guidance with correct shell commands.The troubleshooting section provides practical verification steps that follow shell best practices:
- Proper use of
2>/dev/nullfor silent fallbacks- Clean PATH inspection with
tr ':' '\n'- PATH export guidance matches the install.sh script
The debugging workflow is logical and should help users quickly identify installation issues.
…platform parameter
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
install.sh (2)
36-39: Consider usinggrep -Ffor literal path matching.The current
grep -qxtreats the path as a regex pattern. If a directory path contains special regex characters, it could cause false matches or failures. Usegrep -Fqxfor fixed/literal string matching.🔎 Proposed fix
in_path() { - echo "$PATH" | tr ':' '\n' | grep -qx "$1" + echo "$PATH" | tr ':' '\n' | grep -Fqx "$1" }
89-111: LGTM! Clean initial setup logic.The validation, platform detection, and directory selection logic are sound. All variables use proper scoping and quoting.
Optional: Consider checking executability.
Line 94 could use
-xin addition to-fto verify thatgit-gtris executable, catching configuration issues earlier.🔎 Optional improvement
- if [ ! -f "$GIT_GTR_PATH" ]; then + if [ ! -f "$GIT_GTR_PATH" ] || [ ! -x "$GIT_GTR_PATH" ]; then log_error "Could not find git-gtr at: $GIT_GTR_PATH" + log_error "Make sure the file exists and is executable." log_error "Please run this script from the git-worktree-runner directory." exit 1 fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
install.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for all shell scripts
Use snake_case naming for all functions and local variables in Bash scripts
Use UPPER_CASE naming for constants and environment variables in Bash scripts
Use 2-space indentation (no tabs) in all Bash scripts
Always quote variables and paths in Bash scripts (e.g., use"$var"not$var)
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Maintain POSIX-compatible Bash patterns targeting Bash 3.2+ compatibility
Git worktree operations must work cross-platform on macOS, Linux, and Windows Git Bash
All code must only use git and basic POSIX shell utilities - no external dependencies
Useset -efor error handling in Bash scripts where appropriate
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (9)
install.sh (9)
1-18: LGTM! Excellent setup.The shebang, error handling, color constants, and path detection follow all coding guidelines correctly. Variables are properly quoted, and
${BASH_SOURCE[0]}is the right approach for locating the script directory.
20-34: LGTM! Clean logging abstraction.The logging functions follow snake_case naming conventions and properly quote all variables. The color-coded output will improve user experience.
41-57: LGTM! Comprehensive platform detection.The platform detection correctly handles macOS, Linux, and Windows (Git Bash environments: MINGW/MSYS/CYGWIN), with a sensible fallback for unknown platforms.
59-80: LGTM! Well-designed installation directory selection.The priority order (Homebrew →
/usr/local/bin→~/bin) is sensible, and the logic properly validates directory existence and permissions before selection. The fallback to~/binensures installation can proceed without elevated privileges in most cases.
82-86: LGTM! Correct sudo detection logic.The function correctly identifies when elevated privileges are needed by checking if the directory exists but is not writable. Proper use of local variables and quoting.
112-133: LGTM! Robust handling of existing installations.The logic correctly handles three scenarios: symlink to correct target (already installed), symlink to wrong target (replace), and non-symlink file (error with clear guidance). Proper use of
|| trueforreadlinkerror handling withset -eactive, and conditionalsudousage.
152-166: LGTM! Clear PATH configuration guidance.The PATH check and user instructions are well-designed. The
\$PATHis properly escaped in the echo statement so users can copy-paste the export command directly. Good UX to callverify_installationat the end.
168-195: LGTM! Comprehensive installation verification.The verification logic properly checks both
git-gtravailability andgit gtrcommand execution, with helpful success messages and troubleshooting guidance. Good use ofcommand -vfor POSIX compatibility.
197-197: LGTM! Proper script execution.Correctly invokes the main function with properly quoted arguments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
723-747: Troubleshooting section is practical and covers the most likely root cause.The diagnostic steps (symlink presence, PATH inclusion,
whichverification) effectively narrow down the issue. The remediation paths are clear and align with the install options documented above.Optional refinement: Line 729 checks two potential symlink locations but doesn't indicate which is the "default" or expected location. Consider adding a note like "If neither exists, re-run
./install.shto create the symlink in the platform-appropriate location." This would make the connection between diagnosis and remediation clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md
🧰 Additional context used
📓 Path-based instructions (1)
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
README.md: Update README.md with installation/setup instructions when adding a new editor adapter
Update README.md with installation instructions and use cases when adding a new AI tool adapter
Files:
README.md
🔇 Additional comments (2)
README.md (2)
32-57: Manual installation options are technically sound and cover key scenarios.The three fallback options (Homebrew, system-wide, user-local) are well-chosen and use correct symlink syntax. The PATH guidance for user-local install (lines 53–54) is particularly helpful.
Minor note: Lines 38, 45, and 52 all use
"$(pwd)"to reference the bin directory. Ensure the README or install.sh notes that users must run these commands from the repository root, or consider using an explicit path in the documentation if install.sh handles this differently.
24-30: install.sh exists and implements platform detection, symlink creation, and verification as documented.The script is properly implemented at the repository root and includes:
- Platform detection via
unamefor macOS, Linux, and Windows (lines 42–57)- Symlink creation using
ln -swith automatic sudo handling (lines 141–148)- Installation directory prioritization: Homebrew bin →
/usr/local/bin→~/bin(lines 60–80)- Verification steps checking PATH and running
git gtr version(lines 168–195)- PATH guidance if the install directory is not in PATH (lines 154–162)
Minor note: The README's manual section distinguishes "macOS (Apple Silicon with Homebrew)" from "macOS (Intel) / Linux", but install.sh doesn't explicitly detect Apple Silicon vs Intel architecture—it simply tries Homebrew if installed, regardless of Mac architecture. This is more flexible in practice but differs from the documented distinction.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
install.sh (1)
46-85: Excellent fix for the critical Windows compatibility issue!The dedicated
install_windowsfunction successfully addresses the critical issue raised in the previous review by avoiding sudo and automatic symlink creation on Windows Git Bash. The manual guidance approach is the right solution.However, consider these improvements to the Windows guidance:
Line 67 - mklink path format: The fallback
|| echo "$GIT_GTR_PATH"will output a Unix-style path ifcygpathfails, butmklinkrequires Windows-style paths. Consider adding a note that the user may need to adjust the path format.Line 73 - copy destination: The suggestion to copy to
/usr/bin/git-gtrmight require admin privileges on Windows. Consider suggesting~/binor clarifying that this requires elevated permissions.🔎 Proposed improvements
echo " Open an Administrator Command Prompt and run:" echo - echo " mklink \"C:\\Program Files\\Git\\usr\\bin\\git-gtr\" \"$(cygpath -w "$GIT_GTR_PATH" 2>/dev/null || echo "$GIT_GTR_PATH")\"" + echo " mklink \"C:\\Program Files\\Git\\usr\\bin\\git-gtr\" \"$(cygpath -w "$GIT_GTR_PATH" 2>/dev/null || echo "[Windows-style path to $GIT_GTR_PATH]")\"" echo - echo " Option 3: Copy the script directly" + echo " Option 3: Copy the script directly (requires admin)" echo " ─────────────────────────────────────────────────────" echo " Copy git-gtr to a directory already in your PATH:" echo - echo " cp \"$GIT_GTR_PATH\" /usr/bin/git-gtr" + echo " cp \"$GIT_GTR_PATH\" ~/bin/git-gtr" + echo " # OR to a system directory (requires admin):" + echo " # cp \"$GIT_GTR_PATH\" /usr/bin/git-gtr" echo
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
install.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for Bash scripts
Use snake_case naming for functions in Bash scripts
Use snake_case for local variables and UPPER_CASE for constants/environment variables in Bash scripts
Use 2-space indentation (no tabs) in Bash scripts
Always quote variables and paths in Bash scripts to prevent word splitting and globbing issues
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Useset -efor error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (5)
install.sh (5)
1-19: LGTM! Clean setup following Bash best practices.The shebang, error handling with
set -e, color constants, and script directory detection all follow the coding guidelines correctly.
20-44: Well-implemented utility functions.The logging helpers provide good user feedback, and
in_pathcorrectly usesgrep -Fqxfor exact PATH matching.
87-132: Solid platform detection and directory selection logic.The three-tier fallback strategy in
find_install_diris well-designed:
- Homebrew bin (if installed and writable)
- /usr/local/bin (if exists and in PATH)
- ~/bin (user-local fallback)
The
needs_sudofunction correctly identifies when elevated privileges are required, and the logic ensures thatmkdir -pat line 190 only runs for directories that don't require sudo.
134-218: Robust installation logic with proper error handling.The main installation flow correctly:
- Validates the git-gtr binary exists before proceeding
- Detects platform and routes to Windows-specific handling
- Handles existing installations gracefully (idempotent)
- Uses sudo only when necessary for protected directories
- Provides helpful PATH setup guidance when needed
The early Windows exit (line 154) ensures that the Unix-specific symlink/sudo logic never executes on Windows platforms, fully resolving the critical issue from the previous review.
220-249: Thorough verification with helpful user guidance.The
verify_installationfunction provides valuable feedback by:
- Checking if
git-gtris discoverable in PATH- Testing actual functionality with
git gtr version- Displaying usage examples on successful installation
- Gracefully handling partial installation states
The verification correctly handles command failures without triggering
set -eexit (since they're in conditional statements), and provides actionable guidance when issues are detected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
install.sh (2)
36-39: Quote the argument in grep command.Per coding guidelines, always quote variables to prevent word splitting and globbing issues.
🔎 Proposed fix
in_path() { - echo "$PATH" | tr ':' '\n' | grep -Fqx "$1" + local dir="$1" + echo "$PATH" | tr ':' '\n' | grep -Fqx "$dir" }
41-44: Unused function:is_windowsis defined but never called.The
mainfunction usesdetect_platformdirectly and compares with"windows". This function can be removed to reduce dead code.🔎 Proposed fix
-# Check if running on Windows (Git Bash, MSYS, Cygwin) -is_windows() { - [ "$(detect_platform)" = "windows" ] -} -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
install.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for Bash scripts
Use snake_case naming for functions in Bash scripts
Use snake_case for local variables and UPPER_CASE for constants/environment variables in Bash scripts
Use 2-space indentation (no tabs) in Bash scripts
Always quote variables and paths in Bash scripts to prevent word splitting and globbing issues
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Useset -efor error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (10)
install.sh (10)
1-18: LGTM!Proper shebang,
set -efor error handling, UPPER_CASE for constants, and correctly quoted path variables.
20-34: LGTM!Logging functions follow snake_case convention and provide clear, colored output.
46-85: LGTM!Windows-specific handling is well-designed: provides multiple installation options, uses
cygpathwith fallback, and verifies PATH status. This properly addresses the Windows compatibility concerns from previous reviews.
87-103: LGTM!Clean platform detection with appropriate handling for macOS, Linux, and Windows variants (MINGW, MSYS, CYGWIN).
105-126: LGTM!Sensible priority order: Homebrew bin (if writable) →
/usr/local/bin(if in PATH) →~/bin. Theneeds_sudocheck later handles write permission requirements appropriately.
128-132: LGTM!Correctly determines if sudo is needed based on directory existence and write permissions.
134-155: LGTM!Clean entry point with proper validation: verifies
git-gtrexists, detects platform, and handles Windows separately.
164-185: LGTM!Robust symlink handling: detects existing symlinks, handles same/different targets appropriately, and errors gracefully when a non-symlink file blocks installation.
187-218: LGTM!Directory creation is correctly scoped to
~/bin(since/usr/local/binis only selected if it already exists). PATH warning with actionable guidance is helpful.
220-249: LGTM!Thorough verification with clear feedback: confirms PATH availability, tests the actual command, and provides helpful next steps for both success and failure cases.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
install.sh (1)
134-139: Consider adding executable permission check.The script checks if
git-gtrexists with-f, but doesn't verify it's executable with-x. If the file exists but lacks execute permission, installation appears to succeed but commands fail later.Proposed improvement
# Verify git-gtr exists - if [ ! -f "$GIT_GTR_PATH" ]; then + if [ ! -f "$GIT_GTR_PATH" ]; then log_error "Could not find git-gtr at: $GIT_GTR_PATH" log_error "Please run this script from the git-worktree-runner directory." exit 1 fi + + if [ ! -x "$GIT_GTR_PATH" ]; then + log_warn "git-gtr is not executable, fixing permissions..." + chmod +x "$GIT_GTR_PATH" + fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
install.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.sh
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing
Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true
**/*.sh: Use shebang#!/usr/bin/env bash(not/bin/bashor/bin/sh) for Bash scripts
Use snake_case naming for functions in Bash scripts
Use snake_case for local variables and UPPER_CASE for constants/environment variables in Bash scripts
Use 2-space indentation (no tabs) in Bash scripts
Always quote variables and paths in Bash scripts to prevent word splitting and globbing issues
Check return codes and use|| exit 1or|| return 1for error handling in Bash scripts
Useset -efor error handling in Bash scripts
Uselocalkeyword for function-scoped variables in Bash scripts
Files:
install.sh
**/*.{bash,fish,sh}
📄 CodeRabbit inference engine (.github/instructions/sh.instructions.md)
**/*.{bash,fish,sh}: Bash 3.2+ compatible (macOS default), but 4.0+ features allowed where appropriate
Always quote variables: use"$var"not$var
Use function-scoped variables:local var="value"
Check return codes; functions return 1 on failure
Usesnake_casefor functions and variables,UPPER_CASEfor constants
Prefer[ ]over[[ ]]for POSIX portability; use[[only when needed
Always quote glob inputs; disable unintended globbing withset -ftemporarily if required
Avoid associative arrays in shell scripts; use simple string/loop constructs for Bash 3.2+ compatibility
Avoidreadarrayand process substitution unsupported in older Bash
Debug withbash -x ./bin/gtr <cmd>or wrap suspicious blocks withset -x/set +x
Check function presence withdeclare -f create_worktreeordeclare -f resolve_target
Use stderr for variable inspection:echo "DEBUG var=$var" >&2to keep stdout clean for command substitution
Keep dependencies minimal: only usegit,sed,awk,find,grep; avoid jq/curl unless justified
Check availability of external commands before use when adding new tools
Use"${var}"for variable expansion; for line loops usewhile IFS= read -r line; do ... doneto preserve spaces
Sanitize branch names viasanitize_branch_namefunction; do NOT duplicate logic elsewhere
Everything is sourced at startup withset -eenabled; functions call each other directly; no subshells except for hooks and AI tools
Files:
install.sh
🔇 Additional comments (10)
install.sh (10)
1-18: LGTM!The script setup follows all coding guidelines: correct shebang (
#!/usr/bin/env bash),set -eactive, UPPER_CASE for constants, and proper quoting for path detection usingBASH_SOURCE[0].
20-34: LGTM!Logging functions follow snake_case naming convention and provide clear, color-coded output for different message types.
36-39: LGTM with minor edge case consideration.The implementation is correct for typical PATH entries. Note that trailing slashes in PATH entries (e.g.,
/usr/local/bin/vs/usr/local/bin) could cause false negatives, but this is rare in practice.
41-80: Good Windows handling approach.The script correctly avoids automatic symlink creation on Windows Git Bash and provides clear manual installation options. The
cygpathfallback at line 62 properly handles cases where the command isn't available.
82-98: LGTM!Platform detection covers all expected environments with appropriate pattern matching for Windows variants (MINGW, MSYS, CYGWIN).
100-121: LGTM!The installation directory priority is well-thought-out: writable Homebrew bin takes precedence, followed by existing
/usr/local/bin, with~/binas a safe fallback. The checks ensure appropriate directory selection without requiring sudo unless necessary.
123-127: LGTM!The sudo check logic is correct: returns true only when the directory exists but isn't writable by the current user.
159-180: LGTM!Existing target handling is comprehensive: correctly identifies same-target symlinks, replaces different-target symlinks with warning, and errors appropriately for non-symlink files. The
|| trueguard onreadlinkfollows the coding guidelines for non-critical failures.
182-213: LGTM!Directory and symlink creation logic correctly uses sudo only when needed. The PATH warning provides actionable guidance with shell-specific configuration file suggestions.
215-244: LGTM!The verification function provides good feedback by checking both
command -vavailability and actualgit gtr versionexecution, with helpful guidance when the command isn't immediately available.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.