fix: MCP reconnect via tool passes only server_id to connect_server (#1385)
* fix: MCP reconnect via tool passes only server_id to connect_server connect_server requires name, transport, command, args, env, and url but the reconnect path in do_manage_mcp only passed the server_id, causing a TypeError on every reconnect attempt. Mirror the pattern used in mcp_routes.py reconnect_server. * test: verify MCP reconnect passes full server config to connect_server Mocks the MCP manager and DB to assert that do_manage_mcp reconnect passes name, transport, command, args, env, and url — not just the server_id.
This commit is contained in:
committed by
GitHub
parent
69d6fe44b3
commit
1f2a06facd
@@ -1215,7 +1215,17 @@ async def do_manage_mcp(content: str, owner: Optional[str] = None) -> Dict:
|
|||||||
try:
|
try:
|
||||||
srv = db2.query(McpServer).filter(McpServer.id == sid).first()
|
srv = db2.query(McpServer).filter(McpServer.id == sid).first()
|
||||||
if srv:
|
if srv:
|
||||||
await mcp.connect_server(sid)
|
_args = json.loads(srv.args) if srv.args else []
|
||||||
|
_env = json.loads(srv.env) if srv.env else {}
|
||||||
|
await mcp.connect_server(
|
||||||
|
server_id=sid,
|
||||||
|
name=srv.name,
|
||||||
|
transport=srv.transport,
|
||||||
|
command=srv.command,
|
||||||
|
args=_args,
|
||||||
|
env=_env,
|
||||||
|
url=srv.url,
|
||||||
|
)
|
||||||
st = mcp.get_server_status(sid)
|
st = mcp.get_server_status(sid)
|
||||||
return {"response": f"Reconnected '{srv.name}' ({st.get('tool_count', 0)} tools)", "exit_code": 0}
|
return {"response": f"Reconnected '{srv.name}' ({st.get('tool_count', 0)} tools)", "exit_code": 0}
|
||||||
return {"error": f"Server {sid} not found", "exit_code": 1}
|
return {"error": f"Server {sid} not found", "exit_code": 1}
|
||||||
|
|||||||
46
tests/test_mcp_reconnect_args.py
Normal file
46
tests/test_mcp_reconnect_args.py
Normal file
@@ -0,0 +1,46 @@
|
|||||||
|
"""Verify that MCP reconnect via the agent tool passes full server metadata."""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
import json
|
||||||
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
|
||||||
|
def test_reconnect_passes_full_server_config():
|
||||||
|
"""do_manage_mcp reconnect must pass name/transport/command/args/env/url."""
|
||||||
|
from src.tool_implementations import do_manage_mcp
|
||||||
|
|
||||||
|
fake_mcp = MagicMock()
|
||||||
|
fake_mcp.disconnect_server = AsyncMock()
|
||||||
|
fake_mcp.connect_server = AsyncMock(return_value=True)
|
||||||
|
fake_mcp.get_server_status = MagicMock(return_value={"tool_count": 3})
|
||||||
|
|
||||||
|
fake_srv = SimpleNamespace(
|
||||||
|
id="srv-123",
|
||||||
|
name="test-server",
|
||||||
|
transport="stdio",
|
||||||
|
command="/usr/bin/test",
|
||||||
|
args=json.dumps(["--flag"]),
|
||||||
|
env=json.dumps({"KEY": "val"}),
|
||||||
|
url=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
fake_db = MagicMock()
|
||||||
|
fake_db.query.return_value.filter.return_value.first.return_value = fake_srv
|
||||||
|
|
||||||
|
with patch("src.tool_implementations.get_mcp_manager", return_value=fake_mcp), \
|
||||||
|
patch("core.database.SessionLocal", return_value=fake_db):
|
||||||
|
result = asyncio.run(do_manage_mcp(
|
||||||
|
json.dumps({"action": "reconnect", "server_id": "srv-123"})
|
||||||
|
))
|
||||||
|
|
||||||
|
assert result["exit_code"] == 0
|
||||||
|
fake_mcp.connect_server.assert_called_once_with(
|
||||||
|
server_id="srv-123",
|
||||||
|
name="test-server",
|
||||||
|
transport="stdio",
|
||||||
|
command="/usr/bin/test",
|
||||||
|
args=["--flag"],
|
||||||
|
env={"KEY": "val"},
|
||||||
|
url=None,
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user