feat: implement auto-pagination for MCP server list operations and address review feedback - #4086
feat: implement auto-pagination for MCP server list operations and address review feedback#4086akshay183 wants to merge 2 commits into
Conversation
- Added auto-pagination logic for `list_tools`, `list_prompts`, `list_resources`, and `list_resource_templates` in `MCPServer`. - Implemented an infinite loop guard that prevents duplicate fetching if the server repeatedly returns the same cursor. - Updated existing tests and added dedicated `test_pagination.py` to cover full page accumulation and loop breaking. - Kept all logic backward compatible. Co-authored-by: akshay183 <68906315+akshay183@users.noreply.github.com>
ffa9c24 to
090c337
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffa9c24a7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4936c7c3a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4936c7c to
e6a32e8
Compare
akshay183
changed the title
|
@codex review |
|
Note You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a32e8db1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| async def fetch_tools_page(page_cursor: str | None = _cursor) -> Any: | ||
| return await self._maybe_serialize_request( | ||
| lambda: session.list_tools(cursor=page_cursor) |
There was a problem hiding this comment.
This new cursor keyword needs corresponding updates to the repo's existing session doubles: DummySession.list_tools(self) in tests/mcp/test_client_session_retries.py has no cursor parameter, and test_list_tools_unlimited_retries configures max_retry_attempts=-1, so the resulting deterministic TypeError is retried forever instead of completing. The same signature mismatch exists for the prompt doubles reached by the new prompt pagination path, so update those fakes/adapters to accept cursor before merging.
Useful? React with 👍 / 👎.
| lambda: session.list_tools(cursor=page_cursor) | ||
| ) | ||
|
|
||
| result = await self._run_with_retries(fetch_tools_page) |
There was a problem hiding this comment.
Because _run_with_retries is now invoked once per fetched page, a multi-page list_tools() call receives a fresh max_retry_attempts allowance for every cursor. For servers that expose many pages and hit transient failures, max_retry_attempts=3 can turn into three retries per page rather than for the SDK-level list operation, substantially exceeding the configured retry/latency budget; keep the retry counter across the whole pagination loop or otherwise cap the aggregate attempt count.
Useful? React with 👍 / 👎.
|
@akshay183 Thank you so much for reporting and trying to resolve this issue. I've created a PR including your co-autuhored-by credit at #4094 ; the PR doesn't have any remaining review feedback to resolve, so we'll go with the PR instead. Thanks again fro taking the time for this. |
Great! |
2 participants
This pull request implements automatic pagination for MCP server list operations (
list_toolsandlist_prompts) insrc/agents/mcp/server.pyand resolves review feedback regarding cursor evaluation and log redaction.#4085
Background & Context
When interacting with MCP servers that return paginated tool or prompt lists, the SDK previously fetched only the first page. For large servers, subsequent tools and prompts were omitted. Additionally, review feedback identified two key considerations in the pagination loop:
"") were improperly treated as exhaustion when evaluated with truthiness (if not next_cursor:).cursorandnext_cursorvalues were written tologger.warning(...)calls during loop detection, potentially exposing opaque server tokens or tenant state in logs.Key Changes
list_toolsandlist_promptsinsrc/agents/mcp/server.pyto transparently accumulate all pages untilnextCursorisNone.NoneExhaustion Check: Replaced truthiness checks withnext_cursor is Noneandif next_cursor is not None and next_cursor in seen_cursors:to safely support empty string cursor tokens ("").cursor/next_cursorpayload values fromlogger.warning(...)messages in bothlist_toolsandlist_prompts.tests/mcp/test_pagination.pyverifying multi-page tool/prompt accumulation, metadata merging, repeated-cursor loop termination, and empty string cursor handling.local_mcp_pagination_probe.py) to verify real Streamable HTTP transport pagination.