Repository navigation
Fix Windows MCP build pipeline: spawn npm/npx through a shell with quoted args - #32
Merged
Merged
Conversation
…oted args
On Windows the whole MCP pipeline (gen:mcp -> build:mcp -> pack:mcp) failed
with a misleading "npm install failed in mcp/." Two stacked bugs:
1. npm/npx are .cmd shims that CreateProcess cannot launch directly, so a bare
spawnSync('npm', ...) throws ENOENT. Naming npm.cmd explicitly is not a fix
either -- that throws EINVAL on Node 20+ (CVE-2024-27980 hardening), so
shell: true is required.
2. shell: true alone then breaks any path containing a space: Node hands the
args to the shell as a command string and the shell re-splits on whitespace.
pack:mcp stages under os.tmpdir(), so every user whose profile name has a
space (C:\Users\Sony User\...) had the stage/output paths split into extra
argv entries and mcpb rejected the arg count.
Route all npm/npx spawns through one runTool() helper that sets shell and
quotes whitespace-bearing args on Windows only. On macOS and Linux needsShell
is false, so args pass through untouched with shell: false -- byte-identical
to the previous behavior.
Verified on Windows 11 / Node 22: gen:mcp, build:mcp, and pack:mcp all
complete, producing mcp/alpha-camera.mcpb.
Refs #31
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pack:mcp reported "OpenCV libs not found — assuming static/linked" on Windows
and shipped a bundle that could not start.
opencvLibDir() looked for shared/opencv/Windows/{Release/x64,x64/vc17}/bin, but
extract.ts writes shared/opencv/Windows/x86_64/Release/bin. Neither candidate
matched, so all four OpenCV DLLs were silently omitted.
CameraWebApp.exe load-time imports opencv_core480.dll, opencv_imgcodecs480.dll
and opencv_imgproc480.dll, so the packed server died instantly with
STATUS_DLL_NOT_FOUND before it could log anything -- which surfaced to users as
the MCP server simply failing to auto-start on connect.
Adds the correct path as the first candidate and keeps the previous two as
fallbacks for older SDK layouts. The darwin and linux candidates are untouched;
the macOS entry already matches what extract.ts writes.
Verified on Windows 11: pack:mcp now reports "OpenCV libs 4", and the bundled
server extracted to a clean directory starts and enumerates a camera
(ILCE-1M2) over USB.
Refs #31
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Fixes #31.
On Windows the entire MCP pipeline (
gen:mcp→build:mcp→pack:mcp) fails. Two stacked bugs incli/src/lib/mcp-build.ts; fixing the first surfaces the second.1.
spawnSync('npm'|'npx', …)throws ENOENTnpm/npxare.cmdshims on Windows andCreateProcesswon't resolve the extension, so the spawn never happens — surfacing as a misleadingnpm install failed in mcp/.even though runningnpm installinmcp/by hand works fine.Naming
npm.cmdexplicitly isn't a fix either: that throwsEINVALon Node 20+ (the CVE-2024-27980 hardening).shell: trueis required.2.
shell: truealone breaks paths with spacesWith
shell: trueNode passes args to the shell as a command string, and the shell re-splits on whitespace.pack:mcpstages underos.tmpdir(), so any user whose profile name contains a space (C:\Users\Sony User\…) had the stage/output paths split into extra argv entries:This is latent — it only reproduces when the temp or repo path contains a space, so it won't show up for every contributor.
Fix
One
runTool()helper that both setsshelland quotes whitespace-bearing args, on Windows only. All threenpm/npxcall sites route through it.Scope
Deliberately narrow — one file,
cli/src/lib/mcp-build.ts, +14/-5.api/server/, noshared/core/, no CMake.needsShellisfalseoff Windows, sofinalArgs === argsandshell: false— byte-identical to the previous call. The generation logic itself is untouched; only how the child process is launched changes.openapi.yaml, or lockfiles.Verification
On Windows 11 Pro (10.0.26200) / Node v22.21.0 / npm 11.6.2, from a repo path containing a space, all three stages pass:
Follow-up worth considering (not in this PR)
ensureDeps's failure message swallows the underlying spawn error. Surfacingr.errorwhenr.status === nullwould have turned this into a one-minute diagnosis rather than a long one.🤖 Generated with Claude Code