MM-69467: Add page/per_page pagination to read_channel and read_post agent tools#856
MM-69467: Add page/per_page pagination to read_channel and read_post agent tools#856cursor[bot] wants to merge 7 commits into
Conversation
…tools The read_channel and read_post agent tools were limited to 100 posts per call with no way to paginate. Add page and per_page parameters so callers can retrieve more than 100 results or page through larger result sets. read_channel now forwards page/per_page to GetPostsForChannel (max 200, keeping the deprecated limit as an alias) and surfaces a next-page hint. read_post paginates large threads in memory and continues post numbering across pages. Co-authored-by: mattermost-code <[email protected]>
Seed >100 posts and a multi-post thread against the real Mattermost test server and verify read_channel returns 120 posts in one call (beyond the old 100 cap) with working page navigation, and read_post pages through a thread. Co-authored-by: mattermost-code <[email protected]>
Add coverage for the read_post per_page cap, negative page clamping in both tools, the empty later-page branch in read_channel, and tighten the next-page hint and post-count assertions. Co-authored-by: mattermost-code <[email protected]>
Document that since filters within the fetched page (reducing the shown count) while the next-page hint reflects the number of posts the server returned, not the filtered count. Co-authored-by: mattermost-code <[email protected]>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
🤖 LLM Evaluation ResultsOpenAI
❌ Failed EvaluationsShow 7 failuresOPENAI1. TestReactEval/[openai]_react_cat_message
2. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[openai]_bot_dm_tool_introspection
Anthropic
❌ Failed EvaluationsShow 7 failuresANTHROPIC1. TestReactEval/[anthropic]_react_cat_message
2. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[anthropic]_bot_dm_tool_introspection
This comment was automatically generated by the eval CI pipeline. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds ChangesPagination for
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcpserver/tools/posts.go`:
- Around line 178-202: The pagination logic multiplies `page * perPage` to
calculate `pageStart` without validating that `page` is within a reasonable
upper bound, which can cause integer overflow when `page` is very large (e.g.,
math.MaxInt64), resulting in a negative value that bypasses the bounds check and
causes a panic on slice indexing. Before computing `pageStart`, add validation
to ensure `page` is less than the total number of pages, calculated as
(totalThreadPosts + perPage - 1) / perPage, and return an appropriate
out-of-range error message if the page exceeds this limit. Additionally, add a
regression test case that verifies the pagination logic handles `page =
math.MaxInt64` with `per_page > 0` correctly by returning the out-of-range
message without panicking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 95bc7985-01ec-4d52-8177-6aa3414fa40f
📒 Files selected for processing (6)
mcpserver/README.mdmcpserver/tools/channels.gomcpserver/tools/channels_test.gomcpserver/tools/posts.gomcpserver/tools/posts_test.gomcpserver/tools_integration_test.go
Validate page against total pages before multiplying by per_page to avoid integer overflow panics on extreme page values. Co-authored-by: mattermost-code <[email protected]>
|
Creating a Plugin SpinWick test server |
There was a problem hiding this comment.
Stale comment
Babysit complete — ready for human review
CodeRabbit approved commit
537d5609after the page-overflow guard fix.CI note: flaky
e2e-tool-calling-anthropicfailure (unrelated)The
e2e-tool-calling-anthropicjob failed on the post-fix CI run with a timeout inmultiplayer-tool-calling.spec.ts("Accept tool calls then Keep Private"). This is not related to theread_channel/read_postpagination changes — the same job passed on the initial CI run (28709e09), and the failure is a timing flake in the multiplayer tool-approval UI flow.All other checks are green (lint, plugin-tests, build, e2e shards, other real-API e2e jobs).
Sent by Cursor Automation: Matty Code (Babysit Until Human Review)
|
Plugin Spinwick PR #856 🎉 Test server created! Access here: https://agents-pr-856-qow94.test.mattermost.cloud
Installation ID: Credentials: Posted securely in this Mattermost channel - Look for PR #856 |
nickmisasi
left a comment
There was a problem hiding this comment.
Could we extract some of the pagination logic (in particular things around <=0, setting the default limit per entity) etc - to a helper within the tools package? That way future tools that support pagination could use it too.
|
Looking! Follow along here: https://cursor.com/agents/bc-91fcec2e-f586-4ce5-bc74-ccc7b60c559a |
PR Feedback Resolution SummaryQuestions Answered (0 items): Code Changes Applied (1 item):
Declined Changes (0 items): Needs Human Review (0 items): Verification:
|
|
/smart-cherry-pick release-2.0 |
|
@amyblais please ignore you being added here - was testing smart cherry pick. |
Thanks! |
|
@crspeller — friendly nudge: this PR has been waiting on your review for a couple of days and CodeRabbit has already signed off. Could you take a look when you get a chance? 🙏 |
Reconcile read_channel/read_post pagination with master refactors (typed resolvers, *bool include_thread, renderMembers helper) and fix unit tests to call tool handlers with struct args. Co-authored-by: mattermost-code <[email protected]>
|
New commit detected. SpinWick will update the plugin if a new artifact is available. |
|
Plugin test server update attempted, but encountered an issue: The test server is still available. You can manually download and install the updated plugin using the artifact link below. Updated with git commit
|
There was a problem hiding this comment.
Babysit complete — ready for human review
All acceptance criteria met on commit 67fc44ac.
Changes in this babysit run
- Merge conflicts resolved with
master(typed resolvers,*boolinclude_thread,renderMembershelper) while preservingread_channel/read_postpagination. - Unit tests fixed to call tool handlers with struct args instead of legacy
argsGettercallbacks.
Review feedback
- CodeRabbit (page overflow): already addressed in
537d5609; thread resolved. Latest CodeRabbit status check passed on67fc44ac. - nickmisasi (extract pagination helpers): addressed in
9a352a56via sharedmcpserver/tools/pagination.goused by read_channel, read_post, get_channel_members, get_user_channels, and get_team_members.
CI
All checks green on 67fc44ac (lint, plugin-tests, build, e2e shards, evals, security analyzers).
Handing off for human review.
Sent by Cursor Automation: Matty Code (Babysit Until Human Review)
|
@crspeller — friendly nudge: this PR has been waiting on your review for a couple of days and CodeRabbit has already signed off. Could you take a look when you get a chance? 🙏 |
|
@crspeller — friendly nudge: this PR has been waiting on your review for a couple of days and CodeRabbit has already signed off. Could you take a look when you get a chance? 🙏 |
2 similar comments
|
@crspeller — friendly nudge: this PR has been waiting on your review for a couple of days and CodeRabbit has already signed off. Could you take a look when you get a chance? 🙏 |
|
@crspeller — friendly nudge: this PR has been waiting on your review for a couple of days and CodeRabbit has already signed off. Could you take a look when you get a chance? 🙏 |



Summary
The
read_channelandread_postagent (MCP) tools were limited to 100 posts per call with no way to paginate, so an agent asked for "the last 500 threads" could only report it can pull up 100 recent posts per call. This addspageandper_pageparameters to both tools so callers can retrieve more than 100 results or page through larger result sets.read_channel(mcpserver/tools/channels.go): now forwardspage/per_pagetoGetPostsForChannel(server-side pagination), raises the per-page cap from 100 to 200 (consistent with the other paginated channel tools), keeps the oldlimitas a deprecated alias forper_page(backward compatible), and appends a "More posts available — call read_channel again with page=N" hint when a full page is returned.read_post(mcpserver/tools/posts.go): paginates large threads viapage/per_page(the whole thread is still returned when neither is provided, preserving existing behavior), continues post numbering across pages, and appends a "More posts in this thread" hint when more pages exist.mcpserver/README.md.QA test steps
read_channel/read_posttools and a channel with more than 100 posts.page.page=1,page=2, … and surfaces the next-page hint until the channel is exhausted.Testing / evidence
No LLM provider credentials are available in this environment, so the user-facing agent conversation could not be driven in a browser. The bug and fix live at the MCP tool layer, which is verified end-to-end against a real Mattermost server (Testcontainers) through the real MCP protocol:
TestMCPToolsIntegration/ReadChannelTool/PaginationBeyond100: seeds 130 posts; page 0 withper_page=120returnsFound 120 posts (page 0)(impossible under the old 100 cap) plusMore posts available — call read_channel again with page=1 to retrieve the next 120.; page 1 returns the remainder with no hint, and the oldest post is reachable on the later page.TestMCPToolsIntegration/ReadPostTool/ThreadPagination: pages through a 10-post thread (page 0, showing 4→page 2, showing 2).httptest.Server+ realClient4) cover param forwarding, thelimitalias, the 200 cap on both tools, negative-page clamping, the empty later-page message, the next-page hint contract, and thesince+ pagination interaction.Ticket Link
Jira https://mattermost.atlassian.net/browse/MM-69467
Release Note
Summary by CodeRabbit
Release Notes
New Features
page+per_pagepagination toread_channeland in-threadread_postwhen enabled, including next-page “more posts” hints.per_pagecap of 200 and support for the deprecatedlimitalias.Bug Fixes
sincefiltering behaviour) for page-based results.pagevalues are now clamped to 0.Tests
Documentation
per_pageand the deprecatedlimit.