Conversation
Server authors opt-in by setting list_page_size on FastMCP. Client convenience methods auto-fetch all pages transparently. Use _mcp methods with cursor parameter for manual pagination.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96d4b1b606
ℹ️ 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".
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request adds cursor-based pagination across FastMCP client and server list operations. It introduces pagination utilities (base64-encoded opaque cursors), a configurable Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fastmcp/server/context.py (1)
343-361: Context.list_resources() and list_prompts() return incomplete results when pagination is enabled.These methods call the paginated MCP handlers but don't loop through pages. When
list_page_sizeis configured, they return only the first page. The docstrings claim "List all available resources/prompts," which is misleading—they should either fetch all pages like the Client methods do, or update the docstrings to clarify they return only the first page.This has real impact: the tool injection middleware in
src/fastmcp/server/middleware/tool_injection.py(lines 56, 90) uses these methods to create tools that get injected into the server. When pagination is enabled, users would get incomplete lists from those tools.Consider either:
- Loop through all pages to match Client behavior and docstring promises
- Update docstrings to clarify pagination behavior
- Add pagination tests for Context methods to prevent regression
🧹 Nitpick comments (1)
docs/servers/pagination.mdx (1)
41-66: Make the client examples runnable with an async entrypoint and basic error handling.These snippets rely on an implicit async context and omit error handling; consider adding an entrypoint and a minimal try/except. Apply a similar pattern to the manual pagination block.
♻️ Proposed update (apply similarly to the manual pagination block)
-from fastmcp import Client - -async with Client(server) as client: - # Returns all 200 tools, fetching pages automatically - tools = await client.list_tools() - print(f"Total tools: {len(tools)}") # 200 +import asyncio +from fastmcp import Client + +async def main() -> None: + async with Client(server) as client: + try: + # Returns all 200 tools, fetching pages automatically + tools = await client.list_tools() + except Exception as exc: + print(f"Failed to list tools: {exc}") + return + print(f"Total tools: {len(tools)}") # Expected: 200 + +if __name__ == "__main__": + asyncio.run(main())Please run the updated snippet to confirm the expected output. As per coding guidelines, examples should be runnable and include error handling.
Closes #540, closes #211, closes #1465
Adds pagination support for
tools/list,resources/list,resources/templates/list, andprompts/listoperations per the MCP spec.Server: Opt-in by setting
list_page_size:Results are chunked with opaque cursors. When not configured, behavior is unchanged (all results in one response).
Client: Convenience methods auto-fetch all pages transparently: