Skip to content

fix: Pass cursor parameter to server #745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nbarbettini
Copy link
Contributor

Motivation and Context

Fixes #735 by:

  • Updating request params to correctly follow the pagination spec
  • Updated tests that assert the payload sent to the server conforms to the spec (regardless of whether the server supports it)

See also: PR comments inline
I'm happy to update the approach if it doesn't fit the style of this repo.

How Has This Been Tested?

  • Updated unit tests
  • Manual verification

Breaking Changes

None - internal changes only, no client interface changes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Comment on lines -82 to -87
class PaginatedRequest(Request[RequestParamsT, MethodT]):
cursor: Cursor | None = None
"""
An opaque token representing the current pagination position.
If provided, the server should return results starting after this cursor.
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this, since it was only used for the methods in question.

Comment on lines +88 to +90
@pytest.fixture
def stream_spy():
"""Fixture that provides spies for both client and server write streams.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built this as a fixture so that it can be easily reused in multiple tests.

I needed a way to verify the request payload "on the wire" and this seemed like a good approach, since it did not require any modification of the actual client code (only test fixtures).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursors are not sent to the server correctly
2 participants