Skip to content

Commit 275aad4

Browse files
committed
fix(session-group): don't crash on servers with no components
ClientSessionGroup._aggregate_components deleted the session from _session_exit_stacks whenever a server exposed no prompts, resources, or tools. Via connect_with_session (caller-supplied session, never registered) this raised KeyError; via connect_to_server it dropped the freshly-registered exit stack so a later disconnect_from_server could not close the transport. Remove the erroneous cleanup so empty servers are aggregated like any other, and add regression tests for both entry points.
1 parent f1b6589 commit 275aad4

2 files changed

Lines changed: 66 additions & 7 deletions

File tree

‎src/mcp/client/session_group.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ async def disconnect_from_server(self, session: mcp.ClientSession) -> None:
281281

282282
# Clean up the session's resources via its dedicated exit stack
283283
if session_known_for_stack:
284-
session_stack_to_close = self._session_exit_stacks.pop(session) # pragma: no cover
285-
await session_stack_to_close.aclose() # pragma: no cover
284+
session_stack_to_close = self._session_exit_stacks.pop(session)
285+
await session_stack_to_close.aclose()
286286

287287
async def connect_with_session(
288288
self, server_info: types.Implementation, session: mcp.ClientSession
@@ -414,11 +414,6 @@ async def _aggregate_components(self, server_info: types.Implementation, session
414414
except MCPError as err: # pragma: no cover
415415
logging.warning(f"Could not fetch tools: {err}")
416416

417-
# Clean up exit stack for session if we couldn't retrieve anything
418-
# from the server.
419-
if not any((prompts_temp, resources_temp, tools_temp)):
420-
del self._session_exit_stacks[session] # pragma: no cover
421-
422417
# Check for duplicates.
423418
matching_prompts = prompts_temp.keys() & self._prompts.keys()
424419
if matching_prompts:

‎tests/client/test_session_group.py‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,3 +402,67 @@ async def test_client_session_group_establish_session_parameterized(
402402
# 3. Assert returned values
403403
assert returned_server_info is mock_initialize_result.server_info
404404
assert returned_session is mock_entered_session
405+
406+
407+
@pytest.mark.anyio
408+
async def test_connect_with_session_empty_server_does_not_raise():
409+
"""A server exposing no components connects cleanly via connect_with_session.
410+
411+
Regression: the caller-supplied session is never registered in
412+
_session_exit_stacks, so the old empty-server cleanup deleted a missing key
413+
and raised KeyError.
414+
"""
415+
server_info = mock.Mock(spec=types.Implementation)
416+
server_info.name = "EmptyServer"
417+
session = mock.AsyncMock(spec=mcp.ClientSession)
418+
session.list_tools.return_value = mock.AsyncMock(tools=[])
419+
session.list_resources.return_value = mock.AsyncMock(resources=[])
420+
session.list_prompts.return_value = mock.AsyncMock(prompts=[])
421+
422+
group = ClientSessionGroup()
423+
await group.connect_with_session(server_info, session)
424+
425+
assert session in group._sessions
426+
assert not group.tools
427+
assert not group.resources
428+
assert not group.prompts
429+
assert session not in group._session_exit_stacks
430+
431+
432+
@pytest.mark.anyio
433+
async def test_connect_to_server_empty_server_keeps_exit_stack(
434+
mock_exit_stack: contextlib.AsyncExitStack,
435+
):
436+
"""An empty server connected via connect_to_server retains its exit stack.
437+
438+
Regression: the old cleanup dropped the freshly-registered stack, so a later
439+
disconnect could not close the transport.
440+
"""
441+
server_info = mock.Mock(spec=types.Implementation)
442+
server_info.name = "EmptyServer"
443+
session = mock.AsyncMock(spec=mcp.ClientSession)
444+
session.list_tools.return_value = mock.AsyncMock(tools=[])
445+
session.list_resources.return_value = mock.AsyncMock(resources=[])
446+
session.list_prompts.return_value = mock.AsyncMock(prompts=[])
447+
session_stack = mock.AsyncMock(spec=contextlib.AsyncExitStack)
448+
449+
group = ClientSessionGroup(exit_stack=mock_exit_stack)
450+
451+
async def fake_establish(
452+
server_params: StdioServerParameters,
453+
session_params: ClientSessionParameters,
454+
) -> tuple[types.Implementation, mcp.ClientSession]:
455+
group._session_exit_stacks[session] = session_stack
456+
return server_info, session
457+
458+
with mock.patch.object(group, "_establish_session", side_effect=fake_establish):
459+
await group.connect_to_server(StdioServerParameters(command="test"))
460+
461+
assert session in group._sessions
462+
assert group._session_exit_stacks[session] is session_stack
463+
464+
await group.disconnect_from_server(session)
465+
466+
assert session not in group._sessions
467+
assert session not in group._session_exit_stacks
468+
session_stack.aclose.assert_awaited_once()

0 commit comments

Comments
 (0)