Skip to content

fix(mcp): honor env PATHEXT for stdio commands - #56537

Open
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/mcp-pathext-env-resolution
Open

fix(mcp): honor env PATHEXT for stdio commands#56537
ooiuuii wants to merge 1 commit into
NousResearch:mainfrom
ooiuuii:fix/mcp-pathext-env-resolution

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #56536

What does this PR do?

  • Resolves stdio MCP commands against the same environment that will be passed to the MCP subprocess.
  • Honors Windows PATHEXT from the MCP env when a config supplies both PATH and PATHEXT.
  • Preserves Windows-style Path / Pathext casing instead of creating a duplicate PATH key while prepending the resolved command directory.
  • Adds focused regression coverage for env-scoped PATHEXT and mixed-case Windows env keys.

Why?

_resolve_stdio_command() already supports resolving commands against a filtered MCP PATH, but on Windows shutil.which(..., path=...) still consults the parent process PATHEXT. A stdio MCP config with its own PATHEXT could therefore fail to resolve a command that is valid in the configured subprocess environment.

Proof

  • python -m py_compile tools\\mcp_tool.py tests\\tools\\test_mcp_tool_issue_948.py
  • Focused Windows proof script passed:
process PATHEXT = .EXE
MCP env PATH = <temp>\\bin
MCP env PATHEXT = .CMD
<temp>\\bin\\demo.cmd exists
_resolve_stdio_command(demo, env) resolves demo.cmd
focused MCP PATHEXT proof passed

Limitations

  • python -m pytest tests\\tools\\test_mcp_tool_issue_948.py -q could not run in this checkout because pytest is not installed (No module named pytest).
  • Repo-local/global autoreview helper was not available in this checkout.

@alt-glitch alt-glitch added type/bug Something isn't working tool/mcp MCP client and OAuth platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P3 Low — cosmetic, nice to have labels Jul 1, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the Windows MCP environment-resolution mismatch. Current main still calls shutil.which() from tools/mcp_tool.py:587-589 after _build_safe_env() merges the configured subprocess environment at tools/mcp_tool.py:426-446, so the fix targets a live path.

Problems

  • tests/tools/test_mcp_tool_issue_948.py:87 creates demo.cmd, but the same new test supplies PATHEXT=.CMD. The proposed resolver derives demo.CMD; since the test only patches sys.platform and runs on Linux's case-sensitive filesystem, its os.path.isfile() lookup cannot find the lowercase fixture.

Suggested changes

  • Make the fixture extension match the configured PATHEXT casing (for example demo.CMD), or mock the filesystem check while retaining a platform-independent regression test.

Automated hermes-sweeper review.

bin_dir = tmp_path / "bin"
bin_dir.mkdir()
cmd_path = bin_dir / "demo.cmd"
cmd_path.write_text("@echo off\n", encoding="utf-8")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture is demo.cmd, while the test configures PATHEXT=.CMD. _which_in_env() constructs demo.CMD; with only sys.platform patched, Linux CI uses a case-sensitive filesystem and will not find this fixture. Please match the fixture casing or mock the lookup.

@ooiuuii
ooiuuii force-pushed the fix/mcp-pathext-env-resolution branch from 2c9ae8e to b64baac Compare August 12, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP stdio resolution ignores env PATHEXT on Windows

3 participants