Skip to content

fix(cli): avoid Windows shell for mcp dev#2524

Open
Genmin wants to merge 1 commit into
modelcontextprotocol:mainfrom
Genmin:codex/fix-mcp-dev-windows-shell
Open

fix(cli): avoid Windows shell for mcp dev#2524
Genmin wants to merge 1 commit into
modelcontextprotocol:mainfrom
Genmin:codex/fix-mcp-dev-windows-shell

Conversation

@Genmin
Copy link
Copy Markdown

@Genmin Genmin commented May 1, 2026

Summary

  • remove Windows shell=True from the mcp dev inspector subprocess path
  • resolve the Windows npx executable with shutil.which() instead of probing through a shell
  • add a regression test that exercises a path containing & and verifies the subprocess receives an argument list without shell dispatch

Fixes #1257.

Validation

  • uv run pytest tests/cli -q
  • uv run ruff check src/mcp/cli/cli.py tests/cli/test_utils.py
  • uv run pyright src/mcp/cli/cli.py tests/cli/test_utils.py
  • git diff --check

@Genmin Genmin force-pushed the codex/fix-mcp-dev-windows-shell branch 2 times, most recently from de8aa19 to f351681 Compare May 1, 2026 01:37
@yyzquwu
Copy link
Copy Markdown

yyzquwu commented Jun 1, 2026

Tested this on Windows.

Environment:

  • Windows 11
  • Python 3.13.13
  • PR head a176ccc07a5c48d439a909f1d9dcb9c71287dc81

What passed:

pytest tests/cli -q
# 27 passed

ruff check src/mcp/cli/cli.py tests/cli/test_utils.py
# passed

One thing still fails locally:

pyright src/mcp/cli/cli.py tests/cli/test_utils.py
# tests/cli/test_utils.py:87:29 - "shutil" is not exported from module ".cli"
# tests/cli/test_utils.py:98:29 - "shutil" is not exported from module ".cli"
# tests/cli/test_utils.py:125:29 - "subprocess" is not exported from module ".cli"
# tests/cli/test_utils.py:157:29 - "subprocess" is not exported from module ".cli"
# tests/cli/test_utils.py:204:29 - "subprocess" is not exported from module ".cli"
# tests/cli/test_utils.py:228:29 - "subprocess" is not exported from module ".cli"

The fix itself looks right to me. The remaining issue seems test-side: the new tests monkeypatch cli.shutil and cli.subprocess, which trips reportPrivateImportUsage. Importing shutil and subprocess in the test module and monkeypatching those module objects directly should keep the same behavior without the private import warnings, because cli.py holds the same imported modules.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don’t use shell=True in mcp dev subprocess on Windows (command injection risk)

2 participants