Harden the drain guard against cycling pagination cursors

Comparing only against the immediately preceding cursor catches a server
that echoes the cursor back but not one that alternates between cursors
(a, b, a, ...), which would still page forever. Track every cursor seen
during the drain and raise on any repeat, in both the Client drains and
the ClientSessionGroup aggregation drain.

Author: CJGjr

docs/advanced/pagination.md

@@ -65,10 +65,11 @@ That loop is the same one in every client that pages, so `Client` ships it. The
 `ClientSessionGroup` aggregation drains the same way, so a group fronting several servers reports the full collection instead of each server's first page. That aggregator is **[Session groups](../client/session-groups.md)**.
 
 !!! warning
-    A drain trusts the server to advance the cursor. A server that keeps returning the same
-    `next_cursor` it was handed would page forever, so the drains stop and raise `RuntimeError`
-    the moment a cursor fails to move. A page that does not advance is a broken server, and a
-    loud failure beats a silent hang or a half-read list.
+    A drain trusts the server to advance the cursor. A server that echoes back the
+    `next_cursor` it was handed, or cycles through a longer loop of them, would page forever,
+    so the drains remember every cursor they have seen and raise `RuntimeError` the moment one
+    repeats. A repeated cursor is a broken server, and a loud failure beats a silent hang or a
+    half-read list.
 
 ## The three rules
 

src/mcp/client/client.py

@@ -955,19 +955,20 @@ async def iter_all_tools(self, *, meta: RequestParamsMeta | None = None) -> Asyn
         materializing the full list in memory.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
+        seen_cursors: set[str] = set()
         cursor: str | None = None
         while True:
             result = await self.list_tools(cursor=cursor, meta=meta)
             for tool in result.tools:
                 yield tool
             if result.next_cursor is None:
                 return
-            if result.next_cursor == cursor:
-                raise RuntimeError(
-                    "Server returned a pagination cursor that did not advance; refusing to page forever."
-                )
+            if result.next_cursor in seen_cursors:
+                raise RuntimeError("Server returned a pagination cursor it already returned; refusing to page forever.")
+            seen_cursors.add(result.next_cursor)
             cursor = result.next_cursor
 
     async def list_all_tools(self, *, meta: RequestParamsMeta | None = None) -> list[Tool]:
@@ -978,61 +979,66 @@ async def list_all_tools(self, *, meta: RequestParamsMeta | None = None) -> list
         list.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
         return [tool async for tool in self.iter_all_tools(meta=meta)]
 
     async def iter_all_prompts(self, *, meta: RequestParamsMeta | None = None) -> AsyncIterator[Prompt]:
         """Yield every prompt from the server, paging through `next_cursor`.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
+        seen_cursors: set[str] = set()
         cursor: str | None = None
         while True:
             result = await self.list_prompts(cursor=cursor, meta=meta)
             for prompt in result.prompts:
                 yield prompt
             if result.next_cursor is None:
                 return
-            if result.next_cursor == cursor:
-                raise RuntimeError(
-                    "Server returned a pagination cursor that did not advance; refusing to page forever."
-                )
+            if result.next_cursor in seen_cursors:
+                raise RuntimeError("Server returned a pagination cursor it already returned; refusing to page forever.")
+            seen_cursors.add(result.next_cursor)
             cursor = result.next_cursor
 
     async def list_all_prompts(self, *, meta: RequestParamsMeta | None = None) -> list[Prompt]:
         """List every prompt from the server, draining `next_cursor` across pages.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
         return [prompt async for prompt in self.iter_all_prompts(meta=meta)]
 
     async def iter_all_resources(self, *, meta: RequestParamsMeta | None = None) -> AsyncIterator[Resource]:
         """Yield every resource from the server, paging through `next_cursor`.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
+        seen_cursors: set[str] = set()
         cursor: str | None = None
         while True:
             result = await self.list_resources(cursor=cursor, meta=meta)
             for resource in result.resources:
                 yield resource
             if result.next_cursor is None:
                 return
-            if result.next_cursor == cursor:
-                raise RuntimeError(
-                    "Server returned a pagination cursor that did not advance; refusing to page forever."
-                )
+            if result.next_cursor in seen_cursors:
+                raise RuntimeError("Server returned a pagination cursor it already returned; refusing to page forever.")
+            seen_cursors.add(result.next_cursor)
             cursor = result.next_cursor
 
     async def list_all_resources(self, *, meta: RequestParamsMeta | None = None) -> list[Resource]:
         """List every resource from the server, draining `next_cursor` across pages.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
         return [resource async for resource in self.iter_all_resources(meta=meta)]
 
@@ -1042,26 +1048,28 @@ async def iter_all_resource_templates(
         """Yield every resource template from the server, paging through `next_cursor`.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
+        seen_cursors: set[str] = set()
         cursor: str | None = None
         while True:
             result = await self.list_resource_templates(cursor=cursor, meta=meta)
             for template in result.resource_templates:
                 yield template
             if result.next_cursor is None:
                 return
-            if result.next_cursor == cursor:
-                raise RuntimeError(
-                    "Server returned a pagination cursor that did not advance; refusing to page forever."
-                )
+            if result.next_cursor in seen_cursors:
+                raise RuntimeError("Server returned a pagination cursor it already returned; refusing to page forever.")
+            seen_cursors.add(result.next_cursor)
             cursor = result.next_cursor
 
     async def list_all_resource_templates(self, *, meta: RequestParamsMeta | None = None) -> list[ResourceTemplate]:
         """List every resource template from the server, draining `next_cursor` across pages.
 
         Raises:
-            RuntimeError: The server returned a pagination cursor that did not advance.
+            RuntimeError: The server returned a pagination cursor it already
+                returned, which would page forever.
         """
         return [template async for template in self.iter_all_resource_templates(meta=meta)]
 

src/mcp/client/session_group.py

@@ -78,9 +78,11 @@ async def _drain_paginated(
     the list attribute on the result (e.g. `"tools"`, `"prompts"`).
 
     Raises:
-        RuntimeError: The server returned a pagination cursor that did not advance.
+        RuntimeError: The server returned a pagination cursor it already
+            returned, which would page forever.
     """
     items: list[Any] = []
+    seen_cursors: set[str] = set()
     cursor: str | None = None
     while True:
         params = types.PaginatedRequestParams(cursor=cursor) if cursor is not None else None
@@ -89,8 +91,9 @@ async def _drain_paginated(
         next_cursor = getattr(result, "next_cursor", None)
         if next_cursor is None:
             return items
-        if next_cursor == cursor:
-            raise RuntimeError("Server returned a pagination cursor that did not advance; refusing to page forever.")
+        if next_cursor in seen_cursors:
+            raise RuntimeError("Server returned a pagination cursor it already returned; refusing to page forever.")
+        seen_cursors.add(next_cursor)
         cursor = next_cursor
 
 

tests/client/test_list_all_pagination.py

@@ -70,6 +70,25 @@ async def handler(_ctx: ServerRequestContext, _params: types.PaginatedRequestPar
     return handler
 
 
+def _cycling_cursor_handler(
+    make_item: Callable[[str], ItemT],
+    result_cls: Callable[..., ResultT],
+    items_field: str,
+) -> Callable[[ServerRequestContext, types.PaginatedRequestParams | None], Awaitable[ResultT]]:
+    """Build a malformed handler that alternates between two cursors forever.
+
+    The cursor always advances relative to the previous page, so a guard that
+    only compares against the immediately preceding cursor would page forever.
+    """
+
+    async def handler(_ctx: ServerRequestContext, params: types.PaginatedRequestParams | None) -> ResultT:
+        cursor = params.cursor if params else None
+        next_cursor = "b" if cursor == "a" else "a"
+        return result_cls(**{items_field: [make_item("x")]}, next_cursor=next_cursor)
+
+    return handler
+
+
 def _make_tool(name: str) -> types.Tool:
     return types.Tool(name=name, input_schema={"type": "object"})
 
@@ -269,5 +288,21 @@ async def test_drain_raises_when_cursor_does_not_advance(
     server = build_server()
 
     async with Client(server) as client:
-        with pytest.raises(RuntimeError, match="did not advance"):
+        with pytest.raises(RuntimeError, match="already returned"):
             await getattr(client, client_method)()
+
+
+async def test_drain_raises_when_cursors_cycle():
+    """A server whose cursors cycle (a, b, a, ...) must fail loudly, not loop forever.
+
+    Each cursor differs from the one before it, so this specifically exercises
+    the seen-set guard rather than the simpler stuck-cursor case above.
+    """
+    server = Server(
+        "cycling-tools",
+        on_list_tools=_cycling_cursor_handler(_make_tool, types.ListToolsResult, "tools"),
+    )
+
+    async with Client(server) as client:
+        with pytest.raises(RuntimeError, match="already returned"):
+            await client.list_all_tools()

tests/client/test_session_group.py

@@ -463,5 +463,5 @@ async def test_client_session_group_rejects_non_advancing_cursor(
 
     group = ClientSessionGroup(exit_stack=mock_exit_stack)
     with mock.patch.object(group, "_establish_session", return_value=(mock_server_info, mock_session)):
-        with pytest.raises(RuntimeError, match="did not advance"):
+        with pytest.raises(RuntimeError, match="already returned"):
             await group.connect_to_server(StdioServerParameters(command="test"))