-
Notifications
You must be signed in to change notification settings - Fork 2.6k
refactor(mcp): use getDefaultEnvironment for stdio client transport #4259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Replace manual PATH and HOME env var handling with getDefaultEnvironment() - Improves consistency and reliability of MCP client environment setup - Leverages SDK's built-in environment configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make sure everything works well, we should consider adding a test case in src/services/mcp/__tests__/McpHub.test.ts that specifically verifies the connectToServer method correctly passes the environment from getDefaultEnvironment() (along with any custom configInjected.env) to the StdioClientTransport constructor for stdio-type servers. This could involve spying on the StdioClientTransport constructor or examining the arguments it receives.
This would provide explicit confirmation that McpHub correctly utilizes the SDK's environment function as intended.
|
This looks good, for reference here's the function being used from the SDK, it correctly injects |
thanks for clarify for me :D |
Related GitHub Issue
Closes: #4257
Description
Test Procedure
Type of Change
srcor test files.Pre-Submission Checklist
npm run lint).console.log) has been removed.npm test).mainbranch.npm run changesetif this PR includes user-facing changes or dependency updates.Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
Important
Refactor
McpHub.tsto usegetDefaultEnvironment()for MCP client environment setup, replacing manualPATHandHOMEhandling.PATHandHOMEenvironment variable handling withgetDefaultEnvironment()inMcpHub.ts.getDefaultEnvironmentto imports from@modelcontextprotocol/sdk/client/stdio.js.This description was created by
for e91bfbcd1a96c81c591ffb6b65db6430e9d7da64. You can customize this summary. It will automatically update as commits are pushed.