Fix macOS Claude CLI detection in start.sh#4
Fix macOS Claude CLI detection in start.sh#4honcoops wants to merge 1 commit intoleonvanzyl:masterfrom
Conversation
The script was failing to detect the Claude CLI on macOS because the installer places it in ~/.claude/local/bin/ which may not be in PATH during script execution (non-interactive shell). Changes: - Check common installation paths: ~/.claude/local/bin (macOS) and ~/.local/bin (Linux) - Add found path to PATH for subsequent commands - Use $CLAUDE_CMD variable for the login command - Display the found path in success message for debugging
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
start.sh (1)
55-55: Quote the variable for defensive scripting.While the current paths are unlikely to contain spaces, it's a shell scripting best practice to quote variables during command execution to handle edge cases.
🛡️ Proposed defensive fix
- $CLAUDE_CMD login + "$CLAUDE_CMD" login
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
start.sh
🔇 Additional comments (3)
start.sh (3)
11-24: LGTM! Well-structured multi-path detection.The detection logic correctly prioritizes PATH-available commands before checking platform-specific installation directories. Using
command -vand the-xtest are appropriate for this use case. The PATH exports on lines 19 and 23 are beneficial for any subprocess invocations that might occur later in the script (e.g., from the Python application).
26-26: LGTM! Appropriate error checking.The empty string check using
-zis correct and consistent with the detection logic above.
36-36: LGTM! Helpful debugging information.Displaying the discovered path helps users verify which Claude CLI installation is being used.
The script was failing to detect the Claude CLI on macOS because the installer places it in ~/.claude/local/bin/ which may not be in PATH during script execution (non-interactive shell).
Changes:
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.