Fix/pagination#275
Merged
Merged
Conversation
All list commands (apps, teams, secrets, catalogs, environments, schedules) only fetched the first page from the API, silently truncating results. Added a PaginatedResponse trait and generic fetch_all_pages helper that iterates through all pages. Both CLI and MCP tools benefit since they share the same api.rs layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Mock server now respects page/page_size query params on all list endpoints - Added paginate() helper and mock_max_page_size override for testing - Added /test/seed-apps and /test/reset endpoints for test data setup - Added cli_pagination.feature with scenarios verifying multi-page fetching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The mock server re-adds predeployed-test-app on reset, so JSON mode returns 26 (25 seeded + 1 pre-existing). Changed assertion to "at least 25". Also pinned mock server to Python <3.14 (pyo3 compat). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…erflow Instead of capping page_size server-side (shared mutable state across parallel workers), seed 105 apps so pagination naturally exceeds the CLI's page_size=100, producing 2 pages without any global state changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Include truncated response content in error messages to help debug CI-only deserialization failures. Remove unused debug step from tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This will reveal the exact field/type mismatch causing CI pagination test failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The real Tower API does not include run_results in list app responses. Including it in the mock caused deserialization failures on CI where a cached wheel expected a newer RunResults schema with a 'starting' field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bradhe
approved these changes
May 13, 2026
5 tasks
bradhe
added a commit
that referenced
this pull request
May 14, 2026
* fix: align pagination with API semantics and make it configurable 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. * chore: Reformat the things * chore: Clean up old, broken rust that comes with Homebrew * chore: Go the other way with it--clobber the Homebrew rustup * chore: One more attempt to fix this broken rust toolchain in macos15 images * chore: Another attempt at resolving the issue with the upstream --------- Co-authored-by: Brad Heller <brad@tower.dev>
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
2 participants
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.
Current behavior: CLI and MCP only fetch the first list of resources when listing apps, runs, secrets, etc. Which makes some apps undiscoverable through the CLI/MCP
Desired behavior: Paginate through and return all resources for all
listcommands on the CLI and MCP