feat: detect nvm-windows Node.js installations on Windows#51
Conversation
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
qiuzhi2046
left a comment
There was a problem hiding this comment.
感谢这个 PR,也很感谢你把 nvm-windows 这个真实使用场景补进来,方向我觉得没有问题,而且补的测试也非常好,非常感谢你的贡献。
我这边重新仔细看了一下代码和文案,当前这版我更建议“修改后再合入”,主要有两个原因:
-
这次修改目前还没有真正接到用户会经过的主流程里
这次新增的nvm-windows识别逻辑主要落在node-subprocess-runtime.ts/nvm-node-runtime.ts这一支 helper 路径上,但当前用户在环境检查、启动网关等主流程里,实际走的还是checkNode()。而checkNode()这条路径目前在 Windows 上仍然没有接入新的nvm-windows发现逻辑,所以从用户视角看,这个 PR 还没有完整实现“Qclaw 可以识别 nvm-windows Node”的目标。 -
Windows 路径判断还有一个边界问题没有收干净
resolveNodeInstallStrategy()这次做了路径分隔符归一化,这一步是好的;但当前比较仍然是大小写敏感的。Windows 路径本身是大小写不敏感的,所以如果路径大小写不一致,仍然可能把本该识别为nvm的路径误判成installer,进而影响后续运行时选择。
我比较推荐的优化方向是:
-
优先把主流程统一到同一套 Node 发现逻辑上
比较理想的做法,是让checkNode()也复用现在这套已经补过nvm-windows的运行时发现逻辑,而不是继续保留一套独立的 POSIX 风格nvm检测分支。这样可以避免 helper 已经修好了,但用户主流程还没生效的情况。 -
把 Windows 路径比较改成真正的“Windows 语义”
在比较binDir和nvmDir之前,除了把\统一成/,建议再做一次大小写归一化,这样才能避免路径仅大小写不同导致的误判。 -
测试建议往“用户真实会经过的路径”补一层
目前测试主要覆盖了 helper 和选择逻辑,这很好;但建议再补两类测试:- Windows 下
checkNode()主流程能识别nvm-windows - Windows 路径大小写不一致时,仍能正确识别为
nvm
- Windows 下
简短总结一下:
这个 PR 的方向是好的,底层能力也补了不少,但现在还差最后一段“接到主流程”和“补齐 Windows 路径边界处理”。我会比较建议把这两部分一起补完再合入,这样用户侧收益会更完整,后续也不容易留下“代码看起来支持了,但实际流程还没生效”的问题。
感谢铁铁为Qclaw做出的贡献,欢迎继续提交PR🤗
|
@JasonYang318 @qiuzhi2046 您好!看到这个 nvm-windows 支持的 PR,实现非常完善!我注意到主流程集成和 Windows 路径处理还有优化空间。作为当前 Windows 体验改进的贡献者(PR #16、#24),我很乐意在技术层面提供支持,确保这些改进能无缝集成到现有代码中。需要我协助 review 或提供协调建议吗?🙃 |
…sitive path comparison Made-with: Cursor
|
感谢详细的 review!两个问题都已修正: 修正 1:将 checkNode() 接入 nvm-windows 检测
现在用户流程 修正 2:Windows 路径比较改为大小写不敏感
新增测试
验证
|
…sitive path comparison Made-with: Cursor
…Yang318/Qclaw into feat/nvm-windows-detection
qiuzhi2046
left a comment
There was a problem hiding this comment.
相关类型检查和受影响测试我这边也过了一遍,当前看不会对 macOS 现有稳定性和整体架构造成明显破坏。感谢补上这块兼容性支持,能实实在在改善 Windows 系统的环境检查体验。
感谢铁铁为Qclaw做出贡献,欢迎继续提交PR👍
問題描述
nvm-windows 使用者在 Windows 上安裝的 Node.js 無法被 Qclaw 偵測到,導致 Qclaw 始終回退使用內建 Node,而非使用者透過 nvm-windows 管理的版本。
變更內容
新功能:nvm-windows 偵測支援
electron/main/nvm-node-runtime.tsdetectNvmWindowsDir():優先讀取NVM_HOME環境變數,回退至%APPDATA%\nvmlistInstalledNvmWindowsNodeExePaths():掃描 nvm-windows 目錄下的版本資料夾,回傳排序後的node.exe路徑列表electron/main/node-subprocess-runtime.tsresolveQualifiedNodeRuntime()中,win32平台改呼叫detectNvmWindowsDir與listInstalledNvmWindowsNodeExePaths,取代原本直接跳過的邏輯effectiveNvmDir統一傳入selectPreferredNodeRuntime與resolveNodeInstallStrategy,使installStrategy回傳'nvm'而非'installer'electron/main/node-runtime-selection.tsresolveNodeInstallStrategy()在比對路徑前先將反斜線正規化為斜線,確保 Windows 路徑能正確比對附帶修正:POSIX nvm 測試在 Windows 上因硬編碼路徑失敗
原測試中多處使用硬編碼的 POSIX 路徑(如
/Users/alice/.nvm/...),在 Windows 上以node:path(win32 模式)執行時會產生路徑不一致,導致測試失敗。electron/main/__tests__/nvm-node-runtime.test.tslistInstalledNvmNodeBinDirs、buildNvmNodeBinDir、detectNvmDir的測試呼叫補上pathModule: path.posix注入,確保跨平台使用 POSIX 語意electron/main/__tests__/node-subprocess-runtime.test.tsprobeVersion的比對改用.replace(/\\/g, '/')正規化路徑executablePath的斷言改為expect.stringMatching(...)regex,相容 Windows 與 POSIX 分隔符electron/main/nvm-node-runtime.tsbuildNvmShellPrefix()中path.join(nvmDir, 'nvm.sh')改為 template literal 字串拼接,避免 Windows 路徑模組將/替換為\(shell script 路徑必須保持 POSIX 格式)測試
驗證
npm run typechecknpm test(本分支新增/修改的 28 個測試全數通過;其他失敗為 main 既有問題)npm run build:app備註
npm run build因專案強制 code signing,於本機無法完成(預期行為)openclaw-upgrade-service、openclaw-install-discovery等)已確認在main分支上同樣失敗,與本 PR 無關