Skip to content
This repository was archived by the owner on Nov 22, 2025. It is now read-only.

Commit 1f768e4

Browse files
pacphiclaude
andcommitted
fix(nodejs): remove npm prefix configuration incompatible with NVM
NVM manages global packages in version-specific directories, and setting a custom npm prefix causes conflicts and errors. Updated nodejs extension to use NVM's built-in global package management instead of a custom ~/.npm-global directory. Changes: - Removed npm config set prefix command - Removed custom npm-global directory creation - Added cleanup for existing conflicting prefix configs - Updated validation to check for NVM-managed paths - Updated documentation with proper guidance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent db59dc4 commit 1f768e4

3 files changed

Lines changed: 183 additions & 42 deletions

File tree

docker/lib/extensions.d/README.md

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,13 +452,35 @@ Use provided print functions:
452452
- `print_debug` - Debug output (only when DEBUG=true)
453453

454454
### 5. User-Space Installation
455-
Avoid sudo when possible:
455+
Avoid sudo when possible. Use version managers and user-space installation methods:
456+
456457
```bash
457-
# Good: User-space installation
458-
npm config set prefix "$HOME/.npm-global"
458+
# Good: Use version managers (recommended pattern)
459+
curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.3/install.sh | bash # Node.js
460+
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh # Rust
461+
curl -s "https://get.sdkman.io" | bash # JVM languages
462+
463+
# Good: Language package managers with user flags
464+
# Note: Replace "package" with actual package name (e.g., requests, ripgrep, rails)
465+
pip install --user package # Python
466+
cargo install package # Rust (installs to ~/.cargo/bin)
467+
gem install --user-install package # Ruby
459468

460469
# Avoid: System-wide installation requiring sudo
461470
sudo npm install -g package
471+
sudo pip install package
472+
sudo gem install package
473+
```
474+
475+
**Important for Node.js/NVM**: Do NOT set npm prefix when using NVM:
476+
```bash
477+
# WRONG: Conflicts with NVM
478+
npm config set prefix "$HOME/.npm-global"
479+
480+
# CORRECT: Let NVM manage global packages
481+
# NVM already provides user-space global installs without sudo
482+
# Global packages install to: $NVM_DIR/versions/node/vX.X.X/bin
483+
npm install -g package # No sudo needed, installs to NVM directory
462484
```
463485

464486
### 6. SSH Session Support

docker/lib/extensions.d/nodejs.sh.example

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,13 @@ configure() {
119119
export NVM_DIR="$HOME/.nvm"
120120
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
121121

122-
# Setup npm global directory in user space (avoid sudo for global packages)
123-
local npm_global_dir="$HOME/.npm-global"
122+
# NOTE: Do NOT set npm prefix when using NVM
123+
# NVM manages global packages in version-specific directories:
124+
# $NVM_DIR/versions/node/vX.X.X/lib/node_modules
125+
# Setting a custom prefix is incompatible with NVM and causes errors.
126+
# NVM already provides user-space global installs without sudo.
124127

125-
if [[ ! -d "$npm_global_dir" ]]; then
126-
mkdir -p "$npm_global_dir"
127-
npm config set prefix "$npm_global_dir" --location=global
128-
print_success "npm global directory configured: $npm_global_dir"
129-
fi
130-
131-
# Add NVM and npm global bin to PATH
132-
local npm_bin_path="$npm_global_dir/bin"
128+
# Add NVM to shell configuration
133129
if ! grep -q "NVM_DIR" "$HOME/.bashrc" 2>/dev/null; then
134130
echo "" >> "$HOME/.bashrc"
135131
echo "# ${EXT_NAME} - NVM (Node Version Manager)" >> "$HOME/.bashrc"
@@ -139,26 +135,14 @@ configure() {
139135
print_success "Added NVM loading to .bashrc"
140136
fi
141137

142-
if [[ -d "$npm_bin_path" ]]; then
143-
if ! grep -q "$npm_bin_path" "$HOME/.bashrc" 2>/dev/null; then
144-
echo "" >> "$HOME/.bashrc"
145-
echo "# ${EXT_NAME} - npm global packages" >> "$HOME/.bashrc"
146-
echo "export PATH=\"$npm_bin_path:\$PATH\"" >> "$HOME/.bashrc"
147-
print_success "Added npm global bin to PATH"
148-
fi
149-
150-
# Export for current session
151-
export PATH="$npm_bin_path:$PATH"
152-
fi
153-
154138
# Setup SSH wrapper for node/npm/nvm
155139
if command_exists node; then
156140
local node_path=$(which node)
157141
local npm_path=$(which npm)
158142

159143
if command_exists setup_tool_path 2>/dev/null; then
160144
setup_tool_path "${EXT_NAME}" \
161-
"export NVM_DIR=\"\$HOME/.nvm\"; [ -s \"\$NVM_DIR/nvm.sh\" ] && \\. \"\$NVM_DIR/nvm.sh\"; export PATH=\"$npm_bin_path:\$PATH\""
145+
"export NVM_DIR=\"\$HOME/.nvm\"; [ -s \"\$NVM_DIR/nvm.sh\" ] && \\. \"\$NVM_DIR/nvm.sh\""
162146
fi
163147

164148
if command_exists create_tool_wrapper 2>/dev/null; then
@@ -171,7 +155,14 @@ configure() {
171155
# Set npm to use less verbose output
172156
npm config set loglevel warn 2>/dev/null || true
173157

158+
# Clean up any existing prefix configuration that conflicts with NVM
159+
if npm config get prefix 2>/dev/null | grep -q "npm-global"; then
160+
print_warning "Removing incompatible npm prefix configuration..."
161+
npm config delete prefix 2>/dev/null || true
162+
fi
163+
174164
print_success "Node.js configuration completed"
165+
print_status "Global packages will be installed to: \$NVM_DIR/versions/node/\$(nvm current)/bin"
175166
return 0
176167
}
177168

@@ -243,10 +234,15 @@ validate() {
243234
fi
244235
fi
245236

246-
# Check npm global directory
237+
# Check npm global directory - should be NVM-managed, not custom prefix
247238
local npm_prefix=$(npm config get prefix 2>/dev/null || echo "")
248239
if [[ -n "$npm_prefix" ]]; then
249-
print_status "npm global prefix: $npm_prefix"
240+
# Verify it's an NVM-managed path (contains .nvm in the path)
241+
if [[ "$npm_prefix" == *".nvm"* ]]; then
242+
print_success "npm prefix: $npm_prefix (NVM-managed)"
243+
else
244+
print_warning "npm prefix: $npm_prefix (not NVM-managed, may cause issues)"
245+
fi
250246
fi
251247

252248
# Test NVM version switching capability
@@ -308,9 +304,13 @@ status() {
308304
print_status "Installed Node versions: $installed_versions"
309305
fi
310306

311-
# Show npm global directory
307+
# Show npm global directory (should be NVM-managed)
312308
local npm_prefix=$(npm config get prefix 2>/dev/null || echo "unknown")
313-
print_status "npm global prefix: $npm_prefix"
309+
if [[ "$npm_prefix" == *".nvm"* ]]; then
310+
print_status "npm global prefix: $npm_prefix (NVM-managed ✓)"
311+
else
312+
print_warning "npm global prefix: $npm_prefix (should be NVM-managed)"
313+
fi
314314

315315
# Show global packages count
316316
local global_count
@@ -357,21 +357,12 @@ remove() {
357357
print_warning "NVM is not installed"
358358
fi
359359

360-
# Remove npm global directory
361-
local npm_global_dir="$HOME/.npm-global"
362-
if [[ -d "$npm_global_dir" ]]; then
363-
print_warning "npm global directory exists: $npm_global_dir"
364-
if prompt_confirmation "Remove npm global directory and all global packages?"; then
365-
rm -rf "$npm_global_dir"
366-
print_success "npm global directory removed"
367-
else
368-
print_status "npm global directory preserved"
369-
fi
370-
fi
360+
# Note: NVM manages global packages in version-specific directories
361+
# These are automatically removed when NVM directory is deleted
362+
print_status "Global packages are managed by NVM and will be removed with NVM directory"
371363

372364
# Remove bashrc entries
373365
cleanup_bashrc "# ${EXT_NAME} - NVM"
374-
cleanup_bashrc "# ${EXT_NAME} - npm global packages"
375366

376367
print_success "Node.js and NVM uninstalled"
377368
print_warning "Restart your shell or run: source ~/.bashrc"

test-nodejs-fix.sh

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
#!/bin/bash
2+
# Test script to validate nodejs extension NVM compatibility fix
3+
# This simulates the exact same steps as the CI integration test
4+
5+
set -e
6+
7+
echo "🧪 Testing nodejs extension NVM compatibility fix..."
8+
echo ""
9+
10+
# Cleanup function
11+
cleanup() {
12+
echo ""
13+
echo "🧹 Cleaning up..."
14+
docker rm -f sindri-test-nodejs 2>/dev/null || true
15+
}
16+
17+
# Register cleanup on exit
18+
trap cleanup EXIT
19+
20+
# Build minimal test container
21+
echo "📦 Building test container..."
22+
docker build -t sindri-test-nodejs -f - . <<'DOCKERFILE'
23+
FROM ubuntu:24.04
24+
25+
ENV DEBIAN_FRONTEND=noninteractive
26+
ENV LANG=C.UTF-8
27+
ENV LC_ALL=C.UTF-8
28+
29+
# Install minimal dependencies
30+
RUN apt-get update && apt-get install -y \
31+
curl \
32+
git \
33+
bash \
34+
&& rm -rf /var/lib/apt/lists/*
35+
36+
# Create test user
37+
RUN useradd -m -s /bin/bash developer
38+
39+
# Copy extension files (matching production paths)
40+
COPY docker/lib/extensions.d/nodejs.sh.example /workspace/scripts/lib/nodejs.sh
41+
COPY docker/lib/extensions-common.sh /workspace/scripts/extensions-common.sh
42+
RUN mkdir -p /workspace/scripts/lib/extensions.d
43+
44+
RUN chown -R developer:developer /workspace
45+
46+
USER developer
47+
WORKDIR /home/developer
48+
49+
DOCKERFILE
50+
51+
echo "✅ Test container built"
52+
echo ""
53+
54+
# Run the test in container
55+
echo "🔬 Running nodejs extension test..."
56+
docker run --name sindri-test-nodejs sindri-test-nodejs /bin/bash -c '
57+
set -e
58+
59+
echo "Step 1: Installing nodejs extension..."
60+
cd /workspace/scripts/lib
61+
bash nodejs.sh install
62+
echo "✅ Installation completed"
63+
echo ""
64+
65+
echo "Step 2: Configuring nodejs extension..."
66+
bash nodejs.sh configure
67+
echo "✅ Configuration completed"
68+
echo ""
69+
70+
echo "Step 3: Checking for NVM prefix conflict..."
71+
export NVM_DIR="$HOME/.nvm"
72+
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
73+
74+
# This should NOT show the error anymore
75+
if npm config get prefix 2>&1 | grep -q "npm-global"; then
76+
echo "❌ FAIL: npm prefix is still set to npm-global (conflicts with NVM)"
77+
exit 1
78+
else
79+
echo "✅ PASS: npm prefix is NVM-managed"
80+
fi
81+
echo ""
82+
83+
echo "Step 4: Validating node command (CI validation step)..."
84+
# This is the exact command from the CI test
85+
if source ~/.nvm/nvm.sh 2>/dev/null && command -v node && node --version; then
86+
echo "✅ PASS: node command works"
87+
else
88+
echo "❌ FAIL: node command validation failed"
89+
exit 1
90+
fi
91+
echo ""
92+
93+
echo "Step 5: Checking NVM compatibility..."
94+
# This would show the error if prefix is misconfigured
95+
if nvm current 2>&1 | grep -q "globalconfig.*incompatible"; then
96+
echo "❌ FAIL: NVM shows compatibility error"
97+
exit 1
98+
else
99+
echo "✅ PASS: NVM has no compatibility errors"
100+
fi
101+
echo ""
102+
103+
echo "Step 6: Test npm global install (without sudo)..."
104+
npm install -g cowsay 2>&1 | tee /tmp/npm-install.log
105+
if grep -q "globalconfig.*incompatible" /tmp/npm-install.log; then
106+
echo "❌ FAIL: npm global install shows NVM incompatibility"
107+
exit 1
108+
else
109+
echo "✅ PASS: npm global install works without errors"
110+
fi
111+
echo ""
112+
113+
echo "Step 7: Verify global package location..."
114+
npm_prefix=$(npm config get prefix)
115+
if [[ "$npm_prefix" == *".nvm"* ]]; then
116+
echo "✅ PASS: Global packages install to NVM directory: $npm_prefix"
117+
else
118+
echo "❌ FAIL: Global packages not in NVM directory: $npm_prefix"
119+
exit 1
120+
fi
121+
echo ""
122+
123+
echo "🎉 All tests passed! The nodejs extension is NVM-compatible."
124+
'
125+
126+
echo ""
127+
echo "✅ Test completed successfully!"
128+
echo "The fix resolves the NVM compatibility issue."

0 commit comments

Comments
 (0)