Repository navigation
fix(mcp): follow nextCursor in prompts/list, resources/list and resources/templates/list - #859
Conversation
gloryfromca
left a comment
There was a problem hiding this comment.
Taking this one over #876, which fixes the same thing. Both follow nextCursor on the three verbs, both are CI-green; the difference is where the walk lives.
- Here: one
raven/mcp/paging.pyholding_walkplus four typed walkers, and thetools/listloop from #825 moved onto it, soclient.pyloses its private copy. One implementation, four call sites. - #876:
_walk_paginated_listcopied verbatim intoprompts.pyandresources.py,client.pyuntouched. Three copies of the same walk.
#855's proposed fix asked for the first shape in as many words: "One shared helper in raven/mcp/", and "move the tools/list loop from #825 onto it, so all four verbs share one implementation."
Against the issue's acceptance criteria:
| Criterion | Where it is met |
|---|---|
| Single-page servers behave exactly as before | the existing _Session fakes are unchanged apart from accepting params=None, and still pass |
| Multi-page prompt / resource / template lists return every item | test_a_paging_server_hands_over_every_page in both test files, plus the templates variant |
| A repeated cursor ends the walk | test_a_repeated_cursor_ends_the_walk |
tools/list keeps its behaviour after the move |
tests/test_mcp_client.py:143-157 already covers the walk and the repeated-cursor stop, and now exercises walk_tools |
No deprecated cursor= overload |
params=types.PaginatedRequestParams(cursor=cursor) in _walk |
One criterion the issue listed that is worth naming explicitly, because it is easy to get wrong by wrapping one page instead of the walk: the 30s gate stays around the whole thing, _with_timeout(walk_prompts(session)). A server that never stops paging fails visibly rather than stalling the turn.
The _PagingSession fakes answer by the cursor they were asked for and carry assert len(self.cursors) <= len(self._pages), so a walk that lost its stop rule fails loudly instead of looping. That is the part that makes these tests able to fail.
Not a blocker, just naming the trade: _walk is list[Any] inside and the four public walkers put the real SDK type back on at the boundary. For a page shape that differs only in which attribute carries the items, that is the right place to lose and regain the type.
One disclosure so this is not read as more coverage than it is: mrbot has not reviewed this PR. Its GitHub watch is scoped to MEMBER and OWNER authors, so no automated pass ran on any of the outside-contributor PRs. This is a read of the diff, not a second opinion on top of one.
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: The new paging module must remove its issue-number references to satisfy AGENTS.md section 1.1.
I found one repository-rule blocker; the runtime behavior itself looks correct. I covered the complete github/main...HEAD diff, the three model-facing callers and the existing tools/list connection path through their returned values, the #825 implementation history and #855 acceptance criteria, all live PR review/comment channels, AGENTS.md / CLAUDE.md / CONTEXT-MAP.md, compatibility with the pinned MCP SDK, current-main path overlap and merge-tree, and whether existing tests or assertions were weakened (none were).
Verification:
uv run --frozen --all-extras pytest tests/test_mcp_resources.py tests/test_mcp_prompts.py tests/test_mcp_client.py tests/test_mcp_manager.py -q-> 88 passed. The first default-extra run executed no test bodies and produced 88 setup errors because the fresh worktree lacked theraven_everosdev plugin; the all-extras rerun installed the repository-declared dev plugins and passed.- Ruff check and format check on all six changed files -> passed.
make lint-types-> passed.uv run --frozen --all-extras lint-imports-> 10 contracts kept, 0 broken.- Commit-message validation and the source-language check against
github/main..HEAD-> passed. - Direct SDK inspection confirmed all four methods accept
params=PaginatedRequestParams(...)and expose the expected item field plusnextCursor; a direct two-cursor-cycle probe terminated after[None, A, B]. git diff --checkandgit merge-tree --write-tree HEAD github/main-> clean; currentmainhas no overlapping changes in these six files.- All current GitHub check runs for this head are completed and green or intentionally skipped.
AGENTS.md section 1.1 keeps source comments free of PR and issue references. The docstring keeps its behavioral explanation and loses the ticket numbers, so the module documents the durable pagination constraint without transient issue context.
90a71cd to
66af53d
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full diff and affected callers against AGENTS.md and the context map, including history, backward compatibility, timeout and error behavior, repeated-cursor termination, architecture boundaries, and whether tests were weakened. The shared walker preserves the existing tools/list behavior and correctly follows nextCursor for prompts, resources, and templates. The SDK call shapes and result fields match mcp 1.30.0.
Verification: uv run pytest tests/test_mcp_resources.py tests/test_mcp_prompts.py tests/test_mcp_client.py tests/test_mcp_manager.py -q passed with 88 tests; targeted Ruff, Ruff format, ty, source-language, commit-message, and diff-whitespace checks also passed.
Summary
raven/mcp/paging.pywalks a paginated list verb page by page withparams=PaginatedRequestParams(cursor=...), stopping on an absent or a repeated cursor (the rule fix(mcp): page through tools/list nextCursor #825 introduced fortools/list). The public surface is one typed walker per verb (walk_tools / walk_resources / walk_resource_templates / walk_prompts), so the items a caller loops over carry their real SDK type rather than the Any a session yields at these call sites; the page shape stays behind one private engine.resources/listandresources/templates/listinraven/mcp/resources.py, andprompts/listinraven/mcp/prompts.py. Each site keeps its existing_with_timeoutaround the whole walk, so a server that never stops paging fails visibly instead of stalling the turn.tools/listloop from fix(mcp): page through tools/list nextCursor #825 inraven/mcp/client.pycollapses onto the same helper, so all four verbs share one implementation.paramsargument the walker always passes.Type
Verification
uv run pytest tests/test_mcp_resources.py tests/test_mcp_prompts.py tests/test_mcp_client.py tests/test_mcp_manager.py-> 88 passed, including four new tests (multi-page resources, multi-page templates, repeated cursor, multi-page prompts)raven/changes and running them -> 4 failed; restoring the fix -> passruff check+ruff format --checkon the touched files;ty checkat the pre-change baseline (18 platform-only diagnostics on Windows, no new);lint-imports-> 10 contracts kept, 0 brokenRisk
Related Issues
Fixes #855