Skip to content

Commit 43e6341

Browse files
authored
fix(mcp): capability-gate tools/list so prompt-only MCP servers can connect (#44550)
Port from anomalyco/opencode#31271: only call tools/list when the server advertises the 'tools' capability in InitializeResult.capabilities. Previously, _discover_tools() unconditionally called session.list_tools() right after initialize. Prompt-only / resource-only servers (which omit the tools capability per the MCP spec) raise McpError(-32601 Method not found), which aborted the connection — burning all 3 initial-connect retries and permanently failing the server even though its prompts and resources were perfectly usable. The 180s keepalive had the same problem: it probed with list_tools(), so even a successfully connected prompt-only server would be torn down on the first keepalive cycle. Changes: - MCPServerTask._advertises_tools(): capability check with a legacy fallback (no captured InitializeResult -> behave as before) - _discover_tools(): skip tools/list for non-tool servers - keepalive: use the universal ping request for non-tool servers - _refresh_tools(): guard against tools/list_changed from non-tool servers E2E verified with a real stdio prompt-only FastMCP-style server: on main it fails all 3 connection attempts with Method-not-found; with this fix it connects, lists prompts, answers ping keepalives, and shuts down cleanly.
1 parent ef88bb5 commit 43e6341

2 files changed

Lines changed: 215 additions & 5 deletions

File tree

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""Tests for capability-gated MCP tool discovery and keepalive.
2+
3+
Prompt-only / resource-only MCP servers do not implement the ``tools/*``
4+
request family. Per the MCP spec, ``InitializeResult.capabilities.tools``
5+
is non-None iff the server supports it. Before this fix, Hermes always
6+
called ``tools/list`` during discovery and as the keepalive probe — both
7+
raised ``McpError(-32601 Method not found)`` against such servers, so a
8+
prompt-only server could never stay connected.
9+
10+
Ported from anomalyco/opencode#31271.
11+
"""
12+
import asyncio
13+
from types import SimpleNamespace
14+
from unittest.mock import AsyncMock
15+
16+
import pytest
17+
18+
from tools.mcp_tool import MCPServerTask
19+
20+
21+
def _caps(tools=None, prompts=None, resources=None):
22+
"""Build a fake InitializeResult with the given capability sub-objects."""
23+
return SimpleNamespace(
24+
capabilities=SimpleNamespace(tools=tools, prompts=prompts, resources=resources)
25+
)
26+
27+
28+
class TestAdvertisesTools:
29+
def test_true_when_tools_capability_present(self):
30+
task = MCPServerTask("test")
31+
task.initialize_result = _caps(tools=SimpleNamespace(listChanged=True))
32+
assert task._advertises_tools() is True
33+
34+
def test_false_for_prompt_only_server(self):
35+
task = MCPServerTask("test")
36+
task.initialize_result = _caps(prompts=SimpleNamespace(listChanged=None))
37+
assert task._advertises_tools() is False
38+
39+
def test_false_for_resource_only_server(self):
40+
task = MCPServerTask("test")
41+
task.initialize_result = _caps(resources=SimpleNamespace())
42+
assert task._advertises_tools() is False
43+
44+
def test_legacy_fallback_no_initialize_result(self):
45+
"""No captured capabilities → preserve old always-list_tools behavior."""
46+
task = MCPServerTask("test")
47+
assert task.initialize_result is None
48+
assert task._advertises_tools() is True
49+
50+
def test_legacy_fallback_no_capabilities_attr(self):
51+
task = MCPServerTask("test")
52+
task.initialize_result = SimpleNamespace() # no .capabilities
53+
assert task._advertises_tools() is True
54+
55+
56+
@pytest.mark.asyncio
57+
class TestDiscoverToolsGating:
58+
async def test_skips_list_tools_for_prompt_only_server(self):
59+
task = MCPServerTask("test")
60+
task.initialize_result = _caps(prompts=SimpleNamespace())
61+
task.session = SimpleNamespace(list_tools=AsyncMock())
62+
task._tools = ["stale"]
63+
64+
await task._discover_tools()
65+
66+
task.session.list_tools.assert_not_called()
67+
assert task._tools == []
68+
69+
async def test_calls_list_tools_for_tool_capable_server(self):
70+
task = MCPServerTask("test")
71+
task.initialize_result = _caps(tools=SimpleNamespace())
72+
fake_tool = SimpleNamespace(name="echo")
73+
task.session = SimpleNamespace(
74+
list_tools=AsyncMock(return_value=SimpleNamespace(tools=[fake_tool]))
75+
)
76+
77+
await task._discover_tools()
78+
79+
task.session.list_tools.assert_awaited_once()
80+
assert task._tools == [fake_tool]
81+
82+
async def test_legacy_fallback_still_calls_list_tools(self):
83+
task = MCPServerTask("test")
84+
task.session = SimpleNamespace(
85+
list_tools=AsyncMock(return_value=SimpleNamespace(tools=[]))
86+
)
87+
88+
await task._discover_tools()
89+
90+
task.session.list_tools.assert_awaited_once()
91+
92+
93+
@pytest.mark.asyncio
94+
class TestRefreshToolsGating:
95+
async def test_refresh_noop_for_prompt_only_server(self):
96+
task = MCPServerTask("test")
97+
task.initialize_result = _caps(prompts=SimpleNamespace())
98+
task.session = SimpleNamespace(list_tools=AsyncMock())
99+
100+
await task._refresh_tools()
101+
102+
task.session.list_tools.assert_not_called()
103+
104+
105+
@pytest.mark.asyncio
106+
class TestKeepaliveProbe:
107+
async def _run_one_keepalive_cycle(self, task):
108+
"""Drive _wait_for_lifecycle_event through exactly one keepalive
109+
timeout, then fire shutdown so it returns."""
110+
real_wait = asyncio.wait
111+
cycles = {"n": 0}
112+
113+
async def fake_wait(tasks, timeout=None, return_when=None):
114+
cycles["n"] += 1
115+
if cycles["n"] == 1:
116+
# Simulate keepalive timeout: nothing completed.
117+
return set(), set(tasks)
118+
# Second cycle: let shutdown win.
119+
task._shutdown_event.set()
120+
return await real_wait(
121+
tasks, timeout=0.5, return_when=return_when or asyncio.FIRST_COMPLETED
122+
)
123+
124+
import tools.mcp_tool as mcp_mod
125+
orig = mcp_mod.asyncio.wait
126+
mcp_mod.asyncio.wait = fake_wait
127+
try:
128+
return await task._wait_for_lifecycle_event()
129+
finally:
130+
mcp_mod.asyncio.wait = orig
131+
132+
async def test_keepalive_uses_ping_for_prompt_only_server(self):
133+
task = MCPServerTask("test")
134+
task.initialize_result = _caps(prompts=SimpleNamespace())
135+
task.session = SimpleNamespace(
136+
list_tools=AsyncMock(),
137+
send_ping=AsyncMock(),
138+
)
139+
140+
reason = await self._run_one_keepalive_cycle(task)
141+
142+
assert reason == "shutdown"
143+
task.session.send_ping.assert_awaited_once()
144+
task.session.list_tools.assert_not_called()
145+
146+
async def test_keepalive_uses_list_tools_for_tool_capable_server(self):
147+
task = MCPServerTask("test")
148+
task.initialize_result = _caps(tools=SimpleNamespace())
149+
task.session = SimpleNamespace(
150+
list_tools=AsyncMock(return_value=SimpleNamespace(tools=[])),
151+
send_ping=AsyncMock(),
152+
)
153+
154+
reason = await self._run_one_keepalive_cycle(task)
155+
156+
assert reason == "shutdown"
157+
task.session.list_tools.assert_awaited_once()
158+
task.session.send_ping.assert_not_called()

tools/mcp_tool.py

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,26 @@ def _is_http(self) -> bool:
12021202
"""Check if this server uses HTTP transport."""
12031203
return "url" in self._config
12041204

1205+
def _advertises_tools(self) -> bool:
1206+
"""Whether the server advertises the ``tools`` capability.
1207+
1208+
Per the MCP spec, ``InitializeResult.capabilities.tools`` is non-None
1209+
iff the server implements the ``tools/*`` request family. Prompt-only
1210+
or resource-only servers omit it, and calling ``tools/list`` against
1211+
them raises ``McpError(-32601 Method not found)`` — which previously
1212+
killed the connection during discovery and made every keepalive fail.
1213+
(Ported from anomalyco/opencode#31271.)
1214+
1215+
Returns True when no capability info was captured (legacy fallback:
1216+
preserve the old always-call-list_tools behavior rather than regress
1217+
any server that was working before this gate).
1218+
"""
1219+
init_result = self.initialize_result
1220+
caps = getattr(init_result, "capabilities", None) if init_result is not None else None
1221+
if caps is None:
1222+
return True
1223+
return getattr(caps, "tools", None) is not None
1224+
12051225
# ----- Dynamic tool discovery (notifications/tools/list_changed) -----
12061226

12071227
async def _refresh_tools_task(self):
@@ -1273,6 +1293,12 @@ async def _refresh_tools(self):
12731293
"""
12741294
from tools.registry import registry
12751295

1296+
if not self._advertises_tools():
1297+
# A server that doesn't implement tools/* should never send
1298+
# tools/list_changed, but guard anyway — calling tools/list
1299+
# would raise McpError(-32601).
1300+
return
1301+
12761302
async with self._refresh_lock:
12771303
# Capture old tool names for change diff
12781304
old_tool_names = set(self._registered_tool_names)
@@ -1360,12 +1386,22 @@ async def _wait_for_lifecycle_event(self) -> str:
13601386

13611387
# Timeout — no lifecycle event fired. Send a keepalive
13621388
# to exercise the connection and detect stale sockets.
1389+
# Prompt-only / resource-only servers don't implement
1390+
# ``tools/list`` (McpError -32601), so use the universal
1391+
# ``ping`` request for them instead — otherwise every
1392+
# keepalive cycle would trigger a spurious reconnect.
13631393
if self.session:
13641394
try:
1365-
await asyncio.wait_for(
1366-
self.session.list_tools(),
1367-
timeout=30.0,
1368-
)
1395+
if self._advertises_tools():
1396+
await asyncio.wait_for(
1397+
self.session.list_tools(),
1398+
timeout=30.0,
1399+
)
1400+
else:
1401+
await asyncio.wait_for(
1402+
self.session.send_ping(),
1403+
timeout=30.0,
1404+
)
13691405
except Exception as exc:
13701406
logger.warning(
13711407
"MCP server '%s' keepalive failed, "
@@ -1778,9 +1814,25 @@ async def _strip_auth_on_cross_origin_redirect(response):
17781814
)
17791815

17801816
async def _discover_tools(self):
1781-
"""Discover tools from the connected session."""
1817+
"""Discover tools from the connected session.
1818+
1819+
Capability-gated: prompt-only / resource-only MCP servers don't
1820+
implement ``tools/list``, and calling it raises ``McpError(-32601)``,
1821+
which previously aborted the connection — those servers could never
1822+
stay connected for their prompts/resources. Skip the call when the
1823+
server doesn't advertise the ``tools`` capability.
1824+
(Ported from anomalyco/opencode#31271.)
1825+
"""
17821826
if self.session is None:
17831827
return
1828+
if not self._advertises_tools():
1829+
logger.info(
1830+
"MCP server '%s': does not advertise 'tools' capability — "
1831+
"skipping tools/list (prompts/resources remain available)",
1832+
self.name,
1833+
)
1834+
self._tools = []
1835+
return
17841836
async with self._rpc_lock:
17851837
tools_result = await self.session.list_tools()
17861838
self._tools = (

0 commit comments

Comments
 (0)