Skip to content

fix(node): use matching node in npm shim#9749

Merged
jdx merged 1 commit into
mainfrom
codex/fix-node-npm-shim-prefix
May 9, 2026
Merged

fix(node): use matching node in npm shim#9749
jdx merged 1 commit into
mainfrom
codex/fix-node-npm-shim-prefix

Conversation

@jdx

@jdx jdx commented May 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • update the generated Node npm shim to invoke the installed version's own bin/node
  • keep npm prefix detection tied to the npm shim's install instead of whichever node appears first on PATH
  • add a regression assertion for running one Node install's npm while another Node version is active

Root Cause

The npm shim executed node "$npm_cli", which allowed PATH lookup to select a different active Node version. npm then derived its global prefix from that other Node's process.execPath, so default packages could install into the wrong Node prefix.

Validation

  • hk pre-commit hook during commit, including cargo check --all-features
  • mise run test:e2e e2e/core/test_node_slow

Fixes discussion #9748.

This PR was generated by an AI coding assistant.


Note

Medium Risk
Changes how the npm shim selects the Node executable, which can affect npm behavior and global install locations across Node versions. Scope is small but touches core tooling invocation paths.

Overview
Fixes the generated node_npm_shim to invoke npm via the installed tool’s own bin/node instead of resolving node from PATH, preventing cross-version prefix/global-install mismatches.

Adds an e2e regression check ensuring npm config get prefix for a specific Node install returns that install’s prefix even when another Node version is active.

Reviewed by Cursor Bugbot for commit 527eeb6. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where the npm shim would invoke node via PATH lookup, allowing a different active Node version to be selected, which caused npm to derive its global prefix from the wrong process.execPath. The fix pins node execution to $plugin_dir/bin/node so the npm shim always runs under the same Node installation that owns it.

  • node_npm_shim: Both the reshim and the exec code paths now call \"$node_bin\" \"$npm_cli\" where node_bin is the absolute path to the shim's own bin/node, resolved through plugin_dir (itself derived from the shim's real path via pwd -P).
  • e2e/core/test_node_slow: Adds a regression test that runs hydrogen's npm config get prefix while a different Node version (corepack_version) is active via mise x, asserting the prefix resolves to hydrogen's install path rather than the active version's path.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-targeted, and directly validated by the new e2e assertion.

Both changed files are small and focused. The shim fix correctly derives the node binary path from plugin_dir, which is already resolved with pwd -P earlier in the script, so symlinks and relative paths are handled. The e2e test exercises the exact failure scenario described in the bug report and passes through the cross-version npm invocation end-to-end.

No files require special attention.

Important Files Changed

Filename Overview
src/plugins/core/assets/node_npm_shim Replaces bare node PATH lookup with an explicit $plugin_dir/bin/node path so npm always runs under the same Node version that owns the shim, fixing the wrong-prefix bug.
e2e/core/test_node_slow Adds a regression assertion: running hydrogen's npm while another Node version is active via mise x should still report hydrogen's install path as the npm prefix.

Reviews (1): Last reviewed commit: "fix(node): use matching node in npm shim" | Re-trigger Greptile

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the node_npm_shim to use the local plugin node binary instead of the global node command and adds an E2E test to verify npm prefix retrieval. A review comment suggests quoting the installation path in the test to handle paths with spaces correctly.

Comment thread e2e/core/test_node_slow
mise i node@lts/hydrogen
mise i -f node
hydrogen_path="$(mise where node@lts/hydrogen)"
assert "mise x node@$corepack_version -- $hydrogen_path/bin/npm config get prefix" "$hydrogen_path"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The path $hydrogen_path should be quoted within the command string passed to assert. If the installation path contains spaces (which can occur depending on the user's environment or configuration), the command will fail to execute correctly because the shell will split the path into multiple arguments when the string is evaluated.

assert "mise x node@$corepack_version -- \"$hydrogen_path/bin/npm\" config get prefix" "$hydrogen_path"

@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.5.4 x -- echo 18.5 ± 0.8 16.9 23.2 1.00
mise x -- echo 18.7 ± 1.1 17.0 34.0 1.01 ± 0.08

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.5.4 env 18.4 ± 0.8 16.8 22.1 1.00 ± 0.06
mise env 18.4 ± 0.7 16.7 21.7 1.00

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.5.4 hook-env 19.0 ± 0.8 17.2 23.1 1.00
mise hook-env 19.2 ± 1.0 17.4 25.8 1.01 ± 0.07

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.5.4 ls 15.7 ± 0.7 14.1 19.3 1.00 ± 0.06
mise ls 15.7 ± 0.7 14.1 18.8 1.00

xtasks/test/perf

Command mise-2026.5.4 mise Variance
install (cached) 124ms 124ms +0%
ls (cached) 57ms 58ms -1%
bin-paths (cached) 63ms 62ms +1%
task-ls (cached) 489ms 492ms +0%

@jdx jdx merged commit 0f3bc1c into main May 9, 2026
37 checks passed
@jdx jdx deleted the codex/fix-node-npm-shim-prefix branch May 9, 2026 18:23
mise-en-dev added a commit that referenced this pull request May 10, 2026
### 🚀 Features

- add --inactive option to outdated and upgrade commands for inactive
tools by @roele in [#9640](#9640)

### 🐛 Bug Fixes

- **(aqua)** resolve bin paths for prefixed v tags by @risu729 in
[#9759](#9759)
- **(bun)** create bunx alongside bun.exe on Windows install by
@JamBalaya56562 in [#9732](#9732)
- **(dotnet)** use shared prerelease tool option by @risu729 in
[#9720](#9720)
- **(node)** use matching node in npm shim by @jdx in
[#9749](#9749)
- **(task)** resolve bash deterministically on Windows to avoid WSL
launcher by @JamBalaya56562 in
[#9750](#9750)

### 📚 Documentation

- **(secrets)** clarify age strict mode default by @risu729 in
[#9737](#9737)
- **(tasks)** add bash shebang to conditional-dependencies example by
@JamBalaya56562 in [#9747](#9747)
- update backend tool option docs by @risu729 in
[#9738](#9738)

### 📦 Registry

- remove tools with zero users by @jdx in
[#9725](#9725)
- add scalafmt
([github:scalameta/scalafmt](https://github.com/scalameta/scalafmt)) by
@pokir in [#9757](#9757)
- remove flarectl by @risu729 in
[#9756](#9756)

### Chore

- **(release)** strip pre-existing sponsor block before appending
canonical one by @jdx in [#9745](#9745)

### New Contributors

- @pokir made their first contribution in
[#9757](#9757)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant