fix(vscode): use shell on Windows when spawning npm/ocr in CliService#490
fix(vscode): use shell on Windows when spawning npm/ocr in CliService#490addielaruee wants to merge 1 commit into
Conversation
On Windows, `node` is `node.exe` (spawnable directly) but `npm` and the `ocr` CLI are `.cmd` wrapper scripts, which Node's `child_process.spawn` cannot execute without `shell: true`. This made the extension's environment check false-report "npm not detected" even though `npm --version` works in every terminal, and would also stop `runRaw` from launching the ocr CLI to run a review. `probeCommand` (environment detection) and `runRaw` (review execution) were missing the `shell: process.platform === 'win32'` option that the `install` method already sets. Add it to both, matching `install`'s pattern. Behavior on macOS/Linux is unchanged (`shell` stays `false`). Add CliService.shell.test.ts, which mocks `child_process.spawn` to assert the shell option per platform, verifying the cross-platform fix without needing a live Windows environment. Fixes alibaba#453
|
|
|
✅ OpenCodeReview: No comments generated. Looks good to me. |
|
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. |
|
Hi @addielaruee, thanks for the contribution! We noticed the CLA check has not passed yet. Could you please sign the CLA at your earliest convenience? We're unable to merge this PR until the CLA is signed. Let us know if you run into any issues. Thanks! |
Description
On Windows,
nodeisnode.exe(whichchild_process.spawncan execute directly), butnpmand theocrCLI are.cmdwrapper scripts. Node'sspawncannot execute a.cmdfile unlessshell: trueis set. As reported in #453, this made the VS Code extension's environment-check page false-report "npm not detected" even thoughnpm --versionworks in every terminal. The same gap would also stoprunRawfrom launching theocrCLI to actually run a review on Windows.The
installmethod already setsshell: process.platform === 'win32', but the two other spawn call sites did not:probeCommand— used for thenode/npm/ocrenvironment detection.runRaw— used to run a review (andllm test).This PR adds
shell: process.platform === 'win32'to both, matchinginstall's existing pattern. On macOS/Linux the option evaluates tofalse, so behavior there is unchanged.Type of Change
How Has This Been Tested?
yarn testpasses locally inextensions/vscode(11 suites, 95 tests).CliService.shell.test.ts, which mockschild_process.spawnand asserts theshelloption per platform (win32→true, others →false) for bothprobeCommandandrunRaw. I confirmed the new tests fail when the fix is reverted, so they actually guard the behavior.tsc --noEmit -p tsconfig.extension.jsonandeslintpass (no new warnings).npm.cmdpath was verified by code inspection against the root cause in Windows 环境下vs code插件检测误报"未检测到 npm"（spawn 未使用 shell 模式） #453 and the already-correctinstallmethod, plus the platform-conditional unit tests above. Behavior on macOS/Linux is provably unchanged (shellstaysfalse).Checklist
Related Issues
Closes #453