Repository navigation
Fix Windows npm resolution for extension installs - #85
Conversation
Select npm.cmd when extension package install paths invoke npm on Windows, avoiding the Nightly Validation spawn npm ENOENT failure. Add focused coverage for platform command selection and record pm-arax tracker evidence.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts npm command resolution for extension installs to work on Windows by introducing a platform-aware npm command helper, updates the error messaging, and adds focused unit coverage and changelog/issue artifacts for the fix. Sequence diagram for platform-aware npm command execution in extension installsequenceDiagram
participant CliExtensionInstall
participant runNpmCommand
participant resolveNpmCommandName
participant execFileAsync
CliExtensionInstall->>runNpmCommand: runNpmCommand(args, cwd)
runNpmCommand->>resolveNpmCommandName: resolveNpmCommandName(platform)
resolveNpmCommandName-->>runNpmCommand: npmCommand
runNpmCommand->>execFileAsync: execFileAsync(npmCommand, args, cwd)
alt [execFileAsync succeeds]
execFileAsync-->>runNpmCommand: stdout
runNpmCommand-->>CliExtensionInstall: trimmed stdout
else [execFileAsync throws]
execFileAsync-->>runNpmCommand: error
runNpmCommand-->>CliExtensionInstall: throw PmCliError("npm command failed: " + npmCommand + args)
end
Flow diagram for resolveNpmCommandName platform-specific selectionflowchart TD
A[Call resolveNpmCommandName] --> B{platform === win32}
B -->|Yes| C[Return npm.cmd]
B -->|No| D[Return npm]
C --> E[runNpmCommand uses npm.cmd in execFileAsync]
D --> E[runNpmCommand uses npm in execFileAsync]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 23 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request resolves an issue with npm command resolution on Windows during extension package installations by introducing a platform-aware helper resolveNpmCommandName that selects npm.cmd on Windows. However, a critical issue was pointed out in the review: executing .cmd files via execFile on Windows requires the shell option to be enabled to prevent execution failures. It is recommended to apply the suggested fix to pass shell: process.platform === 'win32' to execFileAsync.
| const npmCommand = resolveNpmCommandName(); | ||
| try { | ||
| const result = await execFileAsync("npm", args, { cwd, encoding: "utf8" }); | ||
| const result = await execFileAsync(npmCommand, args, { cwd, encoding: "utf8" }); |
There was a problem hiding this comment.
On Windows, batch files like npm.cmd cannot be executed directly using child_process.execFile without a shell. Attempting to do so will result in a spawn npm.cmd ENOENT or EINVAL error because Windows requires cmd.exe to interpret .cmd files.
To fix this, you should pass shell: true (or shell: process.platform === 'win32') in the options when executing the command on Windows.
| const result = await execFileAsync(npmCommand, args, { cwd, encoding: "utf8" }); | |
| const result = await execFileAsync(npmCommand, args, { cwd, encoding: "utf8", shell: process.platform === "win32" }); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b95741cdef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const npmCommand = resolveNpmCommandName(); | ||
| try { | ||
| const result = await execFileAsync("npm", args, { cwd, encoding: "utf8" }); | ||
| const result = await execFileAsync(npmCommand, args, { cwd, encoding: "utf8" }); |
There was a problem hiding this comment.
When process.platform is win32, this now passes npm.cmd to execFileAsync, but it still does so without a shell. Node's child_process docs state that .bat/.cmd files are not executable on their own and cannot be launched with execFile() on Windows (https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows); the existing integration helper also uses shell: isWindows for npm.cmd in tests/integration/cli.integration.spec.ts. As a result, Windows extension installs that reach npm pack/npm install will continue to fail instead of fixing the reported spawn npm ENOENT path.
Useful? React with 👍 / 👎.
Run npm.cmd through shell mode on Windows for execFile compatibility and cover the shell decision in tests. Record the Gemini review follow-up and rerun pm-arax linked tests.
|
Review follow-up: addressed Gemini's Windows execFile note in 4dbf1bd by running npm.cmd through shell mode on win32, with focused unit coverage and pm-arax linked tests rerun. |
… PR#564), j8z6 baseline verification Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C7MgYjKgXUkcUtA9Er3ifw
Summary
Validation
Context
This addresses the stale Nightly Validation failure on windows-latest Node 20 after PR #84 merged: extension npm pack/install paths failed with spawn npm ENOENT.
Summary by Sourcery
Fix cross-platform npm command resolution for extension package installs, ensuring Windows uses the correct npm shim and documenting the fix.
Bug Fixes:
Documentation:
Tests:
Chores: