diff --git a/package-lock.json b/package-lock.json index cc695a4..94fdfac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@thatfactory/xcode-cloud-mcp", - "version": "0.2.0", + "version": "0.5.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@thatfactory/xcode-cloud-mcp", - "version": "0.2.0", + "version": "0.5.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.28.0", @@ -924,7 +924,6 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", - "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -1163,7 +1162,6 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.9.tgz", "integrity": "sha512-wy3T8Zm2bsEvxKZM5w21VdHDDcwVS1yUFFY6i8UobSsKfFceT7TOwhbhfKsDyx7tYQlmRM5FLpIuYvNFyjctiA==", "license": "MIT", - "peer": true, "engines": { "node": ">=16.9.0" } @@ -1881,7 +1879,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz", "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/src/api/base-client.ts b/src/api/base-client.ts index f3902ba..a0d2078 100644 --- a/src/api/base-client.ts +++ b/src/api/base-client.ts @@ -36,6 +36,82 @@ export class BaseAPIClient { return this.request(url.toString()); } + /** + * Fetch all pages of a paginated list endpoint. + * Follows `links.next` until no more pages are available. + * If `maxItems` is provided, stops paginating once enough items are collected. + */ + protected async listAll( + path: string, + params?: Record, + maxItems?: number, + ): Promise { + const allData: TData[] = []; + + let response = await this.get(path, params); + if (Array.isArray(response.data)) { + allData.push(...response.data); + } else { + return [response.data]; + } + + while (response.links?.next && (!maxItems || allData.length < maxItems)) { + response = await this.request(this.resolveNextPageUrl(response.links.next)); + if (Array.isArray(response.data)) { + allData.push(...response.data); + } + } + + if (maxItems && allData.length > maxItems) { + return allData.slice(0, maxItems); + } + + return allData; + } + + /** + * Find the first matching item across a paginated list endpoint. + */ + protected async findInList( + path: string, + matches: (item: TData) => boolean, + params?: Record, + maxItems?: number, + ): Promise { + let scannedItems = 0; + let response = await this.get(path, params); + + while (true) { + if (!Array.isArray(response.data)) { + if (!maxItems || scannedItems < maxItems) { + return matches(response.data) ? response.data : undefined; + } + + return undefined; + } + + for (const item of response.data) { + scannedItems += 1; + + if (matches(item)) { + return item; + } + + if (maxItems && scannedItems >= maxItems) { + return undefined; + } + } + + if (!response.links?.next) { + return undefined; + } + + response = await this.request( + this.resolveNextPageUrl(response.links.next), + ); + } + } + protected async patch( path: string, body: TBody, @@ -107,4 +183,17 @@ export class BaseAPIClient { return payload as APIResponse; } + + private resolveNextPageUrl(url: string): string { + const nextUrl = new URL(url); + const baseOrigin = new URL(this.baseUrl).origin; + + if (nextUrl.origin !== baseOrigin) { + throw new Error( + `Unexpected pagination origin: ${nextUrl.origin}. Expected ${baseOrigin}.`, + ); + } + + return nextUrl.toString(); + } } diff --git a/src/api/resources/builds.ts b/src/api/resources/builds.ts index c7c02d9..7ca5134 100644 --- a/src/api/resources/builds.ts +++ b/src/api/resources/builds.ts @@ -5,6 +5,8 @@ import type { CiBuildAction, CiBuildRun } from '../types.js'; * Build run endpoints. */ export class BuildsClient extends BaseAPIClient { + static readonly buildLocatorScanLimit = 2000; + /** * Get a build run by id. */ @@ -14,17 +16,48 @@ export class BuildsClient extends BaseAPIClient { } /** - * List recent build runs for a workflow. + * List build runs for a workflow, paginating through all results. + * Optionally limit the total number of build runs returned. */ async listForWorkflow(workflowId: string, limit?: number): Promise { - const response = await this.get( + return this.listAll( `/v1/ciWorkflows/${workflowId}/buildRuns`, - { - ...(limit ? { limit: String(limit) } : {}), - }, + { limit: '200' }, + limit, ); + } - return response.data; + /** + * Find a build run with a specific build number for a workflow. + */ + async findByNumberForWorkflow( + workflowId: string, + buildNumber: number, + maxItems: number = BuildsClient.buildLocatorScanLimit, + ): Promise { + return this.findInList( + `/v1/ciWorkflows/${workflowId}/buildRuns`, + (buildRun) => buildRun.attributes.number === buildNumber, + { limit: '200' }, + maxItems, + ); + } + + /** + * Find the latest failing build run for a workflow. + */ + async findLatestFailingForWorkflow( + workflowId: string, + maxItems: number = BuildsClient.buildLocatorScanLimit, + ): Promise { + return this.findInList( + `/v1/ciWorkflows/${workflowId}/buildRuns`, + (buildRun) => + buildRun.attributes.completionStatus === 'FAILED' || + buildRun.attributes.completionStatus === 'ERRORED', + { limit: '200' }, + maxItems, + ); } /** diff --git a/src/api/resources/products.ts b/src/api/resources/products.ts index bbfdb5f..5665a63 100644 --- a/src/api/resources/products.ts +++ b/src/api/resources/products.ts @@ -6,13 +6,9 @@ import type { CiProduct } from '../types.js'; */ export class ProductsClient extends BaseAPIClient { /** - * List Xcode Cloud products. + * List all Xcode Cloud products, paginating through all results. */ - async list(limit?: number): Promise { - const response = await this.get('/v1/ciProducts', { - ...(limit ? { limit: String(limit) } : {}), - }); - - return response.data; + async list(): Promise { + return this.listAll('/v1/ciProducts', { limit: '200' }); } -} +} \ No newline at end of file diff --git a/src/api/resources/workflows.ts b/src/api/resources/workflows.ts index f1b118d..843d459 100644 --- a/src/api/resources/workflows.ts +++ b/src/api/resources/workflows.ts @@ -12,17 +12,13 @@ type WorkflowAttributeUpdate = Partial; */ export class WorkflowsClient extends BaseAPIClient { /** - * List workflows belonging to a product. + * List all workflows belonging to a product, paginating through all results. */ - async listForProduct(productId: string, limit?: number): Promise { - const response = await this.get( + async listForProduct(productId: string): Promise { + return this.listAll( `/v1/ciProducts/${productId}/workflows`, - { - ...(limit ? { limit: String(limit) } : {}), - }, + { limit: '200' }, ); - - return response.data; } /** @@ -122,4 +118,4 @@ export class WorkflowsClient extends BaseAPIClient { ): Promise { return this.updateById(workflowId, { actions }); } -} +} \ No newline at end of file diff --git a/src/tools/build-runs.ts b/src/tools/build-runs.ts index 5ad161a..641173a 100644 --- a/src/tools/build-runs.ts +++ b/src/tools/build-runs.ts @@ -22,21 +22,21 @@ export function registerBuildRunTools( 'list_build_runs', { description: - 'List recent build runs for a workflow, optionally filtered by outcome.', + 'List recent build runs for a workflow, optionally filtered by outcome. Automatically paginates through build runs. Use limit to cap the number of results returned.', inputSchema: { workflowId: z.string(), - limit: z.number().int().positive().max(200).optional(), status: z.enum(['all', 'failed', 'pending', 'running', 'succeeded']).optional(), + limit: z.number().int().min(1).max(500).optional().describe('Maximum number of build runs to return. Defaults to 20 if not specified.'), }, }, async ({ workflowId, - limit, status, + limit, }: { workflowId: string; - limit?: number; status?: BuildRunStatusFilter; + limit?: number; }) => { try { const buildRuns = sortBuildRuns( @@ -99,4 +99,4 @@ function filterBuildRuns( return buildRuns.filter( (buildRun) => buildRun.attributes.completionStatus === 'SUCCEEDED', ); -} +} \ No newline at end of file diff --git a/src/tools/discovery.ts b/src/tools/discovery.ts index d436990..620f097 100644 --- a/src/tools/discovery.ts +++ b/src/tools/discovery.ts @@ -16,14 +16,12 @@ export function registerDiscoveryTools( 'list_products', { description: - 'List Xcode Cloud products available to the configured App Store Connect account.', - inputSchema: { - limit: z.number().int().positive().max(200).optional(), - }, + 'List Xcode Cloud products available to the configured App Store Connect account. Automatically paginates through all results.', + inputSchema: {}, }, - async ({ limit }: { limit?: number }) => { + async () => { try { - const products = await client.products.list(limit); + const products = await client.products.list(); return jsonResponse({ products: products.map((product) => ({ @@ -42,17 +40,15 @@ export function registerDiscoveryTools( server.registerTool( 'list_workflows', { - description: 'List workflows for a given Xcode Cloud product.', + description: 'List workflows for a given Xcode Cloud product. Automatically paginates through all results.', inputSchema: { productId: z.string(), - limit: z.number().int().positive().max(200).optional(), }, }, - async ({ productId, limit }: { productId: string; limit?: number }) => { + async ({ productId }: { productId: string }) => { try { const workflows = await client.workflows.listForProduct( parseIdentifier(productId, 'product'), - limit, ); return jsonResponse({ @@ -91,4 +87,4 @@ export function registerDiscoveryTools( } }, ); -} +} \ No newline at end of file diff --git a/src/utils/build-locator.ts b/src/utils/build-locator.ts index 8b46b6b..a38dead 100644 --- a/src/utils/build-locator.ts +++ b/src/utils/build-locator.ts @@ -59,23 +59,21 @@ export async function resolveBuildLocator( input: BuildLocatorInput, ): Promise { const locator = validateBuildLocator(input); + const workflowId = locator.workflowId!; if (locator.buildRunId) { return client.builds.getById(locator.buildRunId); } - const buildRuns = sortBuildRuns( - await client.builds.listForWorkflow(locator.workflowId!, 100), - ); - if (locator.buildNumber !== undefined) { - const matchingBuildRun = buildRuns.find( - (buildRun) => buildRun.attributes.number === locator.buildNumber, + const matchingBuildRun = await client.builds.findByNumberForWorkflow( + workflowId, + locator.buildNumber, ); if (!matchingBuildRun) { throw new Error( - `No build run found for workflow ${locator.workflowId} with build number ${locator.buildNumber}.`, + `No build run found for workflow ${workflowId} with build number ${locator.buildNumber}.`, ); } @@ -83,23 +81,27 @@ export async function resolveBuildLocator( } if (locator.buildSelector === 'latestFailing') { - const latestFailingBuildRun = buildRuns.find((buildRun) => - isFailureStatus(buildRun.attributes.completionStatus), + const latestFailingBuildRun = await client.builds.findLatestFailingForWorkflow( + workflowId, ); if (!latestFailingBuildRun) { throw new Error( - `No failing build runs found for workflow ${locator.workflowId}.`, + `No failing build runs found for workflow ${workflowId}.`, ); } return latestFailingBuildRun; } + const buildRuns = sortBuildRuns( + await client.builds.listForWorkflow(workflowId, 1), + ); + const latestBuildRun = buildRuns[0]; if (!latestBuildRun) { - throw new Error(`No build runs found for workflow ${locator.workflowId}.`); + throw new Error(`No build runs found for workflow ${workflowId}.`); } return latestBuildRun; diff --git a/tests/build-locator.test.ts b/tests/build-locator.test.ts index 4b23961..ea00420 100644 --- a/tests/build-locator.test.ts +++ b/tests/build-locator.test.ts @@ -59,7 +59,13 @@ test('sortBuildRuns returns newest build first', () => { }); test('resolveBuildLocator resolves a concrete build number', async () => { - const client = createClientMock(); + const client = createClientMock({ + findByNumberForWorkflow: async (workflowId, buildNumber) => { + assert.equal(workflowId, 'wf-1'); + assert.equal(buildNumber, 2); + return buildRuns[1]; + }, + }); const buildRun = await resolveBuildLocator(client, { workflowId: 'wf-1', @@ -70,7 +76,12 @@ test('resolveBuildLocator resolves a concrete build number', async () => { }); test('resolveBuildLocator resolves latest failing build', async () => { - const client = createClientMock(); + const client = createClientMock({ + findLatestFailingForWorkflow: async (workflowId) => { + assert.equal(workflowId, 'wf-1'); + return buildRuns[2]; + }, + }); const buildRun = await resolveBuildLocator(client, { workflowId: 'wf-1', @@ -80,6 +91,24 @@ test('resolveBuildLocator resolves latest failing build', async () => { assert.equal(buildRun.id, 'run-3'); }); +test('resolveBuildLocator resolves latest build from a bounded listing', async () => { + let receivedLimit: number | undefined; + const client = createClientMock({ + listForWorkflow: async (_workflowId, limit) => { + receivedLimit = limit; + return buildRuns; + }, + }); + + const buildRun = await resolveBuildLocator(client, { + workflowId: 'wf-1', + buildSelector: 'latest', + }); + + assert.equal(receivedLimit, 1); + assert.equal(buildRun.id, 'run-3'); +}); + test('isFailureStatus matches failed and errored runs only', () => { assert.equal(isFailureStatus('FAILED'), true); assert.equal(isFailureStatus('ERRORED'), true); @@ -87,12 +116,21 @@ test('isFailureStatus matches failed and errored runs only', () => { assert.equal(isFailureStatus('CANCELED'), false); }); -function createClientMock(): AppStoreConnectClient { +function createClientMock( + overrides: Partial = {}, +): AppStoreConnectClient { return { builds: { getById: async (buildRunId: string) => buildRuns.find((buildRun) => buildRun.id === buildRunId)!, listForWorkflow: async () => buildRuns, + findByNumberForWorkflow: async (_workflowId: string, buildNumber: number) => + buildRuns.find((buildRun) => buildRun.attributes.number === buildNumber), + findLatestFailingForWorkflow: async () => + buildRuns.find((buildRun) => + isFailureStatus(buildRun.attributes.completionStatus), + ), + ...overrides, }, } as unknown as AppStoreConnectClient; } diff --git a/tests/pagination.test.ts b/tests/pagination.test.ts new file mode 100644 index 0000000..59b5bf4 --- /dev/null +++ b/tests/pagination.test.ts @@ -0,0 +1,188 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { BaseAPIClient } from '../src/api/base-client.js'; +import type { APIResponse } from '../src/api/types.js'; + +/** + * Testable subclass that overrides request() to return mock data + * without making real HTTP calls. Since both get() and listAll() + * (for subsequent pages) call request(), this is the single point + * of interception needed. + */ +class TestableClient extends BaseAPIClient { + private responses: APIResponse[] = []; + private callIndex = 0; + + constructor(responses: APIResponse[]) { + super( + { + keyId: 'test', + issuerId: 'test', + privateKey: '-----BEGIN EC PRIVATE KEY-----\nMHQCAQEEIOBR1JO5H6GH5RF2U8DT0SP9UAXKKO7H5JF7E6U3L2VJoAcGBSuBBAAi\nZW2CAQBEAPnJ5VqvF2i0QzY3N7Y4C7Q3L8KM0N9QF2L8GK5V0Y5H2X1D4W7R9T6M\n-----END EC PRIVATE KEY-----', + }, + 'https://api.appstoreconnect.apple.com', + ); + this.responses = responses; + } + + protected override async request( + _url: string, + _init?: RequestInit, + ): Promise> { + const response = this.responses[this.callIndex] as unknown as APIResponse; + this.callIndex++; + return response; + } + + public async testListAll(path: string, params?: Record, maxItems?: number): Promise { + return this.listAll(path, params, maxItems); + } +} + +test('listAll collects items from a single page with no next link', async () => { + const client = new TestableClient([ + { + data: [{ id: '1' }, { id: '2' }], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test' }, + }, + ]); + + const result = await client.testListAll('/v1/test'); + + assert.equal(result.length, 2); + assert.equal((result[0] as { id: string }).id, '1'); + assert.equal((result[1] as { id: string }).id, '2'); +}); + +test('listAll follows pagination links across multiple pages', async () => { + const client = new TestableClient([ + { + data: [{ id: '1' }, { id: '2' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test?limit=200', + next: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page2&limit=200', + }, + }, + { + data: [{ id: '3' }, { id: '4' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page2&limit=200', + next: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page3&limit=200', + }, + }, + { + data: [{ id: '5' }], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page3&limit=200' }, + }, + ]); + + const result = await client.testListAll('/v1/test'); + + assert.equal(result.length, 5); + assert.deepEqual( + result.map((item) => (item as { id: string }).id), + ['1', '2', '3', '4', '5'], + ); +}); + +test('listAll rejects pagination links on unexpected origins', async () => { + const client = new TestableClient([ + { + data: [{ id: '1' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test?limit=200', + next: 'https://evil.example.test/v1/test?cursor=page2&limit=200', + }, + }, + ]); + + await assert.rejects( + () => client.testListAll('/v1/test'), + { + message: + 'Unexpected pagination origin: https://evil.example.test. Expected https://api.appstoreconnect.apple.com.', + }, + ); +}); + +test('listAll returns empty array when first page has empty data', async () => { + const client = new TestableClient([ + { + data: [], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test' }, + }, + ]); + + const result = await client.testListAll('/v1/test'); + + assert.equal(result.length, 0); +}); + +test('listAll respects maxItems limit and stops pagination early', async () => { + const client = new TestableClient([ + { + data: [{ id: '1' }, { id: '2' }, { id: '3' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test?limit=200', + next: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page2&limit=200', + }, + }, + { + data: [{ id: '4' }, { id: '5' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page2&limit=200', + next: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page3&limit=200', + }, + }, + { + data: [{ id: '6' }], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test?cursor=page3&limit=200' }, + }, + ]); + + // Request only 4 items — should stop after page 2 (3+2=5 >= 4) + const result = await client.testListAll('/v1/test', undefined, 4); + + assert.equal(result.length, 4); + assert.deepEqual( + result.map((item) => (item as { id: string }).id), + ['1', '2', '3', '4'], + ); +}); + +test('listAll with maxItems larger than available items returns all items', async () => { + const client = new TestableClient([ + { + data: [{ id: '1' }, { id: '2' }], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test' }, + }, + ]); + + const result = await client.testListAll('/v1/test', undefined, 100); + + assert.equal(result.length, 2); +}); + +test('listAll handles two pages correctly', async () => { + const client = new TestableClient([ + { + data: [{ id: 'a' }], + links: { + self: 'https://api.appstoreconnect.apple.com/v1/test', + next: 'https://api.appstoreconnect.apple.com/v1/test?cursor=2', + }, + }, + { + data: [{ id: 'b' }], + links: { self: 'https://api.appstoreconnect.apple.com/v1/test?cursor=2' }, + }, + ]); + + const result = await client.testListAll('/v1/test'); + + assert.equal(result.length, 2); + assert.deepEqual( + result.map((item) => (item as { id: string }).id), + ['a', 'b'], + ); +});