Fix escape issue in client.ts #56
Merged
+1
−1
Conversation
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
Fixes github#51
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an escape issue in the Windows-specific code path for spawning CLI processes. The double quotes around the CLI path were causing issues when using cmd /c to execute non-absolute paths on Windows.
Changes:
- Removed unnecessary double quotes from the CLI path argument when using
cmd /con Windows
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
724
to
730
| } else if (process.platform === "win32" && !isAbsolutePath) { | ||
| // On Windows, spawn doesn't search PATHEXT, so use cmd /c to resolve the executable. | ||
| command = "cmd"; | ||
| spawnArgs = ["/c", `"${this.options.cliPath}"`, ...args]; | ||
| spawnArgs = ["/c", `${this.options.cliPath}`, ...args]; | ||
| } else { | ||
| command = this.options.cliPath; | ||
| spawnArgs = args; |
There was a problem hiding this comment.
The Windows-specific spawn behavior lacks test coverage. Consider adding a test case that verifies the spawn arguments are correctly constructed for non-absolute paths on Windows, especially since this code path is platform-specific and was previously broken.
SteveSandersonMS
approved these changes
Jan 20, 2026
Contributor
SteveSandersonMS
left a comment
SteveSandersonMS
left a comment
There was a problem hiding this comment.
Thanks @doggy8088!
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 #51
The double quote is unnecessary for
spawnon Windows.