Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions tests/tools/test_mcp_tool_session_expired.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,48 @@ async def _raises(*a, **kw):
mcp_tool._server_error_counts.pop("srv", None)


def test_call_tool_handler_lazy_reconnects_missing_server(monkeypatch, tmp_path):
"""If a tool schema survives but the in-process MCP server task is
missing, the first tool call should run discovery once and then call the
freshly connected server instead of forcing a manual gateway restart."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path))

from tools import mcp_tool
from tools.mcp_tool import _make_tool_handler

server, _ = _install_stub_server("lazy")

async def _call_tool(*a, **kw):
result = MagicMock()
result.isError = False
result.content = [MagicMock(type="text", text="lazy reconnect ok")]
result.structuredContent = None
return result

server.session.call_tool = _call_tool
mcp_tool._servers.pop("lazy", None)
mcp_tool._server_error_counts.pop("lazy", None)
calls = {"n": 0}

def _fake_discover():
calls["n"] += 1
mcp_tool._servers["lazy"] = server
return ["mcp_lazy_mytool"]

monkeypatch.setattr(mcp_tool, "discover_mcp_tools", _fake_discover)

try:
handler = _make_tool_handler("lazy", "mytool", 10.0)
out = handler({"arg": "v"})
parsed = json.loads(out)
assert parsed == {"result": "lazy reconnect ok"}
assert calls["n"] == 1
assert mcp_tool._server_error_counts.get("lazy", 0) == 0
finally:
mcp_tool._servers.pop("lazy", None)
mcp_tool._server_error_counts.pop("lazy", None)


def test_session_expired_handler_returns_none_without_loop(monkeypatch):
"""Defensive: if the MCP loop isn't running (cold start / shutdown
race), the handler must fall through cleanly instead of hanging
Expand Down
63 changes: 60 additions & 3 deletions tools/mcp_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,64 @@ async def _connect_server(name: str, config: dict) -> MCPServerTask:
# Handler / check-fn factories
# ---------------------------------------------------------------------------

def _get_connected_server_for_call(server_name: str):
"""Return a connected MCP server, lazily reconnecting if necessary.

Gateway sessions can retain MCP tool schemas while the in-process server
task is missing or has lost its transport session (for example after an
OAuth token is copied in, a config reload races with startup, or a remote
HTTP session is garbage-collected while idle). A direct tool call should
make one best-effort reconnect before surfacing ``server is not connected``
to the model; otherwise the only recovery path is a manual gateway restart.
"""
with _lock:
server = _servers.get(server_name)

if server and getattr(server, "session", None):
return server

# If a stale entry exists, remove it so discover_mcp_tools/register_mcp_servers
# treats the configured server as reconnectable rather than idempotently
# skipping it because the name is already present in _servers.
if server is not None:
logger.info(
"MCP server '%s' has no active session; attempting lazy reconnect",
server_name,
)
try:
with _lock:
if _servers.get(server_name) is server:
_servers.pop(server_name, None)
with _lock:
loop = _mcp_loop
if loop is not None and loop.is_running() and hasattr(server, "shutdown"):
future = asyncio.run_coroutine_threadsafe(server.shutdown(), loop)
future.result(timeout=15)
except Exception as exc:
logger.debug(
"Best-effort shutdown before lazy MCP reconnect for '%s' failed: %s",
server_name,
exc,
)
else:
logger.info(
"MCP server '%s' is not registered in-process; attempting lazy reconnect",
server_name,
)

try:
discover_mcp_tools()
except Exception as exc:
logger.warning("Lazy MCP reconnect for '%s' failed: %s", server_name, exc)

with _lock:
server = _servers.get(server_name)
if server and getattr(server, "session", None):
_reset_server_error(server_name)
return server
return None


def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float):
"""Return a sync handler that calls an MCP tool via the background loop.

Expand Down Expand Up @@ -2189,9 +2247,8 @@ def _handler(args: dict, **kwargs) -> str:
}, ensure_ascii=False)
# Cooldown elapsed → fall through as a half-open probe.

with _lock:
server = _servers.get(server_name)
if not server or not server.session:
server = _get_connected_server_for_call(server_name)
if not server:
_bump_server_error(server_name)
return json.dumps({
"error": f"MCP server '{server_name}' is not connected"
Expand Down