fix: resolve MCP npx stdio connection failures#977
Open
kshitijk4poor wants to merge 1 commit intoNousResearch:mainfrom
Open
fix: resolve MCP npx stdio connection failures#977kshitijk4poor wants to merge 1 commit intoNousResearch:mainfrom
kshitijk4poor wants to merge 1 commit intoNousResearch:mainfrom
Conversation
1 task
56dccd0 to
3ed2660
Compare
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
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.
Problem (Issue #948)
Two MCP startup failure modes were reported:
FileNotFoundError: ... 'npx'whenmcp_servers.<name>.commandused a barenpx.ExceptionGroup: unhandled errors in a TaskGroup/No such file or directory: 'node'whencommandwas an absolute.../npxpath.Root Cause
MCPServerTask._run_stdio()passedcommandthrough without resolving executable location for the subprocess env._build_safe_env), so PATH differences could make bare commands fail.npxpaths still failed in some environments because sibling executable lookup (node) depended on PATH.Fix
1) Deterministic stdio command resolution
Added
_resolve_stdio_command(command, env)intools/mcp_tool.py:~in command paths.shutil.which(..., path=...).npx/npm/node:$HERMES_HOME/node/bin/<cmd>~/.local/bin/<cmd>_prepend_path) so shebang chains like/usr/bin/env noderesolve reliably.2) Correct PATH semantics edge case
Follow-up fix ensures explicit
PATH: ""is respected:Nonetowhich()(use process PATH).""towhich()(no implicit fallback).3) Better connection diagnostics
Added
_format_connect_error()(with nested-exception unwrapping) and updated discovery warnings to include server + command context and clearer missing-executable guidance.Scope / Safety
tools/mcp_tool.pytests/tools/test_mcp_tool_issue_948.py(new)Reviewer Fast Path
Please review these sections first:
tools/mcp_tool.py:_resolve_stdio_command_prepend_path_format_connect_error_run_stdiointegrationdiscover_mcp_toolswarning pathtests/tools/test_mcp_tool_issue_948.py:test_resolve_stdio_command_respects_explicit_empty_pathtest_run_stdio_uses_resolved_command_and_pathValidation
Focused regression command
python -m pytest -o addopts='' tests/tools/test_mcp_tool.py tests/tools/test_mcp_tool_issue_948.py tests/test_model_tools.py -qFull CI-equivalent verification run locally
python -m pytest tests/ -q --ignore=tests/integration --tb=shortFixes #948.