Merge pull request #127 from kernel/fix-offset-pagination-page-skip

Fix offset auto-pagination skipping every other page

Author: IlyaasK

src/core/pagination.ts

@@ -148,15 +148,29 @@ export class OffsetPagination<Item> extends AbstractPage<Item> {
   }
 
   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,
       },
     };
   }

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<string, string>,
+  items: unknown[],
+  offset?: number,
+): OffsetPagination<unknown> {
+  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);
+  });
+});