Skip to content

fix: resolve npm CLI wrappers in Windows watchers - #475

Open
haosenwang1018 wants to merge 2 commits into
the-open-engine:mainfrom
haosenwang1018:fix/windows-provider-cli-spawn
Open

fix: resolve npm CLI wrappers in Windows watchers#475
haosenwang1018 wants to merge 2 commits into
the-open-engine:mainfrom
haosenwang1018:fix/windows-provider-cli-spawn

Conversation

@haosenwang1018

Copy link
Copy Markdown
Contributor

Summary

  • resolve provider CLI wrappers to their real Node entrypoints before watcher spawn on Windows
  • teach both watcher variants to invoke node script.js ...args when the provider binary is an npm-generated .cmd wrapper
  • add regression coverage for wrapper detection and spawn spec resolution

Related Issues

Part of #457

Changes Made

  • add Windows-aware command lookup helpers in lib/provider-detection.js
  • pass the resolved CLI path into watcher config from task-lib/runner.js
  • reuse the Windows spawn resolution in both task-lib/watcher.js and task-lib/attachable-watcher.js
  • cover where lookup plus .cmd wrapper parsing in tests/providers/detection.test.js

Testing

  • ./node_modules/.bin/mocha tests/providers/detection.test.js tests/issue-providers.test.js

Checklist

  • Tests pass (npm test)
  • Documentation updated (if needed)
  • Follows commit guidelines

@tomdps

tomdps commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Status review (2026-07-29):

  • Windows-support cluster: this directly addresses the provider-CLI .cmd spawning failure tracked in Windows compatibility: npm global commands cannot be spawned without shell workaround #457. Current main already uses where in lib/provider-detection.js::commandExists/getCommandPath, but getHelpOutput/getVersionOutput still spawn the command directly; task-lib/watcher.js and task-lib/attachable-watcher.js likewise pass commandSpec.binary directly to spawn/AttachServer.
  • Not merge-ready: GitHub currently reports mergeable: false / mergeable_state: dirty, with no check runs or status contexts on head 49e594d. task-lib/runner.js::buildWatcherConfig now carries a structured commandSpec, so the PR's older config.command integration conflicts with the current interface and needs rebasing/adaptation.
  • The underlying Windows bug remains relevant, but this branch is stale and partly superseded. Changes required are a rebase, integration with the current commandSpec path, and %dp0% wrapper support/regression coverage: the proposed extractNodeScriptFromCmdWrapper regex matches only a quoted %~dp0\...js target, while current npm cmd-shims set SET dp0=%~dp0 and invoke %dp0%\...; that form returns null and falls back to spawning the .cmd directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants