[codex] Follow up pagination fixes on top of #2#3
Merged
Conversation
All list methods (ProductsClient.list, WorkflowsClient.listForProduct, BuildsClient.listForWorkflow) previously returned only the first page of results, silently discarding subsequent pages. With 100+ builds in a workflow, this meant buildSelector=latest returned stale data. Changes: - Add BaseAPIClient.listAll() method that follows links.next URLs to collect all pages of paginated responses - Update BuildsClient.listForWorkflow to use listAll with limit=200 - Update ProductsClient.list to use listAll with limit=200 - Update WorkflowsClient.listForProduct to use listAll with limit=200 - Remove unused limit parameter from client list methods (MCP tools now paginate automatically) - Update build-locator.ts to remove hardcoded limit=100 - Add pagination tests for listAll (single page, multi-page, empty) - All 25 tests pass Fixes the issue where buildSelector=latest would return build #186 instead of the actual latest build #228+.
Addresses Codex review comment on PR #2: without a limit parameter, list_build_runs fetches all paginated build runs for a workflow, which can be slow and exceed response-size limits for workflows with hundreds of runs. Changes: - Add maxItems parameter to BaseAPIClient.listAll() to stop pagination early once enough items are collected - Restore limit parameter to BuildsClient.listForWorkflow() - Add limit parameter to list_build_runs MCP tool (default: 20, max: 500) - Use limit=50 in build-locator for latest/latestFailing lookups - Add 2 new pagination tests for maxItems behavior - All 27 tests pass
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.
Context
This branch is based on the current head of #2 (
fix: add pagination support for App Store Connect API list endpoints).That means this PR includes:
Why a follow-up PR
The original PR fixes a real correctness bug by following paginated
links.nextresponses from App Store Connect.A later revision also restored a
limitforlist_build_runs, which was the right direction, but it introduced two remaining issues:resolveBuildLocatorused a small fixed cap for all lookup modes, which could fail to find older build numbers or the latest failing build if they were outside the newest windowlistAll()still trusted arbitrarylinks.nextURLs and would send the App Store Connect bearer token to themWhat changed in this PR
Builds listing remains bounded
list_build_runsstill supports an optionallimitand defaults to a bounded recent listing.Build locator lookups are now selective instead of bluntly capped
Instead of using the same small fixed limit for every lookup path:
latestfetches only the most recent build runbuildNumberpaginates until the requested build number is foundlatestFailingpaginates until a failing run is foundThese targeted lookups keep the public listing tool bounded without breaking older-history resolution.
Pagination links are origin-checked
Before following
links.next, the client now verifies that the next URL stays on the App Store Connect origin. This prevents forwarding the bearer token to an unexpected host.Tool/behavior impact
This keeps the intended behavior split clear:
Validation
npm testnpm run build