diff --git a/src/core/pagination.ts b/src/core/pagination.ts index 9a507f0e..91bb2032 100644 --- a/src/core/pagination.ts +++ b/src/core/pagination.ts @@ -148,15 +148,29 @@ export class OffsetPagination extends AbstractPage { } nextPageRequestOptions(): PageRequestOptions | null { - const offset = this.next_offset ?? 0; - const length = this.getPaginatedItems().length; - const currentCount = offset + length; + // X-Next-Offset is the absolute start of the next page, or 0 on the last + // page (the API's stop sentinel). The old code added the current page + // length on top, skipping a full page per iteration. Only a positive + // offset advances; 0 is a value the server affirmatively sent to mean + // "no more", so we stop on it (matching the Go SDK pager). + const offset = this.next_offset; + if (offset == null) { + if (this.has_more) { + throw new KernelError( + 'Server reported X-Has-More: true without an X-Next-Offset header; refusing to silently truncate pagination', + ); + } + return null; + } + if (offset === 0) { + return null; + } return { ...this.options, query: { ...maybeObj(this.options.query), - offset: currentCount, + offset, }, }; } diff --git a/tests/pagination.test.ts b/tests/pagination.test.ts new file mode 100644 index 00000000..3e5fadee --- /dev/null +++ b/tests/pagination.test.ts @@ -0,0 +1,58 @@ +import Kernel, { KernelError } from '@onkernel/sdk'; +import { OffsetPagination } from '@onkernel/sdk/core/pagination'; +import type { FinalRequestOptions } from '@onkernel/sdk/internal/request-options'; + +const client = new Kernel({ + apiKey: 'test-api-key', + fetch: () => Promise.reject(new Error('unexpected request')), +}); + +function pageWith( + headers: Record, + items: unknown[], + offset?: number, +): OffsetPagination { + const options: FinalRequestOptions = { + method: 'get', + path: '/proxies', + query: offset === undefined ? {} : { offset }, + }; + return new OffsetPagination(client, new Response('[]', { headers }), items, options); +} + +describe('OffsetPagination', () => { + test('requests the next page at exactly X-Next-Offset', () => { + // X-Next-Offset already holds the next page's start. Adding the current + // page length on top (the old behavior) skipped a full page per iteration. + const page = pageWith({ 'x-next-offset': '100', 'x-has-more': 'true' }, new Array(100).fill({}), 0); + expect(page.nextPageRequestOptions()?.query).toEqual({ offset: 100 }); + }); + + test('stops cleanly when the last page omits X-Next-Offset', () => { + const page = pageWith({ 'x-has-more': 'false' }, new Array(50).fill({}), 100); + expect(page.nextPageRequestOptions()).toBeNull(); + expect(page.hasNextPage()).toBe(false); + }); + + test('stops on the X-Next-Offset 0 sentinel even when X-Has-More is true', () => { + // Call nextPageRequestOptions directly with has_more=true so the has_more + // gate in hasNextPage() cannot mask it: the 0 offset alone must stop. + const page = pageWith({ 'x-next-offset': '0', 'x-has-more': 'true' }, new Array(50).fill({}), 100); + expect(page.nextPageRequestOptions()).toBeNull(); + }); + + test('stops when X-Has-More is false even with a positive X-Next-Offset', () => { + const page = pageWith({ 'x-next-offset': '200', 'x-has-more': 'false' }, new Array(50).fill({}), 100); + expect(page.hasNextPage()).toBe(false); + }); + + test('stops on an empty page', () => { + const page = pageWith({ 'x-next-offset': '300', 'x-has-more': 'true' }, [], 200); + expect(page.hasNextPage()).toBe(false); + }); + + test('refuses to silently truncate when X-Has-More is true but X-Next-Offset is missing', () => { + const page = pageWith({ 'x-has-more': 'true' }, new Array(100).fill({})); + expect(() => page.hasNextPage()).toThrow(KernelError); + }); +});