fix: Windows support (npx spawn, absolute paths, array options)#18
Open
danscMax wants to merge 1 commit into
Open
fix: Windows support (npx spawn, absolute paths, array options)#18danscMax wants to merge 1 commit into
danscMax wants to merge 1 commit into
Conversation
Three platform bugs prevented the MCP server from running on Windows:
1. `spawn npx ENOENT` — runJscpd called execFile("npx", …). On Windows npx
is a `.cmd` shim that execFile can't resolve, and execFile("npx.cmd", …)
throws EINVAL on patched Node (CVE-2024-27980). Add resolveNpx(): on
Windows invoke npm's npx-cli.js with the current `node` binary (no shell,
no `.cmd`, argv passed verbatim), with a `npx.cmd` + shell fallback. Other
platforms keep the plain `npx` call.
2. Empty report on absolute scan paths — jscpd globs the path with fast-glob,
which treats `\` as an escape on every platform, so a Windows absolute path
(e.g. `E:\proj\src`) matches nothing. Add normalizeScanPath() (Windows:
`\`→`/`) and apply it to the positional path in jscpd.js.
3. Array options silently dropped — buildArgs expanded array values (e.g.
`ignore`) into repeated `--flag` entries, but jscpd keeps only the last
occurrence, so all but one ignore pattern were dropped. Join array values
into a single comma-separated flag instead.
servers/jscpd.js regenerated via `npm run build`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Thanks so much for submitting a fix! The array options bug was indeed quite bad, good catch. The tests are failing on that part of your change and I also wanted to merge a fix immediately, so I went ahead and merged that as a separate PR (#19). Your Windows fixes look good, I would like to merge them. I don't have access to a Windows machine right now, so can't truly verify them though. Do you think you could rebase this PR and make sure the tests pass? I also just added Windows runs to CI btw, so hopefully going forward we can maintain better Windows compatibility. |
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
2 participants
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 the MCP server on Windows. Closes #17.
Three independent platform bugs (details in #17):
spawn npx ENOENT— addedresolveNpx(): on Windows it invokes npm'snpx-cli.jswith the currentnodebinary (no shell, no.cmd, argv passed verbatim → no quoting/injection issues), falling back tonpx.cmd+shell: trueif the CLI script isn't found. Non-Windows behaviour is unchanged.normalizeScanPath()(\→/, Windows-only), applied to the positional scan path so fast-glob can match it.buildArgsnow joins array values into a single comma-separated flag (jscpd's expected form) instead of repeating the flag.servers/jscpd.jsis regenerated vianpm run build.Verification (Windows 11, Node 24)
spawn npx ENOENTis gone — a real scan returnsstatus: ok.ignorenow honours every pattern (a vendored directory is correctly excluded: 696 → 242 clones).npm run buildand the existingtest/server.test.jssmoke test pass.Note
npm testas-is matches 0 files on Windows (its glob'test/**/*.test.js'; a separate pre-existing issue), so I rantest/server.test.jsexplicitly.