From 8d4a81f3a074c5396df7a53359027cde392ddd33 Mon Sep 17 00:00:00 2001 From: goingforstudying-ctrl Date: Sun, 11 Oct 2026 03:28:38 -0400 Subject: [PATCH 1/6] fix(api): paginate MCP list_tools past the first page tools/list (and the other list endpoints) can return a nextCursor, but the client never sent it back, so anything past page one never reached the provider. Follow the cursor in MCPClient.list_tools and let the session list methods take one. Stop if the server echoes a cursor. --- api/core/mcp/mcp_client.py | 14 ++++- api/core/mcp/session/client_session.py | 12 ++-- .../core/mcp/session/test_client_session.py | 55 +++++++++++++++++ .../unit_tests/core/mcp/test_mcp_client.py | 59 +++++++++++++++++++ 4 files changed, 134 insertions(+), 6 deletions(-) diff --git a/api/core/mcp/mcp_client.py b/api/core/mcp/mcp_client.py index 387bb40d270506..11a4923a22f1ac 100644 --- a/api/core/mcp/mcp_client.py +++ b/api/core/mcp/mcp_client.py @@ -114,8 +114,18 @@ def list_tools(self) -> list[Tool]: """List available tools from the MCP server""" if not self._session: raise ValueError("Session not initialized.") - response = self._session.list_tools() - return response.tools + tools: list[Tool] = [] + cursor: str | None = None + seen_cursors: set[str] = set() + while True: + response = self._session.list_tools(cursor=cursor) + tools.extend(response.tools) + cursor = response.nextCursor + # A server echoing the same cursor would loop forever, so stop there. + if not cursor or cursor in seen_cursors: + break + seen_cursors.add(cursor) + return tools def invoke_tool(self, tool_name: str, tool_args: dict[str, Any]) -> CallToolResult: """Call a tool""" diff --git a/api/core/mcp/session/client_session.py b/api/core/mcp/session/client_session.py index 1f1f574afabe3b..44b0e89484d591 100644 --- a/api/core/mcp/session/client_session.py +++ b/api/core/mcp/session/client_session.py @@ -188,23 +188,25 @@ def set_logging_level(self, level: types.LoggingLevel) -> types.EmptyResult: types.EmptyResult, ) - def list_resources(self) -> types.ListResourcesResult: + def list_resources(self, cursor: str | None = None) -> types.ListResourcesResult: """Send a resources/list request.""" return self.send_request( types.ClientRequest( types.ListResourcesRequest( method="resources/list", + params=types.PaginatedRequestParams(cursor=cursor) if cursor is not None else None, ) ), types.ListResourcesResult, ) - def list_resource_templates(self) -> types.ListResourceTemplatesResult: + def list_resource_templates(self, cursor: str | None = None) -> types.ListResourceTemplatesResult: """Send a resources/templates/list request.""" return self.send_request( types.ClientRequest( types.ListResourceTemplatesRequest( method="resources/templates/list", + params=types.PaginatedRequestParams(cursor=cursor) if cursor is not None else None, ) ), types.ListResourceTemplatesResult, @@ -265,12 +267,13 @@ def call_tool( request_read_timeout_seconds=read_timeout_seconds, ) - def list_prompts(self) -> types.ListPromptsResult: + def list_prompts(self, cursor: str | None = None) -> types.ListPromptsResult: """Send a prompts/list request.""" return self.send_request( types.ClientRequest( types.ListPromptsRequest( method="prompts/list", + params=types.PaginatedRequestParams(cursor=cursor) if cursor is not None else None, ) ), types.ListPromptsResult, @@ -307,12 +310,13 @@ def complete( types.CompleteResult, ) - def list_tools(self) -> types.ListToolsResult: + def list_tools(self, cursor: str | None = None) -> types.ListToolsResult: """Send a tools/list request.""" return self.send_request( types.ClientRequest( types.ListToolsRequest( method="tools/list", + params=types.PaginatedRequestParams(cursor=cursor) if cursor is not None else None, ) ), types.ListToolsResult, diff --git a/api/tests/unit_tests/core/mcp/session/test_client_session.py b/api/tests/unit_tests/core/mcp/session/test_client_session.py index 0dcccdcf5398db..5cd894bbef4e40 100644 --- a/api/tests/unit_tests/core/mcp/session/test_client_session.py +++ b/api/tests/unit_tests/core/mcp/session/test_client_session.py @@ -440,6 +440,61 @@ def mock_server(): t.join(timeout=1) +@pytest.mark.parametrize( + ("method_name", "wire_method", "result_key"), + [ + ("list_tools", "tools/list", "tools"), + ("list_resources", "resources/list", "resources"), + ("list_resource_templates", "resources/templates/list", "resourceTemplates"), + ("list_prompts", "prompts/list", "prompts"), + ], +) +def test_list_methods_send_cursor(streams, method_name, wire_method, result_key): + read_stream, write_stream = streams + session = ClientSession(read_stream, write_stream) + + def mock_server(): + msg = write_stream.get(timeout=2) + assert msg.message.root.method == wire_method + assert msg.message.root.params == {"cursor": "opaque-cursor-1"} + req_id = msg.message.root.id + resp = types.JSONRPCResponse(jsonrpc="2.0", id=req_id, result={result_key: [], "nextCursor": "cursor-2"}) + read_stream.put(SessionMessage(message=types.JSONRPCMessage(resp))) + + import threading + + t = threading.Thread(target=mock_server, daemon=True) + t.start() + + with session: + result = getattr(session, method_name)(cursor="opaque-cursor-1") + assert result.nextCursor == "cursor-2" + t.join(timeout=1) + + +def test_list_tools_omits_params_without_cursor(streams): + read_stream, write_stream = streams + session = ClientSession(read_stream, write_stream) + + def mock_server(): + msg = write_stream.get(timeout=2) + assert msg.message.root.method == "tools/list" + assert msg.message.root.params is None + req_id = msg.message.root.id + resp = types.JSONRPCResponse(jsonrpc="2.0", id=req_id, result={"tools": []}) + read_stream.put(SessionMessage(message=types.JSONRPCMessage(resp))) + + import threading + + t = threading.Thread(target=mock_server, daemon=True) + t.start() + + with session: + result = session.list_tools() + assert result.tools == [] + t.join(timeout=1) + + def test_send_roots_list_changed(streams): read_stream, write_stream = streams session = ClientSession(read_stream, write_stream) diff --git a/api/tests/unit_tests/core/mcp/test_mcp_client.py b/api/tests/unit_tests/core/mcp/test_mcp_client.py index 507ad07f20f110..90dcb1c1329ed4 100644 --- a/api/tests/unit_tests/core/mcp/test_mcp_client.py +++ b/api/tests/unit_tests/core/mcp/test_mcp_client.py @@ -399,6 +399,65 @@ def test_list_tools_success(self): assert result == expected_tools mock_session.list_tools.assert_called_once() + def test_list_tools_paginates_until_cursor_exhausted(self): + """Test list_tools follows nextCursor across pages.""" + client = MCPClient(server_url="http://test.example.com") + + tool_a = Tool(name="alpha", description="First page tool", inputSchema={"type": "object", "properties": {}}) + tool_b = Tool(name="beta", description="Second page tool", inputSchema={"type": "object", "properties": {}}) + + mock_session = Mock() + mock_session.list_tools.side_effect = [ + ListToolsResult(tools=[tool_a], nextCursor="cursor-1"), + ListToolsResult(tools=[tool_b]), + ] + client._session = mock_session + + result = client.list_tools() + + assert result == [tool_a, tool_b] + assert mock_session.list_tools.call_count == 2 + assert mock_session.list_tools.call_args_list[0].kwargs == {"cursor": None} + assert mock_session.list_tools.call_args_list[1].kwargs == {"cursor": "cursor-1"} + + def test_list_tools_stops_on_repeated_cursor(self): + """Test list_tools does not loop forever when the server echoes a cursor.""" + client = MCPClient(server_url="http://test.example.com") + + tool_a = Tool(name="alpha", description="First page tool", inputSchema={"type": "object", "properties": {}}) + tool_b = Tool(name="beta", description="Second page tool", inputSchema={"type": "object", "properties": {}}) + + mock_session = Mock() + mock_session.list_tools.side_effect = [ + ListToolsResult(tools=[tool_a], nextCursor="cursor-1"), + ListToolsResult(tools=[tool_b], nextCursor="cursor-1"), + ] + client._session = mock_session + + result = client.list_tools() + + # The echoed cursor must not trigger a third request for the same page. + assert result == [tool_a, tool_b] + assert mock_session.list_tools.call_count == 2 + + def test_list_tools_empty_first_page_with_cursor(self): + """Test list_tools keeps paging when an early page returns no tools.""" + client = MCPClient(server_url="http://test.example.com") + + tool = Tool(name="late-tool", description="Only on page two", inputSchema={"type": "object", "properties": {}}) + + mock_session = Mock() + mock_session.list_tools.side_effect = [ + ListToolsResult(tools=[], nextCursor="cursor-1"), + ListToolsResult(tools=[tool]), + ] + client._session = mock_session + + result = client.list_tools() + + assert result == [tool] + assert mock_session.list_tools.call_count == 2 + def test_invoke_tool_not_initialized(self): """Test invoke_tool when session not initialized.""" client = MCPClient(server_url="http://test.example.com") From 969cd4dfaf83cc28d5891ca3aa6d0e8e6428c881 Mon Sep 17 00:00:00 2001 From: goingforstudying-ctrl Date: Sun, 11 Oct 2026 03:36:42 -0400 Subject: [PATCH 2/6] drop getattr from the cursor test, CI lint forbids it --- .../unit_tests/core/mcp/session/test_client_session.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api/tests/unit_tests/core/mcp/session/test_client_session.py b/api/tests/unit_tests/core/mcp/session/test_client_session.py index 5cd894bbef4e40..3f83e81fcd5c26 100644 --- a/api/tests/unit_tests/core/mcp/session/test_client_session.py +++ b/api/tests/unit_tests/core/mcp/session/test_client_session.py @@ -467,7 +467,13 @@ def mock_server(): t.start() with session: - result = getattr(session, method_name)(cursor="opaque-cursor-1") + method = { + "list_tools": session.list_tools, + "list_resources": session.list_resources, + "list_resource_templates": session.list_resource_templates, + "list_prompts": session.list_prompts, + }[method_name] + result = method(cursor="opaque-cursor-1") assert result.nextCursor == "cursor-2" t.join(timeout=1) From 72de5f2b223c8a7cbfaf496045684019109092e2 Mon Sep 17 00:00:00 2001 From: goingforstudying-ctrl Date: Sun, 11 Oct 2026 03:48:46 -0400 Subject: [PATCH 3/6] rework pagination tests to use an in-memory server instead of session mocks CI's no-new-session-mock guard rejects fresh Mock() sessions in tests. Talking to a real ClientSession over queues covers the wire format too. --- .../unit_tests/core/mcp/test_mcp_client.py | 116 ++++++++++++------ 1 file changed, 76 insertions(+), 40 deletions(-) diff --git a/api/tests/unit_tests/core/mcp/test_mcp_client.py b/api/tests/unit_tests/core/mcp/test_mcp_client.py index 90dcb1c1329ed4..0967e30a34e2d0 100644 --- a/api/tests/unit_tests/core/mcp/test_mcp_client.py +++ b/api/tests/unit_tests/core/mcp/test_mcp_client.py @@ -399,64 +399,100 @@ def test_list_tools_success(self): assert result == expected_tools mock_session.list_tools.assert_called_once() - def test_list_tools_paginates_until_cursor_exhausted(self): - """Test list_tools follows nextCursor across pages.""" - client = MCPClient(server_url="http://test.example.com") + def _client_with_paginated_server(self, pages: list[dict], fail_on_extra_page: bool = False): + """Client whose session talks to an in-memory server answering tools/list from pages. - tool_a = Tool(name="alpha", description="First page tool", inputSchema={"type": "object", "properties": {}}) - tool_b = Tool(name="beta", description="Second page tool", inputSchema={"type": "object", "properties": {}}) + Each entry in pages is the raw result dict for one request. The server + records the cursor of every request it sees. With fail_on_extra_page, + any request past the last page gets an error response so a runaway + pagination loop fails fast instead of hanging the test. + """ + import queue + import threading + + from core.mcp.session.base_session import SessionMessage + from core.mcp.session.client_session import ClientSession + from core.mcp.types import JSONRPCError, JSONRPCMessage, JSONRPCResponse + + read_stream, write_stream = queue.Queue(), queue.Queue() + session = ClientSession(read_stream, write_stream) + seen_cursors: list[str | None] = [] + + def mock_server(): + for page in pages: + msg = write_stream.get(timeout=5) + params = msg.message.root.params or {} + seen_cursors.append(params.get("cursor")) + resp = JSONRPCResponse(jsonrpc="2.0", id=msg.message.root.id, result=page) + read_stream.put(SessionMessage(message=JSONRPCMessage(resp))) + if not fail_on_extra_page: + return + try: + msg = write_stream.get(timeout=2) + except queue.Empty: + return + err = JSONRPCError( + jsonrpc="2.0", + id=msg.message.root.id, + error={"code": -32000, "message": "client asked for a page the server never offered"}, + ) + read_stream.put(SessionMessage(message=JSONRPCMessage(err))) - mock_session = Mock() - mock_session.list_tools.side_effect = [ - ListToolsResult(tools=[tool_a], nextCursor="cursor-1"), - ListToolsResult(tools=[tool_b]), + t = threading.Thread(target=mock_server, daemon=True) + t.start() + + client = MCPClient(server_url="http://test.example.com") + client._session = session + return client, session, t, seen_cursors + + def test_list_tools_paginates_until_cursor_exhausted(self): + """Test list_tools follows nextCursor across pages.""" + pages = [ + {"tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], "nextCursor": "cursor-1"}, + {"tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}]}, ] - client._session = mock_session + client, session, t, seen_cursors = self._client_with_paginated_server(pages) - result = client.list_tools() + with session: + result = client.list_tools() + t.join(timeout=5) - assert result == [tool_a, tool_b] - assert mock_session.list_tools.call_count == 2 - assert mock_session.list_tools.call_args_list[0].kwargs == {"cursor": None} - assert mock_session.list_tools.call_args_list[1].kwargs == {"cursor": "cursor-1"} + assert [tool.name for tool in result] == ["alpha", "beta"] + assert seen_cursors == [None, "cursor-1"] def test_list_tools_stops_on_repeated_cursor(self): """Test list_tools does not loop forever when the server echoes a cursor.""" - client = MCPClient(server_url="http://test.example.com") - - tool_a = Tool(name="alpha", description="First page tool", inputSchema={"type": "object", "properties": {}}) - tool_b = Tool(name="beta", description="Second page tool", inputSchema={"type": "object", "properties": {}}) - - mock_session = Mock() - mock_session.list_tools.side_effect = [ - ListToolsResult(tools=[tool_a], nextCursor="cursor-1"), - ListToolsResult(tools=[tool_b], nextCursor="cursor-1"), + pages = [ + {"tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], "nextCursor": "cursor-1"}, + { + "tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}], + "nextCursor": "cursor-1", + }, ] - client._session = mock_session + client, session, t, seen_cursors = self._client_with_paginated_server(pages, fail_on_extra_page=True) - result = client.list_tools() + with session: + result = client.list_tools() + t.join(timeout=5) # The echoed cursor must not trigger a third request for the same page. - assert result == [tool_a, tool_b] - assert mock_session.list_tools.call_count == 2 + assert [tool.name for tool in result] == ["alpha", "beta"] + assert seen_cursors == [None, "cursor-1"] def test_list_tools_empty_first_page_with_cursor(self): """Test list_tools keeps paging when an early page returns no tools.""" - client = MCPClient(server_url="http://test.example.com") - - tool = Tool(name="late-tool", description="Only on page two", inputSchema={"type": "object", "properties": {}}) - - mock_session = Mock() - mock_session.list_tools.side_effect = [ - ListToolsResult(tools=[], nextCursor="cursor-1"), - ListToolsResult(tools=[tool]), + pages = [ + {"tools": [], "nextCursor": "cursor-1"}, + {"tools": [{"name": "late-tool", "inputSchema": {"type": "object", "properties": {}}}]}, ] - client._session = mock_session + client, session, t, seen_cursors = self._client_with_paginated_server(pages) - result = client.list_tools() + with session: + result = client.list_tools() + t.join(timeout=5) - assert result == [tool] - assert mock_session.list_tools.call_count == 2 + assert [tool.name for tool in result] == ["late-tool"] + assert seen_cursors == [None, "cursor-1"] def test_invoke_tool_not_initialized(self): """Test invoke_tool when session not initialized.""" From f81a5a1eac1ec42fef71153e6f75aa154514d17d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 11 Oct 2026 07:51:15 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- api/tests/unit_tests/core/mcp/test_mcp_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/api/tests/unit_tests/core/mcp/test_mcp_client.py b/api/tests/unit_tests/core/mcp/test_mcp_client.py index 0967e30a34e2d0..1e10535748ff66 100644 --- a/api/tests/unit_tests/core/mcp/test_mcp_client.py +++ b/api/tests/unit_tests/core/mcp/test_mcp_client.py @@ -448,7 +448,10 @@ def mock_server(): def test_list_tools_paginates_until_cursor_exhausted(self): """Test list_tools follows nextCursor across pages.""" pages = [ - {"tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], "nextCursor": "cursor-1"}, + { + "tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], + "nextCursor": "cursor-1", + }, {"tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}]}, ] client, session, t, seen_cursors = self._client_with_paginated_server(pages) @@ -463,7 +466,10 @@ def test_list_tools_paginates_until_cursor_exhausted(self): def test_list_tools_stops_on_repeated_cursor(self): """Test list_tools does not loop forever when the server echoes a cursor.""" pages = [ - {"tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], "nextCursor": "cursor-1"}, + { + "tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], + "nextCursor": "cursor-1", + }, { "tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}], "nextCursor": "cursor-1", From 4d544957835b15b7504ffc72afec6cf58769d037 Mon Sep 17 00:00:00 2001 From: goingforstudying-ctrl Date: Tue, 13 Oct 2026 09:31:01 -0400 Subject: [PATCH 5/6] fix(api): reject incomplete MCP tool pagination Preserve empty opaque cursors and fail cursor cycles before a partial tool directory can replace the saved provider tools. Cover wire serialization, client pagination, and service persistence on success and failure. --- api/core/mcp/mcp_client.py | 8 +- .../core/mcp/session/test_client_session.py | 7 +- .../unit_tests/core/mcp/test_mcp_client.py | 44 +++--- ...est_mcp_tools_manage_service_pagination.py | 136 ++++++++++++++++++ 4 files changed, 171 insertions(+), 24 deletions(-) create mode 100644 api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py diff --git a/api/core/mcp/mcp_client.py b/api/core/mcp/mcp_client.py index 11a4923a22f1ac..e8b3dbd92c39bb 100644 --- a/api/core/mcp/mcp_client.py +++ b/api/core/mcp/mcp_client.py @@ -111,7 +111,7 @@ def connect_server(self, client_factory: Callable[..., AbstractContextManager[An self._session.initialize() def list_tools(self) -> list[Tool]: - """List available tools from the MCP server""" + """List all available tools, rejecting cursor cycles rather than returning a partial list.""" if not self._session: raise ValueError("Session not initialized.") tools: list[Tool] = [] @@ -121,9 +121,11 @@ def list_tools(self) -> list[Tool]: response = self._session.list_tools(cursor=cursor) tools.extend(response.tools) cursor = response.nextCursor - # A server echoing the same cursor would loop forever, so stop there. - if not cursor or cursor in seen_cursors: + if cursor is None: break + # A repeated cursor means the directory is incomplete and must not be saved. + if cursor in seen_cursors: + raise MCPConnectionError("MCP server returned a repeated cursor while listing tools") seen_cursors.add(cursor) return tools diff --git a/api/tests/unit_tests/core/mcp/session/test_client_session.py b/api/tests/unit_tests/core/mcp/session/test_client_session.py index 3f83e81fcd5c26..9d3bcf6978ec5b 100644 --- a/api/tests/unit_tests/core/mcp/session/test_client_session.py +++ b/api/tests/unit_tests/core/mcp/session/test_client_session.py @@ -449,14 +449,15 @@ def mock_server(): ("list_prompts", "prompts/list", "prompts"), ], ) -def test_list_methods_send_cursor(streams, method_name, wire_method, result_key): +@pytest.mark.parametrize("cursor", ["opaque-cursor-1", ""]) +def test_list_methods_send_cursor(streams, method_name, wire_method, result_key, cursor: str): read_stream, write_stream = streams session = ClientSession(read_stream, write_stream) def mock_server(): msg = write_stream.get(timeout=2) assert msg.message.root.method == wire_method - assert msg.message.root.params == {"cursor": "opaque-cursor-1"} + assert msg.message.root.params == {"cursor": cursor} req_id = msg.message.root.id resp = types.JSONRPCResponse(jsonrpc="2.0", id=req_id, result={result_key: [], "nextCursor": "cursor-2"}) read_stream.put(SessionMessage(message=types.JSONRPCMessage(resp))) @@ -473,7 +474,7 @@ def mock_server(): "list_resource_templates": session.list_resource_templates, "list_prompts": session.list_prompts, }[method_name] - result = method(cursor="opaque-cursor-1") + result = method(cursor=cursor) assert result.nextCursor == "cursor-2" t.join(timeout=1) diff --git a/api/tests/unit_tests/core/mcp/test_mcp_client.py b/api/tests/unit_tests/core/mcp/test_mcp_client.py index 1e10535748ff66..8dabe86e475ddc 100644 --- a/api/tests/unit_tests/core/mcp/test_mcp_client.py +++ b/api/tests/unit_tests/core/mcp/test_mcp_client.py @@ -421,6 +421,8 @@ def _client_with_paginated_server(self, pages: list[dict], fail_on_extra_page: b def mock_server(): for page in pages: msg = write_stream.get(timeout=5) + if msg is None: + return params = msg.message.root.params or {} seen_cursors.append(params.get("cursor")) resp = JSONRPCResponse(jsonrpc="2.0", id=msg.message.root.id, result=page) @@ -431,6 +433,10 @@ def mock_server(): msg = write_stream.get(timeout=2) except queue.Empty: return + if msg is None: + return + params = msg.message.root.params or {} + seen_cursors.append(params.get("cursor")) err = JSONRPCError( jsonrpc="2.0", id=msg.message.root.id, @@ -445,12 +451,13 @@ def mock_server(): client._session = session return client, session, t, seen_cursors - def test_list_tools_paginates_until_cursor_exhausted(self): - """Test list_tools follows nextCursor across pages.""" + @pytest.mark.parametrize("cursor", ["cursor-1", ""]) + def test_list_tools_paginates_until_cursor_exhausted(self, cursor: str): + """Follow opaque nextCursor values, including the empty string.""" pages = [ { "tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], - "nextCursor": "cursor-1", + "nextCursor": cursor, }, {"tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}]}, ] @@ -461,29 +468,30 @@ def test_list_tools_paginates_until_cursor_exhausted(self): t.join(timeout=5) assert [tool.name for tool in result] == ["alpha", "beta"] - assert seen_cursors == [None, "cursor-1"] - - def test_list_tools_stops_on_repeated_cursor(self): - """Test list_tools does not loop forever when the server echoes a cursor.""" + assert seen_cursors == [None, cursor] + + @pytest.mark.parametrize( + "cursors", + [["cursor-1", "cursor-1"], ["cursor-1", "cursor-2", "cursor-1"], ["", ""]], + ids=["repeated", "cycle", "empty-repeated"], + ) + def test_list_tools_raises_on_repeated_cursor(self, cursors: list[str]): + """Reject incomplete tool directories without requesting a page twice.""" pages = [ { - "tools": [{"name": "alpha", "inputSchema": {"type": "object", "properties": {}}}], - "nextCursor": "cursor-1", - }, - { - "tools": [{"name": "beta", "inputSchema": {"type": "object", "properties": {}}}], - "nextCursor": "cursor-1", - }, + "tools": [{"name": f"tool-{index}", "inputSchema": {"type": "object", "properties": {}}}], + "nextCursor": cursor, + } + for index, cursor in enumerate(cursors) ] client, session, t, seen_cursors = self._client_with_paginated_server(pages, fail_on_extra_page=True) with session: - result = client.list_tools() + with pytest.raises(MCPConnectionError, match="repeated cursor"): + client.list_tools() t.join(timeout=5) - # The echoed cursor must not trigger a third request for the same page. - assert [tool.name for tool in result] == ["alpha", "beta"] - assert seen_cursors == [None, "cursor-1"] + assert seen_cursors == [None, *cursors[:-1]] def test_list_tools_empty_first_page_with_cursor(self): """Test list_tools keeps paging when an early page returns no tools.""" diff --git a/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py b/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py new file mode 100644 index 00000000000000..0e56b818061770 --- /dev/null +++ b/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py @@ -0,0 +1,136 @@ +"""A failed paginated refresh must preserve the saved MCP tool directory.""" + +import json +from datetime import datetime +from unittest.mock import Mock, call + +import pytest + +from core.entities.mcp_provider import MCPProviderEntity +from core.mcp.auth_client import MCPClientWithAuthRetry +from core.mcp.error import MCPConnectionError +from core.mcp.types import ListToolsResult, Tool +from models.tools import MCPToolProvider +from services.tools.mcp_tools_manage_service import MCPToolManageService + + +@pytest.fixture +def provider() -> MCPToolProvider: + provider = MCPToolProvider( + name="Test server", + server_identifier="test-server", + server_url="https://example.com/mcp", + server_url_hash="hash", + icon="", + tenant_id="tenant-id", + user_id="user-id", + authed=True, + tools=json.dumps([{"name": "saved-tool", "inputSchema": {}}]), + ) + provider.created_at = datetime(2024, 1, 1) + provider.updated_at = datetime(2024, 1, 1) + return provider + + +@pytest.fixture +def db_session(provider: MCPToolProvider) -> Mock: + return Mock(scalar=Mock(return_value=provider)) + + +@pytest.fixture +def remote_session() -> Mock: + return Mock() + + +@pytest.fixture +def build_response() -> Mock: + return Mock() + + +@pytest.fixture +def service( + db_session: Mock, remote_session: Mock, build_response: Mock, monkeypatch: pytest.MonkeyPatch +) -> MCPToolManageService: + service = MCPToolManageService(db_session) + client = MCPClientWithAuthRetry(server_url="https://example.com/mcp") + client._session = remote_session + # Keep the real auth wrapper and pagination loop; only transport setup is stubbed. + monkeypatch.setattr(client, "_initialize", Mock()) + monkeypatch.setattr("services.tools.mcp_tools_manage_service.MCPClientWithAuthRetry", Mock(return_value=client)) + monkeypatch.setattr(MCPProviderEntity, "decrypt_server_url", Mock(return_value="https://example.com/mcp")) + monkeypatch.setattr(service, "_prepare_auth_headers", Mock(return_value={})) + monkeypatch.setattr(service, "_build_tool_provider_response", build_response) + return service + + +def _page(name: str, cursor: str | None) -> ListToolsResult: + return ListToolsResult(tools=[Tool(name=name, inputSchema={})], nextCursor=cursor) + + +@pytest.mark.parametrize("cursor", ["next", ""]) +def test_list_provider_tools_saves_all_pages( + service: MCPToolManageService, + provider: MCPToolProvider, + db_session: Mock, + remote_session: Mock, + build_response: Mock, + cursor: str, +) -> None: + remote_session.list_tools.side_effect = [_page("alpha", cursor), _page("beta", None)] + + result = service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + + assert result is build_response.return_value + assert [tool["name"] for tool in json.loads(provider.tools)] == ["alpha", "beta"] + assert all(tool["description"] == "" for tool in json.loads(provider.tools)) + assert provider.authed is True + db_session.flush.assert_called_once_with() + assert remote_session.list_tools.call_args_list == [call(cursor=None), call(cursor=cursor)] + + +@pytest.mark.parametrize( + "cursors", + [["next", "next"], ["a", "b", "a"], ["", ""]], + ids=["repeated", "cycle", "empty-repeated"], +) +def test_list_provider_tools_preserves_saved_tools_on_cursor_cycle( + service: MCPToolManageService, + provider: MCPToolProvider, + db_session: Mock, + remote_session: Mock, + build_response: Mock, + cursors: list[str], +) -> None: + original_tools, original_updated_at = provider.tools, provider.updated_at + remote_session.list_tools.side_effect = [_page(f"tool-{index}", cursor) for index, cursor in enumerate(cursors)] + + with pytest.raises(ValueError, match="Failed to connect to MCP server: .*repeated cursor"): + service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + + assert provider.tools == original_tools + assert provider.updated_at == original_updated_at + assert provider.authed is True + db_session.flush.assert_not_called() + build_response.assert_not_called() + assert remote_session.list_tools.call_args_list == [call(cursor=cursor) for cursor in [None, *cursors[:-1]]] + + +def test_list_provider_tools_preserves_saved_tools_on_later_page_error( + service: MCPToolManageService, + provider: MCPToolProvider, + db_session: Mock, + remote_session: Mock, + build_response: Mock, +) -> None: + original_tools, original_updated_at = provider.tools, provider.updated_at + remote_session.list_tools.side_effect = [_page("alpha", "next"), MCPConnectionError("later page failed")] + + with pytest.raises(ValueError, match="Failed to connect to MCP server: later page failed"): + service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + + assert provider.tools == original_tools + assert provider.updated_at == original_updated_at + assert provider.authed is True + db_session.flush.assert_not_called() + build_response.assert_not_called() + assert remote_session.list_tools.call_args_list == [call(cursor=None), call(cursor="next")] From 346cae650d81dcd303981a541f258c49f6615938 Mon Sep 17 00:00:00 2001 From: goingforstudying-ctrl Date: Wed, 21 Oct 2026 00:15:34 -0400 Subject: [PATCH 6/6] test(api): exercise pagination with real database and MCP sessions --- ...est_mcp_tools_manage_service_pagination.py | 128 +++++++++--------- 1 file changed, 66 insertions(+), 62 deletions(-) diff --git a/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py b/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py index 60a36e88a8b760..844ecad6002316 100644 --- a/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py +++ b/api/tests/unit_tests/services/tools/test_mcp_tools_manage_service_pagination.py @@ -2,65 +2,59 @@ import json from datetime import datetime -from unittest.mock import Mock, call +from uuid import uuid4 import pytest +from sqlalchemy import event +from sqlalchemy.orm import Session from core.entities.mcp_provider import MCPProviderEntity from core.mcp.auth_client import MCPClientWithAuthRetry -from core.mcp.error import MCPConnectionError -from core.mcp.types import ListToolsResult, Tool +from core.mcp.types import ErrorData, ListToolsResult, Tool +from models.account import Account from models.tools import MCPToolProvider from services.tools.mcp_tools_manage_service import MCPToolManageService +from tests.unit_tests.core.mcp.test_mcp_tool_pagination import client_for_pages @pytest.fixture -def provider() -> MCPToolProvider: +def provider(sqlite_session: Session) -> MCPToolProvider: + owner = Account(name="MCP owner", email="mcp-owner@example.com") + sqlite_session.add(owner) provider = MCPToolProvider( name="Test server", server_identifier="test-server", server_url="https://example.com/mcp", server_url_hash="hash", icon="", - tenant_id="tenant-id", - user_id="user-id", + tenant_id=str(uuid4()), + user_id=owner.id, authed=True, tools=json.dumps([{"name": "saved-tool", "inputSchema": {}}]), ) provider.created_at = datetime(2024, 1, 1) provider.updated_at = datetime(2024, 1, 1) + sqlite_session.add(provider) + sqlite_session.flush() return provider @pytest.fixture -def db_session(provider: MCPToolProvider) -> Mock: - return Mock(scalar=Mock(return_value=provider)) +def flushes(sqlite_session: Session, provider: MCPToolProvider) -> list[None]: + assert provider in sqlite_session + flushes: list[None] = [] + event.listen(sqlite_session, "after_flush", lambda *_args: flushes.append(None)) + return flushes @pytest.fixture -def remote_session() -> Mock: - return Mock() - - -@pytest.fixture -def build_response() -> Mock: - return Mock() - - -@pytest.fixture -def service( - db_session: Mock, remote_session: Mock, build_response: Mock, monkeypatch: pytest.MonkeyPatch -) -> MCPToolManageService: - service = MCPToolManageService(db_session) +def client(monkeypatch: pytest.MonkeyPatch) -> MCPClientWithAuthRetry: client = MCPClientWithAuthRetry(server_url="https://example.com/mcp") - client._session = remote_session - # Keep the real auth wrapper and pagination loop; only transport setup is stubbed. - monkeypatch.setattr(client, "_initialize", Mock()) - monkeypatch.setattr("services.tools.mcp_tools_manage_service.MCPClientWithAuthRetry", Mock(return_value=client)) - monkeypatch.setattr(MCPProviderEntity, "decrypt_server_url", Mock(return_value="https://example.com/mcp")) - monkeypatch.setattr(service, "_prepare_auth_headers", Mock(return_value={})) - monkeypatch.setattr(service, "_build_tool_provider_response", build_response) - return service + # Keep the real auth wrapper and session; in-memory JSON-RPC replaces transport setup. + monkeypatch.setattr(client, "_initialize", lambda: None) + monkeypatch.setattr("services.tools.mcp_tools_manage_service.MCPClientWithAuthRetry", lambda **_kwargs: client) + monkeypatch.setattr(MCPProviderEntity, "decrypt_server_url", lambda _self: "https://example.com/mcp") + return client def _page(name: str, cursor: str | None) -> ListToolsResult: @@ -69,23 +63,23 @@ def _page(name: str, cursor: str | None) -> ListToolsResult: @pytest.mark.parametrize("cursor", ["next", ""]) def test_list_provider_tools_saves_all_pages( - service: MCPToolManageService, + sqlite_session: Session, provider: MCPToolProvider, - db_session: Mock, - remote_session: Mock, - build_response: Mock, + flushes: list[None], + client: MCPClientWithAuthRetry, cursor: str, ) -> None: - remote_session.list_tools.side_effect = [_page("alpha", cursor), _page("beta", None)] - - result = service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + service = MCPToolManageService(sqlite_session) + with client_for_pages({None: _page("alpha", cursor), cursor: _page("beta", None)}, client) as (_, requests): + service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) - assert result is build_response.return_value assert [tool["name"] for tool in json.loads(provider.tools)] == ["alpha", "beta"] assert all(tool["description"] == "" for tool in json.loads(provider.tools)) assert provider.authed is True - db_session.flush.assert_called_once_with() - assert remote_session.list_tools.call_args_list == [call(), call(cursor=cursor)] + assert len(flushes) == 1 + assert [request.params.cursor if request.params is not None else None for request in requests] == [None, cursor] + sqlite_session.expire(provider) + assert [tool["name"] for tool in json.loads(provider.tools)] == ["alpha", "beta"] @pytest.mark.parametrize( @@ -94,43 +88,53 @@ def test_list_provider_tools_saves_all_pages( ids=["repeated", "cycle", "empty-repeated"], ) def test_list_provider_tools_preserves_saved_tools_on_cursor_cycle( - service: MCPToolManageService, + sqlite_session: Session, provider: MCPToolProvider, - db_session: Mock, - remote_session: Mock, - build_response: Mock, + flushes: list[None], + client: MCPClientWithAuthRetry, cursors: list[str], ) -> None: original_tools, original_updated_at = provider.tools, provider.updated_at - remote_session.list_tools.side_effect = [_page(f"tool-{index}", cursor) for index, cursor in enumerate(cursors)] - - with pytest.raises(ValueError, match="Failed to connect to MCP server: .*repeated pagination cursor"): - service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + pages = { + None if index == 0 else cursors[index - 1]: _page(f"tool-{index}", cursor) + for index, cursor in enumerate(cursors) + } + service = MCPToolManageService(sqlite_session) + with client_for_pages(pages, client) as (_, requests): + with pytest.raises(ValueError, match="Failed to connect to MCP server: .*repeated pagination cursor"): + service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) assert provider.tools == original_tools assert provider.updated_at == original_updated_at assert provider.authed is True - db_session.flush.assert_not_called() - build_response.assert_not_called() - assert remote_session.list_tools.call_args_list == [call(), *[call(cursor=cursor) for cursor in cursors[:-1]]] + assert not flushes + assert [request.params.cursor if request.params is not None else None for request in requests] == [ + None, + *cursors[:-1], + ] + sqlite_session.expire(provider) + assert provider.tools == original_tools + assert provider.updated_at == original_updated_at def test_list_provider_tools_preserves_saved_tools_on_later_page_error( - service: MCPToolManageService, + sqlite_session: Session, provider: MCPToolProvider, - db_session: Mock, - remote_session: Mock, - build_response: Mock, + flushes: list[None], + client: MCPClientWithAuthRetry, ) -> None: original_tools, original_updated_at = provider.tools, provider.updated_at - remote_session.list_tools.side_effect = [_page("alpha", "next"), MCPConnectionError("later page failed")] - - with pytest.raises(ValueError, match="Failed to connect to MCP server: later page failed"): - service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) + pages = {None: _page("alpha", "next"), "next": ErrorData(code=500, message="later page failed")} + service = MCPToolManageService(sqlite_session) + with client_for_pages(pages, client) as (_, requests): + with pytest.raises(ValueError, match="Failed to connect to MCP server: .*later page failed"): + service.list_provider_tools(tenant_id=provider.tenant_id, provider_id=provider.id) assert provider.tools == original_tools assert provider.updated_at == original_updated_at assert provider.authed is True - db_session.flush.assert_not_called() - build_response.assert_not_called() - assert remote_session.list_tools.call_args_list == [call(), call(cursor="next")] + assert not flushes + assert [request.params.cursor if request.params is not None else None for request in requests] == [None, "next"] + sqlite_session.expire(provider) + assert provider.tools == original_tools + assert provider.updated_at == original_updated_at