fix: guard client pagination loops against misbehaving servers#3167
fix: guard client pagination loops against misbehaving servers#3167
Conversation
Treat empty/falsy nextCursor as end-of-pagination and detect cursor cycles across all client list methods and the server context proxy helper.
WalkthroughThis pull request adds pagination safety mechanisms across multiple list methods in both the client and server by introducing duplicate cursor detection to prevent infinite pagination loops. The changes introduce a Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8b2d91de0
ℹ️ 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".
The auto-pagination loops in
Client.list_tools(),list_resources(),list_resource_templates(), andlist_prompts()had no protection against servers that return bogusnextCursorvalues. A server that always returns a non-null cursor (or cycles through cursors, or returns empty strings) would cause the client to loop forever — appearing to "hang" after connecting.This adds two guards to every client list method and the server-side context proxy helper:
if not result.nextCursorinstead ofis None, so empty strings and other falsy values are treated as end-of-pagination.Fixes #3158