fix: redirect stdio MCP stderr to logs#2259
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a6f9c641
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(server_config, StdioMCPServer): | ||
| log_path = _mcp_stdio_log_path(server_name) | ||
| log_path.parent.mkdir(parents=True, exist_ok=True) | ||
| transport = StdioTransport( | ||
| command=server_config.command, |
There was a problem hiding this comment.
Preserve transforming stdio MCP configs
_build_mcp_client now treats any StdioMCPServer instance as a plain stdio transport and constructs StdioTransport directly, which bypasses server_config.to_transport(). In FastMCP, transforming stdio configs (e.g., with tools/include_tags/exclude_tags) subclass StdioMCPServer, so this branch silently drops those transforms and exposes unfiltered tools whenever users configure a transforming stdio server. The previous Client(MCPConfig(...)) path preserved that behavior, so this is a functional regression for transformed stdio MCP setups.
Useful? React with 👍 / 👎.
|
Addressed the review point. Plain Validated locally:
|
Summary
~/.kimi/logs/mcp/<server>.loginstead of the interactive terminalNotes
This addresses the stderr leak described in #2251. It is separate from MCP logging notifications handled in #1637; those are in-protocol log messages, while this patch covers the child process stderr stream that FastMCP 3.2.4 defaults to sys.stderr.
To verify