fix: add pagination support for App Store Connect API list endpoints#2
fix: add pagination support for App Store Connect API list endpoints#2EdouardF wants to merge 2 commits into
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+.
|
Hey @backslash-f can you please check when your time permits? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2743ff405f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await client.builds.listForWorkflow( | ||
| parseIdentifier(workflowId, 'workflow'), | ||
| limit ?? 20, | ||
| ), |
There was a problem hiding this comment.
For long-lived workflows, this now fetches every paginated build run and then returns every matching item from list_build_runs; before this path passed limit ?? 20, and the removed schema field leaves callers with no way to keep the MCP response bounded. A workflow with hundreds or thousands of historical runs can make routine list_build_runs calls slow or exceed response-size limits even when the caller only wants recent builds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Makes sense. I guess this is debatable. @backslash-f Any take here?
Addresses Codex review comment on PR thatfactory#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
|
Closing this in favor of #3. #3 is based on the current head of this PR, so it includes the pagination fix proposed here and adds the follow-up changes needed to keep the MCP tool behavior aligned with the original goals:
That makes #3 the better review target for the complete fix set. |
[codex] Follow up pagination fixes on top of #2
2 participants
Problem
All list methods (
ProductsClient.list,WorkflowsClient.listForProduct,BuildsClient.listForWorkflow) returned only the first page of results, silently discarding subsequent pages.When a workflow has more builds than the default page limit (typically 100),
buildSelector=latestwould return stale data — e.g., returning build #186 instead of the actual latest build #228+.The Apple App Store Connect API paginates responses using
links.nextURLs in the response envelope, but these were never followed.Root Cause
BaseAPIClient.get()returnsAPIResponse<TData>which includeslinks.next, but all callers just extractedresponse.dataand returned it, ignoring pagination entirely.Changes
BaseAPIClient.listAll()— New protected method that followslinks.nextURLs to collect all pages of paginated responsesBuildsClient.listForWorkflow()— Now useslistAll()withlimit=200, removed unusedlimitparameterProductsClient.list()— Now useslistAll()withlimit=200, removed unusedlimitparameterWorkflowsClient.listForProduct()— Now useslistAll()withlimit=200, removed unusedlimitparameterbuild-locator.ts— Removed hardcodedlimit=100sincelistForWorkflownow fetches all pagesdiscovery.ts— Simplified tool schemas (removed manuallimitparams since pagination is automatic)build-runs.ts— Updated to match new client signaturestests/pagination.test.ts— New test suite covering single page, multi-page, empty, and two-page scenariosTesting
All 25 tests pass, including 4 new pagination-specific tests:
Breaking Changes
BuildsClient.listForWorkflow(workflowId, limit?)→listForWorkflow(workflowId)(removedlimitparameter)ProductsClient.list(limit?)→list()(removedlimitparameter)WorkflowsClient.listForProduct(productId, limit?)→listForProduct(productId)(removedlimitparameter)These were internal API changes only — the MCP tool interface remains compatible.