fix: align pagination with API semantics and make it configurable#279
Merged
Conversation
The CLI pagination loop added in #275 was 0-indexed but the API treats page=0 (or null) as 'return everything in one response' and uses 1-indexed pages otherwise. The loop happened to work against real prod only because the first request asked for 'all' and immediately exited the loop. The mock server was 0-indexed and so diverged from the real API's semantics. Changes: - Add four hidden global flags for paginated list commands: --page-size, --page, --no-paginate, --max-items. Modeled on the AWS CLI's pagination knobs but matched to our page-number API (--starting-token would be cursor-shaped, which our API does not use). - Thread page_size / paginate / page / max_items through Config. - Fix fetch_all_pages: page=0 means 'all in one response and stop'; page>=1 iterates 1..=num_pages. - Align the mock /v1/apps/etc. pagination to the same semantics so the mock and prod behave the same way. - Rewrite cli_pagination.feature to create apps via the real CLI and delete them in after_scenario. With --page 1 --page-size 2 against three apps, the test forces a real two-page traversal against any backend. - Drop the mock-only /test/seed-apps and /test/reset endpoints that the old pagination feature relied on.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bradhe
approved these changes
May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The CLI pagination loop added in #275 was 0-indexed, but the real API treats
page=0(or null) as "return everything in one response" and uses 1-indexed pages otherwise. The loop happened to work against prod only because the first request asked for "all" and immediately exited the loop after one iteration. The mock server, meanwhile, was 0-indexed and diverged from the real API.This PR:
--starting-tokensince our API doesn't expose cursors):--page-size N— page size to request from the server--page N— page to start at;0means "no pagination, return all in one response"--no-paginate— fetch only the first response--max-items N— hard ceiling on total items returnedpage_size/paginate/page/max_itemsthroughConfigso the flags are honored by everyapi::list_*call (CLI and MCP tools alike).fetch_all_pagesto match real API semantics:page=0returns everything in one shot and stops;page>=1iterates1..=num_pages.paginate()helper to the same semantics so behavior is identical against mock and prod.cli_pagination.featureto create apps via the real CLI (and delete them inafter_scenario) so the test exercises real pagination against any backend. With--page 1 --page-size 2against three apps, the loop genuinely walks two pages./test/seed-appsand/test/resetendpoints that the previous pagination test relied on — they only worked against the mock and would silently fail against prod.Test plan
cargo buildclean./tests/integration/run_tests.py— all 43 scenarios passtower --page-size 5 apps listagainst staging shows pages-of-five behaviortower --no-paginate apps listreturns only the first responsetower --max-items 3 apps listcaps the output at 3