-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix NVM Node.js PATH issue for locally installed Claude #346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix NVM Node.js PATH issue for locally installed Claude #346
Conversation
Problem: When opcode is launched from /Applications on macOS, Claude commands fail with 'env: node: No such file or directory' for npm-local Claude installations that depend on Node.js from NVM. Root cause: GUI-launched apps receive minimal PATH that lacks NVM directories, and existing code only added NVM paths when Claude itself was in NVM. Solution: - Added NVM detection in claude_binary.rs for when Claude needs Node from NVM - Fixed unused _std_cmd variable in commands/claude.rs to actually use enhanced PATH - Added targeted override that only applies when Claude is NOT in NVM but needs it This is a minimal, targeted fix with zero impact on other installation types. Tested and confirmed working on macOS where the edge case was occurring. Not tested on other environments or operating systems.
Have written about the process of this contribution here: https://humanwritten.ai/os1/ |
Hi, thanks for the PR. I just ran this PR through claude. Can you take a look at these suggestions? 1. Version Sort AlgorithmCurrent: node_versions.sort_by(|a, b| b.cmp(a)); Issue: Simple string comparison may not work correctly for all semantic versions:
Recommendation: Use semantic version parsing for more robust comparison: node_versions.sort_by(|a, b| {
// Parse semantic version (v20.10.0 -> [20, 10, 0])
let parse_version = |v: &str| {
v.trim_start_matches('v')
.split('.')
.filter_map(|s| s.parse::<u32>().ok())
.collect::<Vec<u32>>()
};
let a_parts = parse_version(a);
let b_parts = parse_version(b);
b_parts.cmp(&a_parts)
}); 2. Homebrew Node.js Not ConsideredCurrent: Only adds NVM Node.js paths. Issue: Users may have Node.js installed via Homebrew ( Recommendation: Consider adding Homebrew paths as fallback: // After NVM check, also check for Homebrew Node.js
if cfg!(target_os = "macos") {
let homebrew_paths = vec![
"/opt/homebrew/bin",
"/usr/local/bin"
];
for hb_path in homebrew_paths {
if !current_path.contains(hb_path) {
let node_check = PathBuf::from(hb_path).join("node");
if node_check.exists() {
let new_path = format!("{}:{}", hb_path, current_path);
cmd.env("PATH", new_path);
break;
}
}
}
} 3. Duplicate Logic Between FilesIssue: Similar PATH enhancement logic exists in both files (claude_binary.rs:492-535 and claude.rs:279-290). Recommendation: Consider extracting to a shared helper function to reduce duplication and improve maintainability. 4. Error HandlingCurrent: Silent failures with Recommendation: Log errors when directory reading fails: let mut node_versions: Vec<String> = entries
.filter_map(|entry| match entry {
Ok(e) => Some(e),
Err(err) => {
log::warn!("Failed to read NVM directory entry: {}", err);
None
}
})
// ... rest of chain 5. PATH OrderingCurrent: Prepends NVM path: Consideration: This gives NVM Node.js highest priority. Verify this is the desired behavior - it will override system Node.js installations. |
@123vivekr - i actually went through a loop of this but I didn't want to change more than was needed to sort the fix for my NVM specific install issue. The sort algorithm is fine and fixed my issue which is all that I'm trying to submit. With regards to the other points they're missing context as homebrew + other conditions already exist throughout the code - I chose purposely not to re-write/refactor these and just to fix my issue as the rest isn't my code. I went through claude analysis loop too and it suggested making many more changes but i guided it to keep it simple and just solve the specific edge case not impacting other logic. |
- Add else clause to detect NVM Node.js when Claude isn't in NVM - Automatically find and add latest NVM Node.js version to PATH - Fix 'env: node: No such file or directory' for npm-local Claude - Only triggers when Claude NOT in NVM but needs Node from NVM - Zero impact on other installation types This resolves PATH issues when opcode launched from /Applications on macOS with npm-local Claude installations requiring NVM Node.js. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added logging to confirm all integrated features work at runtime: - PR winfunc#261 (Mermaid): Track cache hits and render success - PR winfunc#222 (MCP Async): Track sidecar vs regular execution - PR winfunc#346 (NVM PATH): Track Node.js PATH additions All logs prefixed with PR numbers for easy identification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Problem
When opcode is launched from /Applications folder on macOS, it receives a minimal PATH environment that doesn't include NVM directories. This causes Claude commands to fail with "env: node: No such file or directory" for users with npm-local Claude installations that depend on Node.js from NVM.
Root Cause
The existing code had an unused
_std_cmd
variable incommands/claude.rs
that was creating an enhanced PATH but never using it. Additionally, NVM path detection only worked when Claude itself was installed in an NVM directory, not when Claude was elsewhere but needed Node.js from NVM.Solution
This PR provides a minimal, targeted fix:
_std_cmd
so the enhanced PATH is actually usedclaude_binary.rs
to detect and add NVM Node.js paths even when Claude isn't installed via NVMChanges
src-tauri/src/claude_binary.rs
: Added else clause for NVM detection when Claude needs Node but isn't in NVMsrc-tauri/src/commands/claude.rs
: Fixed unused_std_cmd
and added targeted PATH overrideTesting
Impact
Fixes the issue where npm-local Claude installations fail when opcode is launched from /Applications due to missing Node.js in PATH.