Make SDK work on Windows without additional arguments

[JonasHelming]

Is your feature request related to a problem? Please describe.
We experienced that to start MCP servers via the SDK on Windows, you need to start a command interpreter (e.g. cmd.exe) as the server command in order for path lookups to work as expected
e.g. like this:

"filesystem": {
    "command": "cmd",
    "args": ["/C", "npx -y @modelcontextprotocol/server-filesystem /Users/YOUR_USERNAME/Desktop"],
    "env": {
      "CUSTOM_ENV_VAR": "custom-value"
    }
  }

Describe the solution you'd like

Would be nice to hide this in the client SDK to hide the complexity from the user, at least with a sensible default for Windows.

Describe alternatives you've considered
none

Additional context
We have documented this work around for our users: https://theia-ide.org/docs/user_ai/#mcp-integration
See also: https://eclipsesource.com/blogs/2024/12/19/theia-ide-and-theia-ai-support-mcp/

[aiqlcom]

I also encounter the same issue, and I noticed that there is an ongoing pull request addressing it: #95

This pull request aims to replace spawn with cross-spawn to resolve the issue.

ad6ea75#diff-cf9cf0e16fe93b95e6cc378c616f4cf5e884677337106dec23f740caee8707c3R1-R2

My workaround:

I am currently employing an alternative method as a temporary workaround to address this issue:

typescript-sdk/src/client/stdio.ts

Lines 107 to 118 in 3164da6

	return new Promise((resolve, reject) => {
	this._process = spawn(
	this._serverParams.command,
	this._serverParams.args ?? [],
	{
	env: this._serverParams.env ?? getDefaultEnvironment(),
	stdio: ["pipe", "pipe", this._serverParams.stderr ?? "inherit"],
	shell: false,
	signal: this._abortController.signal,
	windowsHide: process.platform === "win32" && isElectron(),
	}
	);

On Linux or macOS, shell: true is usually not required because command-line tools (such as npx) are typically standalone executables.

However, on Windows, shell: true is necessary to ensure that commands are executed correctly through the command interpreter.

Thus, I replaced it with shell: process.platform === "win32" ? true : false to temporarily avoid this issue.

[dinhphieu]

I spent hours today trying to figure out why I kept getting this

I use different shells, wsl, cygwin, etc. so it was hard to pinpoint the issue at first

Error: spawn npx ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at processTicksAndRejections (node:internal/process/task_queues:90:21) {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawn npx',
  path: 'npx',
  spawnargs: [ '-y', '@modelcontextprotocol/server-github' ]
}

I'm also patching shell: true with https://pnpm.io/cli/patch (or https://www.npmjs.com/package/patch-package) to temporarily address this, hope it gets fixed soon

[jspahrsummers]

I replaced it with shell: process.platform === "win32" ? true : false to temporarily avoid this issue.

We used to have it enabled, but it was removed in #55 (cc @anaisbetts) because it creates terminal windows unnecessarily. Are you seeing the same?

[aiqlcom]

I replaced it with shell: process.platform === "win32" ? true : false to temporarily avoid this issue.

We used to have it enabled, but it was removed in #55 (cc @anaisbetts) because it creates terminal windows unnecessarily. Are you seeing the same?

I haven't observed this ‘terminal windows’ issue myself. After switching to 'true', I didn't notice any significant negative impacts. However, this is something we should be mindful of, as different frameworks/shells or environments might yield varying observations.

Would it be possible to make this parameter configurable as an input parameter during Client initialization? This way, even if the default value doesn't suit some developers' current frameworks, they could customize the parameter to better adapt to their specific systems/frameworks.