Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,17 @@ install_nodejs() {
ensure_nvm_loaded --force
nvm use 22 --silent
nvm alias default 22 2>/dev/null || true
info "Node.js installed: $(node --version)"
local installed_version
installed_version="$(node --version)"
info "Node.js installed via nvm: ${installed_version} (default alias)"
# Surface the shell-reload requirement right next to the install line so the
# user isn't left thinking the new Node is already active in their terminal.
# install.sh runs as a subprocess; the parent shell's PATH genuinely cannot
# be mutated from here, so we print the truth and the exact command.
# See issue #2178.
warn "Your current shell still resolves \`node\` to the previous version."
printf " Open a new terminal, or run this in your existing shell:\n"
printf " source ~/.nvm/nvm.sh && nvm use 22\n"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

# ---------------------------------------------------------------------------
Expand Down
19 changes: 19 additions & 0 deletions test/install-preflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,25 @@ fi`,
expect(gitCalls).not.toMatch(/clone/);
expect(gitCalls).not.toMatch(/fetch/);
});

// Issue #2178 — when nvm installs a new Node, the user's parent shell still
// resolves `node` to the old version until the shell is reloaded. The
// installer's upgrade path must surface this loudly and adjacent to the
// "Node.js installed" line, not only in the generic bottom-of-output Next
// block where it's easy to miss.
it("install_nodejs upgrade path emits a Node-specific shell-reload hint", () => {
const script = fs.readFileSync(INSTALLER_PAYLOAD, "utf-8");
const installNodejs = script.match(/install_nodejs\(\)\s*\{[\s\S]*?\n\}/);
expect(installNodejs).not.toBeNull();
const body = installNodejs![0];
// Anchor to the actual warn/printf calls (not the comment) so the test
// fails if the executable statements are removed. A child process can't
// mutate the parent's PATH, so the honest fix is printing the exact
// command the user can run in their existing shell (no exec tricks —
// those create a nested shell that masks the problem; see PR #2298).
expect(body).toMatch(/\n\s*warn\s+"Your current shell still resolves/);
expect(body).toMatch(/\n\s*printf\s+"[^"]*source ~\/\.nvm\/nvm\.sh && nvm use 22/);
});
});

// ---------------------------------------------------------------------------
Expand Down
Loading