Skip to content

Commit 3e4d380

Browse files
zzstoatzzclaude
andauthored
fix(mcp): merge custom env with os.environ for MCPServerStdio (#1296)
When users set env={"FOO": "bar"} on an MCPServerStdio, the subprocess was spawned with only those vars — missing PATH, HOME, etc. This caused the MCP server process to fail during initialization (TimeoutError). Now always merges user env on top of os.environ so custom vars take precedence while the subprocess still inherits the standard environment. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8d9bc95 commit 3e4d380

2 files changed

Lines changed: 69 additions & 6 deletions

File tree

src/marvin/_internal/integrations/mcp.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,21 @@ async def start_servers(self, actor: "Actor") -> list[MCPServer]:
102102
continue
103103

104104
try:
105-
# Set environment variables for stdio servers if not already set
106-
if isinstance(server, MCPServerStdio) and server.env is None:
107-
logger.debug(
108-
f"[MCPManager] Server {server_repr} has no env set. Setting env=dict(os.environ)."
109-
)
110-
server.env = dict(os.environ)
105+
# Ensure stdio servers inherit the full environment,
106+
# merging any user-specified env vars on top of os.environ.
107+
# Without this, a server with env={"FOO": "bar"} would lose
108+
# PATH, HOME, etc., causing the subprocess to fail.
109+
if isinstance(server, MCPServerStdio):
110+
if server.env is None:
111+
logger.debug(
112+
f"[MCPManager] Server {server_repr} has no env set. Setting env=dict(os.environ)."
113+
)
114+
server.env = dict(os.environ)
115+
else:
116+
logger.debug(
117+
f"[MCPManager] Server {server_repr} has custom env. Merging with os.environ."
118+
)
119+
server.env = {**os.environ, **server.env}
111120

112121
await self.exit_stack.enter_async_context(server)
113122
self.active_servers.append(server)

tests/agents/test_mcp_integration.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import os
12
from unittest.mock import AsyncMock, MagicMock, patch
23

34
from pydantic_ai.mcp import MCPServer
@@ -267,6 +268,59 @@ async def mock_start_servers(self, actor):
267268
# Cleanup
268269
await cleanup_thread_mcp_servers()
269270

271+
async def test_mcp_manager_merges_env_with_os_environ(self):
272+
"""MCPServerStdio with custom env should still inherit os.environ.
273+
274+
Regression test: when a user sets env={"FOO": "bar"} on an MCPServerStdio,
275+
the subprocess needs PATH, HOME, etc. to function. The manager should
276+
merge user env on top of os.environ, not use it as-is.
277+
"""
278+
from pydantic_ai.mcp import MCPServerStdio
279+
280+
custom_env = {"HUE_BRIDGE_IP": "192.168.1.100", "HUE_BRIDGE_USERNAME": "test"}
281+
server = MCPServerStdio(command="echo", args=["hello"], env=custom_env)
282+
agent = MagicMock(spec=Agent)
283+
agent.name = "TestAgent"
284+
agent.get_mcp_servers.return_value = [server]
285+
286+
manager = MCPManager()
287+
288+
# Patch enter_async_context to avoid actually starting the server
289+
manager.exit_stack.enter_async_context = AsyncMock()
290+
291+
with patch.dict(
292+
os.environ, {"PATH": "/usr/bin", "HOME": "/home/test"}, clear=True
293+
):
294+
await manager.start_servers(agent)
295+
296+
# The server's env should have both os.environ and custom vars
297+
assert server.env is not None
298+
assert server.env["PATH"] == "/usr/bin"
299+
assert server.env["HOME"] == "/home/test"
300+
# Custom vars should take precedence
301+
assert server.env["HUE_BRIDGE_IP"] == "192.168.1.100"
302+
assert server.env["HUE_BRIDGE_USERNAME"] == "test"
303+
304+
async def test_mcp_manager_sets_env_when_none(self):
305+
"""MCPServerStdio with env=None should get os.environ."""
306+
from pydantic_ai.mcp import MCPServerStdio
307+
308+
server = MCPServerStdio(command="echo", args=["hello"])
309+
assert server.env is None
310+
311+
agent = MagicMock(spec=Agent)
312+
agent.name = "TestAgent"
313+
agent.get_mcp_servers.return_value = [server]
314+
315+
manager = MCPManager()
316+
manager.exit_stack.enter_async_context = AsyncMock()
317+
318+
with patch.dict(os.environ, {"PATH": "/usr/bin"}, clear=True):
319+
await manager.start_servers(agent)
320+
321+
assert server.env is not None
322+
assert server.env["PATH"] == "/usr/bin"
323+
270324
async def test_mcp_manager_tracks_started_servers_by_id(self):
271325
"""MCPManager should track servers by id to avoid restarting the same server."""
272326
manager = MCPManager()

0 commit comments

Comments
 (0)