Support fnm and nvm-windows Node discovery on Windows#359
Merged
Conversation
A GUI-launched MCP host (Claude Desktop) starts `q2 mcp` with a minimal environment. fnm mutates PATH only inside an interactive shell and never persists it, so its Node is invisible to such a host — and the layer-3 well-known fallback only probed Program Files and Volta on Windows, so discovery failed outright even with an fnm default set. Probe the two Windows-native version managers directly: fnm under dirs::config_dir()/fnm (aliases/default junction + the node-versions tree, which has no bin/ subdir on Windows) and nvm-windows under NVM_HOME (flat vX.Y.Z/node.exe). Rather than parameterize the tree walker, highest_version_node just tries a superset of sub-paths; a sub-path for the wrong OS is never a file and is skipped, so unix resolution is unchanged. Adding Windows callers also makes the helper live on Windows, clearing a latent dead_code warning. Also gate an unused-on-Windows PathBuf import in the launcher's node integration tests behind cfg(unix); it only compiled clean because CI is unix-only.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Member
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a GUI app like Claude Desktop launches
q2 mcp, the server starts with a minimal environment and no shell-sourced PATH. fnm on Windows mutates PATH only inside an interactive shell and never persists it, so a GUI-launched host never sees an fnm-managed Node. The layer-3 well-known fallback incrates/quarto-mcp-launcher/src/node.rsonly probed Program Files and Volta on Windows, so discovery failed outright —node: NOT FOUNDeven with an fnm default installed.Fix
Probe the two Windows-native version managers in
default_well_known:dirs::config_dir()/fnm— thealiases/defaultjunction and thenode-versionstree, which has nobin/subdir on Windows (unlike unix fnm).%NVM_HOME%— a flatvX.Y.Z/node.exelayout with no subdir.Rather than parameterize the version-tree walker,
highest_version_nodenow tries a superset of sub-paths covering all four layouts (unix nvm, unix fnm, Windows fnm, nvm-windows). A sub-path for the wrong OS is never a file and is skipped, so unix resolution is unchanged. Adding Windows callers also makes the helper live on Windows, clearing a latent dead_code warning.nvm-windows persists its active version to PATH via the registry, so most GUI apps already inherit it; that probe is defense-in-depth. The confirmed-broken case is fnm.
Test Plan
highest_version_nodeselects the newest version across all four layouts (unit tests)q2 mcp --launcher-inforeports the fnm Node instead of NOT FOUND